mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-02 07:11:49 +00:00
fix(anthropic): skip mcp_ prefix on outgoing tool schemas when already prefixed
Companion to the GH-25255 incoming-strip fix from @hayka-pacha. Without this, build_anthropic_kwargs unconditionally added 'mcp_' to every tool name in step 3, so a native MCP server tool registered as 'mcp_composio_X' was sent as 'mcp_mcp_composio_X' on the wire. The incoming strip only removes ONE prefix, which still worked on first call, but on subsequent calls the model pattern-matched the single-prefixed form from message history and produced names that stripped to 'composio_X' — registry miss, dispatch fail. The history-rewrite block (#4) already has this guard. Apply the same guard to the schema-rewrite block (#3) so round-trip is symmetric. Added 4 outgoing-side tests. Existing 7 incoming-side tests still pass. Author map: hayka-pacha added for PR #25270 salvage attribution. Refs GH-25255.
This commit is contained in:
parent
2f91a8406c
commit
eea9553a9c
3 changed files with 71 additions and 1 deletions
|
|
@ -2122,9 +2122,13 @@ def build_anthropic_kwargs(
|
|||
block["text"] = text
|
||||
|
||||
# 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.
|
||||
if anthropic_tools:
|
||||
for tool in anthropic_tools:
|
||||
if "name" in tool:
|
||||
if "name" in tool and not tool["name"].startswith(_MCP_TOOL_PREFIX):
|
||||
tool["name"] = _MCP_TOOL_PREFIX + tool["name"]
|
||||
|
||||
# 4. Prefix tool names in message history (tool_use and tool_result blocks)
|
||||
|
|
|
|||
|
|
@ -196,6 +196,7 @@ AUTHOR_MAP = {
|
|||
"gonzes7@gmail.com": "aqilaziz", # PR #26406 salvage (preserve native audio outside Telegram)
|
||||
"karthikeyann@users.noreply.github.com": "karthikeyann", # PR #26609 salvage (DM-topic routing pin)
|
||||
"rino.alpin@gmail.com": "kunci115", # PR #27098 salvage (thread-not-found retry)
|
||||
"hayka-pacha@users.noreply.github.com": "hayka-pacha", # PR #25270 salvage (registry-aware mcp_ prefix strip)
|
||||
"237601532+chromalinx@users.noreply.github.com": "chromalinx", # PR #27014 salvage (commands for groups+DM)
|
||||
"booker1207@gmail.com": "booker1207", # PR #25132 salvage (gate profile bots by allowed topics)
|
||||
"kiranvk2011@gmail.com": "kiranvk-2011", # PR #24815 salvage (image documents → vision)
|
||||
|
|
|
|||
|
|
@ -183,3 +183,68 @@ class TestAnthropicMcpPrefixStrip:
|
|||
# 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"
|
||||
|
||||
|
||||
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."""
|
||||
|
||||
def _build(self, tools, is_oauth=True):
|
||||
from agent.anthropic_adapter import build_anthropic_kwargs
|
||||
return build_anthropic_kwargs(
|
||||
model="claude-sonnet-4-6",
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
tools=tools,
|
||||
max_tokens=4096,
|
||||
reasoning_config=None,
|
||||
is_oauth=is_oauth,
|
||||
)
|
||||
|
||||
def test_oauth_adds_prefix_to_bare_tool_name(self):
|
||||
"""OAuth + bare name → prefix added (existing 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"]
|
||||
|
||||
def test_oauth_does_not_double_prefix_native_mcp_tool(self):
|
||||
"""OAuth + already-prefixed native MCP name → left alone."""
|
||||
kwargs = self._build([{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "mcp_composio_COMPOSIO_SEARCH_TOOLS",
|
||||
"description": "x",
|
||||
"parameters": {},
|
||||
},
|
||||
}])
|
||||
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.
|
||||
assert names == ["mcp_composio_COMPOSIO_SEARCH_TOOLS"]
|
||||
|
||||
def test_oauth_mixed_native_and_bare_tools(self):
|
||||
"""Mixed: native MCP preserved, bare names prefixed."""
|
||||
kwargs = self._build([
|
||||
{"type": "function", "function": {"name": "read_file",
|
||||
"description": "x", "parameters": {}}},
|
||||
{"type": "function", "function": {"name": "mcp_composio_SEARCH",
|
||||
"description": "y", "parameters": {}}},
|
||||
{"type": "function", "function": {"name": "terminal",
|
||||
"description": "z", "parameters": {}}},
|
||||
])
|
||||
names = sorted(t["name"] for t in kwargs["tools"])
|
||||
assert names == ["mcp_composio_SEARCH", "mcp_read_file", "mcp_terminal"]
|
||||
|
||||
def test_non_oauth_path_untouched(self):
|
||||
"""Non-OAuth requests never get the prefix — schemas pass through as-is."""
|
||||
kwargs = self._build([
|
||||
{"type": "function", "function": {"name": "read_file",
|
||||
"description": "x", "parameters": {}}},
|
||||
{"type": "function", "function": {"name": "mcp_composio_SEARCH",
|
||||
"description": "y", "parameters": {}}},
|
||||
], is_oauth=False)
|
||||
names = sorted(t["name"] for t in kwargs["tools"])
|
||||
assert names == ["mcp_composio_SEARCH", "read_file"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue