feat(mcp): cross-replica single-flight refresh for the v2 per-user OAuth store [2/2] by tin-berri · Pull Request #31493 · BerriAI/litellm

tin-berri · 2026-06-27T04:07:33Z

Re-open context

Re-opens #31474. Its changes were squash-merged into the #31473 stack branch (litellm_mcp_v2_authz_code_dispatch) before #31473 itself merged, then reverted in #31492. This PR re-applies the same eight commits cleanly on top of the current stack branch; the only merge was in oauth_token_store.py, which auto-merged with the scopes-preservation fix already on the branch. The full outbound_credentials/ suite passes (136 tests).

What this is

Split out of #31269. This is PR 2 of 2, stacked on #31473. It layers cross-replica single-flight refresh onto the single-replica per-user OAuth store from #31473.

#31473 + this PR reproduce the original #31269, plus one fix on top: the cross-replica lock now distinguishes a dead lock backend from a busy holder so a Redis outage degrades to an extra refresh rather than serving a stale token (see the last bullet under Changes)

Why

A per-user OAuth token expires; the same user fires concurrent tool calls landing on different proxy replicas. Each replica independently POSTs the refresh_token grant. Most IdPs rotate the refresh_token (RFC 6749 §6): the first grant invalidates the old token, so every other concurrent grant gets invalid_grant and the user is spuriously bounced into re-auth.

v1 had no coordination for the per-user path (MCPPerUserTokenCache is a bare DualCache get/set); the only single-flight in v1 was an in-memory asyncio.Lock on the client_credentials path, which neither spans replicas nor survives restart.

Changes

RedisDistributedLock — SET NX PX / DEL / EXISTS over the async Redis client. acquire reports a LockAcquisition tristate (ACQUIRED / HELD / ERROR) so a caller can tell a busy holder from an unreachable backend.
RedisRefreshCoordinator — elects one worker per (user, server) across the fleet to run the refresh; losers wait on EXISTS, then re-read the persisted token. PX auto-expiry means a crashed holder can't wedge refresh; it self-heals on the next expiry re-check. Reads take no lock.
DualCacheTokenCacheBackend + OAuthTokenCacheCodec — encrypt + serialize the token into LiteLLM's shared DualCache under the v1-compatible key/encryption, so a token cached by either side is readable across the cutover.
Composition root upgraded to wire these when Redis is present, falling back to in-process defaults otherwise.
Fail open on a lock-backend outage, not stale. The lock is a single-flight load optimization, not a correctness mutex. Previously acquire caught a Redis error and returned False, which the coordinator could not distinguish from "another worker holds it", so on a total Redis outage every worker took the wait-then-reread branch and served the still-expired token upstream (the upstream then 401s) while the docstrings claimed it "degrades to an extra refresh". The ERROR arm of the new tristate makes the coordinator refresh anyway when the backend is unreachable, degrading to the no-coordinator behavior (an extra refresh) instead of a stale bearer. Covered by a regression test that asserts an acquire error refreshes rather than re-reading the expired token.

Type

🆕 New Feature

Proof of fix

Mechanism — deterministic, against real Redis. Drove the production RedisDistributedLock + RedisRefreshCoordinator with 3 separate replica clients sharing one real Redis (8.x), all hitting the same expired (user, server) token concurrently:

