fix(mcp): make the operationId to tool-name map a single owner

Greptile found that the OpenAPI fallback added a commit ago collapsed operation
IDs that registration keeps apart: foo/bar and foo.bar register as two tools but
sanitize_openapi_tool_name rewrites both to foo_bar, so a policy naming either
also decided the other.

The cause was two owners for one map, and picking the wrong one. Registration
names an operationId inline at _register_openapi_tools with
operation_id.replace(" ", "_").lower(), which keeps / and . ; the separate
sanitize_openapi_tool_name replaces every character outside [a-zA-Z0-9_-] and
belongs to register_tools_from_openapi, which has no production caller. Nothing
made the matcher use the one that actually registers, so it used the lookalike.

That inline expression is now openapi_tool_name in utils, and both registration
and the matcher call it. Replaying the registering function is the whole safety
argument, and it is structural rather than a claim: two operationIds that
register as two tools normalize to two names here by construction, because this
is the map that registered them. A coarser lookalike cannot be substituted
without a test failing.

The matcher also loses its exact-then-fallback split. The transform is identity
on native servers and idempotent on already-registered names, so normalizing
both sides is exact matching where no OpenAPI spec is involved. Executable lines
drop by three this round; the branch is +5 over the merge-base for four shared
owners that removed duplication at six call sites.

litellm/proxy/_experimental/mcp_server/mcp_server_manager.py

@@ -118,10 +118,11 @@ from litellm.proxy._experimental.mcp_server.utils import (
     is_short_mcp_tool_prefix_enabled,
     iter_known_server_prefixes,
     iter_known_tool_name_spellings,
-    match_known_tool_name,
     match_known_server_prefix,
+    match_known_tool_name,
     merge_mcp_headers,
     normalize_server_name,
+    openapi_tool_name,
     parse_admin_env_vars,
     strip_known_server_prefix,
     validate_mcp_server_name,
@@ -1785,7 +1786,7 @@ class MCPServerManager:
 
                     # Generate tool name (without prefix initially)
                     operation_id = operation.get("operationId", f"{method}_{path.replace('/', '_')}")
-                    base_tool_name = operation_id.replace(" ", "_").lower()
+                    base_tool_name = openapi_tool_name(operation_id)
 
                     # Add server prefix to tool name
                     prefixed_tool_name = add_server_prefix_to_name(base_tool_name, server_prefix)

litellm/proxy/_experimental/mcp_server/utils.py

@@ -2,7 +2,10 @@
 MCP Server Utilities
 """
 
+import hashlib
+import importlib
 import json
+import os
 import re
 from collections.abc import MutableMapping, MutableSequence
 from typing import (
@@ -17,10 +20,6 @@ from typing import (
     Tuple,
     Union,
 )
-
-import hashlib
-import importlib
-import os
 from urllib.parse import quote
 
 from litellm.types.mcp_server.mcp_server_manager import MCPServer
@@ -339,34 +338,36 @@ def iter_known_tool_name_spellings(tool_name: str, server: MCPServer) -> Iterato
         yield add_server_prefix_to_name(tool_name, prefix)
 
 
+def openapi_tool_name(operation_id: str) -> str:
+    """Return the tool name ``_register_openapi_tools`` registers ``operation_id`` under.
+
+    The single transform between a spec's operationId and the name the gateway serves.
+    Policy recovers the link by replaying this exact function, which is what keeps it from
+    deciding for a tool it does not name: two operationIds that register as two tools
+    necessarily normalize to two names here, because this is the map that registered them.
+    """
+    return operation_id.replace(" ", "_").lower()
+
+
 def match_known_tool_name(tool_name: str, server: MCPServer, names: Iterable[str]) -> str | None:
     """Return the entry of ``names`` that denotes ``tool_name`` on ``server``, else ``None``.
 
     The single question every tool-name-keyed site asks: the allow list, the deny list,
     ``allowed_params`` and the discovery filter, so discovery hides exactly what dispatch
-    refuses. It spans every spelling routing accepts and no more. A tool's identity is its
-    exact name, because routing dispatches two names differing only in case as two tools,
-    and folding case here would let one policy decide both.
+    refuses. It spans every spelling routing accepts and no more, because a tool's identity
+    is the exact name routing dispatches; anything looser lets one policy decide two tools.
 
-    OpenAPI servers are the exception, and not a fuzzy one. ``_register_openapi_tools``
-    rewrites every operationId through ``sanitize_openapi_tool_name``, so configuration
-    holding the spec's own spelling never equals the registered name; replaying that exact
-    rewrite on the entry recovers the link. It cannot merge identities, because every name
-    it produces is lowercased, so no two tools on such a server differ only in case.
+    On an OpenAPI server the configured entry holds the spec's operationId while routing
+    holds :func:`openapi_tool_name` of it, so both sides go through that map first. Doing it
+    with the registering function rather than a lookalike is the whole safety argument: a
+    coarser one collapses operationIds that registration keeps apart.
 
     Callers read the returned entry rather than testing a container's values, which is what
     stops an explicitly empty ``allowed_params`` list from reading as "nothing configured".
     """
-    from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
-        sanitize_openapi_tool_name,
-    )
-
-    spellings = set(iter_known_tool_name_spellings(tool_name, server))
-    exact = next((name for name in names if name in spellings), None)
-    if exact is not None or not getattr(server, "spec_path", None):
-        return exact
-    sanitized = {sanitize_openapi_tool_name(spelling) for spelling in spellings}
-    return next((name for name in names if sanitize_openapi_tool_name(name) in sanitized), None)
+    normalize = openapi_tool_name if getattr(server, "spec_path", None) else str
+    spellings = {normalize(spelling) for spelling in iter_known_tool_name_spellings(tool_name, server)}
+    return next((name for name in names if normalize(name) in spellings), None)
 
 
 def split_server_prefix_from_name(prefixed_name: str) -> Tuple[str, str]:

tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py

@@ -9511,6 +9511,16 @@ class TestServerToolListsHonorThePrefixBoundary:
         assert exc_info.value.status_code == 403
         assert "include_internal" in exc_info.value.detail["error"]
 
+    @pytest.mark.asyncio
+    async def test_an_entry_does_not_decide_an_operation_id_registration_keeps_separate(self):
+        server = self._aliased_server(disallowed_tools=["foo/bar"], spec_path="/specs/petstore.yaml")
+
+        with pytest.raises(HTTPException) as exc_info:
+            await self._run_check(server, "foo/bar")
+
+        assert exc_info.value.status_code == 403
+        await self._run_check(server, "foo.bar")
+
     @pytest.mark.asyncio
     async def test_a_blocklist_entry_does_not_reach_a_case_variant_sibling_tool(self):
         server = self._aliased_server(disallowed_tools=["petstore-getPet"])