test(workspace): add 24-case coverage for builtin_tools/a2a_tools and send_message_wrapper (closes #367)

Added 24 new test cases across three groups:

builtin_tools/a2a_tools:
  - list_peers: 200 response, non-200 response (swallowed), network error
  - delegate_task: empty workspace_id guard, discover 404, discover 200+empty URL,
    A2A 500, result.parts=[], result is str/int/non-dict-part, error dict/string/null,
    POST exception
  - get_peers_summary: empty peers, missing peer fields, healthy roundtrip

send_message_wrapper.safe_send_message:
  - non-string input conversion, HTML entity escaping, truncation at 2000 chars,
    no-truncation under limit, debug logging, label prefix

Also added 2-line empty workspace_id guard in delegate_task (found by test).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

workspace/builtin_tools/a2a_tools.py

@@ -27,6 +27,8 @@ async def list_peers() -> list[dict]:
 
 async def delegate_task(workspace_id: str, task: str) -> str:
     """Send a task to a peer workspace via A2A and return the response text."""
+    if not workspace_id:
+        return "Error: workspace_id is required"
     async with httpx.AsyncClient(timeout=120.0) as client:
         # Discover target URL
         try:

workspace/tests/test_builtin_a2a_tools.py

@@ -0,0 +1,420 @@
+"""Test coverage for ``builtin_tools.a2a_tools`` and ``send_message_wrapper``.
+
+Issue #367: 21 new test cases targeting previously-uncovered branches.
+
+Uses ``respx`` for HTTP mocking — httpx.AsyncClient instantiates the client
+before the mock can intervene (it resolves the host during __init__), so
+patching at the class level is unreliable.  respx intercepts at the transport
+layer, which is safe regardless of how httpx initializes.
+"""
+from __future__ import annotations
+
+import asyncio
+import html
+import os
+import sys
+from types import ModuleType
+
+import pytest
+import respx
+
+
+# ---------------------------------------------------------------------------
+# Session-scoped fixture — reload httpx once at test-session start
+# ---------------------------------------------------------------------------
+
+_httpx_reloaded = False
+
+
+def _reload_httpx_and_real_module():
+    """Force-reload httpx so builtin_tools.a2a_tools imports the real client.
+
+    conftest.py mocks builtin_tools.a2a_tools, which prevents Python from
+    importing the real module from disk (sys.modules takes precedence). This
+    helper removes both sys.modules entries and triggers a fresh import of the
+    real httpx + builtin_tools.a2a_tools chain.
+    """
+    global _httpx_reloaded
+    if _httpx_reloaded:
+        return
+    _httpx_reloaded = True
+
+    # conftest.py set builtin_tools.__path__ = [] — restore so Python can
+    # find builtin_tools/a2a_tools.py on disk.
+    real_builtin = sys.modules.get("builtin_tools")
+    if real_builtin is not None:
+        builtin_dir = os.path.dirname(
+            os.path.dirname(os.path.abspath(__file__))
+        )
+        real_builtin.__path__ = [os.path.join(builtin_dir, "builtin_tools")]
+
+    # Remove the conftest.py mock so the real module loads
+    sys.modules.pop("builtin_tools.a2a_tools", None)
+
+
+# Session-scoped: reload httpx once, not per-test. Per-test fixture only
+# sets env vars (env vars can be set per-test without disturbing httpx).
+@pytest.fixture(scope="session", autouse=True)
+def _reload_httpx_session():
+    _reload_httpx_and_real_module()
+    yield
+
+
+@pytest.fixture(autouse=True)
+def _require_env(monkeypatch):
+    """Per-test: set required env vars. httpx is already reloaded at session start."""
+    monkeypatch.setenv("WORKSPACE_ID", "00000000-0000-0000-0000-000000000001")
+    monkeypatch.setenv("PLATFORM_URL", "http://test.invalid")
+    yield
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+def _run(coro):
+    return asyncio.get_event_loop().run_until_complete(coro)
+
+
+# =============================================================================
+# builtin_tools/a2a_tools — list_peers
+# =============================================================================
+
+class TestListPeers:
+    """Coverage for builtin_tools/a2a_tools.list_peers()."""
+
+    @respx.mock
+    def test_returns_peers_on_200(self):
+        """Successful GET returns the peer list."""
+        from builtin_tools.a2a_tools import list_peers
+
+        peers = [
+            {"id": "ws-1", "name": "Alpha", "role": "sre", "status": "online"},
+            {"id": "ws-2", "name": "Beta",  "role": "dev", "status": "busy"},
+        ]
+        route = respx.get(
+            "http://test.invalid/registry/00000000-0000-0000-0000-000000000001/peers"
+        ).respond(200, json=peers)
+        result = _run(list_peers())
+        assert result == peers
+        assert route.called
+
+    @respx.mock
+    def test_returns_empty_list_on_non_200(self):
+        """list_peers swallows all non-200 responses gracefully."""
+        from builtin_tools.a2a_tools import list_peers
+
+        respx.get(
+            "http://test.invalid/registry/00000000-0000-0000-0000-000000000001/peers"
+        ).respond(500)
+        result = _run(list_peers())
+        assert result == []
+
+    @respx.mock
+    def test_returns_empty_list_on_exception(self):
+        """Network errors must not propagate — list_peers returns []. """
+        from builtin_tools.a2a_tools import list_peers
+
+        # Route that raises so httpx propagates an exception
+        respx.get(
+            "http://test.invalid/registry/00000000-0000-0000-0000-000000000001/peers"
+        ).mock(side_effect=RuntimeError("dns failure"))
+        result = _run(list_peers())
+        assert result == []
+
+
+# =============================================================================
+# builtin_tools/a2a_tools — delegate_task
+# =============================================================================
+
+_DISCOVER_ROUTE = "http://test.invalid/registry/discover/ws-target"
+
+
+class TestDelegateTask:
+    """Coverage for builtin_tools/a2a_tools.delegate_task(workspace_id, task)."""
+
+    def test_empty_workspace_id_returns_error(self):
+        """Empty workspace_id is validated before any network call."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        out = _run(delegate_task("", "do it"))
+        assert "Error" in out
+        assert "workspace_id" in out.lower()
+
+    @respx.mock
+    def test_discover_returns_non_200(self):
+        """Discovery 4xx/5xx → error message with status code."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(404)
+        out = _run(delegate_task("ws-target", "do it"))
+        assert "Error" in out
+        assert "404" in out
+
+    @respx.mock
+    def test_discover_returns_200_with_empty_url(self):
+        """Discovery 200 but no url field → actionable error."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(200, json={"name": "orphan"})
+        out = _run(delegate_task("ws-target", "do it"))
+        assert "Error" in out
+        assert "no URL" in out
+
+    @respx.mock
+    def test_a2a_post_returns_500(self):
+        """A2A send 5xx → Error: sending A2A message."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(500)
+        out = _run(delegate_task("ws-target", "do it"))
+        assert "Error" in out
+        assert "sending A2A message" in out
+
+    @respx.mock
+    def test_result_parts_empty_dict(self):
+        """Regression #279: {"parts": []} → str(result), not "(no text)"."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(
+            200, json={"result": {"parts": []}}
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        # Must return str(result), not "(no text)"
+        assert "parts" in out
+        assert "(no text)" not in out
+
+    @respx.mock
+    def test_result_is_plain_string(self):
+        """A bare string result returns as-is."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(
+            200, json={"result": "just a plain string"}
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        assert out == "just a plain string"
+
+    @respx.mock
+    def test_result_is_number(self):
+        """Non-dict, non-string result → falls through to "(no text)"."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(
+            200, json={"result": 12345}
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        assert out == "(no text)"
+
+    @respx.mock
+    def test_result_parts_non_dict_element(self):
+        """parts[0] is not a dict → falls through to "(no text)".
+
+        The code checks if parts[0] is a dict; since 123 is an int, it hits
+        the else-branch and returns "(no text)".
+        """
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(
+            200, json={"result": {"parts": [123, "also a string"]}}
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        assert out == "(no text)"
+
+    @respx.mock
+    def test_error_dict_form(self):
+        """{"error": {"message": "..."}} → "Error: ..."."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(
+            200, json={"error": {"message": "peer overloaded", "code": 429}}
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        assert out == "Error: peer overloaded"
+
+    @respx.mock
+    def test_error_string_form(self):
+        """{"error": "string error"} → "Error: string error"."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(
+            200, json={"error": "workspace offline"}
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        assert out == "Error: workspace offline"
+
+    @respx.mock
+    def test_error_null(self):
+        """{"error": null} → "Error: None" (edge case — str(null) in message)."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").respond(
+            200, json={"error": None}
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        assert "Error" in out
+
+    @respx.mock
+    def test_a2a_post_raises_exception(self):
+        """Network error during A2A POST → Error: sending A2A message: ..."""
+        from builtin_tools.a2a_tools import delegate_task
+
+        respx.get(_DISCOVER_ROUTE).respond(
+            200, json={"url": "http://peer.invalid/a2a"}
+        )
+        respx.post("http://peer.invalid/a2a").mock(
+            side_effect=ConnectionError("connection refused")
+        )
+        out = _run(delegate_task("ws-target", "do it"))
+        assert "Error" in out
+        assert "connection refused" in out
+
+
+# =============================================================================
+# builtin_tools/a2a_tools — get_peers_summary
+# =============================================================================
+
+_PEERS_ROUTE = (
+    "http://test.invalid/registry/00000000-0000-0000-0000-000000000001/peers"
+)
+
+
+class TestGetPeersSummary:
+    """Coverage for builtin_tools/a2a_tools.get_peers_summary()."""
+
+    @respx.mock
+    def test_empty_peers_returns_no_peers_available(self):
+        from builtin_tools.a2a_tools import get_peers_summary
+
+        respx.get(_PEERS_ROUTE).respond(200, json=[])
+        out = _run(get_peers_summary())
+        assert "No peers" in out
+
+    @respx.mock
+    def test_peer_missing_fields(self):
+        """Peers with missing name/id/role/status must not KeyError/TypeError."""
+        from builtin_tools.a2a_tools import get_peers_summary
+
+        # Peer has only 'id'; name, role, status are absent
+        respx.get(_PEERS_ROUTE).respond(200, json=[{"id": "ws-x"}])
+        out = _run(get_peers_summary())
+        assert "ws-x" in out
+        assert isinstance(out, str)
+
+    @respx.mock
+    def test_healthy_peer_roundtrip(self):
+        """Sanity: normal peer dicts produce a formatted list."""
+        from builtin_tools.a2a_tools import get_peers_summary
+
+        peers = [
+            {"id": "ws-alpha", "name": "Alpha", "role": "sre", "status": "online"},
+        ]
+        respx.get(_PEERS_ROUTE).respond(200, json=peers)
+        out = _run(get_peers_summary())
+        assert "Alpha" in out
+        assert "ws-alpha" in out
+        assert "sre" in out
+        assert "online" in out
+
+
+# =============================================================================
+# send_message_wrapper — safe_send_message
+# =============================================================================
+
+from unittest.mock import patch
+
+from adapters.smolagents.send_message_wrapper import safe_send_message
+
+
+class TestSafeSendMessage:
+    """Coverage for adapters.smolagents.send_message_wrapper.safe_send_message()."""
+
+    def test_non_string_input_converted(self):
+        """Non-str text is str()-converted before escaping."""
+        delivered = []
+        safe_send_message(42, send_fn=lambda s: delivered.append(s))
+        assert delivered == ["[smolagents] 42"]
+        assert isinstance(delivered[0], str)
+
+    def test_html_entities_escaped(self):
+        """< > ' are escaped so rendered UIs cannot be injected.
+
+        The payload <script>alert('xss')</script> has no literal '&', so &amp;
+        does not appear. The escape output is: &lt;script&gt;alert(&#x27;xss&#x27;)&lt;/script&gt;
+        """
+        delivered = []
+        safe_send_message(
+            "<script>alert('xss')</script>",
+            send_fn=lambda s: delivered.append(s),
+        )
+        assert "&lt;" in delivered[0]
+        assert "&gt;" in delivered[0]
+        assert "&#x27;" in delivered[0]
+        assert "&lt;script&gt;" in delivered[0]
+        # The angle brackets and quotes must NOT appear unescaped
+        assert "<script>" not in delivered[0]
+        assert "alert('" not in delivered[0]
+
+    def test_truncation_at_max_len(self):
+        """Text > 2000 chars is truncated; caller is warned."""
+        delivered = []
+        with patch(
+            "adapters.smolagents.send_message_wrapper.logger"
+        ) as mock_logger:
+            long_text = "A" * 2500
+            safe_send_message(long_text, send_fn=lambda s: delivered.append(s))
+            assert len(delivered[0]) < len(long_text)
+            mock_logger.warning.assert_called_once()
+            assert "truncating" in mock_logger.warning.call_args[0][0]
+
+    def test_no_truncation_under_max_len(self):
+        """Text ≤ 2000 chars is passed through intact with no warning."""
+        delivered = []
+        with patch(
+            "adapters.smolagents.send_message_wrapper.logger"
+        ) as mock_logger:
+            text = "A" * 1500
+            safe_send_message(text, send_fn=lambda s: delivered.append(s))
+            expected = f"[smolagents] {text}"
+            assert delivered[0] == expected
+            mock_logger.warning.assert_not_called()
+
+    def test_debug_log_emitted(self):
+        """Every delivery logs at DEBUG with final payload length."""
+        delivered = []
+        with patch(
+            "adapters.smolagents.send_message_wrapper.logger"
+        ) as mock_logger:
+            safe_send_message("hello", send_fn=lambda s: delivered.append(s))
+            mock_logger.debug.assert_called_once()
+            assert "delivering" in mock_logger.debug.call_args[0][0]
+
+    def test_label_prefix_always_present(self):
+        """Every delivered payload starts with '[smolagents]'."""
+        delivered = []
+        safe_send_message("x", send_fn=lambda s: delivered.append(s))
+        assert delivered[0].startswith("[smolagents]")