feat(dynamic-agents): integrate remote agents as tool calls via A2A protocol - #2373
feat(dynamic-agents): integrate remote agents as tool calls via A2A protocol#2373abhinavaditya811 wants to merge 2 commits into
Conversation
sriaradhyula
left a comment
There was a problem hiding this comment.
Thanks for taking this on. I found three blocking concerns in the current patch:
- The client implements an obsolete A2A wire contract; see the inline comments.
- This PR closes #2013 but only implements the client-side tool. The issue also requires the thin netutils A2A server, Docker Compose service/health check, and end-to-end verification. Please either complete those acceptance criteria or stop closing #2013 and track the remaining work explicitly.
- The sole commit is missing its DCO
Signed-off-bytrailer, and the DCO check is currentlyACTION_REQUIRED.
Validation performed: the 11 new tests pass and Ruff passes. The full dynamic-agents suite produced 379 passed, 3 skipped, and one unrelated failure that also reproduces on the parent commit.
|
|
||
| payload = { | ||
| "jsonrpc": "2.0", | ||
| "method": "tasks/send", |
There was a problem hiding this comment.
[P1] Implement a supported A2A wire contract. tasks/send and this legacy payload are not accepted by the intended A2A SDK. A2A 0.3 uses message/send, while current A2A uses SendMessage; both require a message identifier and differ in response shape. Please update request construction and response extraction end to end, and add a contract test against an actual A2A SDK server. See the official specification.
There was a problem hiding this comment.
This and your a2a-sdk suggestion are the same change, so I would rather not fix tasks/send by hand and then throw it away. Can you confirm the SDK as a client dependency?
If yes: pin one version across the dynamic-agents client and the netutils server, use ClientFactory.create_from_url() for discovery and negotiation, move bearer forwarding to a ClientCallInterceptor, and keep RemoteAgentTool as a thin adapter over send_message().
If no: I will implement SendMessage only and let 0.3 servers fail loudly rather than maintain two protocols by hand. Worth flagging why that matters: message/send does not appear in the current spec at all, only in the SDK's compat/v0_3 layer, so hand-rolling 0.3 support means implementing a method name the spec no longer documents.
Either way, your contract test makes a2a-sdk a test dependency of dynamic-agents, so the question is really whether we also take it at runtime.
Also confirming the target: pin 1.x on both sides and let the compat layer handle any third-party 0.3 server, or do you need netutils itself to advertise 0.3?
There was a problem hiding this comment.
If yes: pin one version across the dynamic-agents client and the netutils server, use ClientFactory.create_from_url() for discovery and negotiation, move bearer forwarding to a ClientCallInterceptor, and keep RemoteAgentTool as a thin adapter over send_message().
Let's use the dependency and pin a specific A2A SDK version.
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| AGENT_CARD_PATH = "/.well-known/agent.json" |
There was a problem hiding this comment.
[P1] Use the standard Agent Card path. A2A publishes the public card at /.well-known/agent-card.json. Using agent.json makes discovery fail against conforming servers and silently replaces the advertised name and description with fallback values.
There was a problem hiding this comment.
Fixed. The path is now /.well-known/agent-card.json, with /.well-known/agent.json kept as a fallback; it is tried only on a 404, so pre-0.3.0 servers still resolve. Any other status stops immediately rather than retrying a second path.
| if not urls: | ||
| return [] | ||
|
|
||
| tools = [ |
There was a problem hiding this comment.
[P2] Avoid serial discovery timeouts. Each card request is awaited before starting the next one. An unavailable endpoint can consume the full 10-second timeout, producing linear startup delay across configured agents. Please resolve cards concurrently with per-agent fallback, or cache/lazily fetch them.
There was a problem hiding this comment.
Fixed. _build_remote_agent_tools now resolves cards with asyncio.gather, so the slowest agent sets the cost rather than the sum: five unreachable endpoints cost 10s instead of 50s. Uses return_exceptions=True with per-URL logging, since a bare gather would cost the agent every other remote tool over one bad endpoint.
sriaradhyula
left a comment
There was a problem hiding this comment.
One additional product/authorization gap surfaced while tracing the Agent Builder flow: A2A agents need to be first-class, per-agent tool selections rather than a process-wide environment list.
| # Comma-separated JSON-RPC endpoints of remote A2A agents (env: | ||
| # REMOTE_AGENT_URLS). Each becomes a tool every agent can call to delegate | ||
| # to that remote agent — e.g. "http://netutils-agent:8000/". | ||
| remote_agent_urls: str = "" |
There was a problem hiding this comment.
[P1] Make remote-agent access configurable per dynamic agent. REMOTE_AGENT_URLS is process-wide, and _build_remote_agent_tools() consequently gives every dynamic agent every configured A2A tool. The Agent Builder only persists MCP allowed_tools, so users cannot discover, select, remove, clone, or apply approval/RBAC rules to A2A agents. Please introduce an admin-managed A2A-agent registry plus per-agent references (for example allowed_remote_agents), expose list/probe endpoints, add an A2A section to the Tools step, persist the selection through the TypeScript/Python models and create/update routes, and build only the selected tools at runtime. Avoid storing arbitrary endpoint URLs directly in user-editable agent documents.
There was a problem hiding this comment.
Agreed on the substance, but I would like this as a follow-up issue rather than in this PR.
It is not in #2013, whose acceptance criterion is "REMOTE_AGENT_URLS env var wired into dynamic agents tool assembly". What you describe needs a registry schema, list and probe endpoints, a Tools step section, TS and Python models, and create/update routes. For scale, allowed_tools alone spans 81 files here including the RBAC e2e specs. That is mostly registry, RBAC and frontend work rather than A2A protocol work.
On the security point, which should not wait: nothing stores endpoint URLs in user-editable agent documents today. REMOTE_AGENT_URLS is deployment config, so it is admin controlled and not user reachable. Happy to make "no user-supplied endpoint URLs" an explicit constraint on the follow-up.
Cards are already cached by URL rather than by agent, so per-agent selection will only decide which tools get built.
Shall I open that issue and link it from #2013?
There was a problem hiding this comment.
@abhinavaditya811 - Yes, please track it as new issue
sriaradhyula
left a comment
There was a problem hiding this comment.
A concrete implementation recommendation for the protocol concerns: use the official A2A Python client SDK rather than maintaining a custom JSON-RPC client.
|
|
||
| dynamic-agents ──tool call──> POST {a2a_url} (JSON-RPC ``tasks/send``) | ||
|
|
||
| No A2A SDK is needed on this side — ``tasks/send`` is a JSON-RPC POST over |
There was a problem hiding this comment.
Suggestion: use the official a2a-sdk client here. The hand-written client has already drifted from the protocol in its card path, method name, request identifiers, and response envelope. The official Python SDK supports both A2A 1.0 and 0.3 compatibility and provides create_client() / ClientFactory.create_from_url() for card discovery and transport negotiation. Pin the same SDK version for the dynamic-agents client and netutils server, use an interceptor (or supported HTTP configuration) for the request-scoped bearer token, and keep RemoteAgentTool as the thin LangChain adapter around SDK send_message(). The tradeoff is one dependency, but it removes protocol/version logic from this module and gives us supported handling for task, direct-message, streaming, and error responses. SDK docs: https://a2a-protocol.org/latest/sdk/python/api/a2a.client.client_factory.html
…rotocol Closes cnoe-io#2013 Signed-off-by: Abhinav <aditya811.abhinav@gmail.com>
…ncurrently Addresses two review comments on cnoe-io#2373. **Agent card path.** A2A publishes the public card at `/.well-known/agent-card.json`. We asked for `/.well-known/agent.json`, which 404s against any conforming server, and the failure was silent: the tool was still registered, with a URL-derived name and a generic "Remote agent at ..." description the LLM cannot route on. So it looked healthy and was not. The old path is kept as a fallback, tried only on a 404, so pre-0.3.0 servers still resolve. Any other status stops immediately: a 500 means the server answered and does not want to give us a card, so a second request is noise rather than resilience. For the record, `agent.json` was the canonical value in a2a-sdk 0.2.16, which was current when cnoe-io#2013 was written. It moved in 0.3.0, which kept the old value as `PREV_AGENT_CARD_WELL_KNOWN_PATH` for the transition, and 1.x dropped it. So that acceptance criterion was right at the time and is now stale. **Serial card resolution.** `_build_remote_agent_tools` awaited each card inside a list comprehension, so one unreachable endpoint burned the full card timeout before the next request started and startup delay grew linearly with the number of configured agents. Now resolved with `asyncio.gather`, so the slowest agent sets the cost instead of the sum: five unreachable agents cost 10s, not 50s. `return_exceptions=True` with per-URL logging, because a bare gather propagates the first exception and would cost the agent every other remote tool over one bad endpoint. Cards are cached by URL. Keyed by URL rather than by agent on purpose: the per-agent selection discussed on this PR only decides which tools get built, so it will not change how cards resolve, and two agents pointing at the same remote share one fetch. Failures are deliberately not cached. Otherwise a remote agent that was still starting up kept a useless description for as long as the process lived. Not caching the failure means the next runtime build retries, and with the runtime cache's 600s idle TTL an unreachable agent self-heals. **Tests.** The harness hardcoded `agent.json` in its handler, so it could only ever prove one path. It is now parameterised and records the paths requested. 18 tests, 7 new: current path, a 0.2.x server via the legacy path (asserting the current path is tried first), no card at all (exactly two requests, no retry loop), a 500 (no legacy retry), a resolved card not refetched, a failed card retried on the next build, and concurrent resolution. Verified the tests catch real regressions: reverting the path fails 5 of them, and caching failures alongside successes fails the self-healing one. Not covered: `_build_remote_agent_tools` itself cannot be imported in isolation because `agent_runtime` pulls in `cnoe_agent_utils`, so the gather wiring rests on review and CI rather than on a local test. Noted in the test docstring. The wire contract is unchanged and still `tasks/send`; that is the remaining P1 and depends on the a2a-sdk decision. Signed-off-by: Abhinav <aditya811.abhinav@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5625328 to
c9c1393
Compare
Fixes #2013.
What changed
ai_platform_engineering/dynamic_agents/src/dynamic_agents/config.pyai_platform_engineering/dynamic_agents/src/dynamic_agents/services/agent_runtime.pyai_platform_engineering/dynamic_agents/src/dynamic_agents/services/remote_agent_tool.pyai_platform_engineering/dynamic_agents/tests/test_remote_agent_tool.pyVerification
The project's own test suite was run before and after this change; it introduces no new test failures or lint violations.