Fix OAuthClientProvider._initialize() to enable transparent token refresh

[draesthetic]

Summary

Fix two bugs in OAuthClientProvider._initialize() that combine to force interactive re-authentication on every process restart, even when a valid refresh_token is on disk and the IdP would happily exchange it.

Closes #3250.

The bugs

Bug 1: _initialize() doesn't compute token_expiry_time

_initialize() loads current_tokens from storage but never calls context.update_token_expiry(token). So context.token_expiry_time stays None.

Then is_token_valid():

def is_token_valid(self) -> bool:
    return bool(
        self.current_tokens
        and self.current_tokens.access_token
        and (not self.token_expiry_time or time.time() <= self.token_expiry_time)
    )

When token_expiry_time is None, the second clause is not None or … = True, so the function unconditionally returns True regardless of whether the access_token is expired.

The refresh-on-expiry guard in async_auth_flow:

if not self.context.is_token_valid() and self.context.can_refresh_token():
    refresh_request = await self._refresh_token()
    …

…never fires. Expired access_tokens are sent on every request, the server returns 401, and the user lands in the full-re-auth branch.

This same fix is already applied on the write path: set_tokens() does call update_token_expiry(). The read path (_initialize) just doesn't do the same thing.

Bug 2: _refresh_token() builds the wrong endpoint URL when oauth_metadata isn't loaded

_refresh_token() picks the token endpoint like this:

if self.context.oauth_metadata and self.context.oauth_metadata.token_endpoint:
    token_url = str(self.context.oauth_metadata.token_endpoint)
else:
    auth_base_url = self.context.get_authorization_base_url(self.context.server_url)
    token_url = urljoin(auth_base_url, "/token")

For a server like https://mcp.fold.money/mcp, the fallback path produces https://mcp.fold.money/token — 404. The correct endpoint for Hydra-style servers is https://mcp.fold.money/oauth/token.

oauth_metadata is normally populated via server discovery during the 401-handling flow (after a 401). But the refresh-on-expiry path runs before any 401 — it proactively refreshes when the token is expired, with no 401 yet. So oauth_metadata is never populated, and refresh fails silently with 404.

The fix

In src/mcp/client/auth/oauth2.py, modify OAuthClientProvider._initialize() to:

1. Call context.update_token_expiry(current_tokens) after loading tokens — mirrors what set_tokens does on the write path.
2. Optionally call storage.load_oauth_metadata() via a getattr guard — pre-populates oauth_metadata if the storage implementation provides it. SDK-provided storages that don't implement this method remain unaffected; downstream clients (e.g. hermes-agent's HermesTokenStorage) that already implement it get the fix transparently.

The TokenStorage Protocol is unchanged — load_oauth_metadata is treated as an optional extension. No breaking changes.

async def _initialize(self) -> None:
    """Load stored tokens and client info."""
    self.context.current_tokens = await self.context.storage.get_tokens()
    self.context.client_info = await self.context.storage.get_client_info()
    # Compute the absolute expiry time from the loaded token's relative
    # `expires_in`. Without this, `is_token_valid()` short-circuits to True
    # when `token_expiry_time` is None and the refresh-on-expiry path in
    # `async_auth_flow` never fires — every process restart sends an
    # expired access_token, gets a 401, and lands in the full re-auth
    # branch. Mirrors what `set_tokens` does on the write path.
    if self.context.current_tokens is not None:
        self.context.update_token_expiry(self.context.current_tokens)
    # Optionally load OAuth metadata from storage. Some downstream storage
    # implementations (e.g. ones persisting `.meta.json` from server
    # discovery) implement this; SDK-provided storages do not, so the
    # `getattr` guard keeps this a no-op when absent.
    #
    # Without this, `_refresh_token` falls back to `urljoin(base, "/token")`
    # which gives the wrong endpoint for any IdP that mounts its token
    # endpoint at a non-root path (Hydra/Ory-style: /oauth/token, Auth0:
    # /oauth/token, Keycloak: /protocol/openid-connect/token, etc.) and
    # silently 404s the refresh grant.
    loader = getattr(self.context.storage, "load_oauth_metadata", None)
    if callable(loader):
        try:
            meta = loader()
            if inspect.iscoroutine(meta):
                meta = await meta
            if meta is not None:
                self.context.oauth_metadata = meta  # type: ignore[assignment]
        except Exception:
            # Storage implementations are user-provided; a misbehaving
            # metadata loader must not break auth initialization. The
            # 401-handling path will populate `oauth_metadata` via server
            # discovery as a fallback.
            pass
    self._initialized = True

