From 1c6e82a60e949edb34199d502e7f7528683a22cb Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Tue, 13 Oct 2026 14:39:52 -0700 Subject: [PATCH] fix(mcp): resolve a bare tool name on a single-server scope against that server (#46225) A JSON-RPC tools/call on /{server_name}/mcp (or /mcp with x-mcp-servers naming one server) that uses the upstream's bare tool name answered "Tool not found" on every proxy process that had not yet listed that server, because the per-process tool-name map stays empty for a server whose upstream only lists tools with the caller's own credentials, and mcp SDK 2 no longer lists before an uncached call. call_mcp_tool now hands the single scoped server to execute_mcp_tool as requested_server_id, so the existing REST contract lists the server with the caller's headers and routes the bare name to it, and a bare name shared by two servers goes to the one the path names Co-authored-by: aayushbaluni <73417844+aayushbaluni@users.noreply.github.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> (cherry picked from commit ec28acf322cf48f32a5c0c26524c961be351a117) --- .../_experimental/mcp_server/operations.py | 16 ++++ .../mcp_server/test_mcp_server.py | 41 ++++------ .../mcp_server/test_operations.py | 82 +++++++++++++++++++ 3 files changed, 112 insertions(+), 27 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/operations.py b/litellm/proxy/_experimental/mcp_server/operations.py index 2f0b8e64c9e..af236843a1f 100644 --- a/litellm/proxy/_experimental/mcp_server/operations.py +++ b/litellm/proxy/_experimental/mcp_server/operations.py @@ -2450,6 +2450,20 @@ async def fire_mcp_tool_call_failure_logging( ) +def _single_scoped_server( + mcp_servers: Sequence[str] | None, allowed_mcp_servers: Sequence[MCPServer] +) -> MCPServer | None: + """The one server a ``/{server_name}/mcp`` path or ``x-mcp-servers`` header scoped this call to. + + A bare tool name on such a request resolves against that server, the way a REST ``server_id`` + does, instead of against the per-process tool-name map, which stays empty for a server whose + upstream only lists tools with the caller's own credentials until this process has listed it. + """ + if mcp_servers is None or len(mcp_servers) != 1 or len(allowed_mcp_servers) != 1: + return None + return allowed_mcp_servers[0] + + @client @catalog_operation(lambda: global_mcp_server_manager) async def call_mcp_tool( @@ -2511,6 +2525,7 @@ async def call_mcp_tool( detail="User not allowed to call this tool.", ) + scoped_server: Final = _single_scoped_server(mcp_servers, allowed_mcp_servers) # Delegate to execute_mcp_tool for execution response = await execute_mcp_tool( name=name, @@ -2527,6 +2542,7 @@ async def call_mcp_tool( input_responses=input_responses, request_state=request_state, incoming_bearer_token=incoming_bearer_token, + requested_server_id=scoped_server.server_id if scoped_server is not None else None, **kwargs, ) except Exception as e: diff --git a/tests/unit/proxy/_experimental/mcp_server/test_mcp_server.py b/tests/unit/proxy/_experimental/mcp_server/test_mcp_server.py index 449e437865b..f8ffcd6c31e 100644 --- a/tests/unit/proxy/_experimental/mcp_server/test_mcp_server.py +++ b/tests/unit/proxy/_experimental/mcp_server/test_mcp_server.py @@ -2994,11 +2994,7 @@ async def test_call_mcp_tool_uses_manager_permission_lookup(): "get_mcp_server_by_id", return_value=mock_server, ), - patch.object( - global_mcp_server_manager, - "get_mcp_server_from_tool_name", - return_value=mock_server, - ) as mock_get_server, + patch.object(global_mcp_server_manager, "server_exposes_tool", return_value=True), patch( "litellm.proxy._experimental.mcp_server.operations.global_mcp_tool_registry" ) as mock_tool_registry, @@ -3023,24 +3019,18 @@ async def test_call_mcp_tool_uses_manager_permission_lookup(): assert result == expected_response mock_get_allowed.assert_awaited_once() - # We call `_get_mcp_server_from_tool_name` multiple times: - # - for logging/metadata - # - for resolving the server during dispatch - # - and inside `call_tool` for guardrails/hooks - # The exact count isn't important, only that it is used. - assert mock_get_server.call_count >= 2 - # First call should use the prefixed tool name - assert ( - mock_get_server.call_args_list[0][0][0] - == f"{mock_server.name}/gmail_send_email" - ) + # The request names exactly one server, so that server is the dispatch target + # and the tool name reaches it untouched (the "/" is not a LiteLLM prefix). + mock_handle_managed.assert_awaited_once() + assert mock_handle_managed.await_args.kwargs["server_name"] == mock_server.name + assert mock_handle_managed.await_args.kwargs["name"] == f"{mock_server.name}/gmail_send_email" @pytest.mark.asyncio async def test_call_mcp_tool_resolves_unprefixed_tool_name_and_checks_permissions(): """ - Ensure `call_mcp_tool` correctly resolves the MCP server for an unprefixed tool - name and enforces server-level permissions using that resolved server. + Ensure `call_mcp_tool` resolves an unprefixed tool name on a request scoped to + one server against that server and enforces server-level permissions with it. """ from litellm.proxy._experimental.mcp_server.server import ( call_mcp_tool, @@ -3070,11 +3060,7 @@ async def test_call_mcp_tool_resolves_unprefixed_tool_name_and_checks_permission "get_mcp_server_by_id", return_value=mock_server, ), - patch.object( - global_mcp_server_manager, - "get_mcp_server_from_tool_name", - return_value=mock_server, - ) as mock_get_server, + patch.object(global_mcp_server_manager, "server_exposes_tool", return_value=True), patch( "litellm.proxy._experimental.mcp_server.operations.global_mcp_tool_registry" ) as mock_tool_registry, @@ -3091,7 +3077,7 @@ async def test_call_mcp_tool_resolves_unprefixed_tool_name_and_checks_permission mock_tool_registry.get_tool.return_value = None mock_handle_managed.return_value = expected_response - # Call with UNPREFIXED tool name; server should be resolved via mapping + # Call with UNPREFIXED tool name; the single server the request names is the target result = await call_mcp_tool( name="gmail_send_email", arguments={"body": "hello"}, @@ -3100,8 +3086,9 @@ async def test_call_mcp_tool_resolves_unprefixed_tool_name_and_checks_permission assert result == expected_response mock_get_allowed.assert_awaited_once() - # We should resolve the server at least once using the unprefixed name - assert mock_get_server.call_count >= 1 - assert mock_get_server.call_args_list[0][0][0] == "gmail_send_email" + mock_handle_managed.assert_awaited_once() + assert mock_handle_managed.await_args.kwargs["server_name"] == mock_server.name + assert mock_handle_managed.await_args.kwargs["name"] == "gmail_send_email" # Permissions check should be invoked with the resolved server name mock_is_allowed.assert_called_once() + assert mock_is_allowed.call_args.kwargs["server_name"] == mock_server.name diff --git a/tests/unit/proxy/_experimental/mcp_server/test_operations.py b/tests/unit/proxy/_experimental/mcp_server/test_operations.py index dcc4eb1a570..af547ebf6c7 100644 --- a/tests/unit/proxy/_experimental/mcp_server/test_operations.py +++ b/tests/unit/proxy/_experimental/mcp_server/test_operations.py @@ -565,6 +565,88 @@ async def test_tools_call_warmup_does_not_consume_mcp_server_rpm() -> None: assert [tool.name for tool in listing.tools] == ["echo"] + +def _scoped_rig( + monkeypatch: pytest.MonkeyPatch, +) -> tuple[MCPServer, MCPServer, AsyncMock, AsyncMock]: + """Two config servers that both expose a bare ``list_rules`` on a manager that has listed neither.""" + from mcp.types import CallToolResult, TextContent + + from litellm.proxy import proxy_server + from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager + + manager: Final = MCPServerManager() + memory: Final = MCPServer( + server_id="id-memory", name="memory_server", server_name="memory_server", transport=MCPTransport.http + ) + other: Final = MCPServer( + server_id="id-other", name="other_server", server_name="other_server", transport=MCPTransport.http + ) + manager.config_mcp_servers = {server.server_id: server for server in (memory, other)} + monkeypatch.setattr(proxy_server, "prisma_client", None) + monkeypatch.setattr(operations, "global_mcp_server_manager", manager) + monkeypatch.setattr( + manager, "get_allowed_mcp_servers", AsyncMock(return_value=[memory.server_id, other.server_id]) + ) + monkeypatch.setattr(global_mcp_tool_registry, "published_tools", {}) + listing: Final = AsyncMock() + monkeypatch.setattr(operations, "_get_tools_from_mcp_servers", listing) + handled: Final = AsyncMock( + return_value=CallToolResult(content=[TextContent(type="text", text="ok")], isError=False) + ) + monkeypatch.setattr(operations, "_handle_managed_mcp_tool", handled) + return memory, other, listing, handled + + +@pytest.mark.asyncio +@pytest.mark.parametrize("scope", ["memory_server", "other_server"]) +async def test_bare_tool_name_on_a_single_server_scope_routes_to_that_server_cold( + monkeypatch: pytest.MonkeyPatch, scope: str +) -> None: + """``/{server_name}/mcp`` (or ``x-mcp-servers`` naming one server) with the upstream's bare tool name + must list and call the scoped server on a process whose tool-name map has never seen it, and when two + servers share the bare name the scope decides (BerriAI/litellm#44831).""" + memory, other, listing, handled = _scoped_rig(monkeypatch) + expected: Final = memory if scope == "memory_server" else other + + async with operations.global_mcp_server_manager.catalog.operation(): + result: Final = await operations.call_mcp_tool( + name="list_rules", + arguments={}, + user_api_key_auth=UserAPIKeyAuth(api_key="sk-scoped"), + mcp_servers=[scope], + ) + + assert result.is_error is False + assert listing.await_args.kwargs["mcp_servers"] == [expected.server_id] + assert handled.await_args.kwargs["server_name"] == expected.name + assert handled.await_args.kwargs["name"] == "list_rules" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("mcp_servers", [None, ["memory_server", "other_server"]]) +async def test_bare_tool_name_without_a_single_server_scope_is_still_unresolved_cold( + monkeypatch: pytest.MonkeyPatch, mcp_servers: list[str] | None +) -> None: + """The scope shortcut only applies when the request names exactly one server; an ambiguous bare name + keeps today's answer rather than being guessed onto the first allowed server.""" + from fastapi import HTTPException + + _memory, _other, listing, handled = _scoped_rig(monkeypatch) + + async with operations.global_mcp_server_manager.catalog.operation(): + with pytest.raises(HTTPException) as unresolved: + await operations.call_mcp_tool( + name="list_rules", + arguments={}, + user_api_key_auth=UserAPIKeyAuth(api_key="sk-scoped"), + mcp_servers=mcp_servers, + ) + + assert unresolved.value.status_code == 404 + listing.assert_not_awaited() + handled.assert_not_awaited() + @pytest.mark.asyncio async def test_tools_call_pre_call_check_enforces_mcp_server_rpm() -> None: from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager