Skip to content

fix(docs): improve demo a2a agent docs#3853

Merged
crivetimihai merged 2 commits intomainfrom
3709-improve-demo-a2a-agent-docs
Apr 2, 2026
Merged

fix(docs): improve demo a2a agent docs#3853
crivetimihai merged 2 commits intomainfrom
3709-improve-demo-a2a-agent-docs

Conversation

@marekdano
Copy link
Copy Markdown
Collaborator

📚 Documentation PR

Closes #3709

Summary

Fixes the demo A2A agent to correctly handle ContextForge's request formats and adds missing prerequisites to the documentation.

Changes

Documentation (docs/docs/using/agents/a2a.md)

  • Added Prerequisites section documenting required setup:
    • SSRF_ALLOW_LOCALHOST=true requirement with explanation
    • PLATFORM_ADMIN_EMAIL runtime requirement
  • Fixed token generation command - removed unnecessary --secret parameter (uses config default)
  • Updated API test examples to show correct request format with parameters wrapper
  • Consolidated running instructions into clear Option 1 and Option 2
  • Fixed Admin UI test steps to include agent registration step
  • Fixed MCP tool name from a2a_demo-calculator-agent to a2a-demo-calculator-agent

Demo Agent (scripts/demo_a2a_agent.py)

  • Fixed request parsing - replaced failing Pydantic models with direct JSON dict parsing
  • Added JSONRPC format support for endpoints ending with /
  • Added nested message structure handling for Admin UI test button
  • Added comprehensive debug logging showing raw request body and extraction path
  • Fixed Pydantic models to use Optional[str] = None instead of str = ""
  • Added typing.Optional import for proper type hints
  • Made handler async and added Request import for raw body access

Testing

Tested with:
1. Ran all curl request listed in the docs
2. Admin UI test button with queries like calc: 100/4+25 and weather: Dallas
3. MCP tool invocation via /rpc endpoint

All three methods now work correctly.

Fixes

  • Resolves demo agent returning "Hello" instead of processing queries
  • Addresses three failure modes: missing jti claim, wrong token subject, SSRF rejection
  • Fixes AttributeError when Admin UI sends nested message structure

@marekdano marekdano requested a review from crivetimihai as a code owner March 25, 2026 09:48
@marekdano marekdano requested a review from a-effort March 25, 2026 09:48
@marekdano marekdano added this to the Release 1.0.0 milestone Mar 25, 2026
@marekdano marekdano added documentation Improvements or additions to documentation agents Agent Samples a2a Support for A2A protocol labels Mar 25, 2026
@marekdano marekdano changed the title 3709 improve demo a2a agent docs fix(docs): improve demo a2a agent docs Mar 25, 2026
@marekdano marekdano added the SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release label Mar 27, 2026
@gcgoncalves gcgoncalves force-pushed the 3709-improve-demo-a2a-agent-docs branch from a58bdda to 57e5025 Compare March 27, 2026 10:59
@a-effort
Copy link
Copy Markdown
Collaborator

a-effort commented Mar 27, 2026

Good fixes for blockers users may run into when trying to run the demo!

Here are a few cleanup requests / pieces of feedback from 🤖


What's fixed correctly:

  • SSRF_ALLOW_LOCALHOST=true prerequisite was a silent blocker. Good fix ✅
  • --secret removed from token generation command: good, as it reads from .env
  • Tool name typo fixed: a2a_demo-calculator-agent
  • Admin UI steps now include the agent registration step that was missing ✅
  • Optional [str] = None on Pydantic fields is the right fix (empty string "" is falsy, which broke the or chain) ✅

Changes to consider:

  1. demo_a2a_agent.py run_agent rewrite: the Optional[str] = None fix already solves the root cause. The raw json.loads + manual key-walking approach is harder to maintain than the Pydantic model. Keep the Pydantic path (with Optional) or, if JSONRPC format is genuinely needed, add it as a fallback on top of the Pydantic parse rather than replacing it entirely.
  2. Debug print() statements: several print(f"Raw request body: ...") / print(f"Extracted from ...") lines were left in production code. Move them to logger.debug() or remove them.
  3. import json inside the handler: should be a top-level import.
  4. demo_a2a_agent_auth.py not updated. It has the same str = "" issue on Parameters and A2ARequest (lines 337–349). Apply the same Optional[str] = None fix so both scripts stay consistent.
  5. Minor doc nit: Option 2 generates a token into $TOKEN but the next line, uv run python scripts/demo_a2a_agent.py, doesn't use it. Either pass it in BEARER_TOKEN=$TOKEN or clarify the token is for the curl test commands, not the script startup.

@marekdano marekdano force-pushed the 3709-improve-demo-a2a-agent-docs branch from 57e5025 to a168f17 Compare March 30, 2026 09:10
@marekdano
Copy link
Copy Markdown
Collaborator Author

@a-effort reviewing items have been addressed:

  • json import: Moved to top-level (cleaner code structure)
  • print() statements: Kept as-is for local demo (appropriate for non-production script)
  • demo_a2a_agent_auth.py: Applied Optional[str] = None fix for consistency
  • Documentation: Clarified token usage to avoid confusion

