mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(mcp): do not prefix tool names when listing via scoped /mcp/{server_name} endpoint for stdio servers (#22670)
When an MCP list_tools request arrives via a URL-scoped path such as
/mcp/{server_name}, the client has already disambiguated the server by
the URL. Prefixing the returned tool names with the server name in that
case causes consumers (e.g. the GitHub on-prem stdio MCP server in
#22670) to see names like github_onprem-get_repo instead of get_repo.
The aggregated /mcp endpoint still prefixes tool names so the merged
list stays globally unique.
Implementation:
- Add a transport-agnostic ContextVar (_mcp_request_scoped_to_single_server)
set in extract_mcp_auth_context when a single server is parsed from the
URL path.
- Honour the flag in all four list_* server-fetch call sites (tools,
prompts, resources, resource templates) so behaviour is consistent
across stdio, SSE and HTTP-streamable transports.
Fixes #22670
This commit is contained in:
parent
87e120d958
commit
eb060b1105
3 changed files with 128 additions and 4 deletions
|
|
@ -19,3 +19,16 @@ _mcp_active_toolset_id: ContextVar[Optional[str]] = ContextVar(
|
|||
_mcp_gateway_initialize_instructions: ContextVar[Optional[str]] = ContextVar(
|
||||
"_mcp_gateway_initialize_instructions", default=None
|
||||
)
|
||||
|
||||
# Set when an MCP request arrives via a path-scoped URL like /mcp/{server_name}
|
||||
# (or /{server_name}/mcp). In that case the client has already disambiguated the
|
||||
# server by URL, so tool names from list_tools / list_prompts / list_resources /
|
||||
# list_resource_templates SHOULD NOT be prefixed with the server name. When the
|
||||
# request comes in on the aggregated /mcp endpoint (no server in the path) this
|
||||
# flag stays False and the aggregated list keeps its server-name prefixes so
|
||||
# tool names remain globally unique.
|
||||
# This is intentionally transport-agnostic: stdio, SSE and HTTP-streamable servers
|
||||
# all honour the same flag, so behaviour stays consistent across transports.
|
||||
_mcp_request_scoped_to_single_server: ContextVar[bool] = ContextVar(
|
||||
"_mcp_request_scoped_to_single_server", default=False
|
||||
)
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
|||
from litellm.proxy._experimental.mcp_server.mcp_context import (
|
||||
_mcp_active_toolset_id,
|
||||
_mcp_gateway_initialize_instructions,
|
||||
_mcp_request_scoped_to_single_server,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_debug import MCPDebug
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
|
|
@ -1319,11 +1320,19 @@ if MCP_AVAILABLE:
|
|||
)
|
||||
|
||||
try:
|
||||
# For scoped /mcp/{server_name} requests the client has already
|
||||
# disambiguated by URL, so tool names should NOT be prefixed
|
||||
# (see issue #22670). Otherwise keep the default behaviour of
|
||||
# prefixing tool names with the server name for the aggregated
|
||||
# /mcp endpoint.
|
||||
add_prefix_for_server = (
|
||||
not _mcp_request_scoped_to_single_server.get()
|
||||
)
|
||||
tools = await global_mcp_server_manager._get_tools_from_server(
|
||||
server=server,
|
||||
mcp_auth_header=server_auth_header,
|
||||
extra_headers=extra_headers,
|
||||
add_prefix=True, # Always add server prefix
|
||||
add_prefix=add_prefix_for_server,
|
||||
raw_headers=raw_headers,
|
||||
)
|
||||
filtered_tools = filter_tools_by_allowed_tools(tools, server)
|
||||
|
|
@ -1460,11 +1469,14 @@ if MCP_AVAILABLE:
|
|||
)
|
||||
|
||||
try:
|
||||
# Honour scoped /mcp/{server_name} requests — no prefix when
|
||||
# the URL already disambiguates the server (see issue #22670).
|
||||
add_prefix_for_server = not _mcp_request_scoped_to_single_server.get()
|
||||
prompts = await global_mcp_server_manager.get_prompts_from_server(
|
||||
server=server,
|
||||
mcp_auth_header=server_auth_header,
|
||||
extra_headers=extra_headers,
|
||||
add_prefix=True, # Always add server prefix
|
||||
add_prefix=add_prefix_for_server,
|
||||
raw_headers=raw_headers,
|
||||
)
|
||||
|
||||
|
|
@ -1517,11 +1529,14 @@ if MCP_AVAILABLE:
|
|||
)
|
||||
|
||||
try:
|
||||
# Honour scoped /mcp/{server_name} requests — no prefix when
|
||||
# the URL already disambiguates the server (see issue #22670).
|
||||
add_prefix_for_server = not _mcp_request_scoped_to_single_server.get()
|
||||
resources = await global_mcp_server_manager.get_resources_from_server(
|
||||
server=server,
|
||||
mcp_auth_header=server_auth_header,
|
||||
extra_headers=extra_headers,
|
||||
add_prefix=True, # Always add server prefix
|
||||
add_prefix=add_prefix_for_server,
|
||||
raw_headers=raw_headers,
|
||||
)
|
||||
all_resources.extend(resources)
|
||||
|
|
@ -1572,12 +1587,15 @@ if MCP_AVAILABLE:
|
|||
)
|
||||
|
||||
try:
|
||||
# Honour scoped /mcp/{server_name} requests — no prefix when
|
||||
# the URL already disambiguates the server (see issue #22670).
|
||||
add_prefix_for_server = not _mcp_request_scoped_to_single_server.get()
|
||||
resource_templates = (
|
||||
await global_mcp_server_manager.get_resource_templates_from_server(
|
||||
server=server,
|
||||
mcp_auth_header=server_auth_header,
|
||||
extra_headers=extra_headers,
|
||||
add_prefix=True, # Always add server prefix
|
||||
add_prefix=add_prefix_for_server,
|
||||
raw_headers=raw_headers,
|
||||
)
|
||||
)
|
||||
|
|
@ -2478,6 +2496,14 @@ if MCP_AVAILABLE:
|
|||
raw_headers,
|
||||
) = await MCPRequestHandler.process_mcp_request(scope)
|
||||
mcp_servers = mcp_servers_from_path
|
||||
# When the client disambiguates the MCP server via URL path
|
||||
# (e.g. /mcp/github_onprem), downstream list_* handlers should return
|
||||
# tool / prompt / resource names WITHOUT the server-name prefix, since
|
||||
# there is no ambiguity to resolve. Setting this ContextVar is
|
||||
# transport-agnostic so stdio, SSE and HTTP-streamable servers behave
|
||||
# consistently for scoped requests. See issue #22670.
|
||||
if len(mcp_servers_from_path) == 1:
|
||||
_mcp_request_scoped_to_single_server.set(True)
|
||||
else:
|
||||
(
|
||||
user_api_key_auth,
|
||||
|
|
|
|||
|
|
@ -1297,6 +1297,91 @@ async def test_list_tools_multiple_servers_prefixed_names():
|
|||
assert names == ["jira-toolA", "zapier-toolA"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_tools_scoped_via_url_path_does_not_prefix_stdio_server():
|
||||
"""Regression test for https://github.com/BerriAI/litellm/issues/22670.
|
||||
|
||||
When an MCP list_tools request arrives via the scoped URL path
|
||||
``/mcp/{server_name}`` (rather than the aggregated ``/mcp`` endpoint),
|
||||
the client has already disambiguated the server by the URL, so tool
|
||||
names MUST NOT be prefixed with the server name. This must hold for
|
||||
every transport — stdio included — so that the stdio-backed GitHub
|
||||
on-prem MCP server in issue #22670 does not return names like
|
||||
``github_onprem-get_repo`` when scoped.
|
||||
"""
|
||||
try:
|
||||
from litellm.proxy._experimental.mcp_server.mcp_context import (
|
||||
_mcp_request_scoped_to_single_server,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.server import (
|
||||
_get_tools_from_mcp_servers,
|
||||
set_auth_context,
|
||||
)
|
||||
except ImportError:
|
||||
pytest.skip("MCP server not available")
|
||||
|
||||
user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user")
|
||||
set_auth_context(user_api_key_auth)
|
||||
|
||||
# stdio-transport MCP server matching the reporter's config in #22670.
|
||||
server = MagicMock()
|
||||
server.server_id = "github_onprem_id"
|
||||
server.name = "github_onprem"
|
||||
server.alias = "github_onprem"
|
||||
server.server_name = "github_onprem"
|
||||
server.allowed_tools = None
|
||||
server.disallowed_tools = None
|
||||
server.auth_type = None
|
||||
server.extra_headers = None
|
||||
server.transport = MCPTransport.stdio
|
||||
|
||||
received_add_prefix: dict = {}
|
||||
|
||||
async def mock_get_tools_from_server(
|
||||
server,
|
||||
mcp_auth_header=None,
|
||||
extra_headers=None,
|
||||
add_prefix=True,
|
||||
raw_headers=None,
|
||||
):
|
||||
received_add_prefix["value"] = add_prefix
|
||||
tool = MagicMock()
|
||||
tool.name = f"{server.alias}-get_repo" if add_prefix else "get_repo"
|
||||
tool.description = "desc"
|
||||
tool.inputSchema = {}
|
||||
return [tool]
|
||||
|
||||
mock_manager = MagicMock()
|
||||
mock_manager.get_allowed_mcp_servers = AsyncMock(return_value=["github_onprem_id"])
|
||||
mock_manager.get_mcp_server_by_id = MagicMock(return_value=server)
|
||||
mock_manager.filter_server_ids_by_ip_with_info = lambda ids, _ip: (ids, 0)
|
||||
mock_manager._get_tools_from_server = mock_get_tools_from_server
|
||||
|
||||
# Simulate the scoped /mcp/{server_name} request by setting the
|
||||
# ContextVar that `extract_mcp_auth_context` sets on the real request
|
||||
# path. Setting it here lets us unit-test the decision logic without
|
||||
# bringing up a Starlette app.
|
||||
token = _mcp_request_scoped_to_single_server.set(True)
|
||||
try:
|
||||
with patch(
|
||||
"litellm.proxy._experimental.mcp_server.server.global_mcp_server_manager",
|
||||
mock_manager,
|
||||
):
|
||||
tools = await _get_tools_from_mcp_servers(
|
||||
user_api_key_auth=user_api_key_auth,
|
||||
mcp_auth_header=None,
|
||||
mcp_servers=["github_onprem"],
|
||||
mcp_server_auth_headers=None,
|
||||
)
|
||||
finally:
|
||||
_mcp_request_scoped_to_single_server.reset(token)
|
||||
|
||||
# Core bug: on the scoped URL, tool names must be returned unprefixed.
|
||||
assert received_add_prefix.get("value") is False
|
||||
assert len(tools) == 1
|
||||
assert tools[0].name == "get_repo"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mcp_manager_allows_public_servers_without_permissions():
|
||||
try:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue