mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
fix(mcp): don't block tool listing on missing per-user env vars
Previously a server whose static_headers referenced an unfilled per-user
var contributed 0 tools to the listing (the resolver raised and the list
path swallowed it). Now listing is best-effort: available vars interpolate,
unfilled ${NAME} refs are left untouched, so the server's tools still appear.
The friendly MCPMissingUserEnvVarsError is raised only on the tool-call path,
matching the intended demo flow (connect/list works; you see the missing-vars
error when you actually invoke a tool).
https://claude.ai/code/session_01X5YQzqswkwcVLtsBbk7Qyh
This commit is contained in:
parent
855cf82581
commit
719d562b3a
2 changed files with 57 additions and 13 deletions
|
|
@ -1331,6 +1331,8 @@ class MCPServerManager:
|
|||
self,
|
||||
server: MCPServer,
|
||||
user_api_key_auth: Optional[UserAPIKeyAuth],
|
||||
*,
|
||||
raise_on_missing: bool = True,
|
||||
) -> Optional[Dict[str, str]]:
|
||||
"""Return server.static_headers with ``${NAME}`` interpolated.
|
||||
|
||||
|
|
@ -1338,9 +1340,16 @@ class MCPServerManager:
|
|||
Per-user values come from the ``LiteLLM_MCPUserEnvVars`` row for the
|
||||
calling user.
|
||||
|
||||
Raises ``MCPMissingUserEnvVarsError`` when ``static_headers`` reference
|
||||
a per-user variable that the calling user has not yet supplied. This
|
||||
is converted into a user-facing 412 by the REST layer.
|
||||
When ``raise_on_missing`` is ``True`` (the tool-*call* path), raises
|
||||
``MCPMissingUserEnvVarsError`` if ``static_headers`` reference a per-user
|
||||
variable the calling user has not yet supplied — converted into a
|
||||
user-facing 412 by the REST layer.
|
||||
|
||||
When ``raise_on_missing`` is ``False`` (the tool-*list* path), missing
|
||||
per-user vars are non-blocking: we interpolate whatever is available and
|
||||
leave unfilled ``${NAME}`` references untouched, so the server's tools
|
||||
still appear in the listing. The user only hits the friendly error when
|
||||
they actually invoke a tool that needs the missing value.
|
||||
"""
|
||||
static_headers = server.static_headers
|
||||
env_vars = getattr(server, "env_vars", None)
|
||||
|
|
@ -1362,16 +1371,17 @@ class MCPServerManager:
|
|||
if referenced_user_vars:
|
||||
user_values = await self._load_user_env_vars(server, user_api_key_auth)
|
||||
|
||||
missing = sorted(
|
||||
name for name in referenced_user_vars if not user_values.get(name)
|
||||
)
|
||||
if missing:
|
||||
raise MCPMissingUserEnvVarsError(
|
||||
server_id=server.server_id,
|
||||
server_name=server.server_name or server.name,
|
||||
missing=missing,
|
||||
setup_url=build_env_var_setup_url(server.server_id),
|
||||
if raise_on_missing:
|
||||
missing = sorted(
|
||||
name for name in referenced_user_vars if not user_values.get(name)
|
||||
)
|
||||
if missing:
|
||||
raise MCPMissingUserEnvVarsError(
|
||||
server_id=server.server_id,
|
||||
server_name=server.server_name or server.name,
|
||||
missing=missing,
|
||||
setup_url=build_env_var_setup_url(server.server_id),
|
||||
)
|
||||
|
||||
merged_vars: Dict[str, str] = {**global_values, **user_values}
|
||||
if not static_headers:
|
||||
|
|
@ -1543,8 +1553,12 @@ class MCPServerManager:
|
|||
client = None
|
||||
|
||||
try:
|
||||
# Tool *listing* must not be blocked by missing per-user env vars —
|
||||
# the server's tools should still appear so the client connects. The
|
||||
# friendly "missing vars" error is raised only on the tool-*call*
|
||||
# path (see _call_regular_mcp_tool).
|
||||
resolved_static_headers = await self._resolve_static_headers_with_env_vars(
|
||||
server, user_api_key_auth
|
||||
server, user_api_key_auth, raise_on_missing=False
|
||||
)
|
||||
if resolved_static_headers:
|
||||
if extra_headers is None:
|
||||
|
|
|
|||
|
|
@ -220,6 +220,36 @@ async def test_resolve_static_headers_raises_when_user_vars_missing(
|
|||
assert "fill_env_vars=srv-1" in exc.value.setup_url
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_static_headers_missing_is_non_blocking_for_listing(
|
||||
mock_server, monkeypatch
|
||||
):
|
||||
"""With raise_on_missing=False (the tool-list path), missing per-user vars
|
||||
must NOT raise. Available vars interpolate; unfilled ${NAME} refs are left
|
||||
untouched so the server's tools still appear in the listing."""
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
MCPServerManager,
|
||||
)
|
||||
|
||||
manager = MCPServerManager()
|
||||
|
||||
async def fake_load_user_env_vars(server, user_api_key_auth):
|
||||
# User has only filled in one of the two required vars.
|
||||
return {"CORP_USERNAME": "alice"}
|
||||
|
||||
monkeypatch.setattr(manager, "_load_user_env_vars", fake_load_user_env_vars)
|
||||
|
||||
headers = await manager._resolve_static_headers_with_env_vars(
|
||||
mock_server, user_api_key_auth=object(), raise_on_missing=False
|
||||
)
|
||||
# Globals + the supplied user var are interpolated; the still-missing
|
||||
# CORP_PASSWORD reference is left as a literal rather than blocking listing.
|
||||
assert headers == {
|
||||
"X-DB-URL": "postgres://alice:${CORP_PASSWORD}@db.local/db",
|
||||
"X-Other": "literal",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_static_headers_passthrough_when_no_env_vars():
|
||||
"""Servers without env_vars should keep static_headers untouched."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue