diff --git a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py index 0bafd7da265..4412599dc0c 100644 --- a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py +++ b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py @@ -213,20 +213,69 @@ class SemanticMCPToolFilter: return [] + @staticmethod + def _name_matches_canonical(client_name: str, canonical: str) -> bool: + """ + Return True if a client-side tool name refers to the given canonical + MCP tool name. + + MCP clients (e.g. opencode) commonly wrap the proxy's canonical tool + name with an additive namespace prefix of their own + (````). The prefix can use either a + dash or an underscore as separator regardless of what + ``MCP_TOOL_PREFIX_SEPARATOR`` is set to on the proxy, because the + client doesn't know the proxy's separator. + + The match is anchored: ``canonical`` must form the complete suffix + of ``client_name`` and be preceded by a separator character, so + ``rain_gear`` does not match canonical ``ear``. + """ + if client_name == canonical: + return True + if len(client_name) <= len(canonical): + return False + if not client_name.endswith(canonical): + return False + separator = client_name[-len(canonical) - 1] + return separator in ("_", "-") + def _get_tools_by_names( self, tool_names: List[str], available_tools: List[Any] ) -> List[Any]: - """Get tools from available_tools by their names, preserving order.""" - # Match tools from available_tools (preserves format - dict or MCPTool) - matched_tools = [] - for tool in available_tools: - tool_name, _ = self._extract_tool_info(tool) - if tool_name in tool_names: - matched_tools.append(tool) + """ + Get tools from available_tools by their names, preserving the + semantic router's ordering. - # Reorder to match semantic router's ordering - tool_map = {self._extract_tool_info(t)[0]: t for t in matched_tools} - return [tool_map[name] for name in tool_names if name in tool_map] + Matching is tolerant of client-side namespace prefixes: if an + incoming tool arrived as ``_`` while the + router returned ```` (see + ``_name_matches_canonical``), that tool is still selected. The + returned tool object is the original from ``available_tools``, so + the client-facing name is preserved for tool-call round-trips. + """ + # Build an index of incoming tools by their client-facing name. + # Exact matches win over suffix matches when both are present, and + # each incoming tool is returned at most once even if two canonical + # names happen to be tail-compatible with the same incoming name. + available_by_name: Dict[str, Any] = {} + for tool in available_tools: + client_name, _ = self._extract_tool_info(tool) + if client_name and client_name not in available_by_name: + available_by_name[client_name] = tool + + matched: List[Any] = [] + used_ids: set = set() + for canonical in tool_names: + tool = available_by_name.get(canonical) + if tool is None: + for client_name, candidate in available_by_name.items(): + if self._name_matches_canonical(client_name, canonical): + tool = candidate + break + if tool is not None and id(tool) not in used_ids: + matched.append(tool) + used_ids.add(id(tool)) + return matched def extract_user_query(self, messages: List[Dict[str, Any]]) -> str: """ diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py index 87c597c659b..15917ff8a0f 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py @@ -392,3 +392,140 @@ async def test_semantic_filter_hook_skips_no_tools(): assert result is None, "Hook should skip requests without tools" print("✅ Hook correctly skips requests without tools") + +class TestGetToolsByNames: + """ + Regression coverage for SemanticMCPToolFilter._get_tools_by_names + name-matching behavior (issue #26078). + + The canonical name stored in the router is what the proxy's MCP + registry emits (e.g. ``fc_web_search-firecrawl_scrape``). Some MCP + clients — notably opencode — wrap every tool name with their own + additive namespace prefix before sending it back in ``tools[]``, so + the incoming name is ``litellm_fc_web_search-firecrawl_scrape``. + + Exact-equality matching against the canonical dropped every such + tool, the proxy forwarded ``tools: []`` with ``tool_choice: auto``, + and strict upstream providers returned 400. + """ + + def _make_filter(self): + from litellm.proxy._experimental.mcp_server.semantic_tool_filter import ( + SemanticMCPToolFilter, + ) + + return SemanticMCPToolFilter( + embedding_model="text-embedding-3-small", + litellm_router_instance=Mock(), + top_k=5, + similarity_threshold=0.3, + enabled=True, + ) + + def test_exact_match_unchanged(self): + """Incoming name equals canonical — the historical path still works.""" + filter_instance = self._make_filter() + available_tools = [ + {"name": "get_weather", "description": "fetch weather"}, + {"name": "send_email", "description": "send mail"}, + ] + + matched = filter_instance._get_tools_by_names( + ["send_email"], available_tools + ) + + assert len(matched) == 1 + assert matched[0]["name"] == "send_email" + + def test_client_prefix_with_underscore_separator(self): + """Client wraps canonical with ``_`` (opencode pattern).""" + filter_instance = self._make_filter() + canonical = "fc_web_search-firecrawl_scrape" + client_name = "litellm_" + canonical + available_tools = [{"name": client_name, "description": "scrape"}] + + matched = filter_instance._get_tools_by_names( + [canonical], available_tools + ) + + assert len(matched) == 1 + # Must return the incoming tool unchanged so the client-facing + # name survives, otherwise tool-call round-trips break client-side. + assert matched[0]["name"] == client_name + + def test_client_prefix_with_dash_separator(self): + """Some clients use dash as alias separator; accept that too.""" + filter_instance = self._make_filter() + canonical = "get_weather" + available_tools = [{"name": "mcp-get_weather", "description": "weather"}] + + matched = filter_instance._get_tools_by_names( + [canonical], available_tools + ) + + assert len(matched) == 1 + assert matched[0]["name"] == "mcp-get_weather" + + def test_suffix_without_separator_does_not_match(self): + """ + A bare-substring suffix must not match — ``rain_gear`` is not a + namespaced version of canonical ``ear`` and the user would be + surprised to see it selected. + """ + filter_instance = self._make_filter() + available_tools = [{"name": "rain_gear", "description": "raincoat"}] + + matched = filter_instance._get_tools_by_names(["ear"], available_tools) + + assert matched == [] + + def test_exact_match_preferred_over_prefixed(self): + """ + When both a bare canonical and a client-prefixed variant are + present, the bare one wins so ordering is stable. + """ + filter_instance = self._make_filter() + canonical = "search" + available_tools = [ + {"name": canonical, "description": "plain"}, + {"name": "litellm_" + canonical, "description": "wrapped"}, + ] + + matched = filter_instance._get_tools_by_names( + [canonical], available_tools + ) + + assert len(matched) == 1 + assert matched[0]["name"] == canonical + + def test_same_tool_not_returned_twice(self): + """ + Two canonicals that both suffix-match the same incoming tool + must not produce a duplicate in the output list. + """ + filter_instance = self._make_filter() + available_tools = [ + {"name": "srv_read_file", "description": "read"} + ] + + matched = filter_instance._get_tools_by_names( + ["read_file", "read_file"], available_tools + ) + + assert len(matched) == 1 + + def test_ordering_follows_router_output(self): + """Returned tools follow the order the semantic router chose.""" + filter_instance = self._make_filter() + available_tools = [ + {"name": "litellm_read", "description": "read"}, + {"name": "litellm_write", "description": "write"}, + {"name": "litellm_delete", "description": "delete"}, + ] + + matched = filter_instance._get_tools_by_names( + ["write", "delete", "read"], available_tools + ) + + names = [t["name"] for t in matched] + assert names == ["litellm_write", "litellm_delete", "litellm_read"]