fix(mcp): persist per-user OAuth client_id so tokens auto-refresh

[katzdave]

Relevant issues

Relates to #31222 (per-user MCP OAuth credential management — the UI removal in #30178 that this PR's backend-only scope accounts for).
Companion PR: #31326 — use stored per-user OAuth token on tool calls.

Type

🐛 Bug Fix

Changes

Dynamically-registered MCP OAuth clients (e.g. Guru) mint a per-user client_id during browser-side dynamic client registration, use it for the token exchange, then discard it. At refresh time refresh_user_oauth_token read server.client_id, which is None for a registration-only server, so the refresh POST omitted client_id and the provider rejected it — the access token died after ~1h and the user had to re-authorize.

This persists client_id with the per-user credential across both store paths (the server-side /token store and the manual oauth-user-credential endpoint) and prefers it over the server config when refreshing (same fallback for client_secret). Also exposes has_refresh_token on the credential status so API clients can tell whether a credential will auto-refresh.

Backend-only: the original change also added a dashboard auto-refresh indicator, but the per-user MCP credential UI was removed upstream (#30178, tracked in #31222), so that hunk is intentionally omitted.

Tests

• test_db_credentials.py — client_id persisted, and preferred over server config on refresh
• test_discoverable_endpoints.py — client_id threaded through token exchange → server-side store → DB layer
• test_mcp_management_endpoints.py — client_id reaches store_user_oauth_credential; has_refresh_token surfaced on status

All passing locally (uv run pytest on the three files above).

Pre-Submission checklist

• I have added meaningful tests
• My PR's scope is as isolated as possible; it only solves 1 specific problem

[CLAassistant]

All committers have signed the CLA.

[katzdave]

@greptileai

[greptile-apps]

Greptile Summary

This PR fixes a token auto-refresh regression for MCP servers that use dynamic client registration (e.g. Guru): at refresh time the code was reading client_id from the server config, which is None for registration-only servers, causing the provider to reject the refresh and forcing re-authorization after ~1 h.

• store_user_oauth_credential now accepts and persists client_id; refresh_user_oauth_token prefers the credential-stored client_id with fallback to server.client_id, and re-persists it after rotation so the chain holds across multiple refresh cycles.
• exchange_token_with_server threads resolved_client_id through _store_per_user_token_server_side into the DB layer; both store paths (server-side token endpoint and manual management endpoint) are updated consistently.
• MCPOAuthUserCredentialStatus gains has_refresh_token so API clients can determine whether auto-refresh is configured without inspecting the credential itself.

Confidence Score: 4/5

The change is narrowly scoped to MCP OAuth credential storage and refresh and does not touch the main LiteLLM request path or authentication layer.

The core fix is correct and thoroughly tested with mock-only unit tests covering the full refresh cycle. The only noteworthy asymmetry is cred.get client_secret added alongside cred.get client_id in the refresh function — since client_secret is never persisted to the credential payload, that branch is permanently dead and could mislead future maintainers, but it has no runtime effect today.

litellm/proxy/_experimental/mcp_server/db.py — specifically the client_secret resolution line in refresh_user_oauth_token.

Important Files Changed

Filename	Overview
litellm/proxy/_experimental/mcp_server/db.py	Core fix: store_user_oauth_credential now persists client_id; refresh_user_oauth_token prefers the credential-stored client_id over server config. One asymmetry: cred.get("client_secret") is added alongside cred.get("client_id") but client_secret is never written to the credential payload, making that branch permanently dead.
litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py	Threads resolved_client_id through exchange_token_with_server to _store_per_user_token_server_side to store_user_oauth_credential. Correct for both dynamic (uses caller's client_id) and static (uses server config client_id) servers.
litellm/proxy/_types.py	Adds optional client_id to MCPOAuthUserCredentialRequest and has_refresh_token: bool = False to MCPOAuthUserCredentialStatus; both additions are backward-compatible.
litellm/proxy/management_endpoints/mcp_management_endpoints.py	Passes payload.client_id down to store_user_oauth_credential and populates has_refresh_token from the read-back stored credential. Consistent with the pattern already used for expires_at.
tests/test_litellm/proxy/_experimental/mcp_server/test_db_credentials.py	Adds four new tests: client_id round-trips, absence of empty key when omitted, credential-stored id preferred on refresh, server config fallback, and re-persistence across refresh cycles. All mock-only.
tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py	Two new tests verify client_id threads correctly from exchange_token_with_server into the DB layer. Mock-only, no network calls.
tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py	Two new tests verify client_id reaches store_user_oauth_credential and has_refresh_token is surfaced correctly on store and status responses.

_{Reviews (1): Last reviewed commit: "fix(mcp): persist per-user OAuth client_..." | Re-trigger Greptile}

[greptile-apps]

cred.get("client_secret") is dead code — client_secret is never written to the credential payload in store_user_oauth_credential, so this branch always evaluates to None. The asymmetry with client_id is misleading: it suggests that client secrets can be read from stored credentials, which future maintainers may rely on incorrectly. Either add client_secret persistence to store_user_oauth_credential (with the same encrypted-storage treatment as client_id) or drop the cred.get(…) prefix.

Suggested change

	client_secret: Optional[str] = cred.get("client_secret") or getattr(
	server, "client_secret", None
	)
	client_secret: Optional[str] = getattr(server, "client_secret", None)

[greptile-apps]

Greptile Summary

Fixes MCP OAuth token auto-refresh for dynamically-registered clients by persisting the per-user client_id alongside the credential and preferring it over the server config at refresh time. Also surfaces has_refresh_token on credential status responses so API clients can detect whether a stored credential will auto-refresh.

• store_user_oauth_credential gains a client_id parameter; refresh_user_oauth_token reads it back from the stored credential with fallback to the server config, and re-persists it when rotating the token.
• MCPOAuthUserCredentialRequest gains client_id; MCPOAuthUserCredentialStatus gains has_refresh_token; both new fields are threaded through the management and token-exchange endpoints.
• Seven new unit tests covering round-trip persistence, fallback to server config, re-persistence across multiple refreshes, and has_refresh_token surfacing — all mock-based with no real network calls.

Confidence Score: 4/5

Safe to merge — changes are fully additive and backward-compatible, with well-targeted tests covering the new code paths.

The fix is correct and well-tested. The only notable gap is that cred.get('client_secret') is added to refresh_user_oauth_token for symmetry, but store_user_oauth_credential never accepts or saves a client_secret, so that expression is always None and always falls through to the server config. This could mislead future maintainers into thinking client_secret persists the same way client_id now does.

db.py — specifically the client_secret read from cred in refresh_user_oauth_token, which is dead code under all current call paths.

Important Files Changed

Filename	Overview
litellm/proxy/_experimental/mcp_server/db.py	Adds client_id parameter to store_user_oauth_credential and persists it in the payload; refreshes now prefer the per-user client_id from the stored credential over the server config. The client_secret read from cred is dead code since the store function never accepts it.
litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py	Threads client_id through _store_per_user_token_server_side to store_user_oauth_credential. Change is additive and preserves existing behavior for servers without a per-user client_id.
litellm/proxy/_types.py	Adds client_id field to MCPOAuthUserCredentialRequest and has_refresh_token to MCPOAuthUserCredentialStatus; both are additive with safe defaults.
litellm/proxy/management_endpoints/mcp_management_endpoints.py	Passes payload.client_id to the DB layer and populates has_refresh_token on both the store and status endpoints. Straightforward additions with no regressions.
tests/test_litellm/proxy/_experimental/mcp_server/test_db_credentials.py	Adds five focused unit tests covering round-trip persistence, omission when absent, fallback logic, and re-persistence across refresh. All mock-based, no real network calls.
tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py	Adds two tests verifying client_id propagates through exchange_token_with_server to _store_per_user_token_server_side to DB layer. Correct mocking, no real HTTP calls.
tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py	Adds two tests: one verifying client_id reaches the DB store, one verifying has_refresh_token is correctly surfaced for credentials with and without a refresh token.

_{Reviews (2): Last reviewed commit: "fix(mcp): persist per-user OAuth client_..." | Re-trigger Greptile}

[greptile-apps]

client_secret is read from cred here, but store_user_oauth_credential never accepts or saves a client_secret parameter — so cred.get("client_secret") will always be None through any current code path. This creates a misleading symmetry with client_id: future maintainers may assume client_secret round-trips the same way client_id does, when it actually always falls back to the server config. If per-user client_secret persistence is intended for a future DCR use case, a matching client_secret parameter should also be added to store_user_oauth_credential now (or the dead half of the expression should be removed).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!