mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-19 00:01:29 +00:00
fix(mcp): resolve OpenAPI tool metadata from the local registry before any tools/list
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
0f7105f25e
commit
6c865d1413
3 changed files with 31 additions and 20 deletions
|
|
@ -127,6 +127,7 @@ from litellm.proxy._experimental.mcp_server.outbound_credentials.types import (
|
|||
from litellm.proxy._experimental.mcp_server.sampling_handler import (
|
||||
MCP_SAMPLING_AVAILABLE,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.tool_registry import global_mcp_tool_registry
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
MCP_TOOL_PREFIX_SEPARATOR,
|
||||
MCPMissingUserEnvVarsError,
|
||||
|
|
@ -4242,9 +4243,11 @@ class MCPServerManager:
|
|||
# applied (e.g. "test_petstore-getinventory"). Do NOT pass them
|
||||
# through _create_prefixed_tools — that would add the prefix a second
|
||||
# time producing "test_petstore-test_petstore-getinventory".
|
||||
if add_prefix:
|
||||
return tools
|
||||
prefix: Final = get_server_prefix(server)
|
||||
sep: Final = MCP_TOOL_PREFIX_SEPARATOR
|
||||
bare_tools: Final = [ # mutable-ok: returned through the list[MCPTool] listing contract
|
||||
return [ # mutable-ok: returned through the list[MCPTool] listing contract
|
||||
(
|
||||
t.model_copy(update={"name": t.name[len(prefix) + len(sep) :]})
|
||||
if t.name.startswith(f"{prefix}{sep}")
|
||||
|
|
@ -4252,8 +4255,6 @@ class MCPServerManager:
|
|||
)
|
||||
for t in tools
|
||||
]
|
||||
self._listed_tools_by_server_id[server.server_id] = MappingProxyType({t.name: t for t in bare_tools})
|
||||
return tools if add_prefix else bare_tools
|
||||
else:
|
||||
tools = await self._fetch_tools_with_timeout(client, server.name)
|
||||
self._remember_upstream_initialize_instructions(server, client)
|
||||
|
|
@ -5139,6 +5140,14 @@ class MCPServerManager:
|
|||
return prefixed_tools
|
||||
|
||||
def get_listed_tool(self, server: MCPServer, name: str) -> MCPTool | None:
|
||||
if server.spec_path:
|
||||
bare_name: Final = strip_known_server_prefix(name, server)
|
||||
registered: Final = global_mcp_tool_registry.get_tool(
|
||||
f"{get_server_prefix(server)}{MCP_TOOL_PREFIX_SEPARATOR}{bare_name}"
|
||||
) or global_mcp_tool_registry.get_tool(bare_name)
|
||||
if registered is None:
|
||||
return None
|
||||
return MCPTool(name=bare_name, description=registered.description, inputSchema=registered.input_schema)
|
||||
listed: Final = self._listed_tools_by_server_id.get(server.server_id)
|
||||
if not listed:
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -17,7 +17,6 @@ from mcp.types import (
|
|||
TextContent,
|
||||
TextResourceContents,
|
||||
)
|
||||
from mcp.types import Tool as MCPTool
|
||||
|
||||
from litellm.proxy._types import (
|
||||
LiteLLM_MCPServerTable,
|
||||
|
|
@ -7161,9 +7160,9 @@ async def test_execute_mcp_tool_sets_model_in_model_call_details():
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execute_mcp_tool_hands_openapi_listed_tool_metadata_to_pre_call_hooks():
|
||||
async def test_execute_mcp_tool_hands_openapi_registered_tool_metadata_to_pre_call_hooks():
|
||||
"""OpenAPI-generated tools dispatch through the local registry, so the pre-call hooks must get the
|
||||
listed description and input schema on that path too, not only on the managed-server path."""
|
||||
registered description and input schema on that path too, even when no tools/list ran first."""
|
||||
from litellm.proxy._experimental.mcp_server import server as mcp_module
|
||||
|
||||
petstore = MCPServer(
|
||||
|
|
@ -7179,9 +7178,7 @@ async def test_execute_mcp_tool_hands_openapi_listed_tool_metadata_to_pre_call_h
|
|||
name="petstore-list_pets", description="List the pets", input_schema=schema, handler=lambda: None
|
||||
)
|
||||
manager = mcp_module.global_mcp_server_manager
|
||||
manager._create_prefixed_tools(
|
||||
[MCPTool(name="list_pets", description="List the pets", inputSchema=schema)], petstore
|
||||
)
|
||||
manager._listed_tools_by_server_id.pop(petstore.server_id, None)
|
||||
pre_call_tool_check = AsyncMock(return_value={})
|
||||
|
||||
try:
|
||||
|
|
@ -7206,10 +7203,13 @@ async def test_execute_mcp_tool_hands_openapi_listed_tool_metadata_to_pre_call_h
|
|||
)
|
||||
finally:
|
||||
mcp_module.global_mcp_tool_registry.unregister_tools_with_prefix("petstore-")
|
||||
manager._listed_tools_by_server_id.pop(petstore.server_id, None)
|
||||
|
||||
handed_tool = pre_call_tool_check.call_args.kwargs["tool"]
|
||||
assert handed_tool is not None and (handed_tool.description, handed_tool.inputSchema) == ("List the pets", schema)
|
||||
assert (handed_tool.name, handed_tool.description, handed_tool.inputSchema) == (
|
||||
"list_pets",
|
||||
"List the pets",
|
||||
schema,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -6504,9 +6504,9 @@ class TestMCPServerManager:
|
|||
assert by_prefixed_name is not None and by_prefixed_name.description == "v2"
|
||||
assert manager.get_listed_tool(server, "missing") is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("add_prefix", [True, False])
|
||||
async def test_openapi_listing_records_tool_metadata_for_pre_call_hooks(self, add_prefix):
|
||||
def test_get_listed_tool_reads_openapi_registry_without_a_prior_listing(self):
|
||||
"""OpenAPI tools live in the local registry from registration on, so their metadata must resolve
|
||||
before any tools/list has run and must disappear with the registration."""
|
||||
from litellm.proxy._experimental.mcp_server.tool_registry import global_mcp_tool_registry
|
||||
|
||||
server = MCPServer(
|
||||
|
|
@ -6519,19 +6519,21 @@ class TestMCPServerManager:
|
|||
)
|
||||
schema = {"type": "object", "properties": {"petId": {"type": "integer"}}}
|
||||
manager = MCPServerManager()
|
||||
manager._create_mcp_client = AsyncMock(return_value=AsyncMock())
|
||||
global_mcp_tool_registry.register_tool(
|
||||
name="petstore-get_pet", description="Fetch a pet", input_schema=schema, handler=lambda: None
|
||||
)
|
||||
try:
|
||||
listed = await manager._get_tools_from_server(server=server, add_prefix=add_prefix)
|
||||
for spelling in ("get_pet", "petstore-get_pet"):
|
||||
tool = manager.get_listed_tool(server, spelling)
|
||||
assert tool is not None and (tool.name, tool.description, tool.inputSchema) == (
|
||||
"get_pet",
|
||||
"Fetch a pet",
|
||||
schema,
|
||||
)
|
||||
finally:
|
||||
global_mcp_tool_registry.unregister_tools_with_prefix("petstore-")
|
||||
|
||||
assert [t.name for t in listed] == ["petstore-get_pet" if add_prefix else "get_pet"]
|
||||
for spelling in ("get_pet", "petstore-get_pet"):
|
||||
tool = manager.get_listed_tool(server, spelling)
|
||||
assert tool is not None and (tool.description, tool.inputSchema) == ("Fetch a pet", schema)
|
||||
assert manager.get_listed_tool(server, "petstore-get_pet") is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_allowed_mcp_servers_with_user_api_key_auth(self):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue