Skip to content

fix: serve a2a/unified and agent register route deliver what they advertise - #5974

Merged
praisonai-triage-agent[bot] merged 2 commits into
mainfrom
claude/issue-5973-20261024
Oct 24, 2026
Merged

praisonai-triage-agent[bot] merged 2 commits into
mainfrom
claude/issue-5973-20261024

Conversation

@praisonai-triage-agent

Copy link
Copy Markdown
Contributor

Fixes #5973

All changes are in the wrapper (src/praisonai). There are no new dependencies and no changes to the core SDK or Agent.

Changes

  1. praisonai serve a2a: message/send now runs the --file agents YAML via praisonai.arun. This is the same YAML-to-agent path that serve agents falls back to. Before, it returned Received: <text>.
    • A missing YAML file now fails at startup.
    • An empty text part returns JSON-RPC error -32602.
    • An agent failure returns -32603.
    • Because /a2a now runs tools, binding to a non-localhost host requires --api-key (or PRAISONAI_SERVE_API_KEY), the same rule serve agents and serve unified use. --api-key was already listed as a common option.
  2. praisonai serve unified: the server now advertises only what it mounts.
    • When --file exists, it mounts POST /agents and POST /a2a.
    • It always mounts a2u.
    • Recipe and MCP are removed from discovery, the startup banner and /. They run as their own servers (serve recipe, praisonai mcp serve), so mounting them here would duplicate those servers.
    • The serve API-key gate is now in one shared helper (_resolve_api_key), so both commands use it.
  3. POST /api/v1/agents/{id}/register: now returns 501 instead of a 200 that registered nothing. Agents are registered in-process with register_agent(...).
  4. Related bug, already on main: add_discovery_routes didn't annotate request. As a result, FastAPI returned 422 for /__praisonai__/discovery and /health on every FastAPI serve app. The handlers now annotate request: Request.

Tests

New tests/unit/test_serve_surfaces_honest.py (7 tests) checks that:

  • A2A runs the agent.
  • A2A requires an API key off localhost and rejects a missing file.
  • Unified mounts every provider it advertises, both with and without a YAML file.
  • Discovery returns 200.
  • Register returns 501.

