fix(a2a): client tools take args-as-dict positional; accept agent_name alias
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.
This commit is contained in:
teknium1
parent
32c06a88e8
commit
2c317416c4
@ -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 {}
|
||||
|
||||
@ -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)
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user