feat(mcp): rehash short tool prefix on collision and cache per server
Two MCP servers can natural-hash to the same three-character base62 prefix. With 62**3 = 238_328 slots the birthday bound is ~488 servers for 50% collision probability, so a single proxy hosting more than ~100 MCP servers has a non-trivial chance of seeing a collision in practice — and a collision means tool names from two different servers share a routing key, causing silent mis-routing. Mitigation: - compute_short_server_prefix(server_id, attempt=N) folds an attempt counter into the SHA-256 seed, so rehashes are deterministic and produce a fresh three-char prefix space per attempt. - New MCPServer.short_prefix field caches the resolved (post-dedup) prefix on the model so it stays stable across the process lifetime. - MCPServerManager._assign_unique_short_prefix walks attempts 0..N until it finds a prefix not already used by another server in the combined registry. Logs an INFO line when a rehash happens so operators have a breadcrumb if it ever does. - Wired into every registration path: load_servers_from_config, add_server, update_server, reload_servers_from_database. The database reload path also carries the previously-resolved prefix forward so reloads don't churn it. - get_server_prefix prefers the cached short_prefix when set, so the resolved value (not the raw natural hash) is used everywhere. - iter_known_server_prefixes yields the cached short_prefix too, so reverse-lookup tolerance covers the rehashed form. No-op when LITELLM_USE_SHORT_MCP_TOOL_PREFIX is disabled — the field stays None and behaviour is unchanged. Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
This commit is contained in:
parent
4e827446d2
commit
df3dbd18d6
@ -50,7 +50,9 @@ from litellm.proxy._experimental.mcp_server.oauth2_token_cache import resolve_mc
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
MCP_TOOL_PREFIX_SEPARATOR,
|
||||
add_server_prefix_to_name,
|
||||
compute_short_server_prefix,
|
||||
get_server_prefix,
|
||||
is_short_mcp_tool_prefix_enabled,
|
||||
is_tool_name_prefixed,
|
||||
iter_known_server_prefixes,
|
||||
merge_mcp_headers,
|
||||
@ -365,6 +367,7 @@ class MCPServerManager:
|
||||
aws_session_name=server_config.get("aws_session_name", None),
|
||||
instructions=server_config.get("instructions", None),
|
||||
)
|
||||
self._assign_unique_short_prefix(new_server)
|
||||
self.config_mcp_servers[server_id] = new_server
|
||||
|
||||
# Check if this is an OpenAPI-based server
|
||||
@ -727,6 +730,7 @@ class MCPServerManager:
|
||||
try:
|
||||
if mcp_server.server_id not in self.registry:
|
||||
new_server = await self.build_mcp_server_from_table(mcp_server)
|
||||
self._assign_unique_short_prefix(new_server)
|
||||
self.registry[mcp_server.server_id] = new_server
|
||||
await self._maybe_register_openapi_tools(new_server)
|
||||
verbose_logger.debug(f"Added MCP Server: {new_server.name}")
|
||||
@ -739,6 +743,12 @@ class MCPServerManager:
|
||||
try:
|
||||
if mcp_server.server_id in self.registry:
|
||||
new_server = await self.build_mcp_server_from_table(mcp_server)
|
||||
# Carry the previously-resolved short prefix across so the
|
||||
# tool names stay stable for clients holding cached lists.
|
||||
existing_prefix = self.registry[mcp_server.server_id].short_prefix
|
||||
if existing_prefix and not new_server.short_prefix:
|
||||
new_server.short_prefix = existing_prefix
|
||||
self._assign_unique_short_prefix(new_server)
|
||||
self.registry[mcp_server.server_id] = new_server
|
||||
await self._maybe_register_openapi_tools(new_server)
|
||||
verbose_logger.debug(f"Updated MCP Server: {new_server.name}")
|
||||
@ -1815,6 +1825,63 @@ class MCPServerManager:
|
||||
verbose_logger.warning(f"Error listing tools from {server_name}: {str(e)}")
|
||||
return []
|
||||
|
||||
_SHORT_PREFIX_MAX_REHASH_ATTEMPTS = 1024
|
||||
|
||||
def _assign_unique_short_prefix(self, server: MCPServer) -> None:
|
||||
"""Resolve and cache a collision-free short tool prefix on ``server``.
|
||||
|
||||
Called at registration time for every MCP server entering the
|
||||
registry. Mutates ``server.short_prefix`` in place. No-ops when
|
||||
``LITELLM_USE_SHORT_MCP_TOOL_PREFIX`` is disabled, when the server
|
||||
has no ``server_id`` (synthetic temp-server objects), or when a
|
||||
prefix is already cached.
|
||||
|
||||
Collision strategy: take the natural hash; if it's already used by
|
||||
a *different* server in the combined registry, rehash with an
|
||||
incrementing attempt counter until we find an unused slot. The
|
||||
attempt counter is folded into the hash so the resulting prefix is
|
||||
still deterministic for a given (server_id, set-of-other-server-ids)
|
||||
pair within one process.
|
||||
"""
|
||||
if not is_short_mcp_tool_prefix_enabled():
|
||||
return
|
||||
if server.short_prefix:
|
||||
return
|
||||
if not server.server_id:
|
||||
return
|
||||
|
||||
used: Dict[str, str] = {}
|
||||
for other in self.get_registry().values():
|
||||
if other.server_id == server.server_id:
|
||||
continue
|
||||
if other.short_prefix:
|
||||
used[other.short_prefix] = other.server_id
|
||||
|
||||
for attempt in range(self._SHORT_PREFIX_MAX_REHASH_ATTEMPTS):
|
||||
candidate = compute_short_server_prefix(server.server_id, attempt=attempt)
|
||||
if candidate not in used:
|
||||
server.short_prefix = candidate
|
||||
if attempt > 0:
|
||||
verbose_logger.info(
|
||||
"MCP short-prefix collision resolved for server %s: "
|
||||
"natural hash collided with %s, using rehashed prefix "
|
||||
"%s (attempt=%d).",
|
||||
server.server_id,
|
||||
used.get(
|
||||
compute_short_server_prefix(server.server_id, attempt=0),
|
||||
"<unknown>",
|
||||
),
|
||||
candidate,
|
||||
attempt,
|
||||
)
|
||||
return
|
||||
|
||||
raise RuntimeError(
|
||||
f"Unable to assign a unique short MCP tool prefix for server "
|
||||
f"{server.server_id} after {self._SHORT_PREFIX_MAX_REHASH_ATTEMPTS} "
|
||||
"attempts; the 3-character prefix space is too crowded."
|
||||
)
|
||||
|
||||
def _create_prefixed_tools(
|
||||
self, tools: List[MCPTool], server: MCPServer, add_prefix: bool = True
|
||||
) -> List[MCPTool]:
|
||||
@ -2681,6 +2748,9 @@ class MCPServerManager:
|
||||
previous_registry = self.registry
|
||||
new_registry: Dict[str, MCPServer] = {}
|
||||
|
||||
# Stage one: build every server. Stage two assigns short prefixes
|
||||
# against the *full* set so dedup is deterministic regardless of
|
||||
# iteration order.
|
||||
for server in db_mcp_servers:
|
||||
existing_server = previous_registry.get(server.server_id)
|
||||
|
||||
@ -2704,10 +2774,18 @@ class MCPServerManager:
|
||||
f"Building server from DB: {server.server_id} ({server.server_name})"
|
||||
)
|
||||
new_server = await self.build_mcp_server_from_table(server)
|
||||
# Carry the cached short_prefix from the previous registry entry
|
||||
# (if any) so the prefix is stable across reloads.
|
||||
if existing_server is not None and existing_server.short_prefix:
|
||||
new_server.short_prefix = existing_server.short_prefix
|
||||
new_registry[server.server_id] = new_server
|
||||
await self._maybe_register_openapi_tools(new_server)
|
||||
|
||||
# Swap in the new registry first so _assign_unique_short_prefix
|
||||
# sees the complete set when checking for collisions.
|
||||
self.registry = new_registry
|
||||
for new_server in new_registry.values():
|
||||
self._assign_unique_short_prefix(new_server)
|
||||
|
||||
verbose_logger.debug(
|
||||
"MCP registry refreshed (%s servers in registry)", len(new_registry)
|
||||
|
||||
@ -54,17 +54,22 @@ def is_short_mcp_tool_prefix_enabled() -> bool:
|
||||
return raw.strip().lower() in ("1", "true", "yes", "on")
|
||||
|
||||
|
||||
def compute_short_server_prefix(server_id: str) -> str:
|
||||
def compute_short_server_prefix(server_id: str, attempt: int = 0) -> str:
|
||||
"""Derive the deterministic three-character base62 prefix for a server.
|
||||
|
||||
Uses SHA-256 of the server_id and folds the first eight bytes into a
|
||||
base62 string. An empty server_id raises ValueError — short prefixes
|
||||
require a stable identifier to be deterministic.
|
||||
Uses SHA-256 of ``f"{server_id}#{attempt}"`` and folds the first eight
|
||||
bytes into a base62 string. Pass ``attempt > 0`` to rehash to a
|
||||
different prefix when the natural hash collides with a prefix already
|
||||
assigned to another server (see
|
||||
``MCPServerManager._assign_unique_short_prefix``). An empty server_id
|
||||
raises ValueError — short prefixes require a stable identifier to be
|
||||
deterministic.
|
||||
"""
|
||||
if not server_id:
|
||||
raise ValueError("compute_short_server_prefix requires a non-empty server_id")
|
||||
|
||||
digest = hashlib.sha256(server_id.encode("utf-8")).digest()
|
||||
seed = server_id if attempt == 0 else f"{server_id}#{attempt}"
|
||||
digest = hashlib.sha256(seed.encode("utf-8")).digest()
|
||||
value = int.from_bytes(digest[:8], "big")
|
||||
chars = []
|
||||
for _ in range(SHORT_MCP_TOOL_PREFIX_LENGTH):
|
||||
@ -143,11 +148,18 @@ def get_server_prefix(server: Any) -> str:
|
||||
"""Return the prefix for a server.
|
||||
|
||||
When the short-prefix mode is enabled (``LITELLM_USE_SHORT_MCP_TOOL_PREFIX``)
|
||||
a deterministic three-character base62 ID derived from ``server_id`` is
|
||||
returned. Otherwise we fall back to the historical behaviour: alias if
|
||||
present, else server_name, else server_id.
|
||||
a three-character base62 ID is returned. We prefer the cached
|
||||
``server.short_prefix`` value when set — that field is populated at
|
||||
registration time by ``MCPServerManager._assign_unique_short_prefix``
|
||||
and resolves natural-hash collisions deterministically — and only fall
|
||||
back to the natural hash for ad-hoc / temp-server objects without a
|
||||
cached value. In default mode the historical behaviour is preserved:
|
||||
alias if present, else server_name, else server_id.
|
||||
"""
|
||||
if is_short_mcp_tool_prefix_enabled():
|
||||
cached = getattr(server, "short_prefix", None)
|
||||
if cached:
|
||||
return cached
|
||||
server_id = getattr(server, "server_id", None)
|
||||
if server_id:
|
||||
return compute_short_server_prefix(server_id)
|
||||
@ -177,6 +189,7 @@ def iter_known_server_prefixes(server: Any) -> Iterator[str]:
|
||||
yield value
|
||||
|
||||
yield from _emit(get_server_prefix(server))
|
||||
yield from _emit(getattr(server, "short_prefix", None))
|
||||
|
||||
server_id = getattr(server, "server_id", None)
|
||||
if server_id:
|
||||
|
||||
@ -81,6 +81,12 @@ class MCPServer(BaseModel):
|
||||
# Defaults to the token's expires_in minus the expiry buffer, or
|
||||
# MCP_PER_USER_TOKEN_DEFAULT_TTL when expires_in is absent.
|
||||
token_storage_ttl_seconds: Optional[int] = None
|
||||
# Resolved short-ID tool prefix when LITELLM_USE_SHORT_MCP_TOOL_PREFIX is
|
||||
# enabled. Set by ``MCPServerManager._assign_unique_short_prefix`` at
|
||||
# registration time so that natural-hash collisions between two
|
||||
# different ``server_id`` values are bumped deterministically. Left
|
||||
# ``None`` in default-prefix mode.
|
||||
short_prefix: Optional[str] = None
|
||||
model_config = ConfigDict(arbitrary_types_allowed=True)
|
||||
|
||||
@property
|
||||
|
||||
@ -205,3 +205,84 @@ class TestManagerShortPrefix:
|
||||
prefix = get_server_prefix(server)
|
||||
full = add_server_prefix_to_name("get_repo", prefix)
|
||||
assert len(full) < 60
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Collision-resolution at registration time
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestShortPrefixCollisionResolution:
|
||||
"""``_assign_unique_short_prefix`` must rehash on collision.
|
||||
|
||||
The dedup path is exercised by forcing two distinct ``server_id``
|
||||
values to both hash to the same natural prefix via a monkeypatched
|
||||
``compute_short_server_prefix``.
|
||||
"""
|
||||
|
||||
def test_no_op_when_flag_off(self):
|
||||
manager = MCPServerManager()
|
||||
server = _make_server(server_id="abc")
|
||||
manager._assign_unique_short_prefix(server)
|
||||
assert server.short_prefix is None
|
||||
|
||||
def test_assigns_natural_hash_when_no_collision(self, monkeypatch):
|
||||
from litellm.proxy._experimental.mcp_server import utils as mcp_utils
|
||||
|
||||
monkeypatch.setenv("LITELLM_USE_SHORT_MCP_TOOL_PREFIX", "true")
|
||||
manager = MCPServerManager()
|
||||
server = _make_server(server_id="abc")
|
||||
manager._assign_unique_short_prefix(server)
|
||||
|
||||
assert server.short_prefix == mcp_utils.compute_short_server_prefix("abc")
|
||||
|
||||
def test_rehashes_when_natural_hash_collides(self, monkeypatch):
|
||||
"""Two server_ids that natural-hash to the same prefix get
|
||||
deterministic, distinct short prefixes."""
|
||||
monkeypatch.setenv("LITELLM_USE_SHORT_MCP_TOOL_PREFIX", "true")
|
||||
|
||||
# Force every attempt=0 hash to "AAA" and attempt=1 to "AAB".
|
||||
# That way the second server registered must rehash to "AAB".
|
||||
from litellm.proxy._experimental.mcp_server import utils as mcp_utils
|
||||
|
||||
def _fake_hash(server_id: str, attempt: int = 0) -> str:
|
||||
return "AAA" if attempt == 0 else f"AA{chr(ord('A') + attempt)}"
|
||||
|
||||
monkeypatch.setattr(mcp_utils, "compute_short_server_prefix", _fake_hash)
|
||||
# Also patch the symbol that the manager imported at module load.
|
||||
from litellm.proxy._experimental.mcp_server import (
|
||||
mcp_server_manager as mgr_module,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(mgr_module, "compute_short_server_prefix", _fake_hash)
|
||||
|
||||
manager = MCPServerManager()
|
||||
first = _make_server(server_id="server-1", alias="srv1")
|
||||
second = _make_server(server_id="server-2", alias="srv2")
|
||||
|
||||
# Pretend both are already in the registry so dedup sees both.
|
||||
manager.registry[first.server_id] = first
|
||||
manager._assign_unique_short_prefix(first)
|
||||
manager.registry[second.server_id] = second
|
||||
manager._assign_unique_short_prefix(second)
|
||||
|
||||
assert first.short_prefix == "AAA"
|
||||
assert second.short_prefix == "AAB"
|
||||
assert first.short_prefix != second.short_prefix
|
||||
|
||||
def test_cached_prefix_is_reused(self, monkeypatch):
|
||||
monkeypatch.setenv("LITELLM_USE_SHORT_MCP_TOOL_PREFIX", "true")
|
||||
manager = MCPServerManager()
|
||||
server = _make_server(server_id="abc")
|
||||
server.short_prefix = "ZZZ" # pretend a previous registration set this
|
||||
|
||||
manager._assign_unique_short_prefix(server)
|
||||
|
||||
assert server.short_prefix == "ZZZ"
|
||||
|
||||
def test_get_server_prefix_prefers_cached(self, monkeypatch):
|
||||
monkeypatch.setenv("LITELLM_USE_SHORT_MCP_TOOL_PREFIX", "true")
|
||||
server = _make_server(server_id="abc")
|
||||
server.short_prefix = "Q9q"
|
||||
|
||||
assert get_server_prefix(server) == "Q9q"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user