Tests

Three new tests added in tests/client/test_auth.py (in the existing TestOAuthFallback class):

• test_initialize_computes_token_expiry_time — verifies that _initialize populates context.token_expiry_time from expires_in, and that an expired token (clamped expires_in=0) correctly produces is_token_valid() == False.
• test_initialize_loads_oauth_metadata_from_storage — verifies that _initialize calls storage.load_oauth_metadata() and that the loaded metadata is actually used by _refresh_token() to build the correct endpoint URL (https://hydra.example.com/oauth2/token, not /token).
• test_initialize_metadata_loader_failure_is_non_fatal — verifies that a misbehaving load_oauth_metadata (raises RuntimeError) does not break auth init.

All 3 tests fail against the unpatched code (negative control) and pass against the patched code.

Full test suite: 5,333 tests pass, 10 skipped, 1 xfailed (pre-existing). Pyright clean on both modified files.

Live verification

Patched locally against mcp==1.28.1 on macOS (Hermes agent 0.20.0). 16-minute live repro against https://mcp.fold.money:

1. Login via OAuth → fresh token, mtime T0.
2. Wait 15 min past access_token expiry → token on disk is stale, mtime still T0 (SDK never touched file).
3. Make MCP call with forced-expired access_token + fresh refresh_token.
4. Without the fix: SDK sends expired token, gets 401, falls through to re-auth (browser prompt).
5. With the fix: SDK calls https://mcp.fold.money/oauth/token with grant_type=refresh_token, gets HTTP 200 with fresh rotated pair, writes back to disk. MCP call returns real data (verified: get_total_balance → ₹146,656.35 across 4 bank accounts).

AI disclosure

Drafted with AI assistance (GPT-class model). The bug analysis, code path tracing, fix design, live verification, and tests were all done by a human reviewer who understood every line. The fix itself is 12 lines of source + 3 tests, all under 100 lines total.

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/mcp/client/auth/oauth2.py">

<violation number="1" location="src/mcp/client/auth/oauth2.py:580">
P2: Refresh can fail before any network call when `load_oauth_metadata()` returns non-model data, because `_initialize()` stores it without validation and `_refresh_token()` assumes an `OAuthMetadata` object. Validating/coercing `meta` to `OAuthMetadata` here keeps optional storage extensions non-fatal as intended.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Refresh can fail before any network call when load_oauth_metadata() returns non-model data, because _initialize() stores it without validation and _refresh_token() assumes an OAuthMetadata object. Validating/coercing meta to OAuthMetadata here keeps optional storage extensions non-fatal as intended.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/auth/oauth2.py, line 580:

<comment>Refresh can fail before any network call when `load_oauth_metadata()` returns non-model data, because `_initialize()` stores it without validation and `_refresh_token()` assumes an `OAuthMetadata` object. Validating/coercing `meta` to `OAuthMetadata` here keeps optional storage extensions non-fatal as intended.</comment>

<file context>
@@ -551,6 +552,38 @@ async def _initialize(self) -> None:
+                if inspect.iscoroutine(meta):
+                    meta = await meta
+                if meta is not None:
+                    self.context.oauth_metadata = meta  # type: ignore[assignment]
+            except Exception:
+                # Storage implementations are user-provided; a misbehaving
</file context>

Suggested change

	self.context.oauth_metadata = meta # type: ignore[assignment]
	self.context.oauth_metadata = (
	meta if isinstance(meta, OAuthMetadata) else OAuthMetadata.model_validate(meta)
	)