diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 3a2d3f68e17..db9ee4cd0a3 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -372,7 +372,7 @@ def _detect_claude_code_version() -> str: _CLAUDE_CODE_SYSTEM_PREFIX = "You are Claude Code, Anthropic's official CLI for Claude." -_MCP_TOOL_PREFIX = "mcp_" +_MCP_TOOL_PREFIX = "mcp__" def _get_claude_code_version() -> str: @@ -2349,14 +2349,17 @@ def build_anthropic_kwargs( text = text.replace("Nous Research", "Anthropic") block["text"] = text - # 3. Prefix tool names with mcp_ (Claude Code convention) + # 3. Prefix tool names with mcp__ (Claude Code convention) # Skip names that already begin with the marker — native MCP server # tools (from mcp_servers: in config.yaml) are registered under their # full mcp__ name and would double-prefix otherwise, # breaking round-trip registry lookup in normalize_response. GH-25255. + # Check both single-underscore (legacy MCP) and double-underscore + # (new prefix) to avoid double-prefixing either format. + _MCP_SKIP = ("mcp__", "mcp_") if anthropic_tools: for tool in anthropic_tools: - if "name" in tool and not tool["name"].startswith(_MCP_TOOL_PREFIX): + if "name" in tool and not tool["name"].startswith(_MCP_SKIP): tool["name"] = _MCP_TOOL_PREFIX + tool["name"] # 4. Prefix tool names in message history (tool_use and tool_result blocks) @@ -2366,7 +2369,7 @@ def build_anthropic_kwargs( for block in content: if isinstance(block, dict): if block.get("type") == "tool_use" and "name" in block: - if not block["name"].startswith(_MCP_TOOL_PREFIX): + if not block["name"].startswith(_MCP_SKIP): block["name"] = _MCP_TOOL_PREFIX + block["name"] elif block.get("type") == "tool_result" and "tool_use_id" in block: pass # tool_result uses ID, not name diff --git a/agent/transports/anthropic.py b/agent/transports/anthropic.py index aad49161385..7a53bf909d6 100644 --- a/agent/transports/anthropic.py +++ b/agent/transports/anthropic.py @@ -88,7 +88,7 @@ class AnthropicTransport(ProviderTransport): from agent.transports.types import ToolCall strip_tool_prefix = kwargs.get("strip_tool_prefix", False) - _MCP_PREFIX = "mcp_" + _MCP_PREFIX = "mcp__" text_parts = [] reasoning_parts = [] diff --git a/tests/agent/test_anthropic_mcp_prefix_strip.py b/tests/agent/test_anthropic_mcp_prefix_strip.py index 4806661497d..0aaad3f046b 100644 --- a/tests/agent/test_anthropic_mcp_prefix_strip.py +++ b/tests/agent/test_anthropic_mcp_prefix_strip.py @@ -1,7 +1,7 @@ -"""Tests for GH-25255: Anthropic OAuth mcp_ prefix stripping. +"""Tests for GH-25255: Anthropic OAuth mcp__ prefix stripping. When strip_tool_prefix=True (Anthropic OAuth path), the transport must only -strip the ``mcp_`` prefix from OAuth-injected tools, NOT from Hermes-native +strip the ``mcp__`` prefix from OAuth-injected tools, NOT from Hermes-native MCP server tools that are registered under their full ``mcp__`` name in the tool registry. """ @@ -61,14 +61,14 @@ class TestAnthropicMcpPrefixStrip: return AnthropicTransport() def test_strips_prefix_for_oauth_injected_tool(self): - """OAuth tools: mcp_read_file -> read_file (stripped). + """OAuth tools: mcp__read_file -> read_file (stripped). The tool was registered as 'read_file' in the registry. - Anthropic sees 'mcp_read_file' because Hermes adds the prefix. + Anthropic sees 'mcp__read_file' because Hermes adds the prefix. On response, we must strip it back to 'read_file'. """ transport = self._get_transport() - block = _make_tool_use_block("mcp_read_file") + block = _make_tool_use_block("mcp__read_file") response = _make_response(block) registry = _FakeRegistry({"read_file", "terminal", "web_search"}) @@ -102,7 +102,7 @@ class TestAnthropicMcpPrefixStrip: def test_no_strip_when_flag_false(self): """When strip_tool_prefix=False, names are never modified.""" transport = self._get_transport() - block = _make_tool_use_block("mcp_read_file") + block = _make_tool_use_block("mcp__read_file") response = _make_response(block) registry = _FakeRegistry({"read_file"}) @@ -110,10 +110,10 @@ class TestAnthropicMcpPrefixStrip: result = transport.normalize_response(response, strip_tool_prefix=False) assert len(result.tool_calls) == 1 - assert result.tool_calls[0].name == "mcp_read_file" + assert result.tool_calls[0].name == "mcp__read_file" def test_no_strip_when_not_mcp_prefixed(self): - """Non-mcp_ names are untouched regardless of strip flag.""" + """Non-mcp__ names are untouched regardless of strip flag.""" transport = self._get_transport() block = _make_tool_use_block("web_search") response = _make_response(block) @@ -132,7 +132,7 @@ class TestAnthropicMcpPrefixStrip: since it's what the LLM was told about. """ transport = self._get_transport() - block = _make_tool_use_block("mcp_unknown_tool") + block = _make_tool_use_block("mcp__unknown_tool") response = _make_response(block) registry = _FakeRegistry({"read_file"}) # neither name registered @@ -140,12 +140,12 @@ class TestAnthropicMcpPrefixStrip: result = transport.normalize_response(response, strip_tool_prefix=True) assert len(result.tool_calls) == 1 - assert result.tool_calls[0].name == "mcp_unknown_tool" + assert result.tool_calls[0].name == "mcp__unknown_tool" def test_mixed_tools_same_response(self): """Both OAuth and native MCP tools in the same response.""" transport = self._get_transport() - block1 = _make_tool_use_block("mcp_read_file", block_id="tc_1") + block1 = _make_tool_use_block("mcp__read_file", block_id="tc_1") block2 = _make_tool_use_block("mcp_composio_SEARCH", block_id="tc_2") block3 = _make_tool_use_block("mcp_composio_SEARCH", block_id="tc_3") # also registered natively response = _make_response(block1, block2, block3) @@ -160,33 +160,49 @@ class TestAnthropicMcpPrefixStrip: assert len(result.tool_calls) == 3 # OAuth tool: stripped assert result.tool_calls[0].name == "read_file" - # Native MCP: preserved (both stripped and full are registered, full wins) + # Native MCP: preserved (doesn't start with mcp__ so never enters strip path) assert result.tool_calls[1].name == "mcp_composio_SEARCH" assert result.tool_calls[2].name == "mcp_composio_SEARCH" def test_both_stripped_and_full_registered_prefers_full(self): - """Edge case: both 'foo' and 'mcp_foo' exist in registry. + """Edge case: both 'foo' and 'mcp__foo' exist in registry. - Keep 'mcp_foo' (the original name) since it's what the LLM requested. + Keep 'mcp__foo' (the original name) since it's what the LLM requested. """ transport = self._get_transport() - block = _make_tool_use_block("mcp_foo") + block = _make_tool_use_block("mcp__foo") response = _make_response(block) - registry = _FakeRegistry({"foo", "mcp_foo"}) + registry = _FakeRegistry({"foo", "mcp__foo"}) with patch("tools.registry.registry", registry): result = transport.normalize_response(response, strip_tool_prefix=True) assert len(result.tool_calls) == 1 # Both exist — the condition `get_entry(stripped) and not get_entry(name)` # is False because get_entry(name) IS truthy, so we keep the full name. - assert result.tool_calls[0].name == "mcp_foo" + assert result.tool_calls[0].name == "mcp__foo" + + def test_legacy_single_underscore_native_mcp_not_stripped(self): + """Legacy mcp_ (single underscore) native MCP tools are NOT stripped. + + They don't start with mcp__ so the strip path is never entered. + """ + transport = self._get_transport() + block = _make_tool_use_block("mcp_github_create_issue") + response = _make_response(block) + + registry = _FakeRegistry({"mcp_github_create_issue"}) + with patch("tools.registry.registry", registry): + result = transport.normalize_response(response, strip_tool_prefix=True) + + assert len(result.tool_calls) == 1 + assert result.tool_calls[0].name == "mcp_github_create_issue" class TestAnthropicOAuthOutgoingPrefix: """Verify the outgoing-side companion fix: build_anthropic_kwargs must not - double-prefix tool names that already start with ``mcp_`` (native MCP server - tools registered as ``mcp__``). GH-25255.""" + double-prefix tool names that already start with ``mcp_`` or ``mcp__`` + (native MCP server tools registered as ``mcp__``). GH-25255.""" def _build(self, tools, is_oauth=True): from agent.anthropic_adapter import build_anthropic_kwargs @@ -200,13 +216,13 @@ class TestAnthropicOAuthOutgoingPrefix: ) def test_oauth_adds_prefix_to_bare_tool_name(self): - """OAuth + bare name → prefix added (existing Claude Code convention).""" + """OAuth + bare name → mcp__ prefix added (Claude Code convention).""" kwargs = self._build([{ "type": "function", "function": {"name": "read_file", "description": "x", "parameters": {}}, }]) names = [t["name"] for t in kwargs["tools"]] - assert names == ["mcp_read_file"] + assert names == ["mcp__read_file"] def test_oauth_does_not_double_prefix_native_mcp_tool(self): """OAuth + already-prefixed native MCP name → left alone.""" @@ -219,12 +235,12 @@ class TestAnthropicOAuthOutgoingPrefix: }, }]) names = [t["name"] for t in kwargs["tools"]] - # Must NOT become "mcp_mcp_composio_..." — that breaks the round-trip - # because normalize_response only strips ONE mcp_ prefix. + # Must NOT become "mcp__mcp_composio_..." — that breaks the round-trip + # because normalize_response only strips ONE mcp__ prefix. assert names == ["mcp_composio_COMPOSIO_SEARCH_TOOLS"] def test_oauth_mixed_native_and_bare_tools(self): - """Mixed: native MCP preserved, bare names prefixed.""" + """Mixed: native MCP preserved, bare names prefixed with mcp__.""" kwargs = self._build([ {"type": "function", "function": {"name": "read_file", "description": "x", "parameters": {}}}, @@ -234,7 +250,7 @@ class TestAnthropicOAuthOutgoingPrefix: "description": "z", "parameters": {}}}, ]) names = sorted(t["name"] for t in kwargs["tools"]) - assert names == ["mcp_composio_SEARCH", "mcp_read_file", "mcp_terminal"] + assert names == ["mcp__read_file", "mcp__terminal", "mcp_composio_SEARCH"] def test_non_oauth_path_untouched(self): """Non-OAuth requests never get the prefix — schemas pass through as-is."""