fix(auth): keep retrying a failed OAuth refresh instead of giving up for good - #42
Open
ankit-thebigred wants to merge 1 commit into
Open
Conversation
…for good scheduleTokenRefresh() retried three times over a ~15 second window (5s, 10s) and then set nextRefreshAt = null and returned. Nothing re-arms the timer after that, so the process is left with no scheduled refresh at all. The access token runs out its remaining life and every request 401s with "OAuth access token has expired" until the user quits and relaunches the app. The failures that trigger this are overwhelmingly transient. On a laptop the common one is waking from sleep: the refresh timer is already due, it fires before the network is back, and all three attempts die on "getaddrinfo ENOTFOUND console.anthropic.com" inside 15 seconds. A captive portal or a brief offline moment does the same thing. A refresh token is valid for far longer than an access token, so a failure here almost never means the credential is actually dead. Changes: - On failure, re-arm instead of clearing. Backoff is 30s doubling to a 10 minute cap and keeps running, so the moment connectivity returns the next attempt succeeds and normal expiry-based scheduling resumes. authRequired is still emitted from the third consecutive failure so the UI can surface it, but it no longer means the process has stopped trying. - Single-flight the refresh. The token endpoint rotates refresh_token, so two concurrent POSTs carrying the same one leave the second rejected with 400. ensureOAuthToken() is called at the start of every turn and there is no guard, so a chat turn and a subagent turn starting together can race. Observed in the wild as repeated "token refresh failed: 400". Concurrent callers now share one in-flight request. - Re-check token health when the timer fires. ensureOAuthToken() may already have refreshed while the timer was pending, in which case just re-arm. - ensureOAuthToken() also arms a background retry on failure, so the on-demand path can recover without a restart too. The same give-up-permanently shape exists in src/providers/codex.ts. Left alone here because I have no way to exercise that path.
|
@ankit-thebigred is attempting to deploy a commit to the DevApe Team on Vercel. A member of the Team first needs to authorize it. |
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.
What happens
Chats start failing with
API Error: 401 OAuth access token has expired. Re-authenticate to continue.Quitting and relaunching the app fixes it. It comes back hours later.Why
scheduleTokenRefresh()gives up permanently:Three attempts, 5s and 10s apart, so the whole window is ~15 seconds. After that the process has no scheduled refresh at all. The current access token keeps working until it expires, then every request 401s. Only a relaunch recovers, because that is the only thing that calls
ensureOAuthToken()again from a clean state.What triggers it
From one user's gateway log, 49 refresh attempts and 48 failures, in two flavours:
ENOTFOUND is wake-from-sleep. The timer was already due while the machine slept, so it fires immediately on wake, before the network is back. Three attempts inside 15 seconds is not enough to ride that out.
pmset -g logon the same machine shows maintenance sleep/wake cycles roughly hourly, so it has many chances to land badly. Captive portals and short offline moments do the same thing.A refresh token is valid far longer than an access token, so a failure here almost never means the credential is genuinely dead. Giving up permanently on a transient network error is the wrong default.
400 looks like a different bug feeding the same failure path. The token endpoint rotates
refresh_token, andensureOAuthToken()is called at the top of every turn with no concurrency guard, so a chat turn and a subagent turn starting together both POST the same refresh token. The server accepts the first and rejects the second withinvalid_grant. Three of those in a row and refresh is off for good.The fix
authRequiredstill fires from the third consecutive failure so the UI can surface it, but it no longer means the process has stopped trying.ensureOAuthToken()may already have refreshed while it was pending.ensureOAuthToken()arms a background retry on failure too, so the on-demand path can also recover without a restart.Verification
npm run typecheckclean.npm run test:claude-providerpasses against a real account on the patched build:Plus 8 checks over the compiled output and the new scheduling logic:
Being straight about the limits: the retry path is covered by static assertions against the compiled output plus a behavioural replica of the new scheduler, not by a unit test driving the real module, because the refresh internals are module-scoped and have no injection seam. Happy to add one if you would like the timer and fetch factored out to make that possible.
I also have not reproduced the 400 race directly, only inferred it from the log pattern and the absence of a guard. The single-flight change is cheap and correct regardless, but treat that half as reasoned rather than proven.
Not touched
src/providers/codex.tshas the same give-up-permanently shape. Left alone because I have no way to exercise that path.Related to #38, which reports the same user-visible symptom from the env-propagation angle.