From 82ecff5208e7de2c1e20b5c300861e57bffd9d83 Mon Sep 17 00:00:00 2001 From: JonSherlin <142847529+JonSherlin@users.noreply.github.com> Date: Fri, 8 May 2026 01:48:55 -0400 Subject: [PATCH] fix(agent): repair mcp_ prefix drop in _repair_tool_call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Models sometimes call MCP server tools without the mcp_ prefix that Hermes adds at registration time — e.g. APE_search instead of mcp_APE_search, or discord_send instead of mcp_discord_send. difflib scores ~0.60 for this pattern (just below the 0.7 cutoff), so the existing fuzzy-match path misses it and the call fails with "Tool does not exist". Fix: add an explicit mcp_ prepend check before the candidate-set / fuzzy-match path. Three candidates are tried in order: mcp_ mcp_ mcp_ then a case-folded linear scan for mixed-case server names (e.g. APE_SEARCH -> mcp_APE_search). The check is unambiguous — only resolves on an exact match in valid_tool_names, so it cannot mis-route non-MCP tools. Adds TestMcpPrefixDrop (10 tests) to the existing repair test file, covering: all APE tool variants, other MCP servers, already-correct names, uppercase input, non-MCP tools unaffected, and the no-invent guard. All 28 tests pass; pre-existing test_concurrent_interrupt failures are unrelated and present on main before this change. Discovered while working with an APE MCP server where the model consistently emitted APE_meta / APE_search instead of the prefixed names. --- run_agent.py | 16 +++++ tests/run_agent/test_repair_tool_call_name.py | 65 +++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/run_agent.py b/run_agent.py index 403dba4e7850..e1ffb462ec85 100644 --- a/run_agent.py +++ b/run_agent.py @@ -5629,6 +5629,22 @@ def _strip_tool_suffix(s: str) -> str | None: if normalized in self.valid_tool_names: return normalized + # MCP prefix-strip: models sometimes drop the ``mcp_`` prefix that + # Hermes prepends when registering MCP server tools, e.g. calling + # ``APE_search`` instead of ``mcp_APE_search``. A direct prepend + # check is unambiguous — only resolves when there's an exact match. + # (difflib scores ~0.60 for this pattern, just under the 0.7 cutoff.) + for prefix_candidate in (f"mcp_{tool_name}", f"mcp_{lowered}", f"mcp_{normalized}"): + if prefix_candidate in self.valid_tool_names: + return prefix_candidate + # Also try mcp_ + each valid name stripped of its prefix, case-folded. + # Handles APE_SEARCH -> mcp_APE_search when the registered name has + # mixed case after the server component. + lowered_with_prefix = f"mcp_{lowered}" + for vname in self.valid_tool_names: + if vname.lower() == lowered_with_prefix: + return vname + # Build the full candidate set for class-like emissions. cands: set[str] = {tool_name, lowered, normalized, _camel_snake(tool_name)} # Strip trailing tool-suffix up to twice — TodoTool_tool needs it. diff --git a/tests/run_agent/test_repair_tool_call_name.py b/tests/run_agent/test_repair_tool_call_name.py index 15dfcccad241..0a5e8df36202 100644 --- a/tests/run_agent/test_repair_tool_call_name.py +++ b/tests/run_agent/test_repair_tool_call_name.py @@ -115,3 +115,68 @@ def test_none_passed_as_name(self, repair): def test_very_long_name_does_not_match_by_accident(self, repair): # Fuzzy match should not claim a tool for something obviously unrelated. assert repair("ThisIsNotRemotelyARealToolName_tool") is None + + +MCP_VALID = { + "mcp_APE_meta", + "mcp_APE_search", + "mcp_APE_add", + "mcp_APE_contextualize", + "mcp_APE_graph_nearby", + "mcp_APE_graph_connect", + "mcp_discord_send", + "terminal", + "read_file", +} + + +@pytest.fixture +def mcp_repair(): + """Repair fixture with MCP-prefixed tool names registered.""" + from run_agent import AIAgent + stub = SimpleNamespace(valid_tool_names=MCP_VALID) + return AIAgent._repair_tool_call.__get__(stub, AIAgent) + + +class TestMcpPrefixDrop: + """Regression guard: models drop the ``mcp_`` prefix Hermes adds when + registering MCP server tools (e.g. ``APE_search`` instead of + ``mcp_APE_search``). difflib scores ~0.60 for this pattern — just + below the 0.7 cutoff — so the fuzzy path misses it. The explicit + prefix-prepend check introduced alongside these tests catches it.""" + + def test_ape_meta_without_prefix(self, mcp_repair): + assert mcp_repair("APE_meta") == "mcp_APE_meta" + + def test_ape_search_without_prefix(self, mcp_repair): + assert mcp_repair("APE_search") == "mcp_APE_search" + + def test_ape_add_without_prefix(self, mcp_repair): + assert mcp_repair("APE_add") == "mcp_APE_add" + + def test_ape_contextualize_without_prefix(self, mcp_repair): + assert mcp_repair("APE_contextualize") == "mcp_APE_contextualize" + + def test_ape_graph_nearby_without_prefix(self, mcp_repair): + assert mcp_repair("APE_graph_nearby") == "mcp_APE_graph_nearby" + + def test_other_mcp_server_without_prefix(self, mcp_repair): + # Not APE-specific — any mcp__ pattern should resolve. + assert mcp_repair("discord_send") == "mcp_discord_send" + + def test_already_correct_name_unchanged(self, mcp_repair): + # Already has prefix — fast-path returns it directly, no double-prefix. + assert mcp_repair("mcp_APE_meta") == "mcp_APE_meta" + + def test_uppercase_without_prefix(self, mcp_repair): + # Case-insensitive: APE_SEARCH -> mcp_APE_search via lowered candidate. + assert mcp_repair("APE_SEARCH") == "mcp_APE_search" + + def test_non_mcp_tool_unaffected(self, mcp_repair): + # Non-MCP tools in the valid set must still resolve normally. + assert mcp_repair("TERMINAL") == "terminal" + + def test_prefix_drop_does_not_invent_tools(self, mcp_repair): + # Prepending mcp_ to a non-existent name must not return a wrong match. + assert mcp_repair("APE_nonexistent") is None +