fix(mcp): make per-user env var cleanup best-effort on server delete

The server-row delete is the commit point; a transient failure cleaning the
FK-less per-user env var rows now logs a warning instead of propagating, so a
successful delete is no longer turned into a caller error that triggers a retry
and a 404 for an already-gone server. Also mark the health-check env-var
round-trip test as asyncio so it runs explicitly like its siblings.
This commit is contained in:
mateo-berri 2026-06-04 16:05:15 +00:00 • committed by Claude
parent 67e81f794f
commit 2e948527ce
No known key found for this signature in database
3 changed files with 39 additions and 5 deletions

View file

@ -519,8 +519,10 @@ async def delete_mcp_server(
"""
Delete the mcp server from the db by server_id
Also removes any per-user env var rows for the server, which have no FK
cascade, so deleting a server never leaves orphaned credential rows behind.
The server-row delete is the commit point. Per-user env var rows have no FK
cascade, so they are cleaned up afterwards on a best-effort basis: a transient
failure there leaves only orphaned rows pointing at a now-missing server and
must not turn a successful delete into a caller-visible error.
Returns the deleted mcp server record if it exists, otherwise None
"""
@ -530,9 +532,17 @@ async def delete_mcp_server(
},
)
if deleted_server is not None:
await prisma_client.db.litellm_mcpuserenvvars.delete_many(
where={"server_id": server_id}
)
try:
await prisma_client.db.litellm_mcpuserenvvars.delete_many(
where={"server_id": server_id}
)
except Exception as e:
verbose_proxy_logger.warning(
"MCP server %s deleted but per-user env var cleanup failed; "
"orphaned rows can be removed on a later delete: %s",
server_id,
e,
)
return deleted_server

View file

@ -829,6 +829,29 @@ async def test_delete_mcp_server_skips_env_var_cleanup_when_server_missing():
prisma.db.litellm_mcpuserenvvars.delete_many.assert_not_awaited()
@pytest.mark.asyncio
async def test_delete_mcp_server_succeeds_when_orphan_cleanup_fails():
"""The server-row delete is the commit point: a transient failure cleaning
the FK-less per-user env var rows must not turn a successful delete into a
caller error, otherwise the caller retries and hits a 404 for a server that
is already gone."""
from unittest.mock import AsyncMock
from litellm.proxy._experimental.mcp_server.db import delete_mcp_server
deleted = object()
prisma = _mock_env_vars_prisma()
prisma.db.litellm_mcpservertable.delete = AsyncMock(return_value=deleted)
prisma.db.litellm_mcpuserenvvars.delete_many = AsyncMock(
side_effect=Exception("connection pool exhausted")
)
result = await delete_mcp_server(prisma, "srv-1")
assert result is deleted
prisma.db.litellm_mcpuserenvvars.delete_many.assert_awaited_once()
# ── DB helpers: global env vars encrypted at rest ─────────────────────────

View file

@ -4086,6 +4086,7 @@ class TestRegistryTableConversionPreservesEnvVars:
table = manager._build_mcp_server_table(self._server_with_env_vars())
self._assert_env_vars_round_tripped(table)
@pytest.mark.asyncio
async def test_health_check_server_preserves_env_vars(self):
# OAuth2 without client credentials needs a per-user token, so the
# health check is skipped (no network) and we exercise the table