gcgoncalves
gcgoncalves previously approved these changes Mar 30, 2026
Copy link
Copy Markdown
Collaborator

@vishu-bh vishu-bh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link
Copy Markdown
Collaborator

@vishu-bh vishu-bh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I noticed in scripts/demo_a2a_agent.py:167

The new debug print() statements log the full raw request body and extracted query text verbatim. That leaks prompt/request contents into logs and also allows unsanitized user-controlled text into log output.

For local debugging this may be convenient, but it is still a security-quality regression. If logging is needed, it should be reduced, sanitized, and preferably routed through structured debug logging rather than unconditional print() calls.

@marekdano
Copy link
Copy Markdown
Collaborator Author

@vishu-bh - reviewed it and removed the print statement with the full raw request body. I replaced the other print statements with the hardcoded strings.
There is no point in introducing the logging system in this demo_a2a_agents.py script since we do not log the sensitive data, and the print statements are already used there only.
The script is for demo purposes and will never be run on production.

@marekdano marekdano requested a review from vishu-bh March 30, 2026 15:02
vishu-bh
vishu-bh previously approved these changes Mar 30, 2026
Copy link
Copy Markdown
Collaborator

@vishu-bh vishu-bh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @marekdano LGTM 🚀

@marekdano marekdano added the release-fix Critical bugfix required for the release label Mar 30, 2026
Marek Dano and others added 2 commits April 2, 2026 13:48
- Add Prerequisites section (SSRF_ALLOW_LOCALHOST, PLATFORM_ADMIN_EMAIL)
- Fix token generation command and API test examples
- Add JSONRPC and nested message parsing to demo agent
- Fix Pydantic models to use Optional[str] = None
- Make /run handler async with raw body parsing for format flexibility

Closes #3709

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>
Review fixes for #3853:
- Restore correct MCP tool name prefix (a2a_ not a2a-)
- Read PLATFORM_ADMIN_EMAIL from env in demo script
- Remove unused Pydantic models (Parameters, A2ARequest)
- Add JSON parse error handling in /run endpoint
- Simplify docs running instructions
- Add 23 unit tests for demo agent parsing logic

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai
Copy link
Copy Markdown
Member

Thanks for the contribution, @marekdano — the prerequisites section and JSONRPC/nested-message parsing are solid additions.

I rebased, squashed the commits, and made a few review fixes on top:

Bug fix — MCP tool name prefix: The docs changed a2a_demo-calculator-agent to a2a-demo-calculator-agent, but tool_service.py uses f"a2a_{agent.slug}" (underscore). Reverted to the correct name.

PLATFORM_ADMIN_EMAIL env var: The docs referenced it but the script hardcoded admin@example.com. Updated create_jwt_token() to read os.environ.get("PLATFORM_ADMIN_EMAIL", "admin@example.com") and clarified the docs to describe environment variables (not .env file loading, since the script doesn't use python-dotenv).

Dead code cleanup: Removed the now-unused Parameters and A2ARequest Pydantic models (and the Optional import) from demo_a2a_agent.py since run_agent parses raw JSON directly.

Error handling: Added try/except around json.loads(body) so malformed requests get a clean error instead of a 500.

Docs simplification: Consolidated the Option 1/Option 2 pattern into a single clear flow.

Tests: Added 23 unit tests (tests/unit/scripts/test_demo_a2a_agent.py) covering all request format paths — JSONRPC simple, JSONRPC nested message parts, A2A protocol, simple query/message, empty body, invalid JSON, calculator ops, agent routing, and health endpoint. 100% coverage of the new/changed code.

Copy link
Copy Markdown
Member

@crivetimihai crivetimihai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good — rebased, fixes verified, tests added. Approving.

@crivetimihai crivetimihai merged commit a595706 into main Apr 2, 2026
19 of 37 checks passed
@crivetimihai crivetimihai deleted the 3709-improve-demo-a2a-agent-docs branch April 2, 2026 12:57
jonpspri pushed a commit that referenced this pull request Apr 10, 2026
* fix(docs): improve demo a2a agent docs and request parsing

- Add Prerequisites section (SSRF_ALLOW_LOCALHOST, PLATFORM_ADMIN_EMAIL)
- Fix token generation command and API test examples
- Add JSONRPC and nested message parsing to demo agent
- Fix Pydantic models to use Optional[str] = None
- Make /run handler async with raw body parsing for format flexibility

Closes #3709

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>

* fix(review): correct tool name, remove dead code, add tests

Review fixes for #3853:
- Restore correct MCP tool name prefix (a2a_ not a2a-)
- Read PLATFORM_ADMIN_EMAIL from env in demo script
- Remove unused Pydantic models (Parameters, A2ARequest)
- Add JSON parse error handling in /run endpoint
- Simplify docs running instructions
- Add 23 unit tests for demo agent parsing logic

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Marek Dano <Marek.Dano@ibm.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Marek Dano <Marek.Dano@ibm.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a2a Support for A2A protocol agents Agent Samples documentation Improvements or additions to documentation release-fix Critical bugfix required for the release SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DOCS][A2A]: Demo A2A agent quick-start is missing required prerequisites

5 participants