diff --git a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py index a9c4d2ece46..b6f6c7a87a0 100644 --- a/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py +++ b/litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py @@ -221,25 +221,41 @@ class SemanticMCPToolFilter: 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. + MCP clients commonly wrap the proxy's canonical tool name in one of + two ways: - 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``. + 1. **Prefix** (e.g. opencode): ```` + — the canonical forms the complete suffix of the client name. + 2. **Suffix** (e.g. LibreChat): ```` + — the canonical forms the complete prefix of the client name, + followed by a separator and a client-generated unique identifier + used to avoid naming collisions across multiple MCP servers. - Suffix matching is additionally gated on ``canonical`` itself + In both cases the separator can be either a dash or an underscore + 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 on both sides: + + - **Prefix match**: ``canonical`` must form the complete suffix of + ``client_name`` and be preceded by a separator character, so + ``rain_gear`` does not match canonical ``ear``. + - **Suffix match**: ``canonical`` must form the complete prefix of + ``client_name`` and be followed by a separator character, so + ``fc_web_search-firecrawl_scrape`` does match + ``fc_web_search-firecrawl_scrape_a1b2c3d4`` but does not match + ``fc_web_search-firecrawl_scrape_extra_tool`` (the part after the + canonical must be a single unique-ID segment, not another + ```` pair). + + Both prefix and suffix matching are gated on ``canonical`` itself containing ``MCP_TOOL_PREFIX_SEPARATOR``. Server-registered MCP tools are always emitted as ```` (see ``add_server_prefix_to_name``), so a canonical without the separator is not a namespaced MCP tool and falling back to - suffix matching would spuriously collide with unrelated local - user functions whose names end in the same characters. + anchored matching would spuriously collide with unrelated local + user functions whose names start or end in the same characters. """ if client_name == canonical: return True @@ -247,10 +263,35 @@ class SemanticMCPToolFilter: return False 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 ("_", "-") + + # Prefix match: client_name = + # e.g. "litellm_fc_web_search-firecrawl_scrape" matches + # canonical "fc_web_search-firecrawl_scrape" + if client_name.endswith(canonical): + separator = client_name[-len(canonical) - 1] + if separator in ("_", "-"): + return True + + # Suffix match: client_name = + # e.g. "fc_web_search-firecrawl_scrape_a1b2c3d4" matches + # canonical "fc_web_search-firecrawl_scrape" + if client_name.startswith(canonical): + remainder = client_name[len(canonical):] + # The remainder must be a single segment. + # A unique-ID segment contains no separator (it's a short + # hex or alphanumeric string), so we check that the very + # next character is a separator and the rest contains no + # further MCP_TOOL_PREFIX_SEPARATOR. This prevents + # "svc-search-extra_tool" from matching canonical + # "svc-search" — the remainder after the separator would + # itself contain a separator, indicating it's another + # namespaced tool, not a unique-ID suffix. + if remainder and remainder[0] in ("_", "-"): + rest = remainder[1:] + if MCP_TOOL_PREFIX_SEPARATOR not in rest: + return True + + return False def _get_tools_by_names( self, tool_names: List[str], available_tools: List[Any] 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 2558df8533b..4b02988a4dc 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 @@ -640,3 +640,155 @@ class TestGetToolsByNames: ) assert matched == [] + + # --- Suffix-match tests (issue #26507) --- + # LibreChat and similar MCP clients append a unique-ID suffix to + # tool names (e.g. ``_``) to avoid naming collisions + # across multiple connected MCP servers. The filter must recognise + # these as referring to the canonical tool. + + def test_client_suffix_with_underscore_separator(self): + """LibreChat pattern: canonical followed by ``_``.""" + filter_instance = self._make_filter() + canonical = "fc_web_search-firecrawl_scrape" + client_name = canonical + "_a1b2c3d4" + 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 for tool-call round-trips. + assert matched[0]["name"] == client_name + + def test_client_suffix_with_dash_separator(self): + """Some clients use dash as the suffix separator; accept that too.""" + filter_instance = self._make_filter() + canonical = "weather_svc-get_weather" + client_name = canonical + "-a1b2c3d4" + available_tools = [{"name": client_name, "description": "weather"}] + + matched = filter_instance._get_tools_by_names( + [canonical], available_tools + ) + + assert len(matched) == 1 + assert matched[0]["name"] == client_name + + def test_suffix_does_not_match_another_namespaced_tool(self): + """ + ``svc-search-extra_tool`` must NOT match canonical ``svc-search`` + because the remainder after the separator (``extra_tool``) itself + contains ``MCP_TOOL_PREFIX_SEPARATOR`` (``-``), indicating it is + another namespaced tool, not a unique-ID suffix. + """ + filter_instance = self._make_filter() + available_tools = [ + {"name": "svc-search-extra_tool", "description": "different tool"}, + ] + + matched = filter_instance._get_tools_by_names( + ["svc-search"], available_tools + ) + + assert matched == [] + + def test_suffix_without_separator_in_canonical_does_not_match(self): + """ + If the canonical has no MCP_TOOL_PREFIX_SEPARATOR, suffix matching + must not kick in — same safety guard as the prefix case. + ``my_firecrawl_scrape`` must not match canonical ``firecrawl_scrape`` + because ``firecrawl_scrape`` contains no separator. + """ + filter_instance = self._make_filter() + available_tools = [ + {"name": "my_firecrawl_scrape", "description": "unrelated"}, + ] + + matched = filter_instance._get_tools_by_names( + ["firecrawl_scrape"], # no MCP_TOOL_PREFIX_SEPARATOR + available_tools, + ) + + assert matched == [] + + def test_exact_match_preferred_over_suffixed(self): + """ + When both a bare canonical and a suffixed variant are present, + the bare one wins so ordering is stable. + """ + filter_instance = self._make_filter() + canonical = "svc-search" + available_tools = [ + {"name": canonical, "description": "plain"}, + {"name": canonical + "_a1b2c3d4", "description": "suffixed"}, + ] + + matched = filter_instance._get_tools_by_names( + [canonical], available_tools + ) + + assert len(matched) == 1 + assert matched[0]["name"] == canonical + + def test_prefix_and_suffix_both_match_same_canonical(self): + """ + Both ``litellm_`` (prefix) and ``_`` + (suffix) should resolve to the canonical when present in the + available tools. + """ + filter_instance = self._make_filter() + canonical = "fc_web_search-firecrawl_scrape" + prefixed = "litellm_" + canonical + suffixed = canonical + "_a1b2c3d4" + available_tools = [ + {"name": prefixed, "description": "prefixed"}, + {"name": suffixed, "description": "suffixed"}, + ] + + matched = filter_instance._get_tools_by_names( + [canonical], available_tools + ) + + # Should match the shortest qualifying name (prefix case) + assert len(matched) == 1 + assert matched[0]["name"] == prefixed + + def test_name_matches_canonical_suffix_static(self): + """Direct static-method tests for the suffix branch.""" + from litellm.proxy._experimental.mcp_server.semantic_tool_filter import ( + SemanticMCPToolFilter, + ) + + # Suffix match with underscore + assert SemanticMCPToolFilter._name_matches_canonical( + "fc_web_search-firecrawl_scrape_a1b2c3d4", + "fc_web_search-firecrawl_scrape", + ) + # Suffix match with dash + assert SemanticMCPToolFilter._name_matches_canonical( + "fc_web_search-firecrawl_scrape-a1b2c3d4", + "fc_web_search-firecrawl_scrape", + ) + # No suffix match when remainder contains separator (another tool) + assert not SemanticMCPToolFilter._name_matches_canonical( + "svc-search-extra_tool", + "svc-search", + ) + # No suffix match when canonical has no separator + assert not SemanticMCPToolFilter._name_matches_canonical( + "my_firecrawl_scrape", + "firecrawl_scrape", + ) + # Exact match still works + assert SemanticMCPToolFilter._name_matches_canonical( + "fc_web_search-firecrawl_scrape", + "fc_web_search-firecrawl_scrape", + ) + # Prefix match still works + assert SemanticMCPToolFilter._name_matches_canonical( + "litellm_fc_web_search-firecrawl_scrape", + "fc_web_search-firecrawl_scrape", + )