fix: restore OAuth token expiry across process restarts - #3248
Open
dhruvkej9 wants to merge 2 commits into
Open
Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
OAuthClientProvider._initialize(and the client-credentials providers) reloadscurrent_tokensfrom storage but never restorestoken_expiry_time. The persistedOAuthTokenonly carries the relativeexpires_in, so on a fresh processis_token_valid()returnsTruefor an already-expired access token — a stale Bearer is sent and a 401 round-trip is wasted before re-authentication (mcp2cli issues #50, #57).Fix
Persist the absolute expiry and restore it on init:
expires_at: float | NonetoOAuthToken(absolute unix timestamp), with a doc comment explaining the mcp2cli reproduction that motivated it._handle_token_response,_handle_refresh_response).OAuthContext.restore_token_expiry()and call it from all three_initializemethods (base,ClientCredentialsOAuthProvider,PrivateKeyJwtOAuthProvider).Backwards compatible:
expires_atdefaults toNone; existing stored tokens simply re-auth once, then persist the absolute expiry going forward.Test
test_init_restores_expired_token_expiry— fails onmain(expired token reported valid), passes with the fix. 210 auth tests pass, ruff + pyright clean.