The new tests pass. These failures also happen on unmodified main:

  • 7 tests in test_n8n_agent_invoke (they don't set CALL_SERVER_TOKEN).
  • jobs/test_router.
  • test_code_reachability.
  • test_serve_recipe_gateway_split.
  • test_fast_context in the core SDK.

🤖 Generated with Claude Code

…ertise (fixes #5973)

- serve a2a: message/send runs the --file agents YAML via praisonai.arun
  instead of echoing; missing file rejected; non-localhost requires --api-key.
- serve unified: mounts POST /agents and /a2a when --file exists; discovery,
  banner and / list only mounted providers (recipe/mcp are separate servers).
- POST /agents/{id}/register returns 501 instead of a no-op 200.
- discovery/health handlers annotate Request so FastAPI no longer 422s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d3f2eedf-0e61-4253-88fe-303729f9fb07

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MervinPraison

Copy link
Copy Markdown
Owner

@coderabbitai review

@MervinPraison

Copy link
Copy Markdown
Owner

/review

@MervinPraison MervinPraison added pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet pipeline/final-claude-pending Reviews done; waiting for FINAL @claude labels Oct 24, 2026
@praisonai-triage-agent praisonai-triage-agent Bot added the pipeline/reviews-pending Waiting for CodeRabbit/Qodo/Copilot reviews label Oct 24, 2026
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review skipped.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@praisonai-triage-agent praisonai-triage-agent Bot removed the pipeline/reviews-pending Waiting for CodeRabbit/Qodo/Copilot reviews label Oct 24, 2026
@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; both previous findings are addressed and no new actionable issues were found.

Summary

The PR makes the advertised serve routes run their workflows and returns 501 for unsupported HTTP registration.

  • A2A messages now get replies from the configured agent.
  • The unified server lists only providers it actually mounts.
  • The HTTP registration route now says when it cannot register an agent.
  • FastAPI discovery and health routes now receive their request correctly.

Reviews (2) · Last reviewed commit: "fix: reject non-object /agents bodies an..." · Reviewed by Greptile

Comment thread src/praisonai/praisonai/cli/features/serve.py
Comment thread src/praisonai/tests/unit/test_serve_surfaces_honest.py
@MervinPraison

Copy link
Copy Markdown
Owner

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

MANDATORY READ (before reviewing):

  • Always read src/praisonai-agents/AGENTS.md
  • If this PR touches src/praisonai-ts/, also read src/praisonai-ts/AGENTS.md §2.1.2 (TS triage + PR review checklist)

Phase 1: Review per AGENTS.md

  1. Protocol-driven: check heavy implementations vs core SDK
  2. Backward compatible: ensure zero feature regressions
  3. Performance: no hot-path regressions
  4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
  5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
  6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/
    6b. External tool examples (examples/tools/external/**): If the diff is confined to that path, mark SDK value ❌ for monorepo merge — redirect the contributor to PraisonAI-Tools; do not Approve for merge unless the PR has the maintainer-accept-example label.
    6c. New vendor *_tools.py in core (src/praisonai-agents/praisonaiagents/tools/): If the PR adds a new *_tools.py for a third-party API/SaaS/dataset (e.g. FXMacroData), mark Repo routing ❌ and SDK value ❌ — redirect to MervinPraison/PraisonAI-Tools. Do not Approve because legacy bundled tools (Tavily/Exa/You.com/etc.) already in core are not precedent for new vendor modules. Request changes or recommend closing unless the PR has maintainer-accept-core-tools.

MANDATORY COMMENT FORMAT — include this Phase 1 table in your review comment:

Phase 1 — AGENTS.md review

Check Result
Protocol-driven / no heavy impl in core ✅ or ❌ + one-line rationale
Backward compatible ✅ or ❌ + one-line rationale
Performance (hot path) ✅ or ❌ + one-line rationale
SDK value ✅ or ❌ + one-line rationale (explicitly judge whether the change strengthens the SDK)
No Agent param bloat ✅ or ❌ + one-line rationale
Repo routing ✅ or ❌ + one-line rationale

For TypeScript PRs (src/praisonai-ts/), also add:
| TS types / parity / tests | ✅ or ❌ + one-line rationale (npm run build && npm test) |

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. Never Approve or write “Approve for merge” if any Phase 1 row is ❌, if rule 6b/6c applies, or if the change belongs in PraisonAI-Tools/PraisonAI-Plugins. Use Request changes, redirect to the external repo, and recommend closing the monorepo PR when routing is wrong.
12. Approve only when every Phase 1 row is ✅ (including Repo routing and SDK value) and blocking issues are resolved; otherwise request changes with clear action items

@MervinPraison MervinPraison added pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:cooldown Blocked: post-push or @claude cooldown pipeline/blocked:stale-final Blocked: FINAL stale after new commits and removed pipeline/final-claude-pending Reviews done; waiting for FINAL @claude pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet labels Oct 24, 2026
…lient errors

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

Final architecture review — PR #5974

Phase 1 — AGENTS.md review

Check Result
Protocol-driven / no heavy impl in core ✅ Every change is in the praisonai wrapper (serve CLI, api, endpoints). praisonaiagents is untouched.
Backward compatible ✅ /a2a used to echo Received: … and now runs the YAML workflow. Register used to return a fake 200 and now returns 501. Unified discovery now lists only the providers it actually mounts. Each change fixes a surface that misreported what it did.
Performance (hot path) ✅ Request-time code only. The YAML file is read once when the app is built.
SDK value ✅ Removes placeholder or dishonest surfaces. It reuses praisonai.arun and the existing api-key middleware, and adds no new features.
No Agent param bloat ✅ Agent class untouched.
Repo routing ✅ Serve and CLI code belongs in the wrapper.

Phase 2 — Fixes pushed (71e47ff)

  • src/praisonai/praisonai/cli/features/serve.py
    • Unified POST /agents now returns 400 for JSON bodies that are not objects ([], null, "q"). Before, it crashed with a 500 (Greptile P2).
    • Found myself, same class of bug: POST /a2a crashed with a 500 when params, message, parts or a part had the wrong type (e.g. "params": null). It now returns JSON-RPC -32602.
  • src/praisonai/tests/unit/test_serve_surfaces_honest.py: added tests for the -32602 (no text / malformed params) and -32603 (arun raises) replies, checking the error code and request id, and for 400 on non-object /agents bodies (Greptile P2).

Tests: tests/unit/test_serve_surfaces_honest.py has 9 passing. The wider -k "serve or endpoint or discovery or a2a" run gives 437 passed and 5 failed. The same 5 fail with my change stashed; they come from the local environment (pytest-asyncio not installed, praisonai_bot install path) and not from this PR.

Skipped: nothing from reviewer feedback. CodeRabbit and Qodo raised no actionable items.

Phase 3 — Verdict

✅ Approve. Every Phase 1 row passes and both review findings are fixed.

@MervinPraison MervinPraison removed pipeline/blocked:cooldown Blocked: post-push or @claude cooldown pipeline/blocked:stale-final Blocked: FINAL stale after new commits labels Oct 24, 2026
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

Merge gate scan — eligible for assessment. Claude merge gate will assess and may auto-merge on an APPROVE verdict.

@praisonai-triage-agent praisonai-triage-agent Bot added pipeline/merge-ready Eligible for merge gate auto-merge claude-merge-gate-active and removed pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:ci Blocked: CI not green on HEAD labels Oct 24, 2026
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

MERGE_GATE_VERDICT: APPROVE

  • Author is praisonai-triage-agent[bot], which is on the allowlist. The PR comes from a branch in this repo, not a fork.
  • Size: 4 files, +320/-106 lines, well under the 800-line and 30-file limits.
  • Paths: all changes are in the praisonai wrapper (cli/features/serve.py, api/agent_invoke.py, endpoints/server.py) plus a new unit test file. praisonaiagents, agent.py and Agent params are untouched. No workflows, auth, pyproject.toml or secrets.
  • SDK value: the PR replaces placeholder surfaces with working ones, reusing praisonai.arun and the existing API-key middleware:
    • /a2a used to echo the message back and now runs the agents YAML.
    • The unified server's discovery now lists only the providers it actually mounts.
    • HTTP register now returns 501 instead of a fake 200.
    • FastAPI /__praisonai__/discovery and /health no longer return 422.
  • Scope: no new dependencies, Agent params or exports. Routing is correct; no tools, plugins or sandbox backends were added.
  • Security: serve a2a now refuses to bind a non-localhost host without an API key, through the shared _resolve_api_key helper.
  • Tests: tests/unit/test_serve_surfaces_honest.py covers the A2A happy path and its error codes, the API-key gate, a missing file, unified providers with and without a YAML file, discovery returning 200, and register returning 501.
  • Reviews: the final architecture review is present, with the Phase 1 table and an explicit SDK value row, all ✅, verdict Approve. Greptile gives 5/5. No CHANGES_REQUESTED reviews on HEAD 71e47ff.
  • Labels: none of security, breaking-change, needs-manual-review or release.
  • Not checked: the gate could not read AGENTS.md (permission was denied), so the architecture checks used the rules in the gate checklist.

@praisonai-triage-agent
praisonai-triage-agent Bot merged commit db02a80 into main Oct 24, 2026
42 checks passed
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

Merged by Claude PR merge gate (claude-merge-gate.yml).
Verdict: MERGE_GATE_VERDICT: APPROVE
SHA: 71e47ff
Method: merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merged-by-gate pipeline/merge-ready Eligible for merge gate auto-merge

Projects

None yet

1 participant