From 9bf2dac6b940701a28d2ce6abc0639d277e17a65 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 7 Jun 2026 22:23:35 -0700 Subject: [PATCH] fix(a2a): client tools take args-as-dict positional; accept agent_name alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live Tier-3 testing (CLI agent -> a2a tools -> live peer gateway -> model) surfaced two bugs the kwarg-style unit tests masked: 1. registry.dispatch calls handlers as handler(args, **kwargs) — args is the whole dict positional. The handlers used keyword params (url=, agent=), so the dict bound to the first param and .strip() raised 'dict object has no attribute strip'. Rewrote all three handlers to take args: dict (matching the spotify/google_meet convention). Added a registry-dispatch regression test that exercises the real call path the direct-kwarg tests never hit. 2. The model repeatedly reached for agent_name= instead of agent= (6 retries before success). Accept agent_name/name and message/text/task aliases so a reasonable guess succeeds first try. Verified live: client agent discovers the peer's Agent Card, calls it, and gets the reply back (PONG round-trip confirmed on both client audit log and peer conversation log). 39 plugin tests pass. --- plugins/platforms/a2a/tools.py | 14 ++++--- tests/plugins/test_a2a_plugin.py | 66 ++++++++++++++++++++++++++++---- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/plugins/platforms/a2a/tools.py b/plugins/platforms/a2a/tools.py index 53450ece007..4dc78909055 100644 --- a/plugins/platforms/a2a/tools.py +++ b/plugins/platforms/a2a/tools.py @@ -101,9 +101,9 @@ def _rpc_url(base_url: str, card: Optional[dict]) -> str: # Tool handlers # -------------------------------------------------------------------------- -def a2a_discover(url: str = "", **_: Any) -> str: +def a2a_discover(args: dict, **_: Any) -> str: """Fetch and summarize the Agent Card at ``url``.""" - url = (url or "").strip() + url = str(args.get("url") or "").strip() if not url: return "Error: 'url' is required (e.g. http://localhost:9999)." try: @@ -130,14 +130,16 @@ def a2a_discover(url: str = "", **_: Any) -> str: return "\n".join(lines) -def a2a_call(agent: str = "", message: str = "", context_id: str = "", **_: Any) -> str: +def a2a_call(args: dict, **_: Any) -> str: """Send a task to a peer agent and return its reply. ``agent`` is a configured peer name (from ``a2a_agents``) or a direct URL. ``context_id`` continues a prior exchange (multi-turn) when provided. """ - agent = (agent or "").strip() - message = (message or "").strip() + # Accept common aliases models reach for (observed live: 'agent_name'). + agent = str(args.get("agent") or args.get("agent_name") or args.get("name") or "").strip() + message = str(args.get("message") or args.get("text") or args.get("task") or "").strip() + context_id = str(args.get("context_id") or args.get("contextId") or "").strip() if not agent or not message: return "Error: both 'agent' and 'message' are required." @@ -217,7 +219,7 @@ def _reply_text_from_result(result: Any) -> str: return protocol.extract_text(result) -def a2a_list(**_: Any) -> str: +def a2a_list(args: dict | None = None, **_: Any) -> str: """List configured A2A peers and any persisted conversations.""" cfg = _load_config() peers = cfg.get("a2a_agents") or {} diff --git a/tests/plugins/test_a2a_plugin.py b/tests/plugins/test_a2a_plugin.py index c7cf6a15fed..6567cb36a78 100644 --- a/tests/plugins/test_a2a_plugin.py +++ b/tests/plugins/test_a2a_plugin.py @@ -207,15 +207,15 @@ class TestPersistence: class TestClientTools: def test_call_requires_args(self): - assert "required" in tools.a2a_call(agent="", message="hi") - assert "required" in tools.a2a_call(agent="x", message="") + assert "required" in tools.a2a_call({"agent": "", "message": "hi"}) + assert "required" in tools.a2a_call({"agent": "x", "message": ""}) def test_discover_requires_url(self): - assert "required" in tools.a2a_discover(url="") + assert "required" in tools.a2a_discover({"url": ""}) def test_unknown_peer(self, monkeypatch): monkeypatch.setattr(tools, "_load_config", lambda: {"a2a_agents": {}}) - out = tools.a2a_call(agent="ghost", message="hi") + out = tools.a2a_call({"agent": "ghost", "message": "hi"}) assert "unknown agent" in out def test_discover_summarizes_card(self, monkeypatch): @@ -225,7 +225,7 @@ class TestClientTools: skills=[{"id": "s", "name": "search", "description": "web search"}], ) monkeypatch.setattr(tools, "_http_get_json", lambda url, h, t: card) - out = tools.a2a_discover(url="http://localhost:9999") + out = tools.a2a_discover({"url": "http://localhost:9999"}) assert "researcher" in out assert "search" in out @@ -245,7 +245,7 @@ class TestClientTools: ) monkeypatch.setattr(tools, "_http_post_json", fake_post) - out = tools.a2a_call(agent="r", message="my key sk-abcdefghij1234567890ABCD please") + out = tools.a2a_call({"agent": "r", "message": "my key sk-abcdefghij1234567890ABCD please"}) assert "here is the answer" in out # Outbound redaction applied before sending. sent = captured["body"]["params"]["message"]["parts"][0]["text"] @@ -254,10 +254,62 @@ class TestClientTools: def test_list_no_peers(self, monkeypatch, tmp_path): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) monkeypatch.setattr(tools, "_load_config", lambda: {}) - out = tools.a2a_list() + out = tools.a2a_list({}) assert "No peers configured" in out +class TestRegistryDispatchConvention: + """Tools must accept the args-as-dict positional that registry.dispatch + uses (`entry.handler(args, **kwargs)`), not keyword params. Calling the + handlers with a single dict positional is what the live agent does — this + is the convention the direct-kwarg tests above did NOT exercise, which let + an 'dict has no attribute strip' bug ship to a live Tier-3 run.""" + + def test_register_then_dispatch_via_registry(self, monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr(tools, "_load_config", lambda: {}) + from tools.registry import registry + + class _Ctx: + def register_tool(self, name, toolset, schema, handler, **kw): + registry.register(name=name, toolset=toolset, schema=schema, + handler=handler, override=True, **kw) + + tools.register_tools(_Ctx()) + + # Dispatch each tool the way the agent loop does: args as a dict. + # a2a_discover with empty url should return the 'required' guard + # string, NOT raise AttributeError on a dict. + out = registry.dispatch("a2a_discover", {"url": ""}) + assert "required" in out and "AttributeError" not in out + + out = registry.dispatch("a2a_call", {"agent": "", "message": ""}) + assert "required" in out and "AttributeError" not in out + + out = registry.dispatch("a2a_list", {}) + assert "No peers configured" in out + + def test_a2a_call_accepts_agent_name_alias(self, monkeypatch): + """Models reach for 'agent_name' (observed live). Accept it as an + alias for 'agent' so the call doesn't fail the required-arg guard.""" + monkeypatch.setattr(tools, "_load_config", + lambda: {"a2a_agents": {"peer": {"url": "http://localhost:9999"}}}) + monkeypatch.setattr(tools, "_http_get_json", lambda url, h, t: None) + captured = {} + + def fake_post(url, body, headers, timeout): + captured["sent"] = True + return protocol.jsonrpc_result( + body["id"], + protocol.build_task("t", "c1", protocol.STATE_COMPLETED, "PONG")) + + monkeypatch.setattr(tools, "_http_post_json", fake_post) + # 'agent_name' alias instead of 'agent' + out = tools.a2a_call({"agent_name": "peer", "message": "ping"}) + assert captured.get("sent") is True + assert "PONG" in out + + # -------------------------------------------------------------------------- # End-to-end inbound round-trip (real http.server + mocked agent) # --------------------------------------------------------------------------