setup	refresh_token grants to IdP	token each replica got
in-process per replica (no shared lock — the pre-#31474 multi-replica reality)	3	`FRESH-1`, `FRESH-2`, `FRESH-3`
#31474 — shared Redis `SET NX PX` coordinator	1	`FRESH-1`, `FRESH-1`, `FRESH-1`

One worker won SET NX, refreshed once, and DEL-released; the other two got HELD, polled EXISTS, then re-read the winner's persisted token. On a rotating IdP, the pre-#31474 case is exactly the invalid_grant storm this PR removes.

Unit coverage. The full outbound_credentials/ suite passes (130 tests), including the cross-replica behaviors in test_redis_refresh_coordinator.py: winner refreshes-then-releases, loser waits-then-rereads (refresh_calls == []), loser self-heals after a crashed holder's PX expiry, and lock-backend-error refreshes anyway instead of serving stale.

Live deployment. Stood up two real litellm proxies on this branch sharing one Redis + Postgres. Confirmed the activation gate — litellm.enable_redis_auth_cache=True attaches Redis to user_api_key_cache, so the composition root wires the Redis coordinator (not the in-process fallback) — and that an oauth2 authorization_code MCP server loads and routes through the v2 resolver. (The end-to-end-through-HTTP grant count was not run to completion due to unrelated local proxy env/RBAC plumbing; the deterministic Redis test above proves the same single-flight behavior.)

Note

Medium Risk
Touches OAuth token refresh, Redis locking, and encrypted shared cache—security- and availability-sensitive—but changes are defensive (fail-open, no refresh_token in Redis) with broad unit coverage.

Overview
Adds cross-replica coordination for v2 per-user authorization_code OAuth tokens when Redis is available: one proxy worker refreshes per (user, server) while others wait and re-read the persisted token, avoiding rotating-refresh_token invalid_grant storms across replicas.

Redis path: RedisDistributedLock (SET NX PX, owner-only extend/release) drives RedisRefreshCoordinator (winner refreshes with lease renewal; losers poll then re-read). DualCacheTokenCacheBackend + OAuthTokenCacheCodec store encrypted access tokens only in shared DualCache under v1-compatible keys. per_user_oauth_store wires these when user_api_key_cache.redis_cache is set; otherwise it keeps in-process defaults. LazyPerUserOAuthTokenStore rebuilds once Redis appears, with coordination so in-flight local fetches finish before upgrade and concurrent local fetches stay unsynchronized.

Correctness tweaks: RefreshingTokenStore losers re-read via reread_fresh_token so a failed winner refresh yields None (re-auth), not a stale bearer. Lock acquire returns ERROR vs HELD so Redis outages refresh anyway instead of waiting and serving expired tokens. Cache/lock failures degrade to miss or no-op, not request errors.

^{Reviewed by Cursor Bugbot for commit 9eea30f. Bugbot is set up for automated code reviews on this repo. Configure here.}

greptile-apps · 2026-06-27T04:11:19Z

Greptile Summary

This PR adds Redis-backed coordination for per-user MCP OAuth token refreshes. The main changes are:

Cross-replica single-flight refresh using a Redis SET NX PX lock.
Owner-checked lock renewal and release for long-running refreshes.
Shared encrypted DualCache storage for cached access tokens.
Lazy per-user OAuth store wiring that upgrades to Redis-backed coordination when Redis becomes available.
Tests for lock ownership, Redis outage fallback, loser rereads, cache codec behavior, and lazy store upgrade paths.

Confidence Score: 4/5

The Redis-backed refresh coordination and cache codec changes are broadly covered by focused tests and align with the described OAuth refresh behavior.

The touched code is concentrated in outbound credential storage, Redis locking, and composition wiring, with tests covering lock ownership, backend errors, loser rereads, codec behavior, and lazy Redis upgrade paths. The remaining risk is mainly integration-level behavior across live proxy replicas and real deployment configuration.

No specific files require follow-up based on this review.

T-Rex Logs

What T-Rex did

Observed the Redis single-flight flow by comparing before and after logs, noting refresh_call_count dropped from 2 to 1, tokens changed from ['fresh-1', 'fresh-2'] to ['fresh-1', 'fresh-1'], and Redis lock usage including SET ... NX ... PX 10000 and EVAL del.
Compared the base probe for RedisRefreshCoordinator: the before artifact showed a missing RedisRefreshCoordinator and probe failure, while the after artifact shows LockAcquisition_available=True, an injected ERROR, refresh_calls=1, reread_calls=0, is_held_calls=0, and a redacted returned_token; a targeted pytest attempt is captured but pytest is unavailable.
Assessed the dual-cache token codec flow: the before artifact failed with ModuleNotFoundError for dual_cache_token_backend, and the after artifact reports CONTRACT_OK=True, expected v1 key usage, encrypted stored value inspection, refresh-token absence, manual decrypt, and backend readback.
Reviewed the in-memory vs Redis coordination flow: the before artifact showed in-memory coordinator/cache used even with Redis, lacking the lazy constructor seam, and returning a stale expired token on loser reread, while the after artifact shows no-Redis fallback via InMemoryTokenCacheBackend/InProcessRefreshCoordinator, Redis wired as DualCacheTokenCacheBackend/RedisRefreshCoordinator, a lazy store rebuilt after Redis appears with all concurrent results, and None returned instead of the stale bearer on loser reread.

_{Ran code and verified through T-Rex}

_{Reviews (6): Last reviewed commit: "fix: allow concurrent lazy OAuth fetches..." | Re-trigger Greptile}

codecov · 2026-06-27T04:12:28Z

Codecov Report

❌ Patch coverage is 89.15094% with 23 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...erver/outbound_credentials/per_user_oauth_store.py	67.74%	20 Missing ⚠️
...ver/outbound_credentials/redis_distributed_lock.py	94.87%	2 Missing ⚠️
.../outbound_credentials/redis_refresh_coordinator.py	98.24%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

veria-ai · 2026-06-27T04:20:03Z

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 3 · PR risk: 0/10

…(step 1b §1.5) The serialize+encrypt boundary a cross-replica cache needs: a plaintext bearer in Redis is a leak, so encode() encrypts (NaCl in prod via the injected encrypt, identity in tests). Caches only access_token and expires_at, never the refresh_token - the hot path needs just the bearer, and the long-lived refresh_token stays in the DB (the refresh path is always a cache miss), matching v1. A decoded token always has refresh_token=None. Undecryptable (key rotation) or corrupt entries read as a miss.

The cross-replica TokenCacheBackend implementation that plugs into the foundation's CachedOAuthTokenStore seam: encrypts+serializes the token via the codec and stores it in LiteLLM's shared DualCache under the same per-(user,server) key v1 used, so workers share one refresh and a token cached by v1 or v2 is readable by the other across the cutover. Cache and codec are injected; a non-positive TTL (already-expired token) is not cached, and a missing/corrupt entry reads as a miss.

The cross-replica RefreshCoordinator that plugs into the foundation's RefreshingTokenStore seam: a SET NX PX lock elects one worker to refresh per (user, server) while the rest wait for it and re-read the token it persisted, so a rotating refresh_token is used once across the fleet, not once per worker. The lock self-expires (PX) so a crashed holder can't wedge refresh; a loser falls back to a bounded re-read and the surrounding store re-checks expiry next fetch, so a crash self-heals. The lock (a thin Redis SET NX/DEL/EXISTS wrapper in prod) is injected, so the single-flight logic is testable without Redis.

The concrete DistributedLock the RedisRefreshCoordinator elects refreshers with: acquire is an atomic SET key NX PX ttl (first caller wins, entry self-expires so a crashed holder can't wedge refresh), release is DEL, is_held is EXISTS. The async Redis client is injected (the client from LiteLLM's RedisCache in prod), so it is unit-testable with a fake. Any Redis error degrades to not-acquired / not-held so a cache blip causes an extra refresh, never a crash on the resolve path.

…er store (step 1b §1.5) Upgrade the composition root to use the DualCache-backed cache and SET NX PX refresh coordinator when Redis is wired, falling back to the foundation's in-process defaults on a single replica. Layers the cross-replica path on top of the single-replica dispatch store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The cross-replica refresh coordinator elected refreshers with a boolean acquire: a Redis transport error was caught and returned as False, which is indistinguishable from "another worker holds the lock". On a total Redis outage every worker therefore took the wait-then-reread branch and served the still-expired token upstream (the upstream then 401s), even though the lock and coordinator docstrings claimed a Redis blip "degrades to an extra refresh". Make acquire tristate (LockAcquisition: ACQUIRED / HELD / ERROR) so the coordinator can tell a busy holder from a dead backend, and refresh anyway on ERROR. This single-flight lock is a load optimization, not a correctness mutex, so failing open is correct: it degrades a lock-backend outage to the no-coordinator behavior (an extra refresh), never a stale bearer. Add a regression test asserting an acquire error refreshes rather than re-reading the expired token, and update the docstrings to match.

…format

…winner failed The cross-replica coordinator's losers re-read the token the winner persisted. If the winner's refresh failed, the store still holds the expired token, so the loser re-read it and RefreshingTokenStore handed that expired bearer to the caller (the upstream then 401s) instead of the re-auth challenge the winner returned via None. Make the loser's re-read expiry-aware, mirroring refresh_latest_token: a re-read that is still expired surfaces None so the arm challenges. This only affects the loser path; the winner's freshly refreshed token is returned directly by the coordinator and is unaffected.

When a cached blob cannot be decrypted (e.g. after a salt or master-key rotation) the codec logged a full traceback at error level, since decrypt_value_helper defaults to exception_type=error. v1's MCPPerUserTokenCache passed exception_type=debug on the same path. The blob is ciphertext so this is log noise only, but matching v1 avoids error-level traceback spam on stale entries after a key rotation

… token The Redis lock wrote its key through the raw client from init_async_client(), bypassing RedisCache's namespace, so two deployments sharing one Redis collided on mcp:refresh_lock:<user>:<server> for any overlapping (user, server) and a colliding deployment skipped the refresh and challenged its own users. The lock now runs every key through an injected namespace_key wired to RedisCache.check_and_fix_namespace, matching the namespace its token cache already uses release() also deleted the key unconditionally, so a holder whose lock PX-expired and was re-acquired by another worker could delete the new holder's lock and let a third worker run a duplicate refresh, recreating the rotating refresh_token race. acquire now writes a unique per-acquisition token generated by the coordinator and release deletes only when the key still holds that token, via a compare-and-delete Lua script Adds regression tests: release with a stale token is a no-op while the owner's release deletes; keys are namespaced before reaching Redis; the coordinator acquires and releases with the same token

mateo-berri · 2026-06-27T19:07:00Z

@greptileai

…itellm_mcp_v2_authz_code_xrepl_2 # Conflicts: # litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py

DualCache swallows get/set errors internally but not delete, and the Redis layer underneath re-raises through its circuit breaker. So a Redis outage on the delete() path escaped CachedOAuthTokenStore.fetch()'s unauthorized branch (which deletes before returning None) and invalidate(), turning a cache blip into a 500 instead of the v1-style fallback. Catch in the backend so delete degrades to the TTL-bounded stale entry like get/set already do.

The merge from staging brought in ruff's line-length 120, but these two PR-authored files were still wrapped at the old width, so the diff-scoped ruff format --check in CI flagged them. Pure reformatting; no behavior change.

mateo-berri · 2026-06-27T21:15:52Z

@greptileai

mateo-berri · 2026-06-27T21:15:55Z

bugbot run

cursor

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

✅ Fixed: Lock TTL expires before refresh
- Added owner-only Redis lease renewal while the refresh task runs so the lock does not expire before refresh and persist complete.
✅ Fixed: Lazy store skips Redis forever
- Changed the lazy per-user OAuth store to rebuild a no-Redis chain once Redis becomes available.

_{You can send follow-ups to the cloud agent here.}

mateo-berri

If you could double check the:

Greptile - P1 Undecryptable cache strings raise instead of returning a cache miss
Bugbot - the high and med sev bug

see if they're legit concerns or not, that'd be great

CLAassistant · 2026-06-27T21:29:01Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ tin-berri
❌ cursoragent
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

…failures get/set now degrade a cache or codec failure to the safe value (miss / no-op) in the backend itself rather than relying on DualCache and decrypt_value_helper happening to swallow internally, matching delete() and v1's MCPPerUserTokenCache. This upholds the layer's boundary-failure-is-a-miss contract regardless of the injected collaborators, so a Redis outage or an undecryptable entry reads as a cache miss that re-reads the DB instead of a 500. Adds contract tests for the cache raising on get/set/delete and the codec raising on encode.

Greptile's out-of-diff repro had the decrypt reject a blob with ValueError (bad ciphertext after key rotation); cover that exact raise path, not just the decrypt-returns-None case, so get() is regression-locked to read it as a miss.

Replace the hand-written self._<arg> = arg constructors on OAuthTokenCacheCodec, RedisRefreshCoordinator, RedisDistributedLock, and DualCacheTokenCacheBackend with frozen slotted dataclasses, matching the rest of this layer. Fields take the former parameter names so the constructor API (and the tests' keyword args) are unchanged; KW_ONLY preserves the keyword-only collaborators.

tin-berri · 2026-06-27T22:18:34Z

@greptileai

tin-berri · 2026-06-27T22:18:37Z

bugbot run

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

✅ Fixed: Lazy rebuild races refresh
- Serialized lazy local-store fetches through Redis rebuild so an in-flight in-process refresh cannot overlap a new Redis-backed refresh.

_{You can send follow-ups to the cloud agent here.}

… lease TTL wait_timeout_seconds defaulted to the same 10s as lock_ttl_seconds, but the holder renews its lease while a slow token endpoint runs, so a loser waiting past 10s bailed and re-read the still-expired DB token, challenging the user even though a valid refresh was in flight. Bound the holder's renewal with a refresh budget so its lock-hold is finite, and set the loser's wait to outlast that budget (refresh_budget_seconds + one lease tail) so a loser only re-reads once the holder has finished or its bounded lease has lapsed, never mid-refresh.

mateo-berri · 2026-06-27T22:53:00Z

@greptileai

mateo-berri · 2026-06-27T22:53:02Z

bugbot run

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

✅ Fixed: Lazy lock serializes all fetches
- The lazy store now holds its coordination lock only for store selection and local-fetch accounting, allowing unrelated no-Redis fetches to run concurrently while still deferring Redis rebuilds until in-flight local fetches finish.

_{You can send follow-ups to the cloud agent here.}

mateo-berri · 2026-06-27T23:17:51Z

@greptileai

mateo-berri · 2026-06-27T23:17:54Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 9eea30f. Configure here.}

mateo-berri

LGTM; thanks!

…uth store [2/2] (BerriAI#31493) * feat(mcp): encrypt+serialize codec for caching OAuth tokens in Redis (step 1b §1.5) The serialize+encrypt boundary a cross-replica cache needs: a plaintext bearer in Redis is a leak, so encode() encrypts (NaCl in prod via the injected encrypt, identity in tests). Caches only access_token and expires_at, never the refresh_token - the hot path needs just the bearer, and the long-lived refresh_token stays in the DB (the refresh path is always a cache miss), matching v1. A decoded token always has refresh_token=None. Undecryptable (key rotation) or corrupt entries read as a miss. * feat(mcp): DualCache-backed token cache backend (step 1b §1.5) The cross-replica TokenCacheBackend implementation that plugs into the foundation's CachedOAuthTokenStore seam: encrypts+serializes the token via the codec and stores it in LiteLLM's shared DualCache under the same per-(user,server) key v1 used, so workers share one refresh and a token cached by v1 or v2 is readable by the other across the cutover. Cache and codec are injected; a non-positive TTL (already-expired token) is not cached, and a missing/corrupt entry reads as a miss. * feat(mcp): Redis SET NX PX refresh coordinator (step 1b §1.5) The cross-replica RefreshCoordinator that plugs into the foundation's RefreshingTokenStore seam: a SET NX PX lock elects one worker to refresh per (user, server) while the rest wait for it and re-read the token it persisted, so a rotating refresh_token is used once across the fleet, not once per worker. The lock self-expires (PX) so a crashed holder can't wedge refresh; a loser falls back to a bounded re-read and the surrounding store re-checks expiry next fetch, so a crash self-heals. The lock (a thin Redis SET NX/DEL/EXISTS wrapper in prod) is injected, so the single-flight logic is testable without Redis. * feat(mcp): Redis SET NX PX distributed lock (step 1b §1.5) The concrete DistributedLock the RedisRefreshCoordinator elects refreshers with: acquire is an atomic SET key NX PX ttl (first caller wins, entry self-expires so a crashed holder can't wedge refresh), release is DEL, is_held is EXISTS. The async Redis client is injected (the client from LiteLLM's RedisCache in prod), so it is unit-testable with a fake. Any Redis error degrades to not-acquired / not-held so a cache blip causes an extra refresh, never a crash on the resolve path. * feat(mcp): wire the cross-replica cache + coordinator into the per-user store (step 1b §1.5) Upgrade the composition root to use the DualCache-backed cache and SET NX PX refresh coordinator when Redis is wired, falling back to the foundation's in-process defaults on a single replica. Layers the cross-replica path on top of the single-replica dispatch store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): refresh on lock-backend error instead of serving a stale token The cross-replica refresh coordinator elected refreshers with a boolean acquire: a Redis transport error was caught and returned as False, which is indistinguishable from "another worker holds the lock". On a total Redis outage every worker therefore took the wait-then-reread branch and served the still-expired token upstream (the upstream then 401s), even though the lock and coordinator docstrings claimed a Redis blip "degrades to an extra refresh". Make acquire tristate (LockAcquisition: ACQUIRED / HELD / ERROR) so the coordinator can tell a busy holder from a dead backend, and refresh anyway on ERROR. This single-flight lock is a load optimization, not a correctness mutex, so failing open is correct: it degrades a lock-backend outage to the no-coordinator behavior (an extra refresh), never a stale bearer. Add a regression test asserting an acquire error refreshes rather than re-reading the expired token, and update the docstrings to match. * style(mcp): wrap redis lock signatures at line-length 88 for CI ruff format * fix(mcp): a refresh loser surfaces None, not a stale token, when the winner failed The cross-replica coordinator's losers re-read the token the winner persisted. If the winner's refresh failed, the store still holds the expired token, so the loser re-read it and RefreshingTokenStore handed that expired bearer to the caller (the upstream then 401s) instead of the re-auth challenge the winner returned via None. Make the loser's re-read expiry-aware, mirroring refresh_latest_token: a re-read that is still expired surfaces None so the arm challenges. This only affects the loser path; the winner's freshly refreshed token is returned directly by the coordinator and is unaffected. * fix(mcp): log per-user token decrypt failures at debug, matching v1 When a cached blob cannot be decrypted (e.g. after a salt or master-key rotation) the codec logged a full traceback at error level, since decrypt_value_helper defaults to exception_type=error. v1's MCPPerUserTokenCache passed exception_type=debug on the same path. The blob is ciphertext so this is log noise only, but matching v1 avoids error-level traceback spam on stale entries after a key rotation * fix(mcp): namespace the refresh lock key and fence its release with a token The Redis lock wrote its key through the raw client from init_async_client(), bypassing RedisCache's namespace, so two deployments sharing one Redis collided on mcp:refresh_lock:<user>:<server> for any overlapping (user, server) and a colliding deployment skipped the refresh and challenged its own users. The lock now runs every key through an injected namespace_key wired to RedisCache.check_and_fix_namespace, matching the namespace its token cache already uses release() also deleted the key unconditionally, so a holder whose lock PX-expired and was re-acquired by another worker could delete the new holder's lock and let a third worker run a duplicate refresh, recreating the rotating refresh_token race. acquire now writes a unique per-acquisition token generated by the coordinator and release deletes only when the key still holds that token, via a compare-and-delete Lua script Adds regression tests: release with a stale token is a no-op while the owner's release deletes; keys are namespaced before reaching Redis; the coordinator acquires and releases with the same token * fix(mcp): fail open when the per-user token cache delete errors DualCache swallows get/set errors internally but not delete, and the Redis layer underneath re-raises through its circuit breaker. So a Redis outage on the delete() path escaped CachedOAuthTokenStore.fetch()'s unauthorized branch (which deletes before returning None) and invalidate(), turning a cache blip into a 500 instead of the v1-style fallback. Catch in the backend so delete degrades to the TTL-bounded stale entry like get/set already do. * style(mcp): reformat outbound-credentials files to line-length 120 The merge from staging brought in ruff's line-length 120, but these two PR-authored files were still wrapped at the old width, so the diff-scoped ruff format --check in CI flagged them. Pure reformatting; no behavior change. * fix: harden mcp oauth redis refresh coordination * fix(mcp): make the per-user token cache backend airtight on boundary failures get/set now degrade a cache or codec failure to the safe value (miss / no-op) in the backend itself rather than relying on DualCache and decrypt_value_helper happening to swallow internally, matching delete() and v1's MCPPerUserTokenCache. This upholds the layer's boundary-failure-is-a-miss contract regardless of the injected collaborators, so a Redis outage or an undecryptable entry reads as a cache miss that re-reads the DB instead of a 500. Adds contract tests for the cache raising on get/set/delete and the codec raising on encode. * test(mcp): pin per-user cache get() to a miss when decrypt raises Greptile's out-of-diff repro had the decrypt reject a blob with ValueError (bad ciphertext after key rotation); cover that exact raise path, not just the decrypt-returns-None case, so get() is regression-locked to read it as a miss. * refactor(mcp): use frozen dataclasses for the trivial DI constructors Replace the hand-written self._<arg> = arg constructors on OAuthTokenCacheCodec, RedisRefreshCoordinator, RedisDistributedLock, and DualCacheTokenCacheBackend with frozen slotted dataclasses, matching the rest of this layer. Fields take the former parameter names so the constructor API (and the tests' keyword args) are unchanged; KW_ONLY preserves the keyword-only collaborators. * fix: serialize lazy per-user oauth store rebuild * fix(mcp): stop losers challenging mid-refresh by decoupling wait from lease TTL wait_timeout_seconds defaulted to the same 10s as lock_ttl_seconds, but the holder renews its lease while a slow token endpoint runs, so a loser waiting past 10s bailed and re-read the still-expired DB token, challenging the user even though a valid refresh was in flight. Bound the holder's renewal with a refresh budget so its lock-hold is finite, and set the loser's wait to outlast that budget (refresh_budget_seconds + one lease tail) so a loser only re-reads once the holder has finished or its bounded lease has lapsed, never mid-refresh. * fix: allow concurrent lazy OAuth fetches without Redis --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>

greptile-apps Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/redis_distributed_lock.py Outdated

veria-ai Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py

Base automatically changed from litellm_mcp_v2_authz_code_dispatch to litellm_internal_staging June 27, 2026 04:20

tin-berri and others added 9 commits June 26, 2026 21:22

style(mcp): wrap redis lock signatures at line-length 88 for CI ruff …

698b3d8

…format

tin-berri force-pushed the litellm_mcp_v2_authz_code_xrepl_2 branch from 0b11864 to 49781f6 Compare June 27, 2026 04:24

tin-berri requested a review from mateo-berri June 27, 2026 17:27

greptile-apps Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/dual_cache_token_backend.py Outdated

tin-berri added 3 commits June 27, 2026 13:07

Merge remote-tracking branch 'origin/litellm_internal_staging' into l…

6f17ea9

…itellm_mcp_v2_authz_code_xrepl_2 # Conflicts: # litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py

tin-berri mentioned this pull request Jun 27, 2026

feat(mcp): migrate the token_exchange (OBO) arm to the v2 resolver #31526

Merged

4 tasks

veria-ai Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/dual_cache_token_backend.py Outdated

cursor Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/redis_refresh_coordinator.py Outdated

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py Outdated

mateo-berri reviewed Jun 27, 2026

View reviewed changes

mateo-berri requested changes Jun 27, 2026

View reviewed changes

mateo-berri reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/token_cache_codec.py Outdated

fix: harden mcp oauth redis refresh coordination

8853089

tin-berri added 3 commits June 27, 2026 14:34

greptile-apps Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/redis_refresh_coordinator.py

cursor Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py Outdated

fix: serialize lazy per-user oauth store rebuild

a0eefcf

veria-ai Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py Outdated

cursor Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py Outdated

greptile-apps Bot reviewed Jun 27, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.py Outdated

fix: allow concurrent lazy OAuth fetches without Redis

9eea30f

cursor Bot reviewed Jun 27, 2026

View reviewed changes

tin-berri requested a review from mateo-berri June 27, 2026 23:25

mateo-berri approved these changes Jun 27, 2026

View reviewed changes

tin-berri merged commit 5963b93 into litellm_internal_staging Jun 27, 2026
125 checks passed

tin-berri deleted the litellm_mcp_v2_authz_code_xrepl_2 branch June 27, 2026 23:27

Uh oh!

Uh oh!

Conversation

tin-berri commented Jun 27, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Re-open context

What this is

Why

Changes

Type

Proof of fix

Uh oh!

greptile-apps Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

T-Rex Logs

Uh oh!

Uh oh!

codecov Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

veria-ai Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR overview

Security review

Uh oh!

mateo-berri commented Jun 27, 2026

Uh oh!

Uh oh!

Uh oh!

mateo-berri commented Jun 27, 2026

Uh oh!

mateo-berri commented Jun 27, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mateo-berri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CLAassistant commented Jun 27, 2026

Uh oh!

tin-berri commented Jun 27, 2026

Uh oh!

tin-berri commented Jun 27, 2026

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mateo-berri commented Jun 27, 2026

Uh oh!

mateo-berri commented Jun 27, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mateo-berri commented Jun 27, 2026

Uh oh!

mateo-berri commented Jun 27, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

tin-berri commented Jun 27, 2026 •

edited by cursor Bot

Loading

greptile-apps Bot commented Jun 27, 2026 •

edited

Loading

codecov Bot commented Jun 27, 2026 •

edited

Loading

veria-ai Bot commented Jun 27, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading