From b7c962bf86843dc93f7cb8278079685f33ce8eec Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 3 May 2026 23:14:12 -0700 Subject: [PATCH] feat(mcp): wrap inbound channel content with identity + reply hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the channel-plugin change in Molecule-AI/molecule-mcp-claude-channel#24 so the universal MCP path (in-workspace agents) gets the same self-documenting reply guidance the external channel plugin path now ships. Before: `params.content` was the raw inbound text — Claude saw bare prose from a peer or canvas user with no surrounding context. To reply the agent had to (a) fish the routing fields out of `meta`, (b) recall which platform tool routes to which destination (send_message_to_user for canvas, delegate_task for peer), and (c) construct the call by hand. After: content is wrapped as [from · peer_id=] (or "[from canvas user]") ↩ Reply: The identity comes from the existing registry-enrichment path (peer_name + peer_role from enrich_peer_metadata, with friendly fallbacks when the registry lookup misses). Reply tool name lives in the same module as the notification builder so the `feedback_doc_tool_alignment` drift class can't bite — a future tool rename PR that misses this hint also fails test_format_channel_content_*. Tests: 6 new cases pinning the formatter (canvas_user vs peer_agent, full enrichment, name-only, no enrichment, unknown-kind defensive default, multi-line preservation) plus updated existing assertions in the bridge + content tests. All asserts pin exact strings per `feedback_assert_exact_not_substring`. Co-Authored-By: Claude Opus 4.7 (1M context) --- workspace/a2a_mcp_server.py | 70 ++++++++++- workspace/tests/test_a2a_mcp_server.py | 159 ++++++++++++++++++++++++- 2 files changed, 222 insertions(+), 7 deletions(-) diff --git a/workspace/a2a_mcp_server.py b/workspace/a2a_mcp_server.py index 5e1f87da..388ed016 100644 --- a/workspace/a2a_mcp_server.py +++ b/workspace/a2a_mcp_server.py @@ -458,16 +458,84 @@ def _build_channel_notification(msg: dict) -> dict: # endpoint to hit for capabilities lookup. meta["agent_card_url"] = _agent_card_url_for(safe_peer_id) + # Compose the conversation-turn text Claude actually sees. Header + # carries peer identity (name + role when registry-resolved, peer_id + # always); footer carries the exact reply-tool call shape so the + # model doesn't have to remember which tool to call or what args to + # pass. See _format_channel_content for the rationale + tradeoff on + # coupling display to behaviour. Mirrors the change shipped for the + # external channel-plugin path + # (Molecule-AI/molecule-mcp-claude-channel#24); the universal MCP + # path is the same display surface for in-workspace agents. + content = _format_channel_content( + text=msg.get("text", ""), + kind=meta["kind"], + peer_id=meta["peer_id"], + peer_name=meta.get("peer_name"), + peer_role=meta.get("peer_role"), + ) return { "jsonrpc": "2.0", "method": _CHANNEL_NOTIFICATION_METHOD, "params": { - "content": msg.get("text", ""), + "content": content, "meta": meta, }, } +def _format_channel_content( + *, + text: str, + kind: str, + peer_id: str, + peer_name: str | None = None, + peer_role: str | None = None, +) -> str: + """Prepend identity + append reply-tool example to the inbound text. + + Why this couples display to behaviour: Claude Code surfaces the + notification's ``content`` as the conversation turn. Without context + in the text, the model has to remember (a) who sent the message, + (b) which tool to call to reply, (c) which args to pass. Putting it + in the turn itself makes the reply path self-documenting at the + cost of ~80 extra chars per push. + + The reply-tool names live in the same module as the notification + builder so the ``feedback_doc_tool_alignment`` drift class can't bite: + a future tool-rename PR that misses this hint would also fail + ``test_format_channel_content_*`` below. + + canvas_user → ``send_message_to_user({message: "..."})`` — pushed via + canvas WebSocket, lands in the user's chat panel. + peer_agent → ``delegate_task({workspace_id: peer_id, task: "..."})`` + — sends an A2A reply to the calling peer. + """ + if kind == "canvas_user": + header = "[from canvas user]" + hint = '↩ Reply: send_message_to_user({message: "..."})' + elif kind == "peer_agent": + if peer_name and peer_role: + identity = f"{peer_name} ({peer_role})" + elif peer_name: + identity = peer_name + else: + identity = "peer-agent" + header = f"[from {identity} · peer_id={peer_id}]" + hint = ( + f'↩ Reply: delegate_task({{workspace_id: "{peer_id}", ' + f'task: "..."}})' + ) + else: + # Defensive default — _safe_meta_field already constrains kind to + # _VALID_KINDS, so this branch is unreachable in practice. Emit + # the bare text rather than crash so a future kind value (added + # to the allowlist but not the formatter) degrades gracefully + # instead of breaking every push. + return text + return f"{header}\n{text}\n{hint}" + + # --- MCP Server (JSON-RPC over stdio) --- diff --git a/workspace/tests/test_a2a_mcp_server.py b/workspace/tests/test_a2a_mcp_server.py index 84d145ed..5839f2c2 100644 --- a/workspace/tests/test_a2a_mcp_server.py +++ b/workspace/tests/test_a2a_mcp_server.py @@ -171,9 +171,19 @@ def test_build_channel_notification_method_matches_claude_contract(): assert payload["jsonrpc"] == "2.0" -def test_build_channel_notification_content_is_message_text(): - """`content` is what becomes the agent conversation turn — - pulled directly from the inbox message text.""" +def test_build_channel_notification_content_wraps_text_with_identity_and_reply_hint(): + """`content` is what becomes the agent conversation turn — wrapped + with an identity header AND a reply-tool hint. The wrapping makes the + reply path self-documenting so the agent doesn't have to remember + which platform tool to call (per the cross-codepath fix shipped with + Molecule-AI/molecule-mcp-claude-channel#24). + + Before this change `content == msg["text"]` and the agent had to + reach into meta + recall send_message_to_user / delegate_task on + every push. Now the conversation turn carries the identity inline + and a copy-pasteable reply call, so the model surfaces the right + routing without round-tripping through tool documentation each time. + """ from a2a_mcp_server import _build_channel_notification payload = _build_channel_notification({ @@ -185,7 +195,14 @@ def test_build_channel_notification_content_is_message_text(): "created_at": "2026-05-01T00:00:00Z", }) - assert payload["params"]["content"] == "hello from canvas" + # Exact match — per `feedback_assert_exact_not_substring`, substring + # asserts pass for both correct formatting AND for "raw input echoed" + # regression. Only equality discriminates. + assert payload["params"]["content"] == ( + "[from canvas user]\n" + "hello from canvas\n" + '↩ Reply: send_message_to_user({message: "..."})' + ) def test_build_channel_notification_meta_carries_routing_fields(): @@ -235,7 +252,14 @@ def test_build_channel_notification_no_id_field(): def test_build_channel_notification_handles_missing_fields_gracefully(): """Some fields may be absent on edge-case messages (e.g. cursor bootstrapping with no created_at yet). Default to empty strings - so the wire shape stays valid JSON instead of crashing.""" + so the wire shape stays valid JSON instead of crashing. + + With an empty-kind payload the formatter falls through its + defensive default branch (kind not in _VALID_KINDS) and emits the + bare text — no header, no reply hint. This degrades gracefully + rather than emitting a "[from None]" header that would mislead the + receiving agent about who sent the empty payload. + """ from a2a_mcp_server import _build_channel_notification payload = _build_channel_notification({}) @@ -247,6 +271,120 @@ def test_build_channel_notification_handles_missing_fields_gracefully(): assert meta["kind"] == "" +# ----- _format_channel_content: identity header + reply-tool hint ---------- +# +# Pinned separately from _build_channel_notification so a regression in +# the formatter surfaces with a tight failure message ("expected +# delegate_task hint, got send_message_to_user") rather than buried in a +# generic envelope-shape diff. Per `feedback_assert_exact_not_substring`, +# all asserts pin exact strings. + + +def test_format_channel_content_canvas_user_uses_send_message_to_user(): + """canvas_user → reply via send_message_to_user (canvas WebSocket + push). Header omits peer_id since canvas messages don't carry one.""" + from a2a_mcp_server import _format_channel_content + + out = _format_channel_content( + text="what's the deploy status?", + kind="canvas_user", + peer_id="", + ) + assert out == ( + "[from canvas user]\n" + "what's the deploy status?\n" + '↩ Reply: send_message_to_user({message: "..."})' + ) + + +def test_format_channel_content_peer_agent_with_full_enrichment(): + """peer_agent + name + role → friendly identity, delegate_task hint + with workspace_id arg pinned to the peer's UUID.""" + from a2a_mcp_server import _format_channel_content + + peer_uuid = "11111111-2222-3333-4444-555555555555" + out = _format_channel_content( + text="ping", + kind="peer_agent", + peer_id=peer_uuid, + peer_name="ops-agent", + peer_role="sre", + ) + assert out == ( + f"[from ops-agent (sre) · peer_id={peer_uuid}]\n" + "ping\n" + f'↩ Reply: delegate_task({{workspace_id: "{peer_uuid}", task: "..."}})' + ) + + +def test_format_channel_content_peer_agent_name_only(): + """peer_agent + name (no role) → identity uses bare name. Catches + the regression where role-only or both-missing branches accidentally + print 'None' or '(undefined)' in the header.""" + from a2a_mcp_server import _format_channel_content + + peer_uuid = "11111111-2222-3333-4444-555555555555" + out = _format_channel_content( + text="ping", + kind="peer_agent", + peer_id=peer_uuid, + peer_name="ops-agent", + ) + assert out.startswith(f"[from ops-agent · peer_id={peer_uuid}]\n") + assert "(None)" not in out + assert "(undefined)" not in out + + +def test_format_channel_content_peer_agent_no_enrichment_falls_back(): + """peer_agent without name/role (registry miss) → identity is + 'peer-agent' and peer_id is still surfaced so the reply call has + a value to copy.""" + from a2a_mcp_server import _format_channel_content + + peer_uuid = "11111111-2222-3333-4444-555555555555" + out = _format_channel_content( + text="ping", + kind="peer_agent", + peer_id=peer_uuid, + ) + assert out == ( + f"[from peer-agent · peer_id={peer_uuid}]\n" + "ping\n" + f'↩ Reply: delegate_task({{workspace_id: "{peer_uuid}", task: "..."}})' + ) + + +def test_format_channel_content_unknown_kind_degrades_to_raw_text(): + """Defensive default — _safe_meta_field already constrains kind to + _VALID_KINDS, so this branch is unreachable in practice. But if a + future kind is added to the allowlist before the formatter learns + about it, emitting raw text is better than crashing the push path.""" + from a2a_mcp_server import _format_channel_content + + assert _format_channel_content( + text="something", kind="future_kind", peer_id="", + ) == "something" + + +def test_format_channel_content_preserves_multiline_text(): + """Body text may contain newlines (multi-paragraph user prose, + code blocks). Content composition must not collapse or truncate + them — the agent's reply quality depends on seeing the full + inbound message.""" + from a2a_mcp_server import _format_channel_content + + multi = "first paragraph\n\nsecond paragraph\nstill second" + out = _format_channel_content( + text=multi, kind="canvas_user", peer_id="", + ) + # Body sandwiched between header and hint, separated by single + # newlines. Body itself unchanged. + assert ( + f"[from canvas user]\n{multi}\n" + '↩ Reply: send_message_to_user({message: "..."})' + ) == out + + # ----- Channel envelope enrichment (peer_name / peer_role / agent_card_url) --- # # The bare envelope only carries `peer_id` for peer_agent inbound, so the @@ -1088,7 +1226,16 @@ async def test_inbox_bridge_emits_channel_notification_to_writer(): assert payload["jsonrpc"] == "2.0" assert payload["method"] == "notifications/claude/channel" - assert payload["params"]["content"] == "hello from peer" + # Content is wrapped with the identity header + reply hint — + # see _format_channel_content. The bridge test pins the full + # composition so a regression to "raw text only" surfaces here + # as well as in the per-formatter tests above. + assert payload["params"]["content"] == ( + "[from peer-agent · peer_id=11111111-2222-3333-4444-555555555555]\n" + "hello from peer\n" + '↩ Reply: delegate_task({workspace_id: ' + '"11111111-2222-3333-4444-555555555555", task: "..."})' + ) meta = payload["params"]["meta"] assert meta["source"] == "molecule" assert meta["kind"] == "peer_agent"