From 93b7d9a88a6bfe6532fb194c9ee2f89b82d26a43 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Mon, 11 May 2026 01:22:02 +0000 Subject: [PATCH] fix(a2a_tools): add comment + test coverage for string-form error handling in delegate_task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Staging branch bea89ce4 introduced duplicate dead code after a `return` in the delegate_task error-handling block — the first occurrence was the correct fix (adding isinstance(err, str)), but the second occurrence (now unreachable) made the block fragile. Main already has the correct code; this branch adds an explanatory comment and regression tests. The non-tool delegate_task() in a2a_tools.py uses httpx.AsyncClient directly (not send_a2a_message) and must handle three A2A proxy error shapes: {"error": "plain string"} ← the bug fix: isinstance(err, str) {"error": {"message": "...", ...}} ← pre-existing path {"error": {"nested": "object"}} ← falls through to str(err) Adds TestDelegateTaskDirect: test_string_form_error_returns_error_message — regression for AttributeError test_dict_form_error_returns_error_message — pre-existing path still works test_success_returns_result_text — happy path still works Co-Authored-By: Claude Opus 4.7 --- workspace/builtin_tools/a2a_tools.py | 2 + workspace/tests/test_a2a_tools_impl.py | 99 ++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/workspace/builtin_tools/a2a_tools.py b/workspace/builtin_tools/a2a_tools.py index acdd15cb..d568ee40 100644 --- a/workspace/builtin_tools/a2a_tools.py +++ b/workspace/builtin_tools/a2a_tools.py @@ -77,6 +77,8 @@ async def delegate_task(workspace_id: str, task: str) -> str: return str(result) if isinstance(result, str) else "(no text)" elif "error" in data: err = data["error"] + # Handle both string-form errors ("error": "some string") + # and object-form errors ("error": {"message": "...", "code": ...}). msg = "" if isinstance(err, dict): msg = err.get("message", "") diff --git a/workspace/tests/test_a2a_tools_impl.py b/workspace/tests/test_a2a_tools_impl.py index 801eae80..690b3fc5 100644 --- a/workspace/tests/test_a2a_tools_impl.py +++ b/workspace/tests/test_a2a_tools_impl.py @@ -326,6 +326,105 @@ class TestToolDelegateTask: assert a2a_tools._peer_names.get("ws-nona000") is not None +# --------------------------------------------------------------------------- +# delegate_task (non-tool, direct httpx path — used by adapter templates) +# --------------------------------------------------------------------------- + +class TestDelegateTaskDirect: + + async def test_string_form_error_returns_error_message(self): + """The A2A proxy can return {"error": "plain string"}. Must not raise + AttributeError: 'str' object has no attribute 'get'.""" + import a2a_tools + + # Mock: discover succeeds, A2A POST returns a string-form error + mc = AsyncMock() + mc.__aenter__ = AsyncMock(return_value=mc) + mc.__aexit__ = AsyncMock(return_value=False) + + async def fake_post(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"error": "peer workspace unreachable"}) + return r + + async def fake_get(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"url": "http://peer.svc/a2a"}) + return r + + mc.post = fake_post + mc.get = fake_get + + with patch("a2a_tools.httpx.AsyncClient", return_value=mc): + result = await a2a_tools.delegate_task("ws-peer-123", "do a thing") + + assert "Error" in result + assert "peer workspace unreachable" in result + + async def test_dict_form_error_returns_error_message(self): + """{"error": {"message": "...", "code": ...}} — the pre-existing path.""" + import a2a_tools + + mc = AsyncMock() + mc.__aenter__ = AsyncMock(return_value=mc) + mc.__aexit__ = AsyncMock(return_value=False) + + async def fake_post(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"error": {"message": "internal server error", "code": 500}}) + return r + + async def fake_get(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"url": "http://peer.svc/a2a"}) + return r + + mc.post = fake_post + mc.get = fake_get + + with patch("a2a_tools.httpx.AsyncClient", return_value=mc): + result = await a2a_tools.delegate_task("ws-peer-456", "do a thing") + + assert "Error" in result + assert "internal server error" in result + + async def test_success_returns_result_text(self): + """Happy path: result with parts returns the first text part.""" + import a2a_tools + + mc = AsyncMock() + mc.__aenter__ = AsyncMock(return_value=mc) + mc.__aexit__ = AsyncMock(return_value=False) + + async def fake_post(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={ + "result": { + "parts": [{"kind": "text", "text": "Task done!"}] + } + }) + return r + + async def fake_get(url, **kwargs): + r = MagicMock() + r.status_code = 200 + r.json = MagicMock(return_value={"url": "http://peer.svc/a2a"}) + return r + + mc.post = fake_post + mc.get = fake_get + + with patch("a2a_tools.httpx.AsyncClient", return_value=mc): + result = await a2a_tools.delegate_task("ws-peer-789", "do a thing") + + assert result == "Task done!" + + # --------------------------------------------------------------------------- # tool_delegate_task_async # ---------------------------------------------------------------------------