forked from molecule-ai/molecule-core
Merge pull request #2701 from Molecule-AI/feat/universal-mcp-content-reply-hint
feat(mcp): wrap inbound channel content with identity + reply hint
This commit is contained in:
commit
2e3e36b91f
@ -458,16 +458,84 @@ def _build_channel_notification(msg: dict) -> dict:
|
|||||||
# endpoint to hit for capabilities lookup.
|
# endpoint to hit for capabilities lookup.
|
||||||
meta["agent_card_url"] = _agent_card_url_for(safe_peer_id)
|
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 {
|
return {
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"method": _CHANNEL_NOTIFICATION_METHOD,
|
"method": _CHANNEL_NOTIFICATION_METHOD,
|
||||||
"params": {
|
"params": {
|
||||||
"content": msg.get("text", ""),
|
"content": content,
|
||||||
"meta": meta,
|
"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) ---
|
# --- MCP Server (JSON-RPC over stdio) ---
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@ -171,9 +171,19 @@ def test_build_channel_notification_method_matches_claude_contract():
|
|||||||
assert payload["jsonrpc"] == "2.0"
|
assert payload["jsonrpc"] == "2.0"
|
||||||
|
|
||||||
|
|
||||||
def test_build_channel_notification_content_is_message_text():
|
def test_build_channel_notification_content_wraps_text_with_identity_and_reply_hint():
|
||||||
"""`content` is what becomes the agent conversation turn —
|
"""`content` is what becomes the agent conversation turn — wrapped
|
||||||
pulled directly from the inbox message text."""
|
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
|
from a2a_mcp_server import _build_channel_notification
|
||||||
|
|
||||||
payload = _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",
|
"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():
|
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():
|
def test_build_channel_notification_handles_missing_fields_gracefully():
|
||||||
"""Some fields may be absent on edge-case messages (e.g. cursor
|
"""Some fields may be absent on edge-case messages (e.g. cursor
|
||||||
bootstrapping with no created_at yet). Default to empty strings
|
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
|
from a2a_mcp_server import _build_channel_notification
|
||||||
|
|
||||||
payload = _build_channel_notification({})
|
payload = _build_channel_notification({})
|
||||||
@ -247,6 +271,120 @@ def test_build_channel_notification_handles_missing_fields_gracefully():
|
|||||||
assert meta["kind"] == ""
|
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) ---
|
# ----- Channel envelope enrichment (peer_name / peer_role / agent_card_url) ---
|
||||||
#
|
#
|
||||||
# The bare envelope only carries `peer_id` for peer_agent inbound, so the
|
# 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["jsonrpc"] == "2.0"
|
||||||
assert payload["method"] == "notifications/claude/channel"
|
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"]
|
meta = payload["params"]["meta"]
|
||||||
assert meta["source"] == "molecule"
|
assert meta["source"] == "molecule"
|
||||||
assert meta["kind"] == "peer_agent"
|
assert meta["kind"] == "peer_agent"
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user