mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(proxy): handle client-side unique-ID suffixes in MCP semantic tool filter
MCP clients like LibreChat append a unique-ID suffix to tool names (e.g. `<canonical>_<uid>`) to avoid naming collisions across multiple connected MCP servers. The existing `_name_matches_canonical` method only handled the prefix case (`<alias><sep><canonical>`) introduced in #26117. The symmetric suffix case fell through, causing the filter to drop all tools and forward `tools: []` with `tool_choice: auto`, which strict upstream providers reject with a 400 error. Add a suffix-matching branch that recognises `<canonical><sep><uid>` patterns. The remainder after the canonical must be a single `<sep><unique_id>` segment with no further MCP_TOOL_PREFIX_SEPARATOR, preventing `svc-search-extra_tool` from falsely matching canonical `svc-search`. Fixes #26507
This commit is contained in:
parent
0beec45c13
commit
f13d6b4eda
2 changed files with 209 additions and 16 deletions
|
|
@ -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
|
||||
(``<client_alias><sep><canonical>``). 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): ``<client_alias><sep><canonical>``
|
||||
— the canonical forms the complete suffix of the client name.
|
||||
2. **Suffix** (e.g. LibreChat): ``<canonical><sep><unique_id>``
|
||||
— 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
|
||||
``<sep><tool_name>`` pair).
|
||||
|
||||
Both prefix and suffix matching are gated on ``canonical`` itself
|
||||
containing ``MCP_TOOL_PREFIX_SEPARATOR``. Server-registered MCP
|
||||
tools are always emitted as
|
||||
``<server_name><MCP_TOOL_PREFIX_SEPARATOR><tool_name>`` (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 = <alias><sep><canonical>
|
||||
# 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 = <canonical><sep><unique_id>
|
||||
# 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 <sep><unique_id> 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]
|
||||
|
|
|
|||
|
|
@ -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. ``<canonical>_<uid>``) 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 ``_<unique_id>``."""
|
||||
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_<canonical>`` (prefix) and ``<canonical>_<uid>``
|
||||
(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",
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue