From 28ca2245790f8946707dfca5bc7a81a27401bd3f Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Mon, 17 Aug 2026 12:53:27 +0200 Subject: [PATCH 1/2] fix(auth): a transient poll failure retries; the expiry copy names the window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cli#517. The device-poll loop's `default:` branch was terminal, so anything that was not one of the four RFC 8628 sentinels ended the sign-in — a DNS blip, a backend restart, a proxy 502. Inside a ten-minute human-paced window that is a long exposure, and under the installer it threw away a run that had already built a cluster. The default is inverted: unknown failures retry, and every terminal state is now enumerated in classifyPollError — the four sentinels, a 426 version floor, a cancelled context, and any *APIError that is not 5xx / 408 / 429. So a server's refusal still stops on the first poll; only failures that never reached a verdict are ridden out. Retries are bounded by maxPollFailures consecutive failures (reset by any answer), so an unreachable backend reports itself instead of burning the code's window and then blaming the user. Also from #517: • the expiry message names the window ("sign-in codes are valid for 10 minutes"), derived from the server's expires_in rather than hardcoded — without it a ten-minute timeout reads as an instant failure; • "Run `tracebloc login` to start a new one" is suppressed when TRACEBLOC_INSTALLER is set. That advice is right for a hand-typed login and wrong under the installer, which prints its own next step; the two used to contradict each other on screen. • a Ctrl-C landing mid-request now exits quietly, like one landing between polls, instead of reporting the operator's interrupt as a sign-in failure. Every message stays a literal argument of errors.New / fmt.Errorf so the copy catalog's AST harvest can still see it; TestCopyCatalogSeesTheSignInStrings pins that, because composing copy inside a helper drops it from the catalog silently. Co-Authored-By: Claude Opus 5 --- internal/cli/auth.go | 166 ++++++++- internal/cli/auth_test.go | 340 ++++++++++++++++++ .../cli/testdata/golden/zz-all-strings.golden | 8 +- 3 files changed, 500 insertions(+), 14 deletions(-) diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 6982161c..c4237ebe 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "os" "time" "github.com/spf13/cobra" @@ -130,6 +131,128 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { return nil } +// pollDisposition is what the poll loop does with a failed PollToken call. +type pollDisposition int + +const ( + // pollStop — the sign-in cannot succeed. Report it and exit. + pollStop pollDisposition = iota + // pollAgain — an expected non-answer (not approved yet). Poll again unchanged. + pollAgain + // pollSlower — the server asked us to back off (RFC 8628 §3.5). + pollSlower + // pollRetry — an infrastructure failure, not a verdict on the sign-in. Poll + // again, but count it: a backend that never answers must say so eventually. + pollRetry +) + +// maxPollFailures bounds how many CONSECUTIVE pollRetry outcomes the loop rides +// out before giving up and naming the last one. At the RFC's 5-second floor +// that is a minute of unbroken failure — long enough to cross a wifi handover, +// a DNS blip, or a backend restart; short enough that a genuinely unreachable +// backend reports itself instead of silently burning the code's whole window. +// The code's own expiry still bounds the loop; the counter only makes the +// give-up message honest when the network, not the human, is at fault. +const maxPollFailures = 12 + +// classifyPollError decides whether a PollToken failure ends the sign-in or is +// worth another poll inside the code's remaining window (cli#517). +// +// The default used to be "stop", which made one DNS hiccup fatal to an +// installer run that had already built a cluster. The default is now "retry", +// so every genuinely terminal state is enumerated HERE rather than being the +// leftover case — a refusal must never become an infinite loop: +// +// - the four RFC 8628 §3.5 sentinels are terminal or not by the spec; +// - a 426 means this CLI is below the server's version floor — polling can't +// make it newer; +// - an *APIError carries a server VERDICT: 5xx / 408 / 429 are the server or a +// proxy failing temporarily, every other status is a refusal we must respect; +// - a cancelled context is the operator, not a blip; +// - everything left never reached a server verdict at all — DNS, refused +// connection, TLS, a truncated read, a body we couldn't decode — and is the +// class this function exists to keep alive. +func classifyPollError(err error) pollDisposition { + switch { + case errors.Is(err, api.ErrAuthorizationPending): + return pollAgain + case errors.Is(err, api.ErrSlowDown): + return pollSlower + case errors.Is(err, api.ErrExpiredToken), errors.Is(err, api.ErrAccessDenied): + return pollStop + case errors.Is(err, context.Canceled): + return pollStop + } + var ue *api.UpgradeRequiredError + if errors.As(err, &ue) { + return pollStop + } + var ae *api.APIError + if errors.As(err, &ae) { + switch { + case ae.StatusCode >= 500, + ae.StatusCode == http.StatusRequestTimeout, + ae.StatusCode == http.StatusTooManyRequests: + return pollRetry + default: + return pollStop + } + } + return pollRetry +} + +// withSignInAdvice appends the command that starts a fresh sign-in — or leaves +// the error exactly as it is when the installer is driving us (cli#517). +// "`tracebloc login`" is right when a human typed it and WRONG under the +// installer, where a bare login leaves the client mint and the Helm install +// undone; the installer prints its own, correct next step, and two contradicting +// instructions are worse than one. The installer announces itself with +// TRACEBLOC_INSTALLER. +// +// The advice is appended by wrapping rather than composed into each message, so +// every sentence stays a literal argument of an errors.New / fmt.Errorf call — +// which is what keeps this copy visible to the copy catalog's AST harvest. +func withSignInAdvice(err error) error { + if os.Getenv("TRACEBLOC_INSTALLER") != "" { + return err + } + return fmt.Errorf("%w. Run `tracebloc login` to start a new one", err) +} + +// signInWindow renders the code's advertised lifetime as a clause for the +// expiry copy, or "" when the server didn't say. Named in the message because +// without it a ten-minute timeout reads as an instant failure to anyone who +// stepped away — which is exactly how cli#517 was first reported. Derived from +// expires_in rather than hardcoded, so the sentence cannot outlive a change to +// the backend's DEVICE_CODE_TTL. +func signInWindow(expiresIn int) string { + if expiresIn <= 0 { + return "" + } + d := time.Duration(expiresIn) * time.Second + human := d.Round(time.Second).String() + switch { + case d == time.Minute: + human = "1 minute" + case d%time.Minute == 0: + human = fmt.Sprintf("%d minutes", int(d/time.Minute)) + } + return fmt.Sprintf(" — sign-in codes are valid for %s", human) +} + +// terminalSignInError renders the user-facing copy for a poll outcome the loop +// must stop on. An error we have no bespoke copy for is surfaced verbatim: a +// vague "sign-in failed" would hide the only diagnostic we have. +func terminalSignInError(err error, window string) error { + switch { + case errors.Is(err, api.ErrExpiredToken): + return withSignInAdvice(fmt.Errorf("the sign-in code expired%s", window)) + case errors.Is(err, api.ErrAccessDenied): + return withSignInAdvice(errors.New("sign-in was denied in the browser")) + } + return err +} + // pollForToken runs the RFC 8628 device-token poll loop behind a live wait // spinner, returning the issued token or an *exitError. The spinner is cleared // on every return path (deferred Stop), so the caller prints the ✔ / error line @@ -143,13 +266,18 @@ func pollForToken(ctx context.Context, p *ui.Printer, client *api.Client, dc *ap if dc.ExpiresIn > 0 { deadline = time.Now().Add(time.Duration(dc.ExpiresIn) * time.Second) } + window := signInWindow(dc.ExpiresIn) sp := p.Spinner("Waiting for your browser…", "Ctrl-C to cancel") defer sp.Stop() + // Consecutive infrastructure failures. Reset by any answer from the server — + // a blip mid-way through a long wait must not accumulate toward the cap. + var failures int for { if !deadline.IsZero() && time.Now().After(deadline) { - return "", &exitError{code: exitFailure, err: errors.New("login timed out — re-run `tracebloc login`")} + return "", &exitError{code: exitFailure, err: withSignInAdvice( + fmt.Errorf("the sign-in code expired before it was approved%s", window))} } select { case <-ctx.Done(): @@ -158,21 +286,35 @@ func pollForToken(ctx context.Context, p *ui.Printer, client *api.Client, dc *ap } tok, err := client.PollToken(ctx, dc.DeviceCode) - switch { - case err == nil: + if err == nil { return tok, nil - case errors.Is(err, api.ErrAuthorizationPending): - // not approved yet — keep polling - case errors.Is(err, api.ErrSlowDown): + } + // Ctrl-C landing DURING the request surfaces as a cancelled context on the + // HTTP call, not on the select above — exit quietly there too, rather than + // reporting the operator's own interrupt as a sign-in failure. + if ctx.Err() != nil { + return "", &exitError{code: exitInterrupted} + } + switch classifyPollError(err) { + case pollAgain: + failures = 0 // not approved yet — keep polling + case pollSlower: // RFC 8628 §3.5: on slow_down the client MUST increase the poll // interval by 5 seconds for this and all subsequent polls. + failures = 0 interval += 5 - case errors.Is(err, api.ErrExpiredToken): - return "", &exitError{code: exitFailure, err: errors.New("the sign-in code expired — re-run `tracebloc login`")} - case errors.Is(err, api.ErrAccessDenied): - return "", &exitError{code: exitFailure, err: errors.New("sign-in was denied in the browser")} - default: - return "", &exitError{code: exitFailure, err: err} + case pollRetry: + failures++ + if failures >= maxPollFailures { + // One literal, not a concatenation: the copy catalog's AST harvest + // only sees whole string literals, so a "+"-joined message is copy + // nothing inventories. + return "", &exitError{code: exitFailure, err: fmt.Errorf( + "couldn't reach the backend to finish signing in — %d attempts failed in a row (check your network / HTTPS_PROXY): %w", + failures, err)} + } + default: // pollStop + return "", &exitError{code: exitFailure, err: terminalSignInError(err, window)} } } } diff --git a/internal/cli/auth_test.go b/internal/cli/auth_test.go index 6c46a382..ff8e7edf 100644 --- a/internal/cli/auth_test.go +++ b/internal/cli/auth_test.go @@ -2,6 +2,10 @@ package cli import ( "bytes" + "context" + "errors" + "fmt" + "net" "net/http" "net/http/httptest" "strings" @@ -166,6 +170,342 @@ func TestLogin_Denied(t *testing.T) { } } +// ── cli#517: which poll failures end the sign-in, and which are worth another try ── + +// deviceCodeBody is the /device/code reply the poll tests share: a ten-minute +// window (so the expiry copy has a duration to name) and the RFC's 5s interval. +const deviceCodeBody = `{"device_code":"dc","user_code":"X","verification_uri":"https://x/activate","expires_in":600,"interval":5}` + +// pollBackend serves /device/code + /userinfo/ and hands every /device/token +// poll to tokenH, which sees the 1-based poll number. Returns a pointer to the +// live poll count so a test can assert the loop STOPPED (or kept going). +func pollBackend(t *testing.T, tokenH func(w http.ResponseWriter, poll int)) *int { + t.Helper() + polls := 0 + withTestBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/device/code": + _, _ = w.Write([]byte(deviceCodeBody)) + case "/device/token": + polls++ + tokenH(w, polls) + case "/userinfo/": + _, _ = w.Write([]byte(`{"email":"ds@tracebloc.io"}`)) + default: + t.Errorf("unexpected request path %s", r.URL.Path) + } + }) + return &polls +} + +// TestClassifyPollError_Table pins the retry classification directly, on the +// production function the loop calls (never a re-implementation of it). The +// inputs are written down independently of the matcher: each is the error a +// specific real-world failure produces, not a value read back off the rule. +func TestClassifyPollError_Table(t *testing.T) { + cases := []struct { + name string + err error + want pollDisposition + }{ + // The RFC 8628 §3.5 sentinels, bare and wrapped — PollToken returns them + // bare today, but a future wrapper must not silently reclassify them. + {"pending", api.ErrAuthorizationPending, pollAgain}, + {"pending wrapped", fmt.Errorf("poll: %w", api.ErrAuthorizationPending), pollAgain}, + {"slow_down", api.ErrSlowDown, pollSlower}, + {"slow_down wrapped", fmt.Errorf("poll: %w", api.ErrSlowDown), pollSlower}, + {"expired_token", api.ErrExpiredToken, pollStop}, + {"expired_token wrapped", fmt.Errorf("poll: %w", api.ErrExpiredToken), pollStop}, + {"access_denied", api.ErrAccessDenied, pollStop}, + {"access_denied wrapped", fmt.Errorf("poll: %w", api.ErrAccessDenied), pollStop}, + + // A server verdict we must respect: retrying an identical request cannot + // change any of these, and looping on one would hang the installer. + {"400 unrecognized", &api.APIError{StatusCode: 400, Body: `{"error":"invalid_grant"}`}, pollStop}, + {"401", &api.APIError{StatusCode: 401}, pollStop}, + {"403", &api.APIError{StatusCode: 403}, pollStop}, + {"404 no such endpoint", &api.APIError{StatusCode: 404}, pollStop}, + {"426 upgrade required", &api.UpgradeRequiredError{MinVersion: "1.2.3"}, pollStop}, + {"426 wrapped", fmt.Errorf("poll: %w", &api.UpgradeRequiredError{}), pollStop}, + {"operator cancelled", context.Canceled, pollStop}, + {"operator cancelled wrapped", fmt.Errorf("POST /device/token: %w", context.Canceled), pollStop}, + + // Temporary: the server (or a proxy in front of it) is failing, and the + // human at the browser has done nothing wrong. + {"500", &api.APIError{StatusCode: 500}, pollRetry}, + {"502 proxy", &api.APIError{StatusCode: 502}, pollRetry}, + {"503 deploying", &api.APIError{StatusCode: 503}, pollRetry}, + {"504 gateway timeout", &api.APIError{StatusCode: 504}, pollRetry}, + {"408 request timeout", &api.APIError{StatusCode: 408}, pollRetry}, + {"429 rate limited", &api.APIError{StatusCode: 429}, pollRetry}, + + // Never reached a verdict at all — the class cli#517 exists to keep alive. + {"dns failure", fmt.Errorf("POST /device/token: %w", &net.OpError{ + Op: "dial", Net: "tcp", Err: &net.DNSError{Err: "no such host", Name: "api.tracebloc.io"}}), pollRetry}, + {"connection refused", fmt.Errorf("POST /device/token: %w", + &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connect: connection refused")}), pollRetry}, + {"http client timeout", fmt.Errorf("POST /device/token: %w", context.DeadlineExceeded), pollRetry}, + {"undecodable body", errors.New(`device-token success response missing token (got "")`), pollRetry}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := classifyPollError(tc.err); got != tc.want { + t.Errorf("classifyPollError(%v) = %v, want %v", tc.err, got, tc.want) + } + }) + } +} + +// TestClassifyPollError_CoversPollTokenVocabulary derives the input domain from +// the PRODUCER instead of restating it: every error code RFC 8628 §3.5 lets the +// device-token endpoint return is driven through the real api.PollToken, and the +// error it actually produces is classified. A code the client stops mapping (or +// starts mapping differently) shows up here as a changed disposition — which a +// hand-written list of sentinels could not see. +func TestClassifyPollError_CoversPollTokenVocabulary(t *testing.T) { + want := map[string]pollDisposition{ + "authorization_pending": pollAgain, + "slow_down": pollSlower, + "expired_token": pollStop, + "access_denied": pollStop, + // Not in §3.5's happy vocabulary, but §3.5 defers to RFC 6749 §5.2 for + // the rest; those are refusals of the request, not transient conditions. + "invalid_request": pollStop, + "invalid_grant": pollStop, + } + for code, wantDisp := range want { + t.Run(code, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"` + code + `"}`)) + })) + t.Cleanup(srv.Close) + c := &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} + _, err := c.PollToken(context.Background(), "dc") + if err == nil { + t.Fatalf("PollToken returned no error for %q", code) + } + if got := classifyPollError(err); got != wantDisp { + t.Errorf("%q → %v, want %v (err=%v)", code, got, wantDisp, err) + } + }) + } +} + +// TestLogin_TransientFailureRetriesWithinWindow is the headline cli#517 fix: a +// backend blip mid-poll used to abort the whole sign-in (and with it an +// installer run that had already built a cluster). It must be ridden out. +func TestLogin_TransientFailureRetriesWithinWindow(t *testing.T) { + polls := pollBackend(t, func(w http.ResponseWriter, poll int) { + switch { + case poll <= 3: // three 503s in a row — a backend restart + w.WriteHeader(http.StatusServiceUnavailable) + default: + _, _ = w.Write([]byte(`{"token":"cat_ok"}`)) + } + }) + if _, err := runCmd(t, "login"); err != nil { + t.Fatalf("a transient backend failure must not abort sign-in, got: %v", err) + } + if *polls != 4 { + t.Errorf("polls = %d, want 4 (three 503s ridden out, then the token)", *polls) + } + cfg, _ := config.Load() + if cfg.Current().Token != "cat_ok" { + t.Errorf("stored token = %q, want cat_ok", cfg.Current().Token) + } +} + +// TestLogin_TerminalErrorStopsImmediately is the other half of the same +// contract: making transient errors retryable must NOT turn a refusal into a +// loop. A 400 the client can't map to a §3.5 sentinel is a refusal — one poll, +// then out. +func TestLogin_TerminalErrorStopsImmediately(t *testing.T) { + polls := pollBackend(t, func(w http.ResponseWriter, _ int) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"invalid_grant"}`)) + }) + _, err := runCmd(t, "login") + if err == nil { + t.Fatal("a refused device code must fail the sign-in") + } + if *polls != 1 { + t.Errorf("polls = %d, want 1 — a refusal must not be retried", *polls) + } + if !strings.Contains(err.Error(), "invalid_grant") { + t.Errorf("the refusal must be surfaced verbatim, got: %v", err) + } +} + +// TestLogin_AccessDeniedStopsImmediately: the user said no in the browser. +// Polling past that would ignore an explicit refusal. +func TestLogin_AccessDeniedStopsImmediately(t *testing.T) { + polls := pollBackend(t, func(w http.ResponseWriter, _ int) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"access_denied"}`)) + }) + if _, err := runCmd(t, "login"); err == nil { + t.Fatal("a denied sign-in must fail") + } + if *polls != 1 { + t.Errorf("polls = %d, want 1 — an explicit denial must not be retried", *polls) + } +} + +// TestLogin_TransientFailuresGiveUpAtTheCap: retrying is bounded. A backend +// that never answers must report ITSELF as the cause, not burn the code's whole +// window and then blame the user for being slow. +func TestLogin_TransientFailuresGiveUpAtTheCap(t *testing.T) { + polls := pollBackend(t, func(w http.ResponseWriter, _ int) { + w.WriteHeader(http.StatusServiceUnavailable) + }) + _, err := runCmd(t, "login") + if err == nil { + t.Fatal("an unreachable backend must eventually fail the sign-in") + } + // Written down independently of the constant: a cap of 1 would satisfy the + // equality below while restoring exactly the cli#517 behaviour this fixes — + // one failure, no retry. The loop must ride out at least a few. + if *polls < 5 { + t.Errorf("polls = %d — a transient failure must be retried several times before giving up", *polls) + } + if *polls != maxPollFailures { + t.Errorf("polls = %d, want %d (the consecutive-failure cap)", *polls, maxPollFailures) + } + if !strings.Contains(err.Error(), "couldn't reach the backend") { + t.Errorf("the give-up message must name the network as the cause, got: %v", err) + } +} + +// TestLogin_TransientFailureStreakResets: the cap counts CONSECUTIVE failures. +// A blip every few polls during a long human-paced wait must not accumulate +// into a give-up — that would re-introduce cli#517 on a flaky link. +func TestLogin_TransientFailureStreakResets(t *testing.T) { + polls := pollBackend(t, func(w http.ResponseWriter, poll int) { + switch { + case poll >= 3*maxPollFailures: + _, _ = w.Write([]byte(`{"token":"cat_ok"}`)) + case poll%2 == 0: // every other poll fails — never maxPollFailures in a row + w.WriteHeader(http.StatusBadGateway) + default: + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"authorization_pending"}`)) + } + }) + if _, err := runCmd(t, "login"); err != nil { + t.Fatalf("an intermittent failure must not exhaust the cap, got: %v", err) + } + if *polls != 3*maxPollFailures { + t.Errorf("polls = %d, want %d", *polls, 3*maxPollFailures) + } +} + +// TestLogin_ExpiredNamesTheWindow (cli#517 §4): "the sign-in code expired" with +// no duration reads as an instant failure to a user who stepped away. The +// message must name the window the server advertised. +func TestLogin_ExpiredNamesTheWindow(t *testing.T) { + pollBackend(t, func(w http.ResponseWriter, _ int) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"expired_token"}`)) + }) + _, err := runCmd(t, "login") + if err == nil { + t.Fatal("an expired code must fail the sign-in") + } + if !strings.Contains(err.Error(), "10 minutes") { + t.Errorf("the expiry message must name the 600s window as 10 minutes, got: %v", err) + } +} + +// TestSignInWindow renders expires_in as the clause the copy carries. Derived +// from the server's value, never hardcoded, so it can't outlive a TTL change. +func TestSignInWindow(t *testing.T) { + cases := []struct { + expiresIn int + want string + }{ + {600, " — sign-in codes are valid for 10 minutes"}, + {60, " — sign-in codes are valid for 1 minute"}, + {300, " — sign-in codes are valid for 5 minutes"}, + {90, " — sign-in codes are valid for 1m30s"}, + {0, ""}, // server didn't say — say nothing rather than guess + {-1, ""}, // ditto + } + for _, tc := range cases { + if got := signInWindow(tc.expiresIn); got != tc.want { + t.Errorf("signInWindow(%d) = %q, want %q", tc.expiresIn, got, tc.want) + } + } +} + +// TestCopyCatalogSeesTheSignInStrings guards the guard: the copy catalog +// harvests string LITERALS passed to errors.New / fmt.Errorf / the Printer, so +// composing a message inside a helper (and handing the helper a variable) drops +// it silently out of the catalog — the completeness backstop goes on passing +// while the copy it exists to inventory is invisible. These sentences must stay +// literal arguments; assembling them here proves they still are. +func TestCopyCatalogSeesTheSignInStrings(t *testing.T) { + catalog := strings.Join(harvestMessages(t), "\n") + for _, want := range []string{ + "the sign-in code expired%s", + "the sign-in code expired before it was approved%s", + "sign-in was denied in the browser", + "%w. Run `tracebloc login` to start a new one", + "— sign-in codes are valid for %s", // the harvest trims leading space + "couldn't reach the backend to finish signing in", + } { + if !strings.Contains(catalog, want) { + t.Errorf("copy %q is not reachable by the catalog harvest — keep it a literal argument "+ + "of errors.New/fmt.Errorf, not a variable built inside a helper", want) + } + } +} + +// TestSignInAdvice_ContradictsNobody (cli#517 §3): standalone, the CLI names the +// command that starts a fresh sign-in. Under the installer it must NOT — a bare +// `tracebloc login` there leaves the client mint and the Helm install undone, +// and the installer prints its own, correct next step a line later. +func TestSignInAdvice_ContradictsNobody(t *testing.T) { + t.Run("standalone names the command", func(t *testing.T) { + t.Setenv("TRACEBLOC_INSTALLER", "") + err := terminalSignInError(api.ErrExpiredToken, " — sign-in codes are valid for 10 minutes") + if !strings.Contains(err.Error(), "tracebloc login") { + t.Errorf("a hand-run login should say how to retry, got: %v", err) + } + }) + t.Run("under the installer names nothing", func(t *testing.T) { + t.Setenv("TRACEBLOC_INSTALLER", "1") + err := terminalSignInError(api.ErrExpiredToken, " — sign-in codes are valid for 10 minutes") + if strings.Contains(err.Error(), "tracebloc login") { + t.Errorf("under the installer `tracebloc login` is wrong advice, got: %v", err) + } + if !strings.Contains(err.Error(), "expired") || !strings.Contains(err.Error(), "10 minutes") { + t.Errorf("suppressing the advice must not suppress the FACT, got: %v", err) + } + }) +} + +// TestLogin_InstallerContextSuppressesTheCliAdvice pins the same rule through +// the command, not just the helper — the env var has to reach the message a +// user actually sees. +func TestLogin_InstallerContextSuppressesTheCliAdvice(t *testing.T) { + pollBackend(t, func(w http.ResponseWriter, _ int) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"expired_token"}`)) + }) + t.Setenv("TRACEBLOC_INSTALLER", "1") + _, err := runCmd(t, "login") + if err == nil { + t.Fatal("an expired code must fail the sign-in") + } + if strings.Contains(err.Error(), "tracebloc login") { + t.Errorf("under the installer the CLI must not tell the user to re-run login, got: %v", err) + } + if !strings.Contains(err.Error(), "10 minutes") { + t.Errorf("the window must still be named under the installer, got: %v", err) + } +} + func TestLogout(t *testing.T) { // logout now revokes server-side (cli#112) — route it at a stub, not prod. var revoked bool diff --git a/internal/cli/testdata/golden/zz-all-strings.golden b/internal/cli/testdata/golden/zz-all-strings.golden index 3dec5c03..e2cd6270 100644 --- a/internal/cli/testdata/golden/zz-all-strings.golden +++ b/internal/cli/testdata/golden/zz-all-strings.golden @@ -21,6 +21,7 @@ screen. %s/%d are runtime placeholders. "%d image(s) without an annotation (%s)" "%d mask(s) not named _mask.png (%s)" "%d mask(s) without an image (%s)" +"%d minutes" "%d of %d" "%d pod(s), none crash-looping or stuck Pending" "%d pod(s), none restarted ≥%d times" @@ -63,6 +64,7 @@ screen. %s/%d are runtime placeholders. "%s: %w" "%s=%s,%s=%s" "%v (policy: %v)" +"%w. Run `tracebloc login` to start a new one" "(%d CPU · %d GiB" "(+%d more)" "(Pod phase: %s)" @@ -389,6 +391,7 @@ screen. %s/%d are runtime placeholders. "constructing kubernetes clientset: %w" "context" "could not check — cluster API unreachable (see 'Cluster reachable' above)" +"couldn't reach the backend to finish signing in — %d attempts failed in a row (check your network / HTTPS_PROXY): %w" "couldn't read RESOURCE_REQUESTS from jobs-manager — skipping node-fit" "couldn't read capacity: %v" "couldn't read jobs-manager to resolve image pull secrets — skipping" @@ -471,7 +474,6 @@ screen. %s/%d are runtime placeholders. "loading kubeconfig: %w" "locating mysql pod: %w" "location" -"login timed out — re-run `tracebloc login`" "love from tracebloc 💚" "marshaling submit request: %w" "marshaling synthesized spec: %w" @@ -603,7 +605,8 @@ screen. %s/%d are runtime placeholders. "the cluster API server at %s isn't answering — is the cluster running?" "the duration/time column name" "the label/target column name" -"the sign-in code expired — re-run `tracebloc login`" +"the sign-in code expired before it was approved%s" +"the sign-in code expired%s" "the size your images already are; tracebloc checks it, it never resizes" "this machine has %s, but you asked for %s." "time column" @@ -636,4 +639,5 @@ screen. %s/%d are runtime placeholders. "~%s (requested; server may cap shorter)" "· %d GPU" "· %d classes" +"— sign-in codes are valid for %s" "— this machine could give a run up to cpu=%d,memory=%dGi ('tracebloc resources set max')" From 019d5285c8bee90f36594899486e129725dd5c48 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Mon, 17 Aug 2026 12:56:56 +0200 Subject: [PATCH 2/2] chore(release): VERSION 0.10.8 -> 0.10.9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit version-bump-gate: v0.10.8 is already released and this PR changes a published path (internal/*), so the train would otherwise cut the next tag from a stale file. 0.10.9 is the same pending version cli#518, #519 and #520 bump to — they all ship under it together, and the identical change merges without conflict. Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 1a46c7f1..f314d020 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.10.8 +0.10.9