fix(login): reuse a valid session instead of always starting a device flow (#651)

[saadqbal]

Summary

tracebloc login always started a new device flow, even when the machine already held a valid session for the target env — so on a headless host, "re-run login to be safe" was a hard stop rather than a no-op: the credentials were on disk, but the command insisted on a browser approval it had no way to complete.

login now checks the session it already has before asking for one: a session the backend accepts ends the command at exit 0 with "Already signed in as <account>", --force is the opt-out for re-authenticating anyway, and every fall-through to the device flow first says why — naming only what it can actually tell apart.

Related

Closes #651

Type of change

• Feature
• Bug fix
• Tech-debt / refactor
• Docs
• Security / hardening
• Breaking change

What changed

• Short-circuit. A stored session for the target env is confirmed with a live WhoAmI — not trusted off disk, because a revoked token is still a token on disk — and on success login reports it and exits 0 without requesting a device code.
• --force skips the short-circuit entirely, including the probe: switching accounts, or replacing a session you believe is stale. Same sense as delete --force (proceed despite the state that would otherwise stop you), not --yes (skip a confirmation).
• Honest fall-through copy. The issue asks login to distinguish expired from rejected; it only claims what it can tell apart:
  • a local expires_at that has passed is named to the second, and the dead token is never presented to the backend;
  • a 401/403 is reported as rejected (the backend returns the same status for expired and revoked, so naming a cause there would be a guess);
  • anything else — DNS, a 5xx — is "couldn't check", which is not a verdict on the session. Same distinction auth status --check already draws.
  • a 426 surfaces the upgrade instruction and starts no flow: a fresh flow would hit the same version floor.

"A session for this env" is resolved two ways, so credentials already on disk aren't stranded:

1. the current session, when sessionEnv resolves it to the target — the same predicate auth status --check uses, so login and the installer's probe can't disagree about what "signed in to this env" means;
2. failing that, that env's own profile. Profiles are per-env (R10), so a machine on prod can hold a live dev token; login --env dev adopts it and moves current_env, since answering "already signed in" without moving the pointer would leave every following command on prod.

The profile is written back under the key it was found under, so a v1-migrated "Dev" config doesn't gain a second, lower-cased profile beside the real one.

Test plan

Nine new tests in internal/cli/auth_test.go, each asserting whether /device/code was requested at all — the browser demand is the behaviour that matters, the copy is secondary. Coverage: the issue's reproduction, --force, rejected (401), unverifiable (500, must not read as rejected), locally expired (and not presented to the backend), unexpired-but-still-probed, 426, cross-env profile adoption, raw-key preservation, and the signed-out path (unchanged: one flow, one post-flow confirmation, no extra probe).

go test ./... -count=1                                    # all green
make vet lint lint-full fmt-check file-budget check-style deadcode

Output of the gates:

go vet ./...
==> fmt-check: 227 tracked Go file(s) clean
ok: internal/cli/data.go 250 <= 500 (auth.go is unbudgeted; 542 -> 672)
golangci-lint run -> 0 issues.
errcheck / ineffassign / misspell / staticcheck -> clean
==> deadcode: clean (allowlist: 6 entries)
== tracebloc style guard ==  ok: style + terminology clean

Goldens regenerated (TB_UPDATE_GOLDEN=1) for the new --force flag and copy: 07-login.golden, zz-all-strings.golden. command-surface.golden is unchanged — the surface pins class/shape, and no command was added or re-parented.

Deployment notes

Behaviour change on a happy path, but only in the safe direction: login does strictly less than before when a session is valid, and nothing changes for a signed-out machine. The installer probes with auth status --check before calling login, and that probe's predicate is the one the short-circuit reuses — so the two stay in agreement. No backend change; --force restores the exact prior behaviour.

Checklist

• Tests added / updated and passing locally
• Docs updated if behavior or config changed — login --help carries the new behaviour and the --force opt-out; no CLAUDE.md / BUGBOT.md / RFC statement is made false by this change
• No secrets / credentials in the diff
• For security-sensitive paths: appropriate reviewer requested
• Cross-repo issues use Fixes tracebloc/<repo>#N — N/A, same-repo
• If this depends on a change in another repo — N/A, CLI-only
• go build ./..., go vet, and the Lint job's checks pass locally
• Terminal output follows STYLE.md — new copy goes through Printer (Successf/Hintf/Detailf), no hardcoded colour or emoji; scripts/check-style.sh passes

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes authentication entry behavior and token/session handling on disk, but only short-circuits when the backend confirms the session; signed-out and --force paths are unchanged.

Overview
tracebloc login no longer always starts the OAuth device flow. When the machine already has a session for the target env, it probes the backend with WhoAmI and, if accepted, exits 0 with Already signed in—no browser step. --force skips that path for account switches or stale sessions.

Session lookup covers the current env (aligned with auth status --check) and per-env profiles, with case-insensitive profile keys via shared normalizeEnv so v1 "Dev" keys are not missed. Fall-through to device login explains local expiry, 401/403 rejection, unreachable backend, or 426 upgrade using shared classifyWhoAmIError (also used by auth status --check). Reuse updates current_env and saves under the original profile key.

Tests assert device-code requests are skipped when appropriate; help/golden strings document --force and new messages.

^{Reviewed by Cursor Bugbot for commit e27b36b. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2dfd636. Configure here.}

[saadqbal]

Lukas — your three review points all landed, but two of the threads have since disappeared from the PR (the comment IDs 404), so noting the outcome here:

• Ctrl-C during the probe — fixed in 413fcbf, ctx.Err() early-return ahead of the classification, same guard as pollForToken. Test asserts exit 130, silence, zero device codes.
• Case-sensitive profile lookup — fixed in 413fcbf. You were right that the new tests only covered the current-env case; TestLogin_ReuseFindsARawKeyedProfileThatIsNotCurrent now covers a "Dev" profile that is not current, asserting no flow and no duplicate "dev" profile. Lookup folds the map's own keys, exact match first, sorted tie-break for determinism.
• Shared classifier — e27b36b, replied on that thread.

Thanks — the second one was a real hole in my own test, not just the code.

[LukasWodka]

Reviewed through the last few pushes (head now e27b36b). Verified locally with a scratch checkout: go build ./..., go vet ./..., and the full go test ./... suite (18 packages) are clean.

Confirmed the core mechanism: the short-circuit is a genuine backend round-trip via WhoAmI, not a file-presence check — a revoked-but-present token correctly falls through to the device flow. --force bypasses the check entirely. --env/current-session/cross-env profile lookup and the fall-through messaging (expired vs rejected vs unreachable vs version-floor) are each distinct and accurate.

Both of Bugbot's findings (raw-keyed profile lookup missing a non-current session; Ctrl-C during the probe starting a device flow instead of exiting quietly) are real — I independently reproduced the raw-key one with a standalone test before seeing Bugbot's comment, and it now passes against the fix. The DRY duplication between this probe's error classification and auth status --check's has also been cleaned up into a shared helper.

No remaining code issues from my side. Approving.