Vectrum
Sign In
CodeyPublic
Like0 likesFork0 forks

claudecode: only a refusal counts as a refusal

Re-review caught the predicate being wrong. Refresh wrapped ErrExpired
for every non-200, and Source latched ErrExpired as "this login is
dead" — so a 502, a 503, or a 429 permanently disabled refresh for the
process and reported "your Claude Code login has expired" about a login
that was fine.

That is worst for 429, which is the only response this endpoint has
ever produced. The realistic first production refresh would have killed
auth for the whole session with a misleading message.

Now only 400/401/403 wrap ErrExpired and get remembered. Everything
else reports what happened and is retried on the next request. Being
wrong in the direction of asking again costs a round trip; being wrong
the other way costs the session.

The remembered refusal also keeps the server's reason rather than
collapsing into a bare ErrExpired, so the second and third failures say
as much as the first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Browse Files
Jjonicommitted 1 parent fdd4d83commit8823937

Showing 3 changed files with 173 additions and 22 deletions.

+11−3docs/superpowers/specs/2026-09-17-claude-code-oauth-design.mdView File
@@ -221,9 +221,17 @@ Four more things the write has to survive, none of them hypothetical:
221221 meaning of a field Claude Code relies on; keep the old token's expiry beside
222222 a rotated token and our own next startup reads it, decides the refresh token
223223 is dead, and refuses to send a request the server would have honoured.
224- Instead `Source` asks once and remembers the answer — a refusal sets a flag
225- and every later call returns `ErrExpired` without another round trip. An
226- answer from the server beats a timestamp we would have had to invent.
224+ Instead `Source` asks once and remembers the answer. An answer from the
225+ server beats a timestamp we would have had to invent.
226+
227+ What counts as an answer is the load-bearing part. Only 400/401/403 — the
228+ server saying this grant is no good — wrap `ErrExpired` and get remembered.
229+ A 429 or a 5xx does not, because latching one would disable refresh for the
230+ rest of the session on a login that is fine. 429 is precisely where that
231+ would bite: it is the only response this endpoint has ever given us, and we
232+ still do not know whether it means "slow down" or "no". Being wrong in the
233+ direction of asking again is cheap; being wrong the other way costs the user
234+ their session.
227235
228236 Because the endpoint has never once answered, none of this is verified against
229237 the live service. It is verified against `httptest` and against the real file's
+36−16internal/claudecode/refresh.goView File
@@ -58,12 +58,18 @@ var ErrExpired = errors.New("claudecode: your Claude Code login has expired, run
5858 // credentials and writes nothing — persisting is a separate decision, made by
5959 // Source, because whether it is safe depends on whether the server rotated.
6060 //
61- // A refusal wraps ErrExpired, so callers can classify it with errors.Is rather
62- // than by reading status codes. That includes a 429: on these OAuth surfaces a
63- // 429 is not reliable evidence of a rate limit — the Messages API answers a
64- // request shape it declines the same way, and every probe of this endpoint has
65- // returned one without ever completing an exchange — so advising someone to
66- // wait would be advice that may never come good.
61+ // A refusal — the server saying this grant is no good — wraps ErrExpired, so
62+ // callers can classify it with errors.Is rather than by reading status codes
63+ // back out of a string. Only 400/401/403 mean that.
64+ //
65+ // Everything else, 429 and 5xx included, is reported as an ordinary failure.
66+ // The distinction is load-bearing: the caller remembers a refusal and stops
67+ // asking, so treating a transient failure as one would disable refresh for the
68+ // rest of the session on a login that is perfectly fine. 429 is exactly where
69+ // that would bite, because it is the only response this endpoint has ever
70+ // given us — and we still do not know whether it means "slow down" or "no".
71+ // Being wrong in the direction of asking again is cheap; being wrong the other
72+ // way costs the user their session.
6773 func Refresh(ctx context.Context, h *http.Client, c *Credentials) (*Credentials, error) {
6874 ctx, cancel := context.WithTimeout(ctx, refreshTimeout)
6975 defer cancel()
@@ -91,8 +97,14 @@ func Refresh(ctx context.Context, h *http.Client, c *Credentials) (*Credentials,
9197 if err != nil {
9298 return nil, fmt.Errorf("claudecode: reading refresh response: %w", err)
9399 }
94- if resp.StatusCode != http.StatusOK {
95- return nil, fmt.Errorf("%w (refresh refused: %d %s)", ErrExpired, resp.StatusCode, clip(raw))
100+ switch resp.StatusCode {
101+ case http.StatusOK:
102+ case http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden:
103+ return nil, fmt.Errorf("%w (the server refused the refresh token: %d %s)",
104+ ErrExpired, resp.StatusCode, clip(raw))
105+ default:
106+ return nil, fmt.Errorf("claudecode: refresh failed (%d %s); if it keeps happening, run `claude` to log in again",
107+ resp.StatusCode, clip(raw))
96108 }
97109
98110 var out struct {
@@ -259,12 +271,14 @@ type Source struct {
259271 // home is where a rotated credential is written back. Empty disables
260272 // write-back entirely.
261273 home string
262- // refused records that the server has already rejected this refresh token.
263- // There is no stored refresh-token expiry to consult — the file's is not
264- // updated when a token rotates, so trusting it would refuse a request the
265- // server would have honoured. Asking once and remembering the answer is
266- // both more accurate and cheaper than guessing from a timestamp.
267- refused bool
274+ // refused holds the refusal the server gave us, if it gave one. There is no
275+ // stored refresh-token expiry to consult — the file's is not updated when a
276+ // token rotates, so trusting it would refuse a request the server would
277+ // have honoured. Asking once and remembering the answer is both more
278+ // accurate and cheaper than guessing from a timestamp. The error itself is
279+ // kept rather than a bare flag, so the reason survives to whoever asks
280+ // next instead of collapsing into an unexplained "expired".
281+ refused error
268282
269283 // OnWriteBackFailure is called when the server rotated the refresh token
270284 // but the new one could not be stored. That combination is the one this
@@ -308,15 +322,21 @@ func (s *Source) Token(ctx context.Context) (string, error) {
308322 // Once the server has refused this refresh token, it will refuse it again.
309323 // Remembering that is what keeps a dead login from spending a round trip
310324 // on every subsequent request.
311- if s.refused || s.creds.RefreshToken == "" {
325+ if s.refused != nil {
326+ return "", s.refused
327+ }
328+ if s.creds.RefreshToken == "" {
312329 return "", ErrExpired
313330 }
314331
315332 from := s.creds.RefreshToken
316333 next, err := Refresh(ctx, s.http, s.creds)
317334 if err != nil {
335+ // Only a refusal is remembered. A timeout, a 5xx, a 429 — those say
336+ // nothing about whether the token is good, and latching them would
337+ // turn a blip into a dead session.
318338 if errors.Is(err, ErrExpired) {
319- s.refused = true
339+ s.refused = err
320340 }
321341 return "", err
322342 }
+126−3internal/claudecode/refresh_test.goView File
@@ -85,9 +85,11 @@ func TestRefreshRejectedSaysToLogInAgain(t *testing.T) {
8585 }
8686 }
8787
88- // A 429 from these OAuth surfaces is not reliable evidence of a rate limit:
89- // the Messages API answers a request shape it declines the same way. Telling
90- // the user to wait would be advice that never comes good.
88+ // A 429 from these OAuth surfaces is not reliable evidence of a rate limit —
89+ // the Messages API answers a request shape it declines the same way — so the
90+ // advice points at `claude` rather than at waiting. But it is equally not
91+ // evidence of a refusal, so it must not wrap ErrExpired: that is what stops
92+ // the caller latching it and disabling refresh for the session.
9193 func TestRefreshRateLimitedDoesNotAdviseWaiting(t *testing.T) {
9294 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
9395 w.WriteHeader(http.StatusTooManyRequests)
@@ -103,6 +105,9 @@ func TestRefreshRateLimitedDoesNotAdviseWaiting(t *testing.T) {
103105 if !strings.Contains(err.Error(), "claude") {
104106 t.Errorf("a 429 should still point at `claude`, got %q", err)
105107 }
108+ if errors.Is(err, ErrExpired) {
109+ t.Errorf("a 429 must not be classified as a refusal, or the source latches it: %v", err)
110+ }
106111 }
107112
108113 func TestRefreshTransportErrorIsReported(t *testing.T) {
@@ -623,3 +628,121 @@ func TestRefreshErrorDoesNotDumpTheWholeBody(t *testing.T) {
623628 t.Errorf("error carried %d bytes of body; it should be clipped to %d", strings.Count(err.Error(), "Z"), maxErrorBody)
624629 }
625630 }
631+
632+ // A 429 is the only response this endpoint has ever produced, and we still do
633+ // not know whether it means "slow down" or "no". Latching it would disable
634+ // refresh for the rest of the session on a login that is fine, so it must not
635+ // be remembered as a refusal.
636+ func TestTransientFailuresDoNotPoisonTheSource(t *testing.T) {
637+ for _, status := range []int{
638+ http.StatusTooManyRequests,
639+ http.StatusInternalServerError,
640+ http.StatusBadGateway,
641+ http.StatusServiceUnavailable,
642+ } {
643+ t.Run(http.StatusText(status), func(t *testing.T) {
644+ var hits int
645+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
646+ hits++
647+ if hits == 1 {
648+ w.WriteHeader(status)
649+ io.WriteString(w, `{"error":"transient"}`)
650+ return
651+ }
652+ io.WriteString(w, `{"access_token":"fresh","refresh_token":"r1","expires_in":28800}`)
653+ }))
654+ defer srv.Close()
655+ t.Setenv(tokenURLEnv, srv.URL)
656+
657+ s := NewSource(liveCreds(), srv.Client(), "")
658+ if _, err := s.Token(context.Background()); err == nil {
659+ t.Fatal("the first call should surface the failure")
660+ } else if errors.Is(err, ErrExpired) {
661+ t.Errorf("a %d must not be reported as an expired login: %v", status, err)
662+ }
663+ // The server recovers; so must we.
664+ tok, err := s.Token(context.Background())
665+ if err != nil {
666+ t.Fatalf("the source latched a transient failure: %v", err)
667+ }
668+ if tok != "fresh" {
669+ t.Errorf("Token = %q, want fresh", tok)
670+ }
671+ })
672+ }
673+ }
674+
675+ // A real refusal, by contrast, is remembered — and the reason survives with it
676+ // rather than collapsing into an unexplained "expired".
677+ func TestRefusalIsRememberedWithItsReason(t *testing.T) {
678+ for _, status := range []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden} {
679+ t.Run(http.StatusText(status), func(t *testing.T) {
680+ var hits int
681+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
682+ hits++
683+ w.WriteHeader(status)
684+ io.WriteString(w, `{"error":"invalid_grant"}`)
685+ }))
686+ defer srv.Close()
687+ t.Setenv(tokenURLEnv, srv.URL)
688+
689+ s := NewSource(liveCreds(), srv.Client(), "")
690+ for i := 0; i < 3; i++ {
691+ _, err := s.Token(context.Background())
692+ if !errors.Is(err, ErrExpired) {
693+ t.Fatalf("call %d: err = %v, want ErrExpired", i, err)
694+ }
695+ if !strings.Contains(err.Error(), "invalid_grant") {
696+ t.Errorf("call %d lost the reason: %v", i, err)
697+ }
698+ }
699+ if hits != 1 {
700+ t.Errorf("asked the server %d times, want 1", hits)
701+ }
702+ })
703+ }
704+ }
705+
706+ // Someone else writing a newer credential is good news, not a broken login, so
707+ // it must not reach the warning hook.
708+ func TestStoreChangedIsNotReportedAsAFailure(t *testing.T) {
709+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
710+ io.WriteString(w, `{"access_token":"fresh","refresh_token":"r2-rotated","expires_in":28800}`)
711+ }))
712+ defer srv.Close()
713+ t.Setenv(tokenURLEnv, srv.URL)
714+
715+ // The store holds a token that is not the one we refreshed from.
716+ home := storeWith(t, "somebody-elses-newer-token")
717+ s := NewSource(liveCreds(), srv.Client(), home)
718+ var reported error
719+ s.OnWriteBackFailure = func(err error) { reported = err }
720+
721+ if _, err := s.Token(context.Background()); err != nil {
722+ t.Fatalf("Token: %v", err)
723+ }
724+ if reported != nil {
725+ t.Errorf("a newer credential from someone else was reported as a failure: %v", reported)
726+ }
727+ }
728+
729+ // The design now hinges on refreshTokenExpiresAt surviving as a key we do not
730+ // model, so it gets a named guard rather than riding on another test.
731+ func TestSavePreservesTheRefreshExpiryKeyThroughRotation(t *testing.T) {
732+ home := storeWith(t, "r1")
733+ c, err := Load(home)
734+ if err != nil {
735+ t.Fatal(err)
736+ }
737+ c.RefreshToken = "r2-rotated"
738+ if err := Save(home, c, "r1"); err != nil {
739+ t.Fatalf("Save: %v", err)
740+ }
741+ oauth := readStore(t, home)["claudeAiOauth"].(map[string]any)
742+ if _, ok := oauth["refreshTokenExpiresAt"]; !ok {
743+ t.Error("refreshTokenExpiresAt vanished; a key we do not model is not ours to drop")
744+ }
745+ if oauth["refreshToken"] != "r2-rotated" {
746+ t.Errorf("refreshToken = %v, want the rotated one", oauth["refreshToken"])
747+ }
748+ }