diff --git a/litellm/proxy/_experimental/mcp_server/db.py b/litellm/proxy/_experimental/mcp_server/db.py index b8179f80434..3465cd19398 100644 --- a/litellm/proxy/_experimental/mcp_server/db.py +++ b/litellm/proxy/_experimental/mcp_server/db.py @@ -1073,7 +1073,11 @@ async def delete_user_env_vars( user_id: str, server_id: str, ) -> None: - """Remove the calling user's env var values for ``server_id``.""" - await prisma_client.db.litellm_mcpuserenvvars.delete( - where={"user_id_server_id": {"user_id": user_id, "server_id": server_id}} + """Remove the calling user's env var values for ``server_id``. + + Uses ``delete_many`` so a missing row is a no-op; real DB errors still + propagate to the caller instead of being silently swallowed. + """ + await prisma_client.db.litellm_mcpuserenvvars.delete_many( + where={"user_id": user_id, "server_id": server_id} ) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index b075ab19617..61ff14a92fc 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -2172,9 +2172,7 @@ if MCP_AVAILABLE: static_headers = getattr(server, "static_headers", None) or {} if isinstance(static_headers, str): try: - import json as _json - - static_headers = _json.loads(static_headers) or {} + static_headers = json.loads(static_headers) or {} except (ValueError, TypeError): static_headers = {} referenced = collect_env_var_references(strings=static_headers.values()) @@ -2304,10 +2302,7 @@ if MCP_AVAILABLE: detail={"error": f"MCP Server {server_id} not found"}, ) await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id) - try: - await delete_user_env_vars(prisma_client, user_id, server_id) - except Exception: - pass # Already deleted / didn't exist + await delete_user_env_vars(prisma_client, user_id, server_id) return _compute_user_env_var_status(server=server, stored_values={}) @router.get( diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py index 0d0c4995ed6..ffed81d7aa9 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py @@ -430,7 +430,7 @@ def _mock_env_vars_prisma(row=None): prisma.db.litellm_mcpuserenvvars.find_unique = AsyncMock(return_value=row) prisma.db.litellm_mcpuserenvvars.find_many = AsyncMock(return_value=[]) prisma.db.litellm_mcpuserenvvars.upsert = AsyncMock() - prisma.db.litellm_mcpuserenvvars.delete = AsyncMock() + prisma.db.litellm_mcpuserenvvars.delete_many = AsyncMock() return prisma @@ -529,16 +529,16 @@ async def test_get_user_env_vars_bulk_empty_ids_short_circuits(): @pytest.mark.asyncio -async def test_delete_user_env_vars_calls_unique_key(): +async def test_delete_user_env_vars_is_idempotent_delete_many(): + """Delete must use ``delete_many`` so a missing row is a no-op rather than + raising RecordNotFound; real DB errors are left to propagate.""" from litellm.proxy._experimental.mcp_server.db import delete_user_env_vars prisma = _mock_env_vars_prisma() await delete_user_env_vars(prisma, "alice", "srv-1") - prisma.db.litellm_mcpuserenvvars.delete.assert_awaited_once() - call = prisma.db.litellm_mcpuserenvvars.delete.call_args - assert call.kwargs["where"] == { - "user_id_server_id": {"user_id": "alice", "server_id": "srv-1"} - } + prisma.db.litellm_mcpuserenvvars.delete_many.assert_awaited_once() + call = prisma.db.litellm_mcpuserenvvars.delete_many.call_args + assert call.kwargs["where"] == {"user_id": "alice", "server_id": "srv-1"} # ── REST exception handling ─────────────────────────────────────────────── diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 8c06b9a8883..be8c72cb136 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -3614,7 +3614,7 @@ class TestClearMCPUserEnvVars: assert all(not spec.is_set for spec in result.required) @pytest.mark.asyncio - async def test_delete_error_is_swallowed(self): + async def test_delete_db_error_propagates(self): server = _make_env_var_server( env_vars=_ENV_VARS_MIXED, static_headers=_STATIC_HEADERS_MIXED ) @@ -3628,15 +3628,15 @@ class TestClearMCPUserEnvVars: patch.object( mgmt_endpoints, "delete_user_env_vars", - AsyncMock(side_effect=Exception("already gone")), + AsyncMock(side_effect=Exception("db down")), ), ): - # Should not raise even though delete blows up. - result = await mgmt_endpoints.clear_mcp_user_env_vars( - server_id="srv-1", - user_api_key_dict=generate_mock_user_api_key_auth(user_id="alice"), - ) - assert result.missing_count == 2 + # A real DB failure must surface, not be masked as a successful clear. + with pytest.raises(Exception, match="db down"): + await mgmt_endpoints.clear_mcp_user_env_vars( + server_id="srv-1", + user_api_key_dict=generate_mock_user_api_key_auth(user_id="alice"), + ) @pytest.mark.asyncio async def test_missing_user_id_raises_400(self): diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index 44520839977..9df3d92a10b 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -434,8 +434,6 @@ const CreateMCPServer: React.FC = ({ ...(tokenValidation !== null && { token_validation: tokenValidation }), }; - payload.static_headers = staticHeaders; - payload.env_vars = envVars; const includeCredentials = restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type);