diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 1c89288330f7..b4349379bc79 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -3376,7 +3376,7 @@ def repair_tool_call(agent, tool_name: str) -> str | None: Claude-style models sometimes tack on (TodoTool_tool -> TodoTool -> Todo -> todo). Applied twice so double-tacked suffixes like ``TodoTool_tool`` reduce all the way. - 5. Fuzzy match (difflib, cutoff=0.7). + 5. Fuzzy match (difflib, cutoff=0.7) for non-MCP tools. See #14784 for the original reports (TodoTool_tool, Patch_tool, BrowserClick_tool were all returning "Unknown tool" before). @@ -3448,6 +3448,15 @@ def _strip_tool_suffix(s: str) -> str | None: if c and c in agent.valid_tool_names: return c + # MCP tool names encode both the server and the tool. A fuzzy fallback can + # silently change the requested operation inside the same MCP namespace + # (for example, a hallucinated ghidra ``disassemble_function`` becoming + # ``decompile_function``). That is a semantic substitution, not a safe + # spelling repair, so leave unmatched MCP calls invalid and let the normal + # unknown-tool correction path show the model the available tools. + if normalized.startswith("mcp__"): + return None + # Fuzzy match as last resort. matches = get_close_matches(lowered, agent.valid_tool_names, n=1, cutoff=0.7) if matches: diff --git a/tests/run_agent/test_repair_tool_call_name.py b/tests/run_agent/test_repair_tool_call_name.py index c50f5e0a62c1..d5276361e241 100644 --- a/tests/run_agent/test_repair_tool_call_name.py +++ b/tests/run_agent/test_repair_tool_call_name.py @@ -30,6 +30,12 @@ } +def _bind_repair(valid_tool_names): + from run_agent import AIAgent + stub = SimpleNamespace(valid_tool_names=set(valid_tool_names)) + return AIAgent._repair_tool_call.__get__(stub, AIAgent) + + @pytest.fixture def repair(): """Return a bound _repair_tool_call built on a minimal shell agent. @@ -39,9 +45,7 @@ def repair(): reads self.valid_tool_names. A SimpleNamespace stub is enough to bind the unbound function. """ - from run_agent import AIAgent - stub = SimpleNamespace(valid_tool_names=VALID) - return AIAgent._repair_tool_call.__get__(stub, AIAgent) + return _bind_repair(VALID) class TestExistingBehaviorStillWorks: @@ -125,3 +129,34 @@ def test_leading_quote_falls_through_to_fuzzy_match(self, repair): # rest of the pipeline (fuzzy match at 0.7 cutoff) can still # recover the obvious target. assert repair('"terminal"') == "terminal" + +class TestMcpToolNameRepair: + """MCP names are namespaced; fuzzy repair must not swap tool semantics.""" + + MCP_VALID = { + "mcp__ghidra_mcp__decompile_function", + "mcp__ghidra_mcp__rename_function", + "mcp__ghidra_mcp__set_function_prototype", + "mcp__ghidra_mcp__read_bytes", + } + + def test_mcp_tool_names_do_not_fuzzy_match_to_different_operation(self): + repair = _bind_repair(self.MCP_VALID) + + assert repair("mcp__ghidra_mcp__disassemble_function") is None + + def test_mcp_exact_normalization_still_works(self): + repair = _bind_repair(self.MCP_VALID) + + assert ( + repair("MCP__GHIDRA_MCP__DECOMPILE_FUNCTION") + == "mcp__ghidra_mcp__decompile_function" + ) + + def test_mcp_tool_suffix_strip_still_allows_exact_match(self): + repair = _bind_repair(self.MCP_VALID) + + assert ( + repair("mcp__ghidra_mcp__read_bytes_tool") + == "mcp__ghidra_mcp__read_bytes" + )