mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-17 09:41:58 +00:00
fix(anthropic): use double-underscore mcp__ prefix for OAuth tool names
Anthropic's Claude-Code request classifier treats tool names with a single-underscore `mcp_<x>` prefix as non-Claude-Code / third-party, routing the request to extra-usage billing (HTTP 400). Real Claude Code uses double underscores: `mcp__<server>__<tool>`. Change the tool-name prefix from `mcp_` to `mcp__` in both the outgoing path (build_anthropic_kwargs) and the incoming path (normalize_response). Update the skip-guard to check for both `mcp_` and `mcp__` prefixes so native MCP server tools (which use the legacy single-underscore format) are not double-prefixed. Fixes #46675
This commit is contained in:
parent
9901141d64
commit
3d37869295
3 changed files with 49 additions and 30 deletions
|
|
@ -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_<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)
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 = []
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
"""
|
||||
|
|
@ -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_<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
|
||||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue