Repository navigation
fix(a2a): resolve a remote agent's card and client once across invocations - #7672
ishan-1010 wants to merge 2 commits into
Conversation
…tions The agent tree is cloned for every invocation, and BaseAgent.clone() is a shallow model_copy, so RemoteA2aAgent cached the resolved agent card, the httpx client and the A2A client on a copy that was thrown away. Every turn fetched the card again and opened a new httpx client, even when it never transferred to the remote agent, and cleanup() on the original instance could not close those clients. Keep that state on an object the agent and its clones share. Fixes google#7629
tonydzi
left a comment
There was a problem hiding this comment.
Mycroft here, Anton's synthetic AI cofounder. I read _ensure_resolved and cleanup on this branch (4b07387) against the new shared _SharedResolution.
Sharing the card/client across clones makes cleanup() (_remote_a2a_agent.py, ~L2370) act on state that live invocations also use, and cleanup() only half-resets it:
-
It closes
self._httpx_clientand sets it toNone, but leaves_a2a_client,_a2a_client_factory(whichrebind_client_factory_httpxbound to that client around L1052) and_is_resolveduntouched. The early return at ~L1311 (not per_invocation_card and self._is_resolved and self._a2a_client) therefore keeps handing out a client whose transport is already closed. Before this PR a clone had its own state, so closing one instance did not reach into another; nowRunner.close()on the original instance (or any clone'scleanup()) can break the in-flight and the next invocations of every clone.test_cleanup_closes_client_created_by_cloneassertsclient.is_closedandclone._httpx_client is None, but not that a later_ensure_resolved()rebuilds a working client. Resetting_a2a_client, the factory binding andis_resolvedincleanup()(or dropping the_SharedResolutionobject) would close that. -
The shared path in
_ensure_resolvedhas awaits between check and set (_resolve_agent_card,_ensure_httpx_client, thenself._a2a_client = self._a2a_client_factory.create(...)afterif not self._a2a_client) and no lock._ensure_httpx_clientalready carries the comment "Another caller may have created the client during the await", so the race is known. With per-clone state it required concurrent calls on one instance; with the shared object, every concurrent first request across sessions on a server joins it (N card fetches, N clients, last write wins). Anasyncio.Lockon the_SharedResolutionaround the shared branch would make the "fetched once" claim intest_card_is_fetched_once_across_turnshold under concurrency too, and that test runs turns sequentially, so it cannot show it.
I read the code paths only; I did not run them.
cleanup() closed the httpx client but kept the A2A client, the factory bound to it and the resolved flag, so the next request (from any clone, since they share this state) got a client whose transport was closed. Reset them too, so the next request resolves again. Concurrent first requests across sessions each fetched the card and built a client. Run the shared resolution under a lock the clones share.
|
@tonydzi both fixed in 7836316.
|
Link to Issue or Description of Change
Problem:
The agent tree is cloned for every invocation and
BaseAgent.clone()is a shallowmodel_copy, soRemoteA2aAgentcached the resolved card, httpx client and A2A client on a copy that gets thrown away. Every turn fetched/.well-known/agent-card.jsonagain and opened a newhttpx.AsyncClient, even on turns that never transfer, andcleanup()/runner.close()on the instance you passed in couldn't close them.Solution:
Keep that state (
_agent_card,_a2a_client,_a2a_client_factory,_httpx_client,_httpx_client_needs_cleanup,_is_resolved) on a small_SharedResolutionobject the agent and its clones share. The attributes stay as properties with the same names, so the rest of the class is unchanged. The per-invocation path for authenticated cards still builds its client per invocation and doesn't cache it.Testing Plan
Unit Tests:
New
TestRemoteA2aAgentClonesShareResolution, all three fail on main:test_card_is_fetched_once_across_turns: 3 runner turns through a rootLlmAgent, card resolved once (3 on main)test_clones_reuse_one_card_and_client: two clones get the same A2A clienttest_cleanup_closes_client_created_by_clone:cleanup()on the original closes the httpx client a clone openedManual End-to-End (E2E) Tests:
The repro script from #7629 (real uvicorn A2A server, fake
BaseLlm, 5 turns that never transfer):Checklist