Repository navigation
fix: read refreshed access token from Set-Cookie header - #164
Merged
tim-thacker-nullify merged 1 commit intoAug 21, 2026
Merged
tim-thacker-nullify merged 1 commit into
tim-thacker-nullify merged 1 commit into
Conversation
The /auth/refresh_token endpoint delivers the new access token as an access_token cookie rather than in the JSON body, so every refresh failed with 'refresh returned empty access token'. Fall back to the cookie (and its Max-Age for expiry) when the body carries no token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0192iXsk3yw2yf5r3cbtNDKu
tim-thacker-nullify
approved these changes
Aug 21, 2026
tim-thacker-nullify
enabled auto-merge
August 21, 2026 10:54
tim-thacker-nullify
disabled auto-merge
August 21, 2026 10:56
tim-thacker-nullify
enabled auto-merge
August 21, 2026 10:57
Merged
via the queue into
Nullify-Platform:main
with commit Aug 21, 2026
c477a08
4 of 5 checks passed
tim-thacker-nullify
added a commit
that referenced
this pull request
Aug 21, 2026
…01 retry (#165) * fix(auth,client): cover token refresh with tests and repair the MCP 401 retry internal/auth/login.go had no test coverage, so refreshToken was broken from February until #164 and nothing caught it. The MCP transport's 401 retry resolved its replacement token through GetValidToken, which only refreshes once ExpiresAt has elapsed, so a revoked token yielded the same string and the retry never fired. - add login_test.go: refreshToken cookie/body/expiry paths, GetValidToken expiry branch, ForceRefreshToken - add auth.ForceRefreshToken, which refreshes regardless of stored expiry - add lib.RefreshNullifyToken; fixed token sources (--nullify-token, NULLIFY_TOKEN) report client.ErrTokenNotRefreshable instead of replaying the same value - refreshingAuthTransport takes a distinct refreshProvider for its 401 retry - back off after a failed refresh so a down provider stops putting a blocking refresh in front of every request - skip empty access_token cookies and honour Expires when Max-Age is absent Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QxQZtuPdw7fc8P8nb794hG * fix(client): back off a forced token refresh that makes no progress The 30s backoff added for the TTL path did not cover the 401 path, which this change to refreshProvider had just turned into a network call: every request against rejected credentials re-entered a blocking refresh under the write lock, forever. - refreshRetryAfter is a second clock, separate from the TTL one, because tokenProvider and refreshProvider are separate functions. - Arm it whenever a forced refresh cannot produce a token the caller has not already tried: provider error, the same token back, or an empty one. - The last two now leave the cache alone rather than overwriting a working token with the rejected or empty value. - Correct cookieLifetime's comment: its only caller cannot distinguish "no expiry given" from "expires now", and should not -- storing 0 would read as never-expires in GetValidToken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197YS8554gBqdNQihMBR7HQ * test(lib): cover RefreshNullifyToken's fixed-source branches RefreshNullifyToken shipped with no test, including the auth.ErrNotRefreshable -> client.ErrTokenNotRefreshable translation that decides whether the transport retries a 401. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197YS8554gBqdNQihMBR7HQ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
3 participants
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.
Summary
Token refresh in the CLI always fails with
refresh returned empty access token. The/auth/refresh_tokenendpoint returns 200 with metadata in the JSON body (expiresIn,user,queryParameters, ...) but delivers the new access token as aSet-Cookie: access_token=...; Max-Age=3600header, whichrefreshToken()ininternal/auth/login.gonever reads — it only looks foraccessTokenin the body.This change falls back to the
access_tokenresponse cookie (using itsMax-Agefor expiry) when the body carries no token. Body-based tokens still take precedence if the endpoint ever returns them. This unblocksGetValidTokenand the MCP server's refreshing transport for long-running sessions.Validation
go test ./internal/auth/andgolangci-lint run ./internal/auth/passcli statusrefreshed via the cookie and returned dataGenerated by Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.