Repository navigation
Conversation
maybeRefreshAPIToken cleared the stored refresh token on any refresh error. A single 503, 429, network error or malformed response therefore removed the only credential that could recover the session, forcing a new `render login` once the access token expired - even though the server would have accepted the refresh moments later. pkg/client/oauth now returns a typed *ResponseError carrying the HTTP status, OAuth error code and Retry-After (Error() strings are unchanged, so the device flow's authorization_pending check still works). The refresh path clears the refresh token only for a definitive invalid_grant (400/401); transport errors, deadlines, 429, 5xx, temporarily_unavailable, slow_down and malformed 200s keep it. There is still exactly one bounded refresh attempt.
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.
Summary
maybeRefreshAPITokencleared the stored refresh token on any refresh error. One transient 503, 429, network error or malformed response permanently removed the only credential that could recover the session. Because the access token can still be valid, the triggering command may even succeed — the user only finds out later, when the access token expires and every command demands a newrender login.This change:
pkg/client/oauthreturn a typed*ResponseErrorcarrying the HTTP status, OAuth error code andRetry-After(theError()strings are unchanged, so the device flow'sauthorization_pendinghandling is unaffected);invalid_grant(HTTP 400/401). Transport errors, deadlines, 429, 5xx,temporarily_unavailable,slow_downand malformed 200 responses keep it;Motivation
Reproduced with a token endpoint that briefly answered 503: after one command, the config no longer had a refresh token, and the next command after access-token expiry required a fresh login even though the endpoint was healthy again.
How to verify
go test ./pkg/client/... ./pkg/tui/views/...pkg/client/clientrefresh_test.go: 503/429/500/malformed/other-400 preserve the refresh token;invalid_grantclears it; a transport error leaves the config untouched; a later process recovers and rotates both tokens.pkg/client/oauth/oauth_test.go: each failure keeps its status, code and Retry-After and is classified correctly.pkg/tui/views/login_test.go: device polling still continues throughauthorization_pending.client.gochange fails all three refresh tests.go test ./...passes excepte2e/TestE2E, which fails identically onmainwithout credentials.Open questions, deliberately unchanged here: device-grant
slow_downstill aborts login, andRetry-Afteris recorded but not yet used for backoff.Release notes