Skip to content

Commit 0585af1

Browse files
fix(auth): force re-login on dead refresh token + enrich Sentry auth telemetry (#864)
WIZARD-CLI-F auth failures fire 3-10 min into runs across many wizard versions — not long-run token expiry, so PR #862's mid-run refresh didn't help. Root cause: an llm-gateway 401 whose stored refresh token is ALSO dead (`invalid_grant`) was routed to "just re-run", which silently re-mints the same rejected token and loops on the identical failure. Recovery: - token-refresh: `isDeadRefreshToken()` classifies 400 invalid_grant / 401 as unrecoverable (NOT 5xx / network) and flags reauth-required - agent-runner: the AUTH_ERROR outro now clears the stored OAuth session and sets promptLogin when reauth is required, so the next run forces a clean browser login instead of looping; new copy steers the user to log in again - docs/flows.md: Outro flow documents recoverable vs re-login-required Observability: - scrubber-safe Sentry tags (failure_surface, recovery, run_elapsed_bucket), readable/filterable where `auth subkind` is [Filtered] - fingerprint split by failure_surface so gateway vs mcp auth failures become separate issues / alerts instead of one undifferentiated group - breadcrumbs at the three auth-detection points Refs BA-239 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6f749b4 commit 0585af1

6 files changed

Lines changed: 316 additions & 71 deletions

File tree

docs/flows.md

Lines changed: 83 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2,49 +2,59 @@
22

33
## Back navigation
44

5-
Pressing **Esc** on a decision-point screen steps the user back to the
6-
previous decision so they can change their answer. Implemented declaratively
7-
via `FlowEntry.revert` callbacks in `src/ui/tui/flows.ts` — each entry that
8-
supports back-nav describes how to un-complete itself, and the router walks
9-
backwards to find the most recent revertible entry.
5+
Pressing **Esc** on a decision-point screen steps the user back to the previous
6+
decision so they can change their answer. Implemented declaratively via
7+
`FlowEntry.revert` callbacks in `src/ui/tui/flows.ts` — each entry that supports
8+
back-nav describes how to un-complete itself, and the router walks backwards to
9+
find the most recent revertible entry.
1010

1111
Entries without a `revert` act as a **back-stop wall**:
1212

13-
- **Run** — the agent has executed; backing past it would re-run instrumentation.
13+
- **Run** — the agent has executed; backing past it would re-run
14+
instrumentation.
1415
- **Intro** — first screen; nothing to back to.
1516

1617
Entries whose `revert` returns `false` are transparent — the router walks
1718
through them. Setup uses this to walk past when there are no user-answered
18-
questions to pop; CreateProject uses this so back-nav from DataSetup lands
19-
on Auth (not the create-project form).
19+
questions to pop; CreateProject uses this so back-nav from DataSetup lands on
20+
Auth (not the create-project form).
2021

2122
Per-screen Esc behavior:
2223

23-
| Screen | Esc action |
24-
| --------------------- | ------------------------------------------------------------- |
25-
| Auth | Back → SignupFullName / ToS / SignupEmail (whichever has the most recent meaningful revert on the create-account path), else RegionSelect |
26-
| SignupFullName | Back → SignupEmail (clears email + ceremony state via `setSignupEmail(null)`'s bound reset, so the next pass re-probes from scratch) |
27-
| ToS | Back → SignupEmail (clears email + ceremony state — same reason as SignupFullName) |
28-
| SigningUp | Transparent (`revert: () => false`) — back-walk skips this entry; no clean undo for an in-flight network call |
29-
| SignupEmail | Back → RegionSelect (clears captured email *and* `signupRequiredFields` / `signupAuth` / `signupAbandoned` via `setSignupEmail(null)`) |
30-
| DataSetup | Back → Auth (clears org/project selection) |
31-
| ActivationOptions | Back → DataSetup (re-runs activation check) |
32-
| Setup | Pops one answered question; if none, walks back further |
33-
| Slack | Back → DataIngestionCheck or Mcp |
34-
| DataIngestionCheck | Back → Mcp; **Enter** or **q** skip verification (with confirm if no events yet); **x** exits to resume later |
35-
| CreateProject | Cancel (existing) — also functions as back to Auth |
36-
| FeatureOptIn | Skip (confirms with no features selected — Esc=skip is the |
37-
| | one screen that breaks the convention; hint bar makes it |
38-
| | explicit) |
39-
| Intro | Cancel wizard (existing) |
40-
| Outro | Close report dialog (existing) |
41-
| RegionSelect / Mcp | No-op (no revertible step before them) |
24+
| Screen | Esc action |
25+
| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------- |
26+
| Auth | Back → SignupFullName / ToS / SignupEmail (whichever has the most recent meaningful revert on the create-account path), else RegionSelect |
27+
| SignupFullName | Back → SignupEmail (clears email + ceremony state via `setSignupEmail(null)`'s bound reset, so the next pass re-probes from scratch) |
28+
| ToS | Back → SignupEmail (clears email + ceremony state — same reason as SignupFullName) |
29+
| SigningUp | Transparent (`revert: () => false`) — back-walk skips this entry; no clean undo for an in-flight network call |
30+
| SignupEmail | Back → RegionSelect (clears captured email _and_ `signupRequiredFields` / `signupAuth` / `signupAbandoned` via `setSignupEmail(null)`) |
31+
| DataSetup | Back → Auth (clears org/project selection) |
32+
| ActivationOptions | Back → DataSetup (re-runs activation check) |
33+
| Setup | Pops one answered question; if none, walks back further |
34+
| Slack | Back → DataIngestionCheck or Mcp |
35+
| DataIngestionCheck | Back → Mcp; **Enter** or **q** skip verification (with confirm if no events yet); **x** exits to resume later |
36+
| CreateProject | Cancel (existing) — also functions as back to Auth |
37+
| FeatureOptIn | Skip (confirms with no features selected — Esc=skip is the |
38+
| | one screen that breaks the convention; hint bar makes it |
39+
| | explicit) |
40+
| Intro | Cancel wizard (existing) |
41+
| Outro | Close report dialog (existing) |
42+
| RegionSelect / Mcp | No-op (no revertible step before them) |
4243

4344
### Signup ceremony invariants
4445

45-
- `setSignupEmail(null)` clears **all** ceremony state (`signupRequiredFields`, `signupAuth`, `signupAbandoned`) so any back-nav path that rewinds to the email step automatically invalidates the prior probe response. The ceremony is a single conceptual unit keyed to the email being present.
46-
- `SignupFullName.revert` and `ToS.revert` return `false` when the screen was skipped (server never asked, value never set) so the back-walk continues past them rather than firing a no-op revert that traps the user.
47-
- `SigningUpScreen` is the only signup screen with network I/O. Its `useAsyncEffect` writes one of `signupAuth` (success) / `signupRequiredFields` (needs more info) / `signupAbandoned` (redirect or error). The auth task in `default.ts` waits on this settle before opening browser OAuth (see `isAuthTaskGateReady` in `src/commands/helpers.ts`).
46+
- `setSignupEmail(null)` clears **all** ceremony state (`signupRequiredFields`,
47+
`signupAuth`, `signupAbandoned`) so any back-nav path that rewinds to the
48+
email step automatically invalidates the prior probe response. The ceremony is
49+
a single conceptual unit keyed to the email being present.
50+
- `SignupFullName.revert` and `ToS.revert` return `false` when the screen was
51+
skipped (server never asked, value never set) so the back-walk continues past
52+
them rather than firing a no-op revert that traps the user.
53+
- `SigningUpScreen` is the only signup screen with network I/O. Its
54+
`useAsyncEffect` writes one of `signupAuth` (success) / `signupRequiredFields`
55+
(needs more info) / `signupAbandoned` (redirect or error). The auth task in
56+
`default.ts` waits on this settle before opening browser OAuth (see
57+
`isAuthTaskGateReady` in `src/commands/helpers.ts`).
4858

4959
The `[Esc] Back` hint appears in `KeyHintBar` only when back is actually
5060
available, so it never lies about what the keystroke will do.
@@ -57,20 +67,20 @@ The CLI keeps a persistent prompt open at all times (like Claude). Slash
5767
commands can be run at any point during the wizard to change settings or trigger
5868
actions.
5969

60-
| Command | Description |
61-
| ------------ | ----------------------------------------------------------------- |
62-
| `/region` | Switch the data-center region (US or EU) — re-triggers data setup |
63-
| `/org` | Switch the active org |
64-
| `/project` | Switch the active project |
65-
| `/login` | Re-authenticate |
66-
| `/logout` | Clear stored credentials |
67-
| `/whoami` | Show current user, org, and project |
68-
| `/mcp` | Install or remove the Amplitude MCP server |
69-
| `/slack` | Set up Amplitude Slack integration |
70-
| `/feedback` | Send product feedback (optionally with opt-in system diagnostics) |
71-
| `/test` | Run a prompt-skill demo (confirm + choose) |
72-
| `/snake` | Play Snake |
73-
| `/exit` | Exit the wizard |
70+
| Command | Description |
71+
| ----------- | ----------------------------------------------------------------- |
72+
| `/region` | Switch the data-center region (US or EU) — re-triggers data setup |
73+
| `/org` | Switch the active org |
74+
| `/project` | Switch the active project |
75+
| `/login` | Re-authenticate |
76+
| `/logout` | Clear stored credentials |
77+
| `/whoami` | Show current user, org, and project |
78+
| `/mcp` | Install or remove the Amplitude MCP server |
79+
| `/slack` | Set up Amplitude Slack integration |
80+
| `/feedback` | Send product feedback (optionally with opt-in system diagnostics) |
81+
| `/test` | Run a prompt-skill demo (confirm + choose) |
82+
| `/snake` | Play Snake |
83+
| `/exit` | Exit the wizard |
7484

7585
---
7686

@@ -203,10 +213,10 @@ flowchart TD
203213
SUSI -. overlay .-> OUTAGE["OutageScreen"]
204214
```
205215

206-
> The `SettingsOverrideScreen` overlay was removed. The wizard now scopes
207-
> its gateway env to `.claude/settings.local.json` (machine-local,
208-
> gitignored) so the user's checked-in `.claude/settings.json` is never
209-
> touched. See `src/lib/claude-settings-scope.ts`.
216+
> The `SettingsOverrideScreen` overlay was removed. The wizard now scopes its
217+
> gateway env to `.claude/settings.local.json` (machine-local, gitignored) so
218+
> the user's checked-in `.claude/settings.json` is never touched. See
219+
> `src/lib/claude-settings-scope.ts`.
210220
211221
---
212222

@@ -291,11 +301,10 @@ flowchart TD
291301
> **Partially implemented.** `DataSetupScreen` sets `activationLevel` (none /
292302
> partial / full). `DataIngestionCheckScreen` polls for events; the user can
293303
> skip verification (Enter or q, with a confirm step when nothing has been
294-
> observed yet) or exit to resume later (x).
295-
> `ChecklistScreen` offers first chart and first dashboard via browser
296-
> deep-links. Taxonomy agent and direct GraphQL chart/dashboard creation are
297-
> planned. See `features/05-data-setup-flow.feature` for the full target
298-
> behaviour.
304+
> observed yet) or exit to resume later (x). `ChecklistScreen` offers first
305+
> chart and first dashboard via browser deep-links. Taxonomy agent and direct
306+
> GraphQL chart/dashboard creation are planned. See
307+
> `features/05-data-setup-flow.feature` for the full target behaviour.
299308
300309
```mermaid
301310
---
@@ -362,9 +371,28 @@ title: Outro flow
362371
flowchart TD
363372
OUTRO["OutroScreen"] --> OUTCOME{Outcome?}
364373
OUTCOME -->|success| SUCCESS["Show changes, events, docs/continue URLs"]
365-
OUTCOME -->|error| ERR["Show error message"]
374+
OUTCOME -->|error| ERR{Auth error?}
366375
OUTCOME -->|cancel| CANCEL["Show cancel message"]
376+
ERR -->|no| ERR_GENERIC["Show error message"]
377+
ERR -->|"auth — recoverable<br/>(access token expired mid-run,<br/>refresh token still valid)"| ERR_RERUN["Show 'session expired, re-run to refresh'<br/>promptLogin = false · creds preserved"]
378+
ERR -->|"auth — re-login required<br/>(refresh token rejected: invalid_grant,<br/>OR Amplitude OAuth needs-auth)"| ERR_REAUTH["Show 'couldn't refresh your session — log in again'<br/>promptLogin = true · stored session cleared<br/>(next run forces fresh browser OAuth)"]
367379
SUCCESS --> EXIT["Press key to exit"]
368-
ERR --> EXIT
380+
ERR_GENERIC --> EXIT
381+
ERR_RERUN --> EXIT
382+
ERR_REAUTH --> EXIT
369383
CANCEL --> EXIT
370384
```
385+
386+
**Auth-failure recovery routing.** Not all `AUTH_ERROR` outros are equal:
387+
388+
- **Recoverable** — the access token expired mid-run but the stored refresh
389+
token is still good. Silent refresh on the next launch fixes it, so we keep
390+
credentials and tell the user to simply re-run (`promptLogin = false`).
391+
- **Re-login required** — the refresh token itself was rejected (`invalid_grant`
392+
from the OAuth token endpoint) or Amplitude OAuth reported `needs-auth`.
393+
Re-running can only re-refresh the same dead token and loops on the identical
394+
failure. The wizard clears the stored OAuth session (`clearStoredCredentials`)
395+
so the next launch forces a fresh browser login, sets `promptLogin = true`,
396+
and the copy steers the user to log in again (equivalent to `/logout`
397+
`/login`). This breaks the repeat-failure loop observed in Sentry
398+
`WIZARD-CLI-F`.

src/lib/agent-interface.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ import {
121121
refreshGatewayBearer,
122122
startGatewayBearerRefreshTimer,
123123
} from './llm-gateway-bearer-refresh.js';
124+
import { addBreadcrumb } from './observability/index.js';
124125
import {
125126
mergeTrackerHooks,
126127
startAiTelemetryAttempt,
@@ -4065,6 +4066,11 @@ export async function runAgent(
40654066
// copy would be misleading. See AuthErrorSubkind.
40664067
recordAuthSubkind('llm-gateway');
40674068
logToFile('Auth error detected in result message');
4069+
addBreadcrumb(
4070+
'auth',
4071+
'LLM gateway returned authentication_error (result message)',
4072+
{ surface: 'gateway', detected_at: 'result' },
4073+
);
40684074
}
40694075

40704076
// Capture the SDK-reported `retry_delay_ms` from every `api_retry`
@@ -4191,6 +4197,11 @@ export async function runAgent(
41914197
logToFile(
41924198
'Auth retries exceeded threshold — aborting agent query',
41934199
);
4200+
addBreadcrumb(
4201+
'auth',
4202+
`LLM gateway 401 retry storm exceeded threshold (${authRetryCount}/${AUTH_RETRY_LIMIT}) — aborting`,
4203+
{ surface: 'gateway', detected_at: 'retry_storm' },
4204+
);
41944205
analytics.wizardCapture('agent auth retry aborted', {
41954206
'retry count': authRetryCount,
41964207
attempt: attempt + 1,
@@ -4240,6 +4251,11 @@ export async function runAgent(
42404251
logToFile(
42414252
'Auth error detected: amplitude-wizard MCP needs-auth',
42424253
);
4254+
addBreadcrumb(
4255+
'auth',
4256+
'Amplitude MCP reported needs-auth at init — bearer rejected on first MCP call',
4257+
{ surface: 'mcp', detected_at: 'mcp_init' },
4258+
);
42434259
}
42444260
}
42454261
}

0 commit comments

Comments
 (0)