Repository navigation
feat(mcp): cross-replica single-flight refresh for the v2 per-user OAuth store [2/2] #31474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
tin-berri
merged 8 commits into
litellm_mcp_v2_authz_code_dispatch
from
litellm_mcp_v2_authz_code_xrepl
Jun 27, 2026
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1b436b9
feat(mcp): encrypt+serialize codec for caching OAuth tokens in Redis …
tin-berri 494d1ca
feat(mcp): DualCache-backed token cache backend (step 1b §1.5)
tin-berri 7bbc859
feat(mcp): Redis SET NX PX refresh coordinator (step 1b §1.5)
tin-berri c53b95b
feat(mcp): Redis SET NX PX distributed lock (step 1b §1.5)
tin-berri 70e1dbe
feat(mcp): wire the cross-replica cache + coordinator into the per-us…
tin-berri d29e2dc
fix(mcp): refresh on lock-backend error instead of serving a stale token
tin-berri 33c8333
style(mcp): wrap redis lock signatures at line-length 88 for CI ruff …
tin-berri 75ea65d
fix(mcp): a refresh loser surfaces None, not a stale token, when the …
tin-berri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
65 changes: 65 additions & 0 deletions
65
litellm/proxy/_experimental/mcp_server/outbound_credentials/dual_cache_token_backend.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| """Cross-replica ``TokenCacheBackend``: stores the token in LiteLLM's shared ``DualCache``. | ||
|
|
||
| Plugs into the foundation's ``CachedOAuthTokenStore`` via the ``TokenCacheBackend`` seam. The token is | ||
| encrypted + serialized by the injected codec and written under a per-``(user, server)`` key with the | ||
| given TTL, so every worker reads one refresh rather than each re-reading and re-refreshing - matching | ||
| v1's ``MCPPerUserTokenCache`` (same NaCl encryption and key, so a token cached by either is readable by | ||
| the other across the cutover). A missing or undecryptable entry reads as a miss. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import Protocol | ||
|
|
||
| from litellm.proxy._experimental.mcp_server.outbound_credentials.oauth_token_store import ( | ||
| OAuthToken, | ||
| ) | ||
| from litellm.proxy._experimental.mcp_server.outbound_credentials.token_cache_codec import ( | ||
| OAuthTokenCacheCodec, | ||
| ) | ||
|
|
||
|
|
||
| class AsyncCache(Protocol): | ||
| """The slice of LiteLLM's ``DualCache`` this backend needs (Redis-backed, shared across workers).""" | ||
|
|
||
| async def async_get_cache(self, key: str) -> object | None: ... | ||
|
|
||
| async def async_set_cache( | ||
| self, key: str, value: str, ttl: float | None = None | ||
| ) -> None: ... | ||
|
|
||
| async def async_delete_cache(self, key: str) -> None: ... | ||
|
|
||
|
|
||
| class DualCacheTokenCacheBackend: | ||
| def __init__( | ||
| self, | ||
| cache: AsyncCache, | ||
| codec: OAuthTokenCacheCodec, | ||
| *, | ||
| key_prefix: str = "mcp:per_user_token:", | ||
| ) -> None: | ||
| self._cache = cache | ||
| self._codec = codec | ||
| self._key_prefix = key_prefix | ||
|
|
||
| def _key(self, user_id: str, server_id: str) -> str: | ||
| return f"{self._key_prefix}{user_id}:{server_id}" | ||
|
|
||
| async def get(self, user_id: str, server_id: str) -> OAuthToken | None: | ||
| blob = await self._cache.async_get_cache(self._key(user_id, server_id)) | ||
| return self._codec.decode(blob) if isinstance(blob, str) else None | ||
|
|
||
|
tin-berri marked this conversation as resolved.
Resolved
|
||
| async def set( | ||
| self, user_id: str, server_id: str, token: OAuthToken, ttl_seconds: float | ||
| ) -> None: | ||
| if ttl_seconds <= 0: | ||
| return | ||
| await self._cache.async_set_cache( | ||
| self._key(user_id, server_id), | ||
| self._codec.encode(token), | ||
| ttl=ttl_seconds, | ||
| ) | ||
|
|
||
| async def delete(self, user_id: str, server_id: str) -> None: | ||
| await self._cache.async_delete_cache(self._key(user_id, server_id)) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
litellm/proxy/_experimental/mcp_server/outbound_credentials/redis_distributed_lock.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| """Concrete ``DistributedLock`` over a Redis client: ``SET NX PX`` / ``DEL`` / ``EXISTS``. | ||
|
|
||
| The cross-replica lock the ``RedisRefreshCoordinator`` elects refreshers with. ``acquire`` is an | ||
| atomic ``SET key NX PX ttl`` (only the first caller wins; the entry self-expires so a crashed holder | ||
| can't wedge refresh), ``release`` is ``DEL``, ``is_held`` is ``EXISTS``. The Redis client is injected | ||
| (in production the async client from LiteLLM's ``RedisCache``), so the lock is unit-testable with a | ||
| fake. A transport error on ``acquire`` returns ``LockAcquisition.ERROR`` - distinct from ``HELD`` - so | ||
| the coordinator refreshes anyway instead of mistaking a dead backend for a busy holder; a Redis blip | ||
| degrades to an extra refresh, never a stale bearer. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import Protocol | ||
|
|
||
| from litellm._logging import verbose_logger | ||
| from litellm.proxy._experimental.mcp_server.outbound_credentials.redis_refresh_coordinator import ( | ||
| LockAcquisition, | ||
| ) | ||
|
|
||
|
|
||
| class RedisCommands(Protocol): | ||
| """The slice of the async Redis client this lock needs.""" | ||
|
|
||
| async def set( | ||
| self, name: str, value: str, *, nx: bool = False, px: int | None = None | ||
| ) -> object | None: ... | ||
|
|
||
| async def delete(self, *names: str) -> int: ... | ||
|
|
||
| async def exists(self, *names: str) -> int: ... | ||
|
|
||
|
|
||
| class RedisDistributedLock: | ||
| def __init__(self, client: RedisCommands) -> None: | ||
| self._client = client | ||
|
|
||
| async def acquire(self, key: str, ttl_seconds: float) -> LockAcquisition: | ||
| try: | ||
| result = await self._client.set( | ||
| key, "1", nx=True, px=int(ttl_seconds * 1000) | ||
| ) | ||
| # Degrade on any Redis client error: redis.exceptions narrows only via an import that | ||
| # is Unknown under basedpyright, and the lock must never crash the resolve path. | ||
| except Exception as exc: # noqa: BLE001 | ||
| verbose_logger.warning("RedisDistributedLock.acquire failed: %s", exc) | ||
| return LockAcquisition.ERROR | ||
| return LockAcquisition.ACQUIRED if result is not None else LockAcquisition.HELD | ||
|
|
||
| async def release(self, key: str) -> None: | ||
| try: | ||
| await self._client.delete(key) | ||
| # Degrade on any Redis client error: redis.exceptions narrows only via an import that | ||
| # is Unknown under basedpyright, and the lock must never crash the resolve path. | ||
| except Exception as exc: # noqa: BLE001 | ||
| verbose_logger.warning("RedisDistributedLock.release failed: %s", exc) | ||
|
|
||
| async def is_held(self, key: str) -> bool: | ||
| try: | ||
| return await self._client.exists(key) > 0 | ||
| # Degrade on any Redis client error: redis.exceptions narrows only via an import that | ||
| # is Unknown under basedpyright, and the lock must never crash the resolve path. | ||
| except Exception as exc: # noqa: BLE001 | ||
| # On error, report "not held" so a waiter stops waiting and re-reads rather than blocking. | ||
| verbose_logger.warning("RedisDistributedLock.is_held failed: %s", exc) | ||
| return False |
102 changes: 102 additions & 0 deletions
102
litellm/proxy/_experimental/mcp_server/outbound_credentials/redis_refresh_coordinator.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| """Cross-replica ``RefreshCoordinator``: one refresh per ``(user, server)`` across all workers. | ||
|
|
||
| Plugs into the foundation's ``RefreshingTokenStore`` via the ``RefreshCoordinator`` seam. A ``SET NX | ||
| PX`` lock elects one worker to run the refresh 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 | ||
| auto-expires (``PX``), so a crashed holder can't wedge refresh; a loser that times out (or whose | ||
| holder crashed mid-refresh) falls back to a re-read, and the surrounding store re-checks expiry on the | ||
| next fetch, so a crash self-heals rather than serving stale forever. Reading needs no lock, so losers | ||
| don't serialize behind each other. The lock is injected (a thin Redis ``SET NX``/``DEL``/``EXISTS`` | ||
| wrapper in production, a fake in tests). | ||
|
|
||
| The lock is a single-flight optimization, not a correctness mutex, so it fails open: when the lock | ||
| backend is unreachable, ``acquire`` reports ``ERROR`` (distinct from ``HELD``) and this coordinator | ||
| refreshes anyway rather than wait on a holder that may not exist and then serve a still-expired token. | ||
| That degrades a Redis outage to the no-coordinator behavior (each worker may refresh), never a stale | ||
| bearer the upstream would 401. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| import time | ||
| from collections.abc import Awaitable, Callable | ||
| from enum import Enum | ||
| from typing import Protocol | ||
|
|
||
| from litellm.proxy._experimental.mcp_server.outbound_credentials.oauth_token_store import ( | ||
| OAuthToken, | ||
| ) | ||
|
|
||
|
|
||
| class LockAcquisition(Enum): | ||
| """Outcome of a best-effort ``acquire``. ``ERROR`` is kept distinct from ``HELD`` so a caller can | ||
| tell "someone else is refreshing" (wait and re-read) from "the lock backend is down" (no election | ||
| happened, so refresh anyway) instead of conflating both into a single ``False``.""" | ||
|
|
||
| ACQUIRED = "acquired" # won the election; this worker refreshes | ||
| HELD = "held" # another worker holds it; wait then re-read | ||
| ERROR = "error" # lock backend unreachable; holder unknown, so refresh anyway | ||
|
|
||
|
|
||
| class DistributedLock(Protocol): | ||
| """A best-effort cross-replica lock. ``acquire`` is ``SET key NX PX ttl`` reported as a | ||
| ``LockAcquisition`` (won / held by another / backend error); ``release`` is ``DEL``; ``is_held`` | ||
| is ``EXISTS`` (so a waiter can poll without taking the lock).""" | ||
|
|
||
| async def acquire(self, key: str, ttl_seconds: float) -> LockAcquisition: ... | ||
|
|
||
| async def release(self, key: str) -> None: ... | ||
|
|
||
| async def is_held(self, key: str) -> bool: ... | ||
|
|
||
|
|
||
| class RedisRefreshCoordinator: | ||
| def __init__( | ||
| self, | ||
| lock: DistributedLock, | ||
| *, | ||
| key_prefix: str = "mcp:refresh_lock:", | ||
| lock_ttl_seconds: float = 10.0, | ||
| wait_timeout_seconds: float = 10.0, | ||
| poll_interval_seconds: float = 0.05, | ||
| sleep: Callable[[float], Awaitable[None]] = asyncio.sleep, | ||
| clock: Callable[[], float] = time.monotonic, | ||
| ) -> None: | ||
| self._lock = lock | ||
| self._key_prefix = key_prefix | ||
| self._lock_ttl_seconds = lock_ttl_seconds | ||
| self._wait_timeout_seconds = wait_timeout_seconds | ||
| self._poll_interval_seconds = poll_interval_seconds | ||
| self._sleep = sleep | ||
| self._clock = clock | ||
|
|
||
| def _key(self, user_id: str, server_id: str) -> str: | ||
| return f"{self._key_prefix}{user_id}:{server_id}" | ||
|
|
||
| async def run( | ||
| self, | ||
| user_id: str, | ||
| server_id: str, | ||
| refresh: Callable[[], Awaitable[OAuthToken | None]], | ||
| reread: Callable[[], Awaitable[OAuthToken | None]], | ||
| ) -> OAuthToken | None: | ||
| key = self._key(user_id, server_id) | ||
| match await self._lock.acquire(key, self._lock_ttl_seconds): | ||
| case LockAcquisition.ACQUIRED: | ||
| try: | ||
| return await refresh() | ||
| finally: | ||
| await self._lock.release(key) | ||
| case LockAcquisition.ERROR: | ||
| # No election happened (lock backend down), so waiting would just re-read the | ||
| # still-expired token. Refresh anyway; worst case is an extra refresh, not a stale bearer. | ||
| return await refresh() | ||
| case LockAcquisition.HELD: | ||
| # Another worker holds the lock; wait for it to finish (release or PX-expiry), then read | ||
| # the token it persisted - the winner wrote the fresh token to the store, so a plain | ||
| # re-read sees it without us refreshing again. | ||
| deadline = self._clock() + self._wait_timeout_seconds | ||
| while self._clock() < deadline and await self._lock.is_held(key): | ||
| await self._sleep(self._poll_interval_seconds) | ||
| return await reread() |
37 changes: 37 additions & 0 deletions
37
litellm/proxy/_experimental/mcp_server/outbound_credentials/token_cache_codec.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| """Serialize + encrypt boundary for caching an OAuth token in a shared (Redis) cache. | ||
|
|
||
| A cross-replica cache must serialize the token, and a plaintext bearer in Redis is a leak, so this | ||
| encrypts the value (NaCl in production via the injected ``encrypt``, identity in tests). It caches | ||
| **only** the ``access_token``: the hot path needs just the bearer, expiry is carried by the cache | ||
| entry's TTL (set from the token's ``expires_at`` by the cache), and the long-lived refresh_token stays | ||
| in the DB - the refresh path is always a cache miss that re-reads it - so it never reaches Redis. A | ||
| decoded token therefore carries only the bearer (``expires_at`` and ``refresh_token`` both None); the | ||
| TTL, not the value, bounds its life. An empty/undecryptable blob (e.g. master-key rotation) is a miss. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from collections.abc import Callable | ||
|
|
||
| from litellm.proxy._experimental.mcp_server.outbound_credentials.oauth_token_store import ( | ||
| OAuthToken, | ||
| ) | ||
|
|
||
|
|
||
| class OAuthTokenCacheCodec: | ||
| def __init__( | ||
| self, | ||
| encrypt: Callable[[str], str], | ||
| decrypt: Callable[[str], str | None], | ||
| ) -> None: | ||
| self._encrypt = encrypt | ||
| self._decrypt = decrypt | ||
|
|
||
| def encode(self, token: OAuthToken) -> str: | ||
| return self._encrypt(token.access_token) | ||
|
|
||
| def decode(self, blob: str) -> OAuthToken | None: | ||
| access_token = self._decrypt(blob) | ||
| if not access_token: | ||
| return None | ||
| return OAuthToken(access_token=access_token, refresh_token=None) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async_get_cache()reads throughDualCache, and on a Redis hitDualCachewrites the blob into its in-memory cache without the token's remaining Redis TTL (dual_cache.pylines 247-250). BecauseOAuthTokenCacheCodec.decode()returnsexpires_at=None, later calls hit that local entry for the default in-memory TTL and serve the bearer after the original OAuth expiry/skew window. This breaks the expiry guarantee for tokens learned from Redis; bypass the local layer here or store/validate expiry in the cached value.Artifacts
Repro: focused runtime script exercising DualCache token read-through TTL behavior
Stack trace captured during the T-Rex run