mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-10 22:41:41 +00:00
fix(mcp): is_tool_name_prefixed validates against known server prefixes (#25085)
Fixes #25081. is_tool_name_prefixed() checked for the presence of MCP_TOOL_PREFIX_SEPARATOR (default '-') anywhere in the tool name. Any non-MCP tool whose name contains a hyphen (e.g. 'text-to-speech', 'code-review') was silently misclassified as an MCP-prefixed tool. When the semantic tool filter is enabled, these tools would be routed through semantic matching and potentially dropped. Fix: accept an optional known_server_prefixes set. When supplied, the function extracts the candidate prefix (text before the first separator) and checks it against the normalised set of registered server prefixes. Only a genuine match returns True. Without the set, legacy behaviour is preserved for backward compatibility. Updated _get_mcp_server_from_tool_name() to build the prefix set from the live registry and pass it through. 9 new tests. Co-authored-by: d 🔹 <258577966+voidborne-d@users.noreply.github.com>
This commit is contained in:
parent
9088b46b90
commit
cf94f4d8b7
3 changed files with 123 additions and 6 deletions
|
|
@ -2552,7 +2552,12 @@ class MCPServerManager:
|
|||
return server
|
||||
|
||||
# If not found and tool name is prefixed, try extracting server name from prefix
|
||||
if is_tool_name_prefixed(tool_name):
|
||||
known_prefixes = {
|
||||
normalize_server_name(get_server_prefix(s))
|
||||
for s in self.get_registry().values()
|
||||
if get_server_prefix(s)
|
||||
}
|
||||
if is_tool_name_prefixed(tool_name, known_server_prefixes=known_prefixes):
|
||||
(
|
||||
original_tool_name,
|
||||
server_name_from_prefix,
|
||||
|
|
|
|||
|
|
@ -100,17 +100,39 @@ def split_server_prefix_from_name(prefixed_name: str) -> Tuple[str, str]:
|
|||
return prefixed_name, ""
|
||||
|
||||
|
||||
def is_tool_name_prefixed(tool_name: str) -> bool:
|
||||
def is_tool_name_prefixed(
|
||||
tool_name: str,
|
||||
known_server_prefixes: Optional[set] = None,
|
||||
) -> bool:
|
||||
"""
|
||||
Check if tool name has server prefix
|
||||
Check if tool name has a known MCP server prefix.
|
||||
|
||||
When ``known_server_prefixes`` is provided the function verifies that the
|
||||
substring before the first separator is an actual registered server
|
||||
prefix. Without it the check falls back to the legacy heuristic
|
||||
(separator present anywhere in the name), which can produce false
|
||||
positives for non-MCP tools whose names contain hyphens
|
||||
(e.g. ``text-to-speech``, ``code-review``).
|
||||
|
||||
Args:
|
||||
tool_name: Tool name to check
|
||||
tool_name: Tool name to check.
|
||||
known_server_prefixes: Optional set of normalised server prefixes
|
||||
currently registered in the MCP manager. Pass this whenever
|
||||
the caller has access to the server registry so that the check
|
||||
is accurate.
|
||||
|
||||
Returns:
|
||||
True if tool name is prefixed, False otherwise
|
||||
True if tool name is prefixed, False otherwise.
|
||||
"""
|
||||
return MCP_TOOL_PREFIX_SEPARATOR in tool_name
|
||||
if MCP_TOOL_PREFIX_SEPARATOR not in tool_name:
|
||||
return False
|
||||
|
||||
if known_server_prefixes is not None:
|
||||
candidate_prefix = tool_name.split(MCP_TOOL_PREFIX_SEPARATOR, 1)[0]
|
||||
return normalize_server_name(candidate_prefix) in known_server_prefixes
|
||||
|
||||
# Legacy fallback – separator present somewhere in the name.
|
||||
return True
|
||||
|
||||
|
||||
def validate_mcp_server_name(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,90 @@
|
|||
"""
|
||||
Tests for is_tool_name_prefixed with known_server_prefixes parameter.
|
||||
|
||||
Verifies fix for https://github.com/BerriAI/litellm/issues/25081
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.utils import is_tool_name_prefixed
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Legacy behaviour (no known_server_prefixes passed)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestLegacyBehaviour:
|
||||
"""Without known_server_prefixes the function falls back to heuristic."""
|
||||
|
||||
def test_plain_name_returns_false(self):
|
||||
assert is_tool_name_prefixed("get_weather") is False
|
||||
|
||||
def test_hyphenated_name_returns_true_legacy(self):
|
||||
"""Legacy heuristic: any hyphen → True (the bug this issue reports)."""
|
||||
assert is_tool_name_prefixed("text-to-speech") is True
|
||||
|
||||
def test_prefixed_name_returns_true_legacy(self):
|
||||
assert is_tool_name_prefixed("myserver-get_weather") is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# New behaviour (known_server_prefixes supplied)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestWithKnownPrefixes:
|
||||
"""When known_server_prefixes is supplied, only real prefixes match."""
|
||||
|
||||
PREFIXES = {"myserver", "weather_api", "code_tools"}
|
||||
|
||||
def test_known_prefix_returns_true(self):
|
||||
assert (
|
||||
is_tool_name_prefixed(
|
||||
"myserver-get_weather", known_server_prefixes=self.PREFIXES
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
def test_hyphenated_non_mcp_tool_returns_false(self):
|
||||
"""This is the core fix: 'text-to-speech' is NOT an MCP-prefixed tool."""
|
||||
assert (
|
||||
is_tool_name_prefixed(
|
||||
"text-to-speech", known_server_prefixes=self.PREFIXES
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
def test_code_review_not_misclassified(self):
|
||||
assert (
|
||||
is_tool_name_prefixed(
|
||||
"code-review", known_server_prefixes=self.PREFIXES
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
def test_no_separator_returns_false(self):
|
||||
assert (
|
||||
is_tool_name_prefixed(
|
||||
"simple_tool", known_server_prefixes=self.PREFIXES
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
def test_empty_prefixes_set_rejects_all(self):
|
||||
"""With an empty registry, nothing can be prefixed."""
|
||||
assert (
|
||||
is_tool_name_prefixed("myserver-get_weather", known_server_prefixes=set())
|
||||
is False
|
||||
)
|
||||
|
||||
def test_prefix_normalisation(self):
|
||||
"""Server names with spaces are normalised to underscores."""
|
||||
prefixes = {"my_server"}
|
||||
# add_server_prefix_to_name normalises spaces → underscores
|
||||
assert (
|
||||
is_tool_name_prefixed(
|
||||
"my_server-list_files", known_server_prefixes=prefixes
|
||||
)
|
||||
is True
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue