Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions agent/anthropic_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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_<server>_<tool> 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)
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion agent/transports/anthropic.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse:
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 = []
Expand Down
66 changes: 41 additions & 25 deletions tests/agent/test_anthropic_mcp_prefix_strip.py
Original file line number Diff line number Diff line change
@@ -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_<server>_<tool>``
name in the tool registry.
"""
Expand Down Expand Up @@ -61,14 +61,14 @@ def _get_transport(self):
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"})
Expand Down Expand Up @@ -102,18 +102,18 @@ def test_preserves_native_mcp_server_tool_name(self):
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"})
with patch("tools.registry.registry", registry):
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)
Expand All @@ -132,20 +132,20 @@ def test_preserves_name_when_neither_in_registry(self):
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
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_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)
Expand All @@ -160,33 +160,49 @@ def test_mixed_tools_same_response(self):
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_<server>_<tool>``). GH-25255."""
double-prefix tool names that already start with ``mcp_`` or ``mcp__``
(native MCP server tools registered as ``mcp_<server>_<tool>``). GH-25255."""

def _build(self, tools, is_oauth=True):
from agent.anthropic_adapter import build_anthropic_kwargs
Expand All @@ -200,13 +216,13 @@ def _build(self, tools, is_oauth=True):
)

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."""
Expand All @@ -219,12 +235,12 @@ def test_oauth_does_not_double_prefix_native_mcp_tool(self):
},
}])
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": {}}},
Expand All @@ -234,7 +250,7 @@ def test_oauth_mixed_native_and_bare_tools(self):
"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."""
Expand Down
Loading