feat(signup): server-driven needs_information field collection#234
Closed
bird-m wants to merge 52 commits into
Closed
feat(signup): server-driven needs_information field collection#234bird-m wants to merge 52 commits into
bird-m wants to merge 52 commits into
Conversation
Contributor
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Direct
session.regionread bypasses zone resolution- Replaced the direct
s.region!read withuseResolvedZone(store.session)so the signup POST uses the four-tier zone-resolution priority instead of the raw session field.
- Replaced the direct
Or push these changes by commenting:
@cursor push b2ce2b7530
Preview (b2ce2b7530)
diff --git a/src/ui/tui/screens/SigningUpScreen.tsx b/src/ui/tui/screens/SigningUpScreen.tsx
--- a/src/ui/tui/screens/SigningUpScreen.tsx
+++ b/src/ui/tui/screens/SigningUpScreen.tsx
@@ -34,6 +34,7 @@
import { BrailleSpinner } from '../components/BrailleSpinner.js';
import { performSignupOrAuth } from '../../../utils/signup-or-auth.js';
import { KNOWN_REQUIRED_FIELDS, fieldPresentOnSession } from '../flows.js';
+import { useResolvedZone } from '../hooks/useResolvedZone.js';
interface SigningUpScreenProps {
store: WizardStore;
@@ -41,6 +42,7 @@
export const SigningUpScreen = ({ store }: SigningUpScreenProps) => {
useWizardStore(store);
+ const zone = useResolvedZone(store.session);
useEffect(() => {
let cancelled = false;
@@ -52,7 +54,7 @@
{
email: s.signupEmail,
fullName: s.signupFullName,
- zone: s.region!,
+ zone,
},
{ signal: controller.signal },
);You can send follow-ups to the cloud agent here.
bird-m
added a commit
that referenced
this pull request
Apr 24, 2026
Five small changes in response to the review pass: 1. SigningUpScreen — re-read store.session after the await for the unmet-field check. The pre-await snapshot was technically stale because nanostores returns a new object on each setKey; in practice only a /region slash command could mutate state during the POST, but the freshest view costs nothing and removes a latent risk. 2. SigningUpScreen — add an explanatory comment above the empty-deps useEffect documenting that the request-time fields are guaranteed non-null by the SigningUp flow predicate. Future flow reorders that break the invariant would otherwise fail silently. 3. direct-signup.ts — include the response `type` discriminant (when present) in the "unexpected response" error message and log entry. Materially improves triage when a future server arm appears in prod before the wizard knows how to parse it. `type` is just a tag, no PII risk. 4. flows.ts — add `s.region !== null` to the SigningUp `show` predicate. The region check is redundant in normal flow ordering (RegionSelect's isComplete already gates this), but making it local lets SigningUpScreen rely on `s.region !== null` without chasing the invariant across files. Removes the implicit non-null assertion's dependency on a sibling flow entry. 5. bin.ts — comment near the browser_fallback_after_signup trackSignupAttempt call documenting that the wrapper has already emitted user_fetch_failed; the two events together describe a real two-layer degradation, not a double-count. Plus a stale comment cleanup in the BDD step definitions (referenced a terminal-hold timer that no longer exists after the SigningUpScreen render simplification earlier in this branch). All 1240 unit tests + 96 BDD scenarios pass; typecheck and lint clean. Refs MCP-194.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing try/catch causes deadlock on thrown error
- Wrapped
performSignupOrAuthinSigningUpScreenwith try/catch that emitswrapper_exceptiontelemetry and callssetSignupAbandoned(true)sosignupCeremonySettledcan resolve and the auth task no longer deadlocks.
- Wrapped
Or push these changes by commenting:
@cursor push 3c07f6a97d
Preview (3c07f6a97d)
diff --git a/src/ui/tui/screens/SigningUpScreen.tsx b/src/ui/tui/screens/SigningUpScreen.tsx
--- a/src/ui/tui/screens/SigningUpScreen.tsx
+++ b/src/ui/tui/screens/SigningUpScreen.tsx
@@ -32,7 +32,10 @@
import { useWizardStore } from '../hooks/useWizardStore.js';
import { Colors } from '../styles.js';
import { BrailleSpinner } from '../components/BrailleSpinner.js';
-import { performSignupOrAuth } from '../../../utils/signup-or-auth.js';
+import {
+ performSignupOrAuth,
+ trackSignupAttempt,
+} from '../../../utils/signup-or-auth.js';
import { KNOWN_REQUIRED_FIELDS, fieldPresentOnSession } from '../flows.js';
interface SigningUpScreenProps {
@@ -61,14 +64,28 @@
// SignupEmail / SignupFullName aren't mounted simultaneously, so
// they can't write while we're posting).
const s = store.session;
- const result = await performSignupOrAuth(
- {
- email: s.signupEmail,
- fullName: s.signupFullName,
- zone: s.region!,
- },
- { signal: controller.signal },
- );
+ let result: Awaited<ReturnType<typeof performSignupOrAuth>>;
+ try {
+ result = await performSignupOrAuth(
+ {
+ email: s.signupEmail,
+ fullName: s.signupFullName,
+ zone: s.region!,
+ },
+ { signal: controller.signal },
+ );
+ } catch {
+ // The wrapper itself threw (e.g. disk/permission failure while
+ // persisting tokens). Mirror the non-TUI path in
+ // `runDirectSignupIfRequested`: emit `wrapper_exception` and
+ // mark the ceremony abandoned so the auth task in bin.ts can
+ // proceed to browser OAuth instead of waiting forever on
+ // `signupCeremonySettled`.
+ if (cancelled) return;
+ trackSignupAttempt({ status: 'wrapper_exception', zone: s.region! });
+ store.setSignupAbandoned(true);
+ return;
+ }
if (cancelled) return;
switch (result.kind) {You can send follow-ups to the cloud agent here.
Collaborator
Author
|
bugbot run |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing default branch risks silent wizard hang
- Added exhaustive
defaultbranches withneverassertions to bothswitch (result.kind)blocks; SigningUpScreen falls back to setSignupAbandoned and bin.ts routes to the fallback log instead of the success path.
- Added exhaustive
Or push these changes by commenting:
@cursor push 046bbfbc60
Preview (046bbfbc60)
diff --git a/bin.ts b/bin.ts
--- a/bin.ts
+++ b/bin.ts
@@ -530,6 +530,17 @@
`Direct signup did not produce credentials; continuing to ${fallbackLabel}.`,
);
return;
+ default: {
+ // Exhaustiveness guard: if a new arm is added to
+ // PerformSignupOrAuthResult, fail closed by routing to the
+ // fallback path rather than silently treating it as success.
+ const _exhaustive: never = result;
+ void _exhaustive;
+ getUI().log.info(
+ `Direct signup did not produce credentials; continuing to ${fallbackLabel}.`,
+ );
+ return;
+ }
}
getUI().log.info('Direct signup succeeded; using newly created account.');
diff --git a/src/ui/tui/screens/SigningUpScreen.tsx b/src/ui/tui/screens/SigningUpScreen.tsx
--- a/src/ui/tui/screens/SigningUpScreen.tsx
+++ b/src/ui/tui/screens/SigningUpScreen.tsx
@@ -135,6 +135,16 @@
case 'error':
store.setSignupAbandoned(true);
return;
+ default: {
+ // Exhaustiveness guard: if a new arm is added to
+ // PerformSignupOrAuthResult, fail closed by abandoning so
+ // bin.ts's signupCeremonySettled wait resolves and the
+ // wizard doesn't hang indefinitely on "Signing up…".
+ const _exhaustive: never = result;
+ void _exhaustive;
+ store.setSignupAbandoned(true);
+ return;
+ }
}
})();You can send follow-ups to the cloud agent here.
bird-m
added a commit
that referenced
this pull request
Apr 25, 2026
Five small changes in response to the review pass: 1. SigningUpScreen — re-read store.session after the await for the unmet-field check. The pre-await snapshot was technically stale because nanostores returns a new object on each setKey; in practice only a /region slash command could mutate state during the POST, but the freshest view costs nothing and removes a latent risk. 2. SigningUpScreen — add an explanatory comment above the empty-deps useEffect documenting that the request-time fields are guaranteed non-null by the SigningUp flow predicate. Future flow reorders that break the invariant would otherwise fail silently. 3. direct-signup.ts — include the response `type` discriminant (when present) in the "unexpected response" error message and log entry. Materially improves triage when a future server arm appears in prod before the wizard knows how to parse it. `type` is just a tag, no PII risk. 4. flows.ts — add `s.region !== null` to the SigningUp `show` predicate. The region check is redundant in normal flow ordering (RegionSelect's isComplete already gates this), but making it local lets SigningUpScreen rely on `s.region !== null` without chasing the invariant across files. Removes the implicit non-null assertion's dependency on a sibling flow entry. 5. bin.ts — comment near the browser_fallback_after_signup trackSignupAttempt call documenting that the wrapper has already emitted user_fetch_failed; the two events together describe a real two-layer degradation, not a double-count. Plus a stale comment cleanup in the BDD step definitions (referenced a terminal-hold timer that no longer exists after the SigningUpScreen render simplification earlier in this branch). All 1240 unit tests + 96 BDD scenarios pass; typecheck and lint clean. Refs MCP-194.
bird-m
force-pushed
the
followup/signup-needs-information
branch
from
5c21384 to
6a82584
Compare
7 tasks
Collaborator
Author
|
bugbot run |
bird-m
added a commit
that referenced
this pull request
Apr 25, 2026
… switches Adds a `default:` arm with `result satisfies never` to both switches that consume `PerformSignupOrAuthResult` — SigningUpScreen's effect and bin.ts's runDirectSignupIfRequested. If a future arm is added to the discriminated union (the backend just proved this is a live axis by forcing us to add `needs_information` in this same PR), TypeScript now fails at compile time at every consumer site rather than letting the mistake slip past review. Runtime behavior matches the existing non-success arms in each site: - SigningUpScreen falls closed via `setSignupAbandoned(true)` so bin.ts's signupCeremonySettled wait resolves and the wizard doesn't hang on "Signing up…". - bin.ts routes to the same "continuing to fallback" log path as requires_redirect / error, rather than falling through to the success log + onSuccess() call with no credentials. Third instance in this PR of the "SigningUpScreen must always write a terminal session value" protection class, alongside the existing signupCeremonySettled wait and the try/catch around performSignupOrAuth. Raised by Cursor Bugbot on PR #234.
Contributor
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6a82584. Configure here.
Collaborator
Author
|
bugbot run |
Contributor
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b856c50. Configure here.
This was referenced Apr 25, 2026
kelsonpw
added a commit
that referenced
this pull request
Apr 25, 2026
The loadCheckpoint() tests dynamic-import ./registry.js, which pulls in all 18 framework configs. Under parallel test-runner load this regularly exceeds vitest's default 5s testTimeout, producing intermittent failures across PRs (confirmed reproduces on origin/main, independent of any in-flight branch). Fix is local to the affected describe block via vitest 4.x's options-object syntax. Keeps the change off vitest.config.ts (touched by PR #234) so it doesn't collide with parallel work. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restores client-side handling of the needs_information response from the provisioning endpoint (amplitude/javascript PR #103683). fullName becomes optional on DirectSignupInput; full_name is omitted from the request body when null so the server can decide whether to require it.
…on arm Replaces the AmplitudeAuthResult | null return shape with an explicit discriminated union so callers can distinguish 'no credentials produced' outcomes (requires_redirect, needs_information, error) from success. Adds the 'needs_information' telemetry status. Drops the early return on missing fullName — the server now decides whether full_name is required. bin.ts call sites are updated in a later task.
Three new UI-state fields on WizardSession that drive the server-driven signup collection flow introduced in the prior two commits: - signupRequiredFields: string[] — field names the endpoint asked for on the needs_information response. Read by flow predicates that route the user into the collection screen (e.g. SignupFullNameScreen). - signupAuth: SignupSuccessResult | null — tokens + userInfo from the success arm. Narrowed to the success arm of PerformSignupOrAuthResult because SigningUpScreen only writes here on 'success'. bin.ts reads it to skip the OAuth browser round-trip. - signupAbandoned: boolean — set when the direct-signup path is terminally done (redirect / error / unknown required field). Flow falls through to AuthScreen after this flips true. Not persisted to the session checkpoint: these are per-run UI state and the checkpoint schema deliberately excludes credentials and in-flight auth flow state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five new setters wire the session fields added in the previous commit through the reactive store so screens can write them and the router re-evaluates the flow pipeline on each change. setSignupFullName trims whitespace before persisting so downstream callers can rely on a clean value regardless of how the screen's text input collects it.
Collects the new-account email when --signup is passed without --email.
Validates via the shared EMAIL_REGEX. Writes trimmed value to
session.signupEmail via store.setSignupEmail.
Also lands the testing infra this task (and the two following signup
screen tasks) depend on:
- Add ink-testing-library as a devDependency. The repo had no prior
screen unit tests, so no existing harness to reuse; the plan lists
it as required tech stack.
- Extend vitest.config.ts include globs to match *.test.tsx (was
only *.test.ts). Without this the new screen test file is skipped
silently.
Test deviations from the plan's literal snippet:
- WizardStore constructor takes (flow?: Flow), not ({ installDir }).
Use new WizardStore() and assign store.session directly to seed
signup: true (the $session atom is private).
- Split stdin writes for the value and the Enter keystroke — a
single-write "email\\r" gets delivered to ink as one data event
and doesn't trigger key.return.
Parameterized cases covering every resolvable Screen the router might pick during a signup ceremony: SignupEmail, SigningUp (initial + retry), SignupFullName, and the fallback-to-Auth paths for signupAuth set, abandoned, and unknown-field guard.
Three new fast-check properties: - SigningUp never fires when signupAbandoned=true (flow falls through to AuthScreen for the OAuth browser dance) - SigningUp never fires when signupAuth is set (tokens obtained; bin.ts consumes them and skips OAuth) - SigningUp never fires when a requiredField is outside the client's allowlist — prevents the screen from re-POSTing with fields the server asked for but the wizard can't collect
Drops the inline performSignupOrAuth call at the TUI auth ceremony site. SigningUpScreen now owns the POST, the telemetry, and the error handling — this block just reads the screen's output from session.signupAuth. The downstream 'pending user data / browser fallback' logic at bin.ts:1303+ is untouched. Also update cli.test.ts mocks so the mocked store session and buildSession factory default signupAuth to null, matching the real session shape. Without this the new strict `!== null` check would treat the undefined sentinel as a signup success and skip the OAuth path the tests assert on.
Updates the non-TUI direct-signup wrapper to consume the new
discriminated union from Task 2. needs_information is mapped to the
existing "didn't produce credentials; continuing to fallback" bucket —
classic/agent/CI get no interactive re-prompt (out of scope).
Also removes the dead 'if (tokens === null)' check at bin.ts:516,
which was unreachable under the new union (null is no longer a
possible return value; tsc didn't flag it because the comparison
silently evaluated to false).
Fixes the cli.test.ts default mock for performSignupOrAuth, which
previously returned bare undefined. The old code tolerated it
accidentally (via the unreachable `=== null` false-path falling
through to a no-op `onSuccess`); the new switch on result.kind
crashes on undefined. Default the mock to { kind: 'error' } so
tests that don't opt into signup success exercise the fallback
path deterministically.
Four scenarios exercising the end-to-end router/session transitions for the --signup flow's server-driven field collection: - --signup only: email screen + two signing-up passes + name screen - --signup --email: two signing-up passes + name screen, email skipped - --signup + both flags: single signing-up pass, no collection screens - requires_redirect: abandoned bit flips, flow falls through to Auth Adapted from the task's literal scenarios — the existing BDD suite is a router-level integration layer (no HTTP mocking, no TUI rendering), so "first POST" / "second POST" are modeled as the SigningUpScreen being mounted by the router, with the server response simulated by writing its side-effect back onto the session (setSignupRequiredFields / setSignupAuth / setSignupAbandoned). Preserves the four user-journey shapes the original scenarios described while matching the codebase's idiom. 96 scenarios pass (92 pre-existing + 4 new).
Adds a signupCeremonySettled predicate to the auth-ceremony wait condition in bin.ts so the OAuth browser doesn't open while SigningUpScreen's POST is still in flight. Without this, bin.ts races the screen-driven POST: as soon as introConcluded && region flip true, bin.ts reads session.signupAuth (still null because the POST hasn't resolved) and falls through to performAmplitudeAuth, opening the browser mid-signup. The new predicate gates on (!signup || signupAuth !== null || signupAbandoned), which terminates on every SigningUpScreen outcome — success (tokens), abandon (redirect/error/unknown-field), or non-signup runs — so it never deadlocks and bin.ts proceeds exactly when the signup flow has produced a session fact it can act on. Refs MCP-194.
Makes the --signup ceremony (SignupEmail → SigningUp → SignupFullName
→ SigningUp) feel like a single screen that updates in place, instead
of three screens that each wipe into the next. Preserves the "screens
are passive" convention — no network I/O moves into the input
screens, the POST stays in SigningUpScreen.
Two pieces:
1. TRANSITION_GROUPS map in App.tsx — generic primitive that collapses
related screens into a shared DissolveTransition key. When the key
doesn't change, the wipe animation is suppressed. The three signup
screens share the 'signup' group; dissolves still fire around the
group (Region → signup, signup → Auth) because those are real
step transitions.
2. SigningUpScreen renders a mimic of the previous input screen —
same heading, the user's submitted value shown read-only, plus a
"Signing up…" spinner. Combined with the suppressed dissolve, the
screen-swap looks like the input screen updating in place. Removes
the ~1s terminal-hold + "Opening momentarily" message; the
fall-through context now lives on AuthScreen instead (gated on
session.signup).
AuthScreen gains a one-line signup-aware context ("Please sign up or
log in from the browser to continue.") above the OAuth URL when
session.signup is true. Shown whenever SigningUpScreen abandons
(redirect / error / unknown-field / already-sent field), which is
also when the browser is about to open.
SigningUpScreen tests updated: drops the terminal-message +
1s-hold expectations, adds coverage for the two render modes
(email-screen layout vs name-screen layout) and keeps the unmount-
cancellation invariant.
Refs MCP-194. The adjacent MCP-195 (AuthScreen post-auth summary
flash) is a separate follow-up, not touched here.
Five small changes in response to the review pass: 1. SigningUpScreen — re-read store.session after the await for the unmet-field check. The pre-await snapshot was technically stale because nanostores returns a new object on each setKey; in practice only a /region slash command could mutate state during the POST, but the freshest view costs nothing and removes a latent risk. 2. SigningUpScreen — add an explanatory comment above the empty-deps useEffect documenting that the request-time fields are guaranteed non-null by the SigningUp flow predicate. Future flow reorders that break the invariant would otherwise fail silently. 3. direct-signup.ts — include the response `type` discriminant (when present) in the "unexpected response" error message and log entry. Materially improves triage when a future server arm appears in prod before the wizard knows how to parse it. `type` is just a tag, no PII risk. 4. flows.ts — add `s.region !== null` to the SigningUp `show` predicate. The region check is redundant in normal flow ordering (RegionSelect's isComplete already gates this), but making it local lets SigningUpScreen rely on `s.region !== null` without chasing the invariant across files. Removes the implicit non-null assertion's dependency on a sibling flow entry. 5. bin.ts — comment near the browser_fallback_after_signup trackSignupAttempt call documenting that the wrapper has already emitted user_fetch_failed; the two events together describe a real two-layer degradation, not a double-count. Plus a stale comment cleanup in the BDD step definitions (referenced a terminal-hold timer that no longer exists after the SigningUpScreen render simplification earlier in this branch). All 1240 unit tests + 96 BDD scenarios pass; typecheck and lint clean. Refs MCP-194.
Replaces direct `s.region!` read with the shared `useResolvedZone`
hook, matching the pattern used by sibling TUI screens
(CreateProjectScreen, DataIngestionCheckScreen, SlackScreen) and by
ConsoleView. Behaviorally equivalent in the normal flow — the
SigningUp predicate gates on region being set — but it removes an
unsafe non-null assertion and falls back to DEFAULT_AMPLITUDE_ZONE
instead of crashing if the predicate is ever bypassed (race or
future flow refactor).
Note on the previous reviewer's framing: the claim that the old code
"correctly used resolveZone(session, fallback, { readDisk: false })"
to consult ampli.json + stored-user-zone tiers is incorrect — that
form of resolveZone is literally `session.region ?? fallback`, no
disk reads. The real argument for using the hook here is consistency
with the established TUI pattern and defense-in-depth, not
additional tier coverage.
Refs MCP-194.
Without this catch, an exception inside `performSignupOrAuth` (most likely from `replaceStoredUser` — disk / permission failure on ~/.ampli.json, which the wrapper intentionally lets propagate per its own docstring at signup-or-auth.ts) leaves the screen with no terminal session write. The bin.ts auth-ceremony wait gates on signupCeremonySettled (signupAuth || signupAbandoned), which never satisfies, and the wizard hangs indefinitely on "Signing up…". Mirrors runDirectSignupIfRequested's existing handling for the non-TUI path: emit `wrapper_exception` telemetry, set signupAbandoned=true so bin.ts proceeds to OAuth fallback. The user sees a brief AuthScreen with the "Please sign up or log in from the browser to continue" copy, then the browser opens — degraded but non-fatal, and traceable in telemetry. Adds a unit test that mocks performSignupOrAuth to reject; asserts signupAbandoned flips and the telemetry fires. Without the catch, the test would hang waiting for the assertion. Caught by Bugbot review on commit 39e0a52. Refs MCP-194.
… switches Adds a `default:` arm with `result satisfies never` to both switches that consume `PerformSignupOrAuthResult` — SigningUpScreen's effect and bin.ts's runDirectSignupIfRequested. If a future arm is added to the discriminated union (the backend just proved this is a live axis by forcing us to add `needs_information` in this same PR), TypeScript now fails at compile time at every consumer site rather than letting the mistake slip past review. Runtime behavior matches the existing non-success arms in each site: - SigningUpScreen falls closed via `setSignupAbandoned(true)` so bin.ts's signupCeremonySettled wait resolves and the wizard doesn't hang on "Signing up…". - bin.ts routes to the same "continuing to fallback" log path as requires_redirect / error, rather than falling through to the success log + onSuccess() call with no credentials. Third instance in this PR of the "SigningUpScreen must always write a terminal session value" protection class, alongside the existing signupCeremonySettled wait and the try/catch around performSignupOrAuth. Raised by Cursor Bugbot on PR #234.
SignupEmail/SigningUp/SignupFullName completed vacuously when --signup is off, but lacked revert callbacks so canGoBack hit a wall before RegionSelect. Remove duplicate setSignupEmail/setSignupFullName methods (keep trim + telemetry). Co-authored-by: Cursor <cursoragent@cursor.com>
When --signup runs in a directory that previously hosted another account, clear install-dir-keyed caches (API key store, checkpoint, ampli.json) before replaceStoredUser. Adds clearStaleProjectState helper and installDir on SignupOrAuthInput. Refs MCP-196. Co-authored-by: Cursor <cursoragent@cursor.com>
LogoutScreen previously inlined the same three calls (`clearApiKey`, `clearCheckpoint`, `clearAuthFieldsInAmpliConfig`) that the signup success path now composes via `clearStaleProjectState`. Switch logout to call the helper so the two entry points share one definition of "per-project state to wipe when this directory is about to belong to a different account" — any future surface added to the helper shows up in both paths automatically. Behavior unchanged: same three functions in the same order. `clearStoredCredentials()` stays inline because logout still wipes the entire `~/.ampli.json` (signup uses `replaceStoredUser` instead, which is a more conservative atomic swap that preserves non-User keys). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #223 refactored `api-key-store.ts` from `execSync` to `execFileSync` for keychain operations (shell-injection safety). The wipe helper was written before that landed and mocked the wrong function — the keychain delete assertion silently saw zero matching calls because the mock was never on the path. Update the mock and the assertion to match the production code's `execFileSync(file, args)` signature, mirroring the convention in `api-key-store.test.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cleanup at port-detection.test.ts:121 called `rmSync(link, { force: true })`
on a symlink that points to a directory. Older Node versions treated this as
file-removal (the desired symlink-only semantics); Node 23 treats it as
directory-removal and refuses without `recursive: true` — which would also
delete the target directory, defeating the explicit two-step cleanup.
Use `unlinkSync` so the intent ("remove just the symlink") is unambiguous
across Node versions. Verified the test now passes on Node 23.
Drive-by fix surfaced while running the pre-commit hook on Node 23 in
the MCP-196 PR; the test was passing on supported Node versions
(20 / 22 / >=24) so this never broke CI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Symmetric counterpart to the signup-side wipe added earlier in this PR. On a successful fresh-OAuth completion (browser-returned code → tokens exchanged → about to persist via storeToken), wipe pre-existing install-dir-keyed state so the new account doesn't inherit the prior account's API key, workspace binding, or session checkpoint. Without this, the bug class fixed by the signup-side wipe still leaks through the OAuth path: a user with cached state for account A who logs in to account B via the browser would have B's tokens persisted to ~/.ampli.json but A's API key still in keychain / .env.local / project ampli.json — events would silently route into A's tenancy. Earlier discussion in this PR considered a read-side validation primitive instead (validate cached state belongs to the active session on every credential read). The wipe-on-fresh-auth-completion design is strictly simpler: - Idempotent in the same-account case: clearing the cached key just forces one extra backend round-trip on the next read; user ends up in the same valid state. - Correct in the different-account case: stale state is gone before the new tokens are persisted, so downstream credential resolution fetches the new account's key. The wipe fires only on the fresh-OAuth path (after the browser callback). The cached-token short-circuit at oauth.ts:238-261 returns existing tokens unchanged — no auth event has occurred, the cached project state belongs to that same user, no wipe needed. `installDir` is now required on `performAmplitudeAuth`'s options. The compiler caught all four call sites: setup-utils.ts:494 (classic mode funnel), bin.ts:1390 (TUI main flow), bin.ts:1443 (TUI token-expired retry), bin.ts:1770 (the `wizard login` subcommand). The deprecated performOAuthFlow wrapper at oauth.ts:417 has zero callers; updated to default installDir to process.cwd(). Not adding a unit test for the wipe call inside performAmplitudeAuth in this commit: oauth.ts's internal helpers (startCallbackServer, exchangeCodeForToken, opn browser interaction) aren't exported and mocking them properly would require significantly more harness than the one-line behavior change warrants. The compiler-required installDir signature plus the parallel test coverage on performSignupOrAuth's wipe (which shares the helper) cover the contract sufficiently. Happy to add a focused oauth.test.ts in a follow-up if reviewers want it. Refs MCP-196. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use AMPLITUDE_WIZARD_CACHE_DIR + getCheckpointFile for checkpoint removal. Replace obsolete keychain exec assertion with per-user cache API key clear. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve helpers auth gate: keep SigningUp ceremony gating on top of accountCreationFlow + ToS. Align flows/AuthScreen and BDD steps with accountCreationFlow (replaces session.signup on WizardSession). Co-authored-by: Cursor <cursoragent@cursor.com>
…te wipe) - Restore session.accountCreationFlow from CLI --signup for SigningUp gate - Combine isCreateAccountOnboarding ToS check with signup ceremony settling - Logout uses clearStaleProjectState (symmetric with post-signup wipe) - cli.test wizard-session mock uses importOriginal + accountCreationFlow - Split auth-gate tests: menu create-account vs --signup blocking Co-authored-by: Cursor <cursoragent@cursor.com>
- Align flows/screen-registry with EmailCapture, ToS, accountCreationFlow - Dedupe signup helpers and legacy s.signup flow entries in flows.ts - direct-signup: needs_information + structured errors with optional code - signup-or-auth: dashboardUrl on success, single SignupOrAuthOptions - SigningUpScreen passes installDir; AuthScreen OAuth copy uses accountCreationFlow - Tests and Cucumber steps updated for installDir and session flags Co-authored-by: Cursor <cursoragent@cursor.com>
kelsonpw
added a commit
that referenced
this pull request
May 1, 2026
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Auth task makes duplicate signup POST after SigningUpScreen succeeds
- Added a
s.signupAuth !== nullbranch in the bin.ts auth task that hydratesauth,signupUserInfo, and the magic-link URL directly from the SigningUpScreen result, short-circuiting before the duplicateperformSignupOrAuthcall.
- Added a
Or push these changes by commenting:
@cursor push 1c1368b081
Preview (1c1368b081)
diff --git a/src/commands/default.ts b/src/commands/default.ts
--- a/src/commands/default.ts
+++ b/src/commands/default.ts
@@ -758,7 +758,29 @@
'../utils/signup-or-auth.js'
);
const s = tui.store.session;
- if (
+ if (s.signupAuth !== null) {
+ // SigningUpScreen already completed the direct-signup POST
+ // (success arm), wrote tokens to ~/.ampli.json via
+ // replaceStoredUser, and stored the result on the session.
+ // Hydrate `auth` directly from that result so we don't
+ // re-POST to the provisioning endpoint (which would return
+ // requires_redirect for the now-existing account and fall
+ // through to a spurious browser OAuth flow).
+ auth = {
+ idToken: s.signupAuth.idToken,
+ accessToken: s.signupAuth.accessToken,
+ refreshToken: s.signupAuth.refreshToken,
+ zone: s.signupAuth.zone,
+ };
+ signupUserInfo = s.signupAuth.userInfo;
+ signupTokensObtained = true;
+ tui.store.setSignupMagicLinkUrl(
+ s.signupAuth.dashboardUrl ?? null,
+ );
+ getUI().log.info(
+ 'Direct signup succeeded; using newly created account.',
+ );
+ } else if (
isCreateAccountOnboarding(s) &&
s.signupEmail &&
s.signupFullName &&You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 087aef4. Configure here.
| accessToken: signupResult.accessToken, | ||
| refreshToken: signupResult.refreshToken, | ||
| zone: signupResult.zone, | ||
| }; |
Contributor
There was a problem hiding this comment.
Auth task makes duplicate signup POST after SigningUpScreen succeeds
High Severity
After SigningUpScreen writes signupAuth to the session (releasing the isAuthTaskGateReady gate), the auth task reads s = tui.store.session and enters the block at line 762 because signupTokensObtained is never set by SigningUpScreen — only by the older email-capture path. This causes a second performSignupOrAuth call. The provisioning endpoint will likely return requires_redirect for the already-created account, so auth stays null and the code falls through to performAmplitudeAuth, unnecessarily opening browser OAuth after signup already succeeded. The condition needs an additional s.signupAuth === null guard.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 087aef4. Configure here.
Member
6 tasks
bird-m
added a commit
that referenced
this pull request
May 6, 2026
…doc rot **emailCaptureComplete cleanup** Three test files still spread `emailCaptureComplete` into session overrides: `auth-gate.test.ts:64`, `session-checkpoint-events.test.ts:106`, `AuthScreen.coaching.test.tsx:54,85`. The field was removed from `WizardSession` in commit `e5df61b3`; tsc didn't catch the orphans because `tsconfig.json` excludes `src/**/__tests__/**/*`. The auth-gate test "blocks create-account runs that have only completed email capture" was the most concerning — its name described behavior the new ceremony doesn't have a notion of, and the assertion was load-bearing on a now-nonexistent gate condition. Renamed to "blocks create-account runs while ToS is unaccepted" — that case is still real (a user entering create-account without `--accept-tos` should hold the gate until they accept), and the test now exercises it cleanly. **TRANSITION_GROUPS doc rot** `SigningUpScreen.tsx:104-107` claimed "the TRANSITION_GROUPS map in App.tsx collapses the dissolve between SignupEmail/FullName/SigningUp." No such map exists — the closed PR #234 mentioned it but the implementation didn't ship; I copied the comment forward by accident. Each screen swap is a discrete dissolve today, which is fine for our purposes. Drop the misleading sentence. 2919/2919 tests pass; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bird-m
added a commit
that referenced
this pull request
May 7, 2026
* feat: parse needs_information arm in direct-signup
The agentic provisioning endpoint can return a `needs_information` response
when the email belongs to a new user but the request didn't include the
fields the server needs to create the account (today: `full_name`). This
arm lives at amplitude/javascript: server/packages/thunder/src/lib/
agentic-provisioning/wizard/signup.ts and is gated by the
AgenticWizardNewUserSignup feature flag.
Until now the wizard parsed only the oauth / requires_auth / error arms
and reported `needs_information` as "Unexpected response" — fail-closed,
not actionable.
Changes:
- `NeedsInformationSchema` parses `{ type: 'needs_information',
needs_information: { type: 'object', properties, required: [...] } }`.
`properties` is preserved as opaque (record of unknown) so future
field additions don't fail-closed the parse; only `required` is
consumed.
- `DirectSignupResult` gains `{ kind: 'needs_information',
requiredFields: string[] }`. Doc-comment guidance for callers updated.
- `DirectSignupInput.fullName` is now optional. The request body conditionally
includes `full_name` only when set, matching the server's
`full_name: z.string().min(1).optional()` schema. Sending an empty
string would either be rejected or silently accepted depending on
coercion — neither is what we want.
- Tests cover the new arm, the email-only POST shape, and the
full-name-included POST shape.
This is purely additive in production: `signup-or-auth.ts` always passes
`fullName`, so the new branches are unreachable until that wrapper is
updated to support email-only probing in a follow-up PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): handle needs_information arm in signup-or-auth wrapper
Adding `needs_information` to the `DirectSignupResult` union (prior
commit) breaks type narrowing in `performSignupOrAuth`, which previously
only filtered out `requires_redirect` and `error` before reaching for
`result.tokens`. Without this fix, `tsc --noEmit` errs at the success-arm
property accesses.
The wrapper today is single-shot and can't collect missing fields — it's
called from non-TUI paths where both `email` and `fullName` are gated
upstream by the caller. Reaching the `needs_information` branch from
this wrapper means the wizard sent a valid `full_name` but the server
still asked for more (a future-field case the wrapper isn't equipped
to handle). Treat it as a redirect-style fallback: log, emit a
`needs_information` telemetry status, return null. The TUI path that
handles `needs_information` properly — collect the missing field via a
screen and re-POST — comes in a follow-up PR (server-driven field
collection).
Also adds `'needs_information'` to `SignupAttemptStatus` so this
outcome surfaces distinctly from `requires_redirect` in the agentic
signup attempted event.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(signup): clarify TUI-vs-non-interactive scope of signup CLI flags
The signup-related CLI flags `--auth-onboarding`, `--email`, and
`--accept-tos` were originally designed for non-interactive (`--ci` /
`--agent`) modes — there's no TUI to render the Intro picker, signup-email
screen, or ToS screen, so the flags carry that information instead.
In interactive TUI runs they're functionally inert today: the Intro
picker overwrites whatever `--auth-onboarding` set; the EmailCaptureScreen
silently pre-fills from `--email` and the ToS screen silently pre-accepts
from `--accept-tos`, both bypassing user-facing confirmation steps that
exist for a reason. `--help` doesn't signal any of this — every flag
appears in one flat Options block with no mode scope.
This PR cleans both up:
1. Tighten the `--help` describe text so each flag explicitly says
"(ignored in interactive TUI; …)". `--auth-onboarding` says the Intro
menu is canonical; `--email` says the email screen always renders;
`--accept-tos` says the ToS screen always renders.
2. Drop the three flags at session-build time when the resolved
execution mode is `interactive`. `buildSession` gains an optional
`executionMode` parameter and a single doorstep override that strips
`authOnboardingPath` / `authOnboarding` / `signup` /
`accountCreationFlow` / `signupEmail` / `acceptTos` before any
downstream resolver sees them. Every later read site (zod parse,
`resolveAuthOnboardingPathFromArgs`, default assignment) treats them
as unset.
3. `--full-name` is intentionally NOT gated. Pre-fill is just metadata
and bypasses no confirmation step — power users can keep using
`--full-name "Jane Doe"` to skip the name screen in TUI.
4. `buildSessionFromOptions` (the per-command entry point) resolves the
mode via `resolveMode` and threads `executionMode` through to
`buildSession`. The per-command callers (`apply`, `default`, `region`,
`mcp`, etc.) inherit the gating automatically.
5. When `executionMode` is omitted, `buildSession` preserves the legacy
"honor every flag" contract so existing callers (TUI store init,
tests, anything not yet routed through `buildSessionFromOptions`)
keep working unchanged.
Tests cover all four arms (interactive ignores each flag individually,
ci/agent honor every flag, omitted-mode preserves legacy behavior, and
`--full-name` is honored in interactive). 47 wizard-session tests pass;
2834 unit tests + 100 BDD scenarios green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): preserve signup flags in classic mode
Classic mode (--classic / AMPLITUDE_WIZARD_CLASSIC=1) is interactive in
the prompt sense — it uses terminal prompts via @clack/prompts — but it
does NOT have the Ink TUI screens (Intro picker / EmailCaptureScreen /
ToSScreen) that own signup-flag UX in real TUI mode. Classic also has
its own direct-signup call site at default.ts:348 that depends on
--auth-onboarding / --email / --full-name populating the session.
`resolveMode` knows nothing about --classic, so it sees a TTY without
--ci/--yes/--force/--agent and classifies that as 'interactive'. The
prior commit on this branch then made `buildSession` strip
authOnboardingPath / signupEmail / acceptTos in interactive mode —
silently regressing classic-mode direct signup. The classic branch's
runDirectSignupIfRequested call would no-op because
accountCreationProvisioningInputsReady checks all three of the now-
stripped fields, and the wizard would fall through to performAmplitudeAuth
(browser OAuth) — defeating the point of passing --auth-onboarding
create-account --email X --full-name Y in the first place.
Fix: in `buildSessionFromOptions`, detect --classic / AMPLITUDE_WIZARD_CLASSIC
and omit `executionMode` so `buildSession` falls through to its
legacy "honor every flag" path. Real TUI mode keeps the gating; classic,
ci, agent all keep their flags.
The abstraction in the prior commit assumed `executionMode === 'interactive'`
was a proxy for "the Ink TUI owns the signup screens" — which holds for
real TUI mode but not classic. Either the classic-detection at the call
site (this commit) or a finer ExecutionMode union (e.g. add 'classic')
would solve it; the call-site approach is minimal and keeps the existing
ExecutionMode contract unchanged.
New test file `src/commands/__tests__/build-session-from-options.test.ts`
covers the four-axis matrix (classic flag, classic env var, real TUI,
ci, agent) so this can't regress silently again. Caught by Cursor Bugbot
on PR #535.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(signup): semantic mode scoping in --help describe text
The describe text on `--auth-onboarding` and `--email` previously phrased
TUI gating as a parenthetical ("...ignored in interactive TUI; the menu
is canonical"). That reads as an exception to a default of "supported
everywhere", which is the wrong mental model — these flags exist
specifically for non-interactive entry points.
Rewrite as a positive enumeration of supported modes followed by an
explicit "ignored otherwise" so --help readers get the scope at a
glance:
Supported in --agent, --ci, and --classic modes; ignored otherwise.
`--accept-tos` is intentionally left as-is (its TUI-only ToS screen
behavior is materially different from the other two flags' behavior, so
the bespoke wording earns its keep). `--full-name` also unchanged — it's
honored in every mode so the supported/ignored framing doesn't apply.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(signup): tighten --help describe text for signup CLI flags
Refine the describe text on --auth-onboarding, --email, and --accept-tos
so each flag's intended scope is the focal point rather than the
gating exception:
- --auth-onboarding: phrase as "intended for use in --agent and --ci
modes; interactive modes prompt for selection" — clearer that the
Intro picker is the canonical interactive UX.
- --email: name the modes it's meant for inline ("in --agent or --ci
mode") instead of a trailing supported/ignored clause.
- --accept-tos: drop the parenthetical about TUI-only ToS rendering;
the inline mode scope already conveys it.
No behavior change. Just shorter, more direct help text.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(signup): wrapper discriminated union + ceremony session fields
Foundation for server-driven signup field collection in the TUI. No
user-visible behavior change yet — wires up the data model and the
contract the SigningUpScreen will consume.
**Wrapper refactor (`src/utils/signup-or-auth.ts`):**
`performSignupOrAuth` previously returned `T | null`, conflating four
distinct outcomes — success, redirect, error, and "not enough input
provided". Callers that wanted to distinguish them had to look at side
effects (telemetry events) or guess. The new shape is a discriminated
union mirroring `performDirectSignup`:
| { kind: 'success'; idToken; accessToken; ...; userInfo; dashboardUrl }
| { kind: 'needs_information'; requiredFields: string[] }
| { kind: 'redirect' }
| { kind: 'error'; message: string }
The wrapper now also accepts a missing `fullName` and probes the server
with email-only — required for the TUI's two-POST ceremony where the
first POST asks "is this user new?" and the server's needs_information
response decides what to collect next. Non-TUI callers (CI / agent /
classic) still gate on both being present upstream and never hit the
needs_information arm in practice.
`wrapper_exception` is now a distinct telemetry status (was previously
collapsed into `signup_error`), letting orchestrators distinguish a
thrown network/transport failure from the server's clean error arm.
**Caller updates:**
- `src/commands/default.ts` — TUI auth task narrows on `kind === 'success'`
and extracts the four token fields explicitly. needs_information /
redirect / error all fall through to the existing OAuth path with no
behavior change vs. today's null return.
- `src/commands/helpers.ts::runDirectSignupIfRequested` — same narrowing,
with a slightly more informative log message that names the non-success
arm.
**Session fields (`src/lib/wizard-session.ts`):**
Three new fields on `WizardSession` for the upcoming SigningUpScreen
ceremony:
- `signupRequiredFields: string[] | null` — set by SigningUpScreen on
`needs_information`; collection screens render iff their field key is
present.
- `signupAuth: { idToken; accessToken; refreshToken; zone; userInfo;
dashboardUrl } | null` — set on the success arm; drives the auth
task gate (next commit).
- `signupAbandoned: boolean` — set on redirect / error; releases the
auth gate to fall through to OAuth.
Plus matching setters / a single `resetSignupCeremony` resetter on
`WizardStore` for back-nav from collection screens.
**Tests:**
Existing signup-or-auth tests updated to assert the new discriminated
union (`result.kind === 'redirect'` instead of `result === null`, etc.)
plus a new test covering the email-only probe path. cli.test.ts mock
default switched from `vi.fn()` (returns undefined) to
`vi.fn().mockResolvedValue({ kind: 'error' })` so existing test paths
that don't override the mock still narrow safely.
2842/2842 unit tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(signup): server-driven field collection screens + flow rewire
Replaces the legacy EmailCaptureScreen + always-on ToSScreen with a
three-screen ceremony driven by the agentic provisioning endpoint's
needs_information response arm.
**Why**
Today's signup flow asks for email + name + ToS up front, then POSTs.
That means an existing customer (whose email is already on file) is
forced to provide name and ToS before the server can tell them "you
need to log in via the browser instead" — wasted prompts. The server
already knows whether agentic signup is happening based on email alone;
this PR moves the ceremony to match.
**Flow shape**
SignupEmail — collect email
↓
SigningUp — POST email-only; route on response
↓ ↓ ↓
oauth needs_information requires_auth / error
(success) (server asked for (existing user OR
more) server rejected)
↓ ↓ ↓
Auth ToS → SignupFullName → SigningUp (re-POST) → Auth
↓
(fall through to
browser OAuth via
signupAbandoned)
The router's flow-pipeline predicates implement the routing — there's
no imperative branching, no MAX_POSTS counter. SigningUp's `show`
predicate goes false the moment needs_information arrives (because
required fields are unsatisfied), the router walks to the collection
screens, and SigningUp re-shows once the fields are filled. Returning
users hit redirect → signupAbandoned → fall through, never seeing ToS
or the name prompt.
**New screens** (`src/ui/tui/screens/`)
- `SignupEmailScreen.tsx` — passive: TextInput → store.setSignupEmail.
Replaces EmailCaptureScreen's first step. Validates email format,
shows error inline. Esc rewinds to Welcome.
- `SigningUpScreen.tsx` — coordinator: only screen with network I/O.
useAsyncEffect calls performSignupOrAuth, then writes ONE of
`signupAuth` / `signupRequiredFields` / `signupAbandoned` based on
the discriminated-union response. fullName is included in the POST
only when ToS is accepted (avoids creating an account before the
user confirms ToS).
- `SignupFullNameScreen.tsx` — passive: TextInput → store.setSignupFullName.
Renders only when `signupRequiredFields` includes 'full_name' AND
the session doesn't already have a name (e.g. from --full-name).
Esc clears email + ceremony state so the user can correct a typo.
**Flow rewire** (`src/ui/tui/flows.ts`)
Replaces the EmailCapture + ToS create-account block with four entries
(SignupEmail / SigningUp / ToS / SignupFullName) and matching show /
isComplete / revert predicates. ToS now gates on `signupRequiredFields
!== null` so it ONLY renders after the server confirmed agentic signup.
Screen enum gains `SignupEmail`, `SigningUp`, `SignupFullName`. The
old `EmailCapture` enum value and screen file are removed.
**Auth task gate** (`src/commands/helpers.ts::isAuthTaskGateReady`)
Was: gates on tosAccepted on the create-account path.
Now: gates on `signupAuth !== null || signupAbandoned`. The auth task
runs concurrently with the screens, so without this gate it would race
SigningUpScreen and open a browser OAuth tab while the screen-driven
POST is still in flight. New unit tests cover both released arms
(signupAuth captured, ceremony abandoned to OAuth) plus the blocked
state.
**JourneyStepper** stays accurate — Auth-section screen list updated to
include the three new screens.
**BDD scenarios** updated in `features/02-wizard-flow.feature` to
reflect the new ceremony order. Added a new step
`the signup probe returns needs_information for "<field>"` that
mirrors what SigningUpScreen writes on that response arm. 100/100
scenarios pass.
**Tests**
- 2844 unit tests pass (router + flow-invariants + auth-gate + screens)
- 100 BDD scenarios pass
- Typecheck + lint clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(signup): router resolution coverage for the four ceremony shapes
11 new router tests pinning down each step of the create-account
ceremony's flow shapes:
Shape 1 (no flags) : SignupEmail → SigningUp → ToS →
SignupFullName → SigningUp → Auth
Shape 2 (--email only) : SigningUp → ToS → SignupFullName →
SigningUp → Auth
Shape 3 (email + full_name) : SigningUp (probe) → ToS →
SigningUp (success) → Auth
Shape 4 (requires_redirect) : SigningUp → Auth (browser OAuth via
signupAbandoned)
Each `it` exercises ONE router resolution along the path so a
predicate-level regression (wrong show condition, missing isComplete
arm) fails one specific test instead of fanning out across the whole
flow. Plus a sign-in-path negative test asserting none of the new
screens render when authOnboardingPath !== 'create_account'.
These complement the existing flow-invariants property tests (which
cover global properties like "router never resolves SignupEmail when
authOnboardingPath is sign_in") with concrete step-level pins.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): skip TUI auth task's wrapper call after ceremony abandon
After PR 3, `isAuthTaskGateReady` releases on either `signupAuth !== null`
or `signupAbandoned` (so a SigningUpScreen abandon to OAuth doesn't leave
the auth task waiting forever). On the abandon path the auth task body's
direct-signup gate at default.ts:761-766 then sees:
isCreateAccountOnboarding ✓
signupEmail ✓ (set by SignupEmailScreen)
signupFullName ✓ (set by SignupFullNameScreen)
!signupTokensObtained ✓ (no tokens — the screen abandoned)
…and re-POSTs the same args that SigningUpScreen just got redirect/error
on. Same response, falls through to OAuth, but burns an extra round-trip
and double-counts the telemetry event for the attempt.
Practical impact is bounded — the user's outcome is the same — but it's
a clean regression to fix. Add `!s.signupAbandoned` to the gate so the
abandon path goes straight to the OAuth fallback below.
Caught by Cursor Bugbot on PR #539. The architectural cleanup the smell
points at (auth task should read `signupAuth` directly instead of using
`signupTokensObtained` as a proxy + reading from disk) is deferred —
this fix is the minimal correct change matching the surrounding style.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): correct needs_information schema shape to match server
The agentic provisioning endpoint wraps the JSON-Schema payload in an
extra `schema` field. The earlier zod schema put `type` / `properties` /
`required` directly under `needs_information`, so every probe POST
fell through to the generic "unrecognized response shape" error and
silently routed users to OAuth — the same symptom reported live: a
new email gets `needs_information`, but the user is sent to the
redirect link instead of a name prompt.
Verified via direct curl against `https://app.amplitude.com/t/agentic/signup/v1`
on 2026-05-06:
{
"type": "needs_information",
"needs_information": {
"schema": {
"type": "object",
"properties": { "full_name": { ... } },
"required": ["full_name"]
}
}
}
Update `NeedsInformationSchema` to expect the `schema` wrapper, and
update the read site to drill `parsedNeeds.data.needs_information.schema.required`.
Test fixtures updated to mirror the real shape (an inline comment now
points at the curl-verified date so future drift gets caught).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(signup): pin properties-value opacity in needs_information schema
Document and lock the wire contract: each `properties` value in the
needs_information JSON-Schema is opaque to the wizard. The server today
emits `{ type, description }` per property, but `description` is purely
cosmetic and may be removed (or any other inner field added) without
coordination — we only consume `required`.
The schema already uses `z.unknown()` for property values, so the
behavior is correct today. This commit:
1. Extends the schema's leading comment to call out the opacity
contract explicitly, with a "do not tighten without re-verifying
against the live server" guard.
2. Adds a regression test (`accepts properties values with or without
optional metadata`) that parses a response with `{type}` (no
description) AND an entirely empty `{}` property value. Pins the
looseness so a future "tighten the inner shape" change can't
accidentally break wire compatibility — the server could remove
`description` tomorrow and the wizard would still parse.
22/22 direct-signup tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): bind ceremony reset to setSignupEmail(null); guard analytics on clears
Two related back-nav holes addressed in one setter rewrite:
**1. Stale ceremony state across back-nav (the bigger one)**
The signup ceremony has shared state — `signupRequiredFields`,
`signupAuth`, `signupAbandoned` — that's set by `SigningUpScreen` (from
the server response) and consumed by multiple downstream entries' `show`
predicates (`ToS`, `SignupFullName`, the re-show of `SigningUp`).
The per-entry `revert` callbacks only un-did the field that *they* set.
None cleared the shared ceremony state. Concrete failure mode:
1. User on `ToS`, presses Esc → router walks back to `SignupEmail`,
calls its revert → `signupEmail` set to null.
2. `signupRequiredFields` stays `['full_name']` from the prior probe.
3. User retypes any email → `SigningUp.show` is false because
the gate `(signupRequiredFields === null || tosAccepted === true)`
fails (tosAccepted got reset between steps).
4. Router lands the user back on `ToS` without a fresh probe POST —
consuming the stale `needs_information` against a possibly
different email.
Bind the ceremony reset to `setSignupEmail(null)` itself: any path
that rewinds the user back to an empty email state automatically
invalidates the prior probe. The previous `resetSignupCeremony`
helper is removed (its only caller was `SignupFullNameScreen`'s Esc
handler, which now gets the reset for free).
**2. False "captured" analytics on back-nav (Cursor Bugbot finding)**
PR #539 widened `setSignupEmail` and `setSignupFullName` to accept
`null` so revert handlers could clear the values. The setters fired
`'signup email captured'` / `'signup full name captured'` analytics
events unconditionally — including on `null` clears with `{ 'has email':
false }` payloads. That polluted the top-of-funnel "captured" metric
with non-capture events (every Esc-out emitted one).
Fix: guard the analytics calls behind `value !== null`. Drop the
now-always-true `'has email'` / `'has name'` properties at the same
time — they were defensive payloads that became meaningless once the
event only fires on positive captures.
**Tests**
5 new store tests pin the contract:
- `setSignupEmail(string)` fires analytics; `setSignupEmail(null)` does not.
- `setSignupEmail(null)` resets `signupRequiredFields`, `signupAuth`,
`signupAbandoned`.
- Same shape for `setSignupFullName`.
2861 unit tests pass (was 2856 + 5 new); typecheck + lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): make ToS / SignupFullName reverts return false when skipped
Closes the abandon-path back-nav trap. Walks the user past entries
that were "complete because they were skipped" rather than firing a
no-op revert that stops the walk on a state change that doesn't exist.
**The bug**
After `requires_redirect → signupAbandoned=true → browser OAuth`, the
user is on `Auth`. Pressing Esc:
1. Walk back from Auth-1 (`SignupFullName`).
2. `SignupFullName.isComplete` returns true via the
`signupRequiredFields === null` arm — it was skipped because the
server never asked for `full_name`.
3. `SignupFullName.revert` fires → `setSignupFullName(null)`. But
signupFullName was never set; the call is a no-op.
4. Revert returns void (truthy) → router stops walking.
5. Forward resolve: SignupFullName.show is false; everything else
skips back to `Auth`.
User pressed Esc and the screen didn't change. `ToS.revert` had the
same shape (`tosAccepted` was never set, `resetToS` was a no-op,
walk stopped on a phantom revert).
**The fix**
Both reverts now return `false` when `isComplete` is satisfied via
the "screen was skipped" arms — there's nothing to undo, so the
back-walk should continue.
- `ToS.revert`: returns false when `signupRequiredFields === null`
OR `tosAccepted === null`.
- `SignupFullName.revert`: returns false when
`signupRequiredFields === null` OR
`!signupRequiredFields.includes('full_name')` OR
`signupFullName === null`.
In the abandon path, both return false in sequence; the walk reaches
`SignupEmail`, whose revert clears email + ceremony state (per the
prior fix), and forward resolve correctly lands the user on
`SignupEmail` with everything reset.
In the success path, `SignupFullName.revert` IS meaningful
(signupFullName was set), so the walk stops there. The user lands on
SignupFullName and re-types — known leaky because the server-side
account already exists, but at least back-nav is responsive. Tracked
separately.
**Tests**
Two new router tests:
- `Esc on Auth in the abandon path walks past skipped signup screens
to SignupEmail` — the regression that motivated this fix.
- `Esc on Auth in the success path walks back to SignupFullName` —
pins the known-leaky case so we notice if it changes.
Stub store extended to mirror the production `setSignupEmail(null)`
ceremony-reset contract so the test's back-walk produces a faithful
forward-resolve.
2863/2863 unit tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): backToWelcome must clear ceremony state alongside signupEmail
`backToWelcome` writes `signupEmail = null` via raw `$session.setKey` —
bypassing `setSignupEmail(null)`'s ceremony-reset contract that commit
2e521d16 bound to the setter. Result: a user mid-ceremony who hits Esc
back to Welcome leaves stale `signupRequiredFields` / `signupAuth` /
`signupAbandoned` on the session.
Failure mode:
1. User submits email; server returns needs_information; session has
signupRequiredFields=['full_name'].
2. User Esc's back to Welcome.
3. User restarts with a fresh email — but signupRequiredFields is
still set, so SigningUp.show is false and ToS lands directly with
a stale required-fields cache. Or worse: if the user's already
hit the success arm (signupAuth populated), the auth gate
releases on stale tokens.
`backToWelcome` can't *call* `setSignupEmail(null)` here because it
batches multiple raw setKey writes and emits one change at the end —
calling the setter mid-batch would emit twice. Inline the ceremony
reset directly, with a comment pointing back to `setSignupEmail`'s
contract so the two stay in sync.
The existing test (`store.test.ts:933`) didn't assert any of the new
ceremony fields, so it passed despite the broken contract. Extended
to cover (a) the standard mid-ceremony Esc-back case and (b) the
edge case where signupAuth is populated before backToWelcome fires.
152 store tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(signup): remove EmailCaptureScreen vestige
PR #539 deleted EmailCaptureScreen.tsx but left behind:
- `WizardStore.markEmailCaptureComplete` / `resetEmailCapture` — the
setter/resetter pair the deleted screen called. No production
callers remain; only the BDD step defs and the legacy
`backToWelcome` write referenced them.
- `WizardSession.emailCaptureComplete` — a boolean that was the
isComplete signal for the old EmailCapture FlowEntry. The new flow
gates on `signupEmail !== null` directly (`SignupEmail.isComplete`
in flows.ts). The field is now write-only — never read.
- `'email capture complete'` and `'back navigation' { to:
'email-capture' }` analytics events emitted by those setters.
- BDD step defs in `wizard-flow.steps.ts` set `emailCaptureComplete =
true` after Then-actions and asserted on it in `the signup flow
should switch to regular auth`. Tautological under the new gate;
replaced with a meaningful `signupEmail === null` assertion.
Removing all four lets the next reader navigate the create-account
section without false leads. The `'tosAccepted'` clears in
`backToWelcome` stay — they're still load-bearing.
`signupTokensObtained` doc-comment refreshed: it used to point at
`EmailCaptureScreen`; now points at the SigningUp ceremony with a
note on why we keep it as a flag separate from `signupAuth`.
2864 unit tests pass; 100 BDD scenarios pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(signup): drop empty wizardCapture args + tighten --full-name describe
**Empty `{}` props on `wizardCapture`**
`SignupEmailScreen` and `SignupFullNameScreen` Esc handlers fired
`analytics.wizardCapture('signup ... screen back', {})` — the empty
object adds nothing the API needs. Other call sites in this codebase
omit the second arg when there are no properties; matching that style.
**`--full-name` describe text**
Pre-PR-535 the describe said "requires --auth-onboarding create-account".
Post-PR-535 `--auth-onboarding` is silently ignored in interactive TUI
(the Intro menu is canonical) — so the "requires" framing is false on
that path. Rewrite to acknowledge both:
Required in --agent or --ci mode; in interactive TUI it pre-fills
the name screen as a metadata-only shortcut.
Mirrors the language used on `--email` and `--accept-tos` post-PR-535.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(default): document auth-task gate's settle dependency
The TUI auth task's `await isAuthTaskGateReady` now depends on
`SigningUpScreen` writing `signupAuth` or `signupAbandoned`. That's a
non-obvious data dependency — the auth task lives in default.ts and
the screen lives in src/ui/tui/screens/, separated by 1500+ lines of
infrastructure.
Add an inline block explaining the three settle paths
(success / non-success / timeout-then-abandon) and the load-bearing
invariant: don't remove `REQUEST_TIMEOUT_MS` from `direct-signup.ts`
without re-thinking this gate. A raw async-effect with no timeout
would let the gate hang forever on a network hang, with no way for
the user to force a settle short of `/exit`.
No behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(signup): sync flows.md to the server-driven ceremony
CLAUDE.md calls out flows.md as source of truth for the wizard's UX
and asks for it to be updated *first* on flow changes. PR #539
shipped the new ceremony screens and shipped the code-side rewire,
but the docs side was missed. This catches up.
**Mermaid diagram (flows.md + wizard-flow.mmd):**
Replaced the EmailCapture → ToS → Auth chain with the
server-driven path:
SignupEmail
↓
SigningUp (probe POST)
↓
┌──────────┬──────────────────┬─────────┬─────────────────┐
│ oauth │ requires_auth │ error │ needs_information │
↓ ↓ ↓ ↓
Auth Auth Auth ToS → SignupFullName → SigningUp (re-POST) → Auth
The diagram makes explicit that ToS and SignupFullName only render
on the `needs_information` arm — the regression behind PR #539's
existence (forcing ToS+name on existing-user redirects).
**Esc / back-nav table:**
Rewrote per-screen rows for the four new ceremony screens.
SignupFullName / ToS rows note the "rewinds to SignupEmail with
ceremony state cleared" behavior bound by `setSignupEmail(null)`.
Added a SigningUp row noting the `revert: () => false` transparent
walk-past contract. Added an "invariants" subsection capturing:
- `setSignupEmail(null)` clears all ceremony state.
- `SignupFullName.revert` / `ToS.revert` return `false` on the
"screen was skipped" arms.
- `SigningUpScreen` is the only signup screen with network I/O.
**Side-effect updates:**
`pnpm flows` regenerated three other .mmd files where flows.md had
drifted (data-setup-flow title, framework-detection-flow plan-step
rewrite, susi-flow ampli-migration wording). Including them here
because they're "sync the diagrams to the prose" output of the same
command — leaving them stale would just mean the next person who
runs `pnpm flows` ships them with their unrelated PR.
The renderer also spat out a misnamed `back-navigation.mmd` that
was a near-duplicate of `top-level-commands.mmd` (the renderer's
section detector matched my new "Back navigation" subsection
header). Deleted — bug to track separately if it recurs.
`pnpm flows` re-runnable cleanly. Tests + BDD still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: drop --classic-mode workaround now that --classic is gone
Main's #558 (`chore: remove --classic mode`) deleted the `--classic`
flag and `AMPLITUDE_WIZARD_CLASSIC` env-var entirely. The
classic-mode workaround in `buildSessionFromOptions` (added in PR
#535's bugbot fix to avoid stripping signup flags in classic-mode
TTY runs) is now reading flags that no longer exist —
`Boolean(options.classic)` and `process.env.AMPLITUDE_WIZARD_CLASSIC
=== '1'` are always false, so the gate is dead code.
Drop the gate and the two tests that exercised it. The remaining
three tests (TUI / CI / agent mode) still cover the live behavior:
TUI strips signup flags, non-interactive modes honor them.
2911 / 2911 tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): treat needs_information shapes other than ['full_name'] as terminal
The wizard's TUI ceremony has exactly one collection screen
(`SignupFullNameScreen`). The flow predicates and the post-response
plumbing assume the server's `needs_information.schema.required` is
either empty (probe-only) or the literal `['full_name']`. Any other
shape — an unknown field name, a mix, or an empty array — drops the
user into a UI freeze on the SigningUp spinner: the every() in
`SigningUp.show` returns true for unknown fields (the ternary's
else-branch is `true` for anything not equal to `'full_name'`), so
the screen re-mounts and re-POSTs once more, then sits forever
because no collection screen is showable for a field we don't know.
Today's blast radius: low — server only sends `['full_name']`. But
every wizard release in the wild is one server change away from
silently freezing standard-mode runs. This commit makes the
contract explicit at the wrapper layer.
**Behavior**
`performSignupOrAuth` now validates the `requiredFields` array
shape against an allowlist (currently `['full_name']`). On match,
behavior is unchanged. On any other shape — including extra
fields, missing fields, empty array, substituted field — the
wrapper returns `{ kind: 'error', message: ... }` and emits a new
`needs_information_unsupported` telemetry status. The screen's
existing `error` handling routes the user to the OAuth fallback via
`signupAbandoned`, the same path it takes on `requires_redirect`.
The schema layer (`direct-signup.ts`) stays intentionally loose —
`z.unknown()` for property values, `min(1)` on `required` to reject
malformed empties at the wire layer. Wrapper handles the semantic
"do we know how to act on this" gate.
**Why a distinct telemetry status?**
`needs_information_unsupported` is separate from `signup_error` so
the wire-contract drift is visible in the funnel. If this status
starts firing in production, it's a hard signal that the server has
added a required field the wizard doesn't yet handle — one
dashboard query reveals the drift before users start filing tickets.
**Tests**
Five new wrapper tests pin the contract:
- 4× `it.each` covering unknown-only / mixed / empty / substituted
shapes — assert error return + `needs_information_unsupported`
telemetry.
- 1× passes-through test for the supported `['full_name']` shape —
asserts unchanged behavior + regular `needs_information`
telemetry.
20/20 signup-or-auth tests pass. 2916/2916 unit tests pass overall.
Typecheck + lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(signup): move needs_information shape gate to the zod schema
Last commit's wrapper-level allowlist worked but split the wire-shape
contract across two layers — `direct-signup.ts` parsed any well-formed
`needs_information` arm; `signup-or-auth.ts` then re-validated the
`required` content against `['full_name']`. Cleaner to enforce in one
place: the schema.
**The change:**
`NeedsInformationSchema.required` gains a `.refine()` constraining it
to exactly `SUPPORTED_REQUIRED` (currently `['full_name']`). Anything
else fails the parse. To preserve the distinct telemetry signal —
"server changed contract" vs. "server returned garbage" —
`direct-signup.ts` now peeks at `response.data.type` after the parse
chain and, if it's `'needs_information'`, returns
`{ kind: 'error', code: 'unsupported_required_shape', message: ... }`
instead of falling through to the generic "Unexpected response"
error. The wrapper maps that code to
`needs_information_unsupported` telemetry.
**Why this shape:**
- The schema is now the single source of truth for the supported
`required` shape. To extend (e.g. add a `SignupCompanyScreen`),
update `SUPPORTED_REQUIRED` once. The wrapper has nothing to know.
- Schema's overall contract stays "loose by design" everywhere it
matters — `properties` values remain `z.unknown()`, the JSON-Schema
`description` field stays optional. The refine targets only the
semantic axis where the wizard's UX is actually opinionated.
- Type-aware fall-through means a real malformed response (e.g. body
doesn't even have `type`) still surfaces as plain "Unexpected
response" → `signup_error` telemetry, distinct from the contract-
drift signal.
**Tests**
- `direct-signup.test.ts`: 5 new schema-rejection cases (table) plus
one positive shape pin. All exercise the real schema via MSW.
- `signup-or-auth.test.ts`: prior table-driven wrapper cases collapse
to two — one mapping `code: 'unsupported_required_shape'` →
`needs_information_unsupported` telemetry, one verifying generic
errors stay on `signup_error`.
2919/2919 unit tests pass. Typecheck + lint clean. Wrapper drops 22
lines; schema-layer adds 19 lines + the refine.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): thread AbortSignal through performSignupOrAuth → axios
`SigningUpScreen.useAsyncEffect`'s `signal` was checked AFTER the
wrapper returned, but the wrapper itself didn't accept or honor the
signal — so the in-flight axios POST kept running after unmount, and
on the success arm `replaceStoredUser` wrote tokens to disk even when
the user had already navigated away. Net: `/exit` mid-POST left the
user "signed in" on the next launch (Intro picker hid the
create-account option) without their having completed the ceremony.
**Plumbing**
- `DirectSignupInput` and `SignupOrAuthInput` gain an optional
`signal: AbortSignal`.
- `performDirectSignup` passes it to both `axios.post` calls
(provisioning + token exchange) so axios cancels in-flight requests
on abort.
- After each axios call, an `if (input.signal?.aborted)` guard returns
`{ kind: 'error', code: 'aborted' }` so we don't continue to token
exchange / persistence after a cancelled provisioning POST, and
don't continue to persistence after a cancelled token exchange.
- `performSignupOrAuth` adds a final guard immediately before
`replaceStoredUser` — covers the window between the last network
call and the disk write. Returns `{ kind: 'error', message:
'aborted' }`, which the screen routes through its existing abandon
arm to OAuth fallback.
- `SigningUpScreen.useAsyncEffect` passes its `signal` to
`performSignupOrAuth`. The existing `if (signal.aborted) return`
guard on the screen side stays as a backstop.
**Why three guard points, not one**
The wrapper does three things in sequence: provisioning POST, token
exchange POST, user-info fetch. A guard between each ensures the
wrapper can't slip past abort to a downstream step. The
`replaceStoredUser` guard is the most important — it's the
disk-leaking step.
**No new abort code in `performAmplitudeAuth`**
OAuth fallback path is initiated externally (browser opens, user
interacts) — there's no in-flight `await` to cancel from the screen
side.
2919 / 2919 unit tests pass. Typecheck + lint clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(signup): drop emailCaptureComplete orphans + TRANSITION_GROUPS doc rot
**emailCaptureComplete cleanup**
Three test files still spread `emailCaptureComplete` into session
overrides: `auth-gate.test.ts:64`,
`session-checkpoint-events.test.ts:106`,
`AuthScreen.coaching.test.tsx:54,85`. The field was removed from
`WizardSession` in commit `e5df61b3`; tsc didn't catch the orphans
because `tsconfig.json` excludes `src/**/__tests__/**/*`.
The auth-gate test "blocks create-account runs that have only
completed email capture" was the most concerning — its name described
behavior the new ceremony doesn't have a notion of, and the assertion
was load-bearing on a now-nonexistent gate condition. Renamed to
"blocks create-account runs while ToS is unaccepted" — that case is
still real (a user entering create-account without `--accept-tos`
should hold the gate until they accept), and the test now exercises
it cleanly.
**TRANSITION_GROUPS doc rot**
`SigningUpScreen.tsx:104-107` claimed "the TRANSITION_GROUPS map in
App.tsx collapses the dissolve between SignupEmail/FullName/SigningUp."
No such map exists — the closed PR #234 mentioned it but the
implementation didn't ship; I copied the comment forward by accident.
Each screen swap is a discrete dissolve today, which is fine for our
purposes. Drop the misleading sentence.
2919/2919 tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(signup): single _resetCeremonyKeys() helper for ceremony reset
`setSignupEmail(null)` and `backToWelcome` were both inlining the
same ceremony-reset writes — `signupRequiredFields` / `signupAuth` /
`signupAbandoned` — with a comment in `backToWelcome` saying "keep
this in sync with `setSignupEmail`." That's exactly the fragile
pattern the original review flagged.
Extract `_resetCeremonyKeys()` as a private method on `WizardStore`.
Both call sites use it. The "keep in sync" comment goes away because
there's nothing left to sync.
**Defensive scope expansion**
The helper also clears `signupFullName` and `tosAccepted` — the
inputs the ceremony fed. Originally I argued the bug-as-described
(stale `signupFullName` after Esc-back leaks into the next probe)
isn't reachable because `TextInput`'s draft state doesn't lift to
session without an explicit submit. That's still true today.
But the broader "ceremony reset = whole ceremony" semantic is
consistent with the comment we already ship, costs ~3 lines, and
covers a class of future regressions: any change that adds
`onChange={(v) => setSignupFullName(v)}` for live persistence would
otherwise activate the leak. `tosAccepted` has a related
stale-but-load-bearing risk — `SigningUpScreen` only includes
`full_name` in the POST when `tosAccepted === true`, so a stale
`true` after a back-out would change POST semantics on the next
forward pass.
Cheap insurance, and it makes the contract uniform.
**Tests**
Existing `setSignupEmail(null) resets ceremony state` test renamed
and extended to assert that `signupFullName` and `tosAccepted` are
also cleared. The defensive coverage is documented inline so a
future reader sees why we clear "more than the strictly-needed"
fields.
2919/2919 tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(signup): pin abort-signal contract for cancellation path
Two regression tests for the abort plumbing landed in the prior
commit:
1. **`aborting the signal pre-call returns error without persisting
tokens`** — pins the load-bearing guard inside
`performSignupOrAuth`: even if the success arm of
`performDirectSignup` resolves (mocked) and the user-info fetch
succeeds (mocked), an aborted signal must skip
`replaceStoredUser`. Without that, the disk-write happens behind
the back of an unmounted screen and the user lands as "signed in"
on the next launch despite explicitly cancelling.
2. **`threads signal through to performDirectSignup`** — pins the
wire-level pass-through. Without this, the screen's
`useAsyncEffect` AbortController couldn't actually cancel the
axios POSTs on unmount; we'd be relying on a guard that the screen
never reaches because `performSignupOrAuth` blocks on a
non-cancelable promise.
Together they cover both halves of the abort contract: the
network-cancel half (signal reaches axios) and the persistence-skip
half (signal blocks `replaceStoredUser`). A regression in either
half re-introduces the disk-leak failure mode flagged on the review.
2921/2921 unit tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(signup): exhaustive switch on PerformSignupOrAuthResult arms
Reviewer's pushback was sharper than my original counter — the
`rate_limited / retryAfter` example surfaced a failure mode I'd
dismissed: TS catches new arms whose properties get accessed in
existing code (the success-arm `result.tokens` access), but does NOT
catch new arms that should have bespoke handling and instead silently
route to the deny-by-default fallback. The "easy to add later"
framing was weak — "later" is "after a contributor adds an arm and
nobody notices the semantics drift."
The cost asymmetry tilts in favor of `assertNever`:
- Now: ~3-5 lines per call site, plus a one-line helper.
- Later: rediscover every consumer of the union and re-verify each
was correct.
`assertNever` doesn't change the deny-by-default behavior — every
existing call site still routes non-success arms to OAuth fallback.
What it does is force every call site to *list* the arms it's
fallback-ing. When the union grows, TypeScript points at each one.
**Changes**
New `src/utils/assert-never.ts` — bog-standard exhaustiveness helper
with a runtime throw as defense-in-depth.
Three call sites restructured to exhaustive switch on
`PerformSignupOrAuthResult.kind`:
- `SigningUpScreen.tsx` — was an if-chain ending in an unguarded
`setSignupAbandoned(true)` for "everything else." Now lists
`redirect | error` explicitly with `default: assertNever`.
- `default.ts` (TUI auth task) — was `if (success) {...}` with a
"fall through to OAuth" comment. Now lists each non-success arm
in a single combined case with the same fall-through behavior,
plus the assertNever default.
- `helpers.ts::runDirectSignupIfRequested` (CI/agent path) — was
`if (kind !== 'success') return;` (negation, so a new arm would
auto-route to fallback without TS noticing). Now exhaustive.
Behavior is unchanged. The compile-time check is the upgrade.
2918+/2918+ tests pass (3 unrelated oauth-server failures from a
stale port-listener process not connected to this change). Typecheck
clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: prettier --write on signup-or-auth.ts and assert-never.ts
CI's `pnpm lint:prettier` flagged two files. Husky's pre-commit hook
runs `prettier --write` on staged files but appears to have missed
these on the prior commits — likely the new-file path on
`assert-never.ts`, with the cascading import edit on
`signup-or-auth.ts` carrying the same drift. No code change; pure
whitespace/formatting per the project's prettier config.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): abandon on needs_information for an already-satisfied field
If the server returns `needs_information` with a field the wizard has
already populated (e.g. re-requesting `full_name` after we sent
`full_name`), the ceremony can't make progress:
1. SigningUpScreen mounts with `useAsyncEffect` deps
`[email, fullName]`.
2. Effect runs, POSTs, server returns
`needs_information: ['full_name']`.
3. Handler calls `setSignupRequiredFields(['full_name'])` — but
`signupRequiredFields` isn't a useAsyncEffect dep.
4. React re-renders. SigningUp.show stays true (no unmount).
useAsyncEffect deps unchanged. No re-fire.
5. User wedges on the spinner forever; only escape is Ctrl+C.
Today this only fires as a server bug (the schema's `.refine()`
already rejects unsupported `required` shapes, so the only way to
reach this path is the server requesting the same supported shape it
just received). Probability low; failure mode bad enough that the
one-branch guard is worth the cost.
**Fix**
In the `needs_information` arm, check whether every requested field
is already populated in the session. If yes, the wrapper can't make
progress — log, abandon to OAuth, route the user to browser auth.
This is the same fall-through `unsupported_required_shape` already
uses; we just generalize the "I don't know what to do with this" set
to include "you asked for something I gave you."
The check only knows about `full_name` today (the single supported
field), matching the schema's allowlist. If `SUPPORTED_REQUIRED`
grows, this list grows alongside.
Caught by reviewer subagent on PR #539.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): include signupTokensObtained in _resetCeremonyKeys
`_resetCeremonyKeys` (PR #539's helper consolidating ceremony reset
across `setSignupEmail(null)` and `backToWelcome`) was clearing five
fields but missing `signupTokensObtained`. `backToWelcome` cleared
it explicitly; `setSignupEmail(null)` didn't.
Failure mode: user signs up successfully on attempt 1
(`signupTokensObtained=true`) → backs out via an Esc that walks
through `setSignupEmail(null)` → `signupTokensObtained` stays true →
next forward pass hits `default.ts:807` (the post-success hydration
branch), reads tokens from `~/.ampli.json`, and silently re-signs the
user in as the prior account even though they explicitly rewound to
start fresh.
The fix is to clear `signupTokensObtained` inside the helper so all
ceremony-reset paths land in the same state. The redundant explicit
write in `backToWelcome` (post-helper-call) becomes unnecessary —
removed.
Existing test extended: the `setSignupEmail(null) resets the entire
ceremony as one unit` case now pre-seeds `signupTokensObtained=true`
and asserts it's cleared post-call. Comment explains why this field
matters operationally so a future reader doesn't drop it as
"redundant flag."
Caught by reviewer subagent on PR #539. Related to MCP-245
(architectural cleanup to remove `signupTokensObtained` entirely in
favor of `signupAuth`-as-discriminator); this commit is the targeted
fix while that broader refactor stays scoped to its own work.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(default): delete dead post-gate signup wrapper call in TUI
PR #539's auth-task gate (`isAuthTaskGateReady`) requires
`signupAuth !== null || signupAbandoned` before releasing on the
create-account path. That guarantees, by the time the TUI auth-task
block runs:
- Success: `signupAuth` set AND `signupTokensObtained=true` (set
synchronously by SigningUpScreen alongside `signupAuth`).
- Abandon: `signupAbandoned=true`.
The block at `default.ts:746-806` then gated on
`!s.signupTokensObtained && !s.signupAbandoned` — false on both axes
in either gated state — so it could never fire in TUI mode. Pure
dead code: ~60 lines of unreachable switch-with-assertNever, a
redundant `performSignupOrAuth` call (SigningUpScreen already made
it), and the only remaining caller of `assertNever` in this file.
CI / agent / classic modes have their own signup path
(`runDirectSignupIfRequested` in `helpers.ts`), so deleting this
block doesn't affect them.
**What's left**
The `else if (s.signupTokensObtained)` branch survives — it's the
actual hydration path that reads tokens from disk and avoids
opening a spurious browser OAuth on a fresh-install-dir success.
Promoted from `else if` to `if` since the prior block is gone.
Comment updated: it referenced the deleted `EmailCaptureScreen`
("EmailCaptureScreen already called replaceStoredUser…"); now points
at the live writer (`SigningUpScreen settled the ceremony…`).
`trackSignupAttempt` import moved out of the deleted block (still
used by the `browser_fallback_after_signup` telemetry call further
down). `assertNever` import dropped — no other callers in this file.
**Why this works as a deletion, not an "add a comment" preservation**
If a future change weakens `isAuthTaskGateReady` (e.g. allows the
gate to release before SigningUpScreen settles), the dead block
becomes live again — and would re-introduce the redundant POST that
PR #539's bugbot fix `caa57a30` was specifically about preventing.
Better to delete now and force the next change author to think
through the gate semantics, rather than leave a dormant trap.
Caught by reviewer subagent on PR #539 (issues #3 and #4 — dead
block + stale comment).
2909/2909 unit tests pass (3 unrelated oauth-server failures from a
stale port-listener process). Typecheck + prettier clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(signup): fold markSignupTokensObtained into setSignupAuth
The TUI auth-task gate (`isAuthTaskGateReady`) releases on
`signupAuth !== null`; the post-gate hydration branch in `default.ts`
reads `signupTokensObtained` to decide whether to pull tokens from
disk vs. open browser OAuth. Both fields are part of the same logical
"ceremony settled successfully" event — but they were two separate
calls (`setSignupAuth(...)` followed by
`store.markSignupTokensObtained()`), with `setSignupMagicLinkUrl`
sandwiched in between.
That ordering quietly relied on event-loop semantics: nanostores
notifies subscribers synchronously, so the gate-listener could in
principle fire its microtask continuation between the two writes and
observe `signupAuth` set with `signupTokensObtained` still false —
then open browser OAuth even though valid tokens were already in
hand. The shape was a footgun waiting for a refactor.
Fold the `signupTokensObtained=true` write into `setSignupAuth` so
both fields land in the same synchronous setKey + emitChange block.
Add a test pinning the contract: `setSignupAuth(non-null)` flips
`signupTokensObtained=true`; `setSignupAuth(null)` leaves it false
(so a ceremony reset can't accidentally re-open the gate).
* refactor(signup): funnel switchToLogin through _resetCeremonyKeys
`switchToLogin` was inlining a subset of the ceremony reset
(`signupEmail = null`, `signupFullName = null`) but missing the rest:
`signupRequiredFields`, `signupAuth`, `signupAbandoned`, `tosAccepted`,
`signupTokensObtained`. Today nothing in the SignIn path reads those
fields, so the omission is latent — but it's the same shape of bug
that prompted `_resetCeremonyKeys` in the first place. The moment
someone adds a new ceremony field (or wires a sign-in code path that
reads one of the existing ones), this drifts.
Funnel the cleanup through the shared helper so any new ceremony
field added to `_resetCeremonyKeys` is automatically cleared on the
signup→login switch too. Pin the contract with a test that pre-seeds
the full ceremony state and asserts every field clears.
* docs(signup): tighten ceremony comments and auth-gate test names
Bundle of small wording fixes flagged in peer review of PR #539. No
behavior change; everything here is comments + test names.
* SignupFullName Esc handler: the previous comment said "Esc rewinds
to the email screen so the user can correct a typo" — but
`setSignupEmail(null)` clears the email entirely (it doesn't
preserve the prior value), and crucially also resets the rest of
the ceremony state via `_resetCeremonyKeys`. Reword to reflect both
effects so a future reader doesn't think Esc is a soft rewind.
* `auth-gate.test.ts`: two tests asserted the gate blocks "until ToS
is accepted" / "while ToS is unaccepted", but the gate itself no
longer checks `tosAccepted` (PR-539 moved the predicate to "signup
ceremony has settled" — `signupAuth !== null || signupAbandoned`).
Rename both to make explicit that the *ceremony-unsettled* check is
what holds the gate, and document the pre-PR-539 history in a
shared describe-block comment so the redundancy with "ceremony in
flight" is intentional and explained.
* `default.ts` post-gate hydration branch: comment referenced the
removed `markSignupTokensObtained` setter. Rewrite to point at the
new atomic write inside `setSignupAuth`.
* `_resetCeremonyKeys` `signupTokensObtained` reset: cross-link to
`setSignupAuth` so a reader landing on the reset side knows where
the forward-direction write lives (and why it's folded together).
* `flows.ts` SigningUp `revert: () => false`: expand the comment to
explain WHY there's no in-band undo (the in-flight POST may have
created or abandoned the account on the server) and HOW back-nav
still works (walks past to a screen whose own revert clears the
inputs and resets the ceremony).
* test(signup): pin direct-signup inner abort guards
The two `if (input.signal?.aborted)` checks inside `performDirectSignup`
(post-provisioning, post-token-exchange) only had transitive coverage
through the wrapper-level abort tests. If a future refactor splits the
token-exchange step out or moves the wrapper's persistence gate, the
"skip parsing on abort" behavior could regress without any test failure.
Stubs `axios.post` to deterministically land the abort between the
await resolving and the post-await guard — MSW operates below axios
and can't synchronize that window. One test per guard, asserting the
function returns `{kind:'error', code:'aborted'}` and (for the first
guard) that token exchange never fires.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(signup): switch+assertNever in performSignupOrAuth wrapper
Convert the if-chain on result.kind to a switch with default:
assertNever(result). TS narrowing already protects the wrapper from a
silent fall-through into the success block (the success path reads
result.tokens, which would fail typecheck on any new arm), but the
explicit pattern matches what every other consumer of
PerformSignupOrAuthResult / DirectSignupResult uses (SigningUpScreen,
runDirectSignupIfRequested). The wrapper is the closest layer to the
wire and the most worth-protecting site for an exhaustiveness check.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): hydrate signupUserInfo from session.signupAuth
The auth task declared signupUserInfo as `const null`, making the
`if (signupUserInfo)` skip-fetch branch unreachable. The wrapper had
already fetched the user profile (with provisioning retry) and
SigningUpScreen mirrored it onto session.signupAuth.userInfo, but
the auth task never read it — falling through to a redundant
fetchAmplitudeUser call on every successful signup happy path.
Read from tui.store.session.signupAuth?.userInfo so the skip-fetch
branch becomes live: one fewer RTT per successful signup, and the
inline storeToken (already in the else branch) gets correctly skipped
since the wrapper's replaceStoredUser covered it. When the wrapper's
fetch failed (provisioning lag exhausted retries),
signupAuth.userInfo is null and we fall through to the probe path —
same behavior as before.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(signup): note intentional signal omission in non-TUI helper
runDirectSignupIfRequested doesn't pass `signal` to performSignupOrAuth
because CI / agent / classic modes have no in-band cancellation
surface — no Esc handler, no unmount lifecycle, nothing to thread
through. Add a short note at the call site so the omission doesn't
read as an oversight when the TUI's SigningUpScreen passes a signal a
few files away.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(signup): fold signupEmail into _resetCeremonyKeys
Every existing caller of `_resetCeremonyKeys` (setSignupEmail,
switchToLogin, backToWelcome) was already clearing signupEmail
explicitly around the call — the helper's "callers handle email"
contract was a footgun waiting to be tripped by a future caller that
forgot the matched line. Fold the clear into the helper so the
ceremony reset is a single atomic operation.
setSignupEmail's null branch now goes only through the helper (no
redundant double-write); the non-null branch still does its own
setKey + analytics capture. switchToLogin and backToWelcome shed
their explicit clears. The backToWelcome doc comment that listed
specific keys is replaced with a pointer to the helper as the
source of truth — listing keys inline created drift risk every
time a new ceremony field was added.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): wipe ceremony state on /region
setRegionForced cleared every other zone-scoped key (credentials,
OAuth intermediates, org/project selection, framework state, etc.)
but left the signup ceremony untouched. signupAuth.zone is pinned
to the old region and signupRequiredFields is a cached snapshot
from the old zone's probe POST — both would silently leak into the
next forward pass on the new zone, steering the user through the
wrong field-collection screens or short-circuiting the
needs_information probe entirely.
Funnel through the shared `_resetCeremonyKeys` helper alongside the
existing zone-scoped clears so any future ceremony field gets reset
automatically. New store test seeds all seven ceremony keys
(signupEmail, signupFullName, tosAccepted, signupRequiredFields,
signupAuth, signupAbandoned, signupTokensObtained) and asserts they
clear after setRegionForced — the canary for the contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): wipe ceremony state on auth→region back-nav
resetAuthForRegionChange clears every other zone-scoped key
(credentials, pendingOrgs, selected*, etc.) but skipped the signup
ceremony. Same zone-scoping reasoning as setRegionForced:
signupAuth.zone is pinned to the old region and signupRequiredFields
cached the old zone's probe response — both would silently leak
across the region change.
Dormant today: tracing the back-nav path from Auth → RegionSelect on
the create-account flow shows that the per-screen reverts of the
signup screens already clear individual fields before this revert
fires, so no current reader observes a stale signupAuth here. The
fix preserves the invariant that every reset path goes through
_resetCeremonyKeys — adding a new caller (or changing the back-nav
order) won't have to remember the matched-line contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(signup): walk past collection screens on back-nav after abandonment
When the second POST errored after ToS+name collection,
signupAbandoned=true while signupRequiredFields, tosAccepted, and
signupFullName were all populated. Esc on Auth would land on
SignupFullName, the user types a new name, the next forward pass
skips SigningUp (its show predicate gates on !signupAbandoned), and
the user is dumped back on Auth without any retry — a confusing
dead-end where typing a name accomplishes nothing.
Make ToS.revert and SignupFullName.revert return false when
signupAbandoned is true so the back-walk continues to
SignupEmail.revert. SignupEmail.revert calls setSignupEmail(null),
which funnels through _resetCeremonyKeys and clears the whole
ceremony — including signupAbandoned — giving the user a clean
restart.
Stub fix: makeStubStore's setSignupEmail(null) was missing
signupFullName / tosAccepted / signupTokensObtained from its mutate
patch, …
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
3 participants
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
Replaces today's "all fields required up front" behavior for
--signupwith server-driven interactive collection. The wizard POSTs to the provisioning endpoint with whatever it has; if the server respondsneeds_information, the wizard collects the requested field(s) via TUI screens and retries once.Linear: MCP-194 (part of the Wizard — direct signup UX project)
Scope
--classic,--agent,--ci— these retain today's hard-fail on missing--email/--full-name. Classic-mode interactive prompting is tracked separately.amplitude/javascriptrestores theneeds_informationresponse arm (removed in commitc9b560eof PR #103683). The wizard may ship ahead of that restoration — if so, theneeds_informationarm is dead code until the server catches up.What's in
SignupEmailScreen,SigningUpScreen,SignupFullNameScreen— three new TUI screens wired into the flow pipeline.SigningUpScreen— reads session, callsperformSignupOrAuth, writes outcome. Screens stay passive.signupCeremonySettledwait condition inbin.ts— blocks the OAuth browser handoff until the signup ceremony terminates.AuthScreengains a signup-aware context line ("Please sign up or log in from the browser to continue.") when falling through to OAuth.TRANSITION_GROUPSmap inApp.tsx— new generic primitive that collapsesDissolveTransitionanimations for screens in the same logical step. Used here so the signup ceremony feels like one screen updating in place.SigningUpScreenrenders a mimic of the previous input screen (heading + submitted value + "Signing up…") so the screen-swap looks continuous.direct-signup.ts/signup-or-auth.tsnow parse theneeds_informationarm and return a discriminated union.signupAuthon the session is narrowed to the success arm at the type level.Architecture
SigningUpScreenonly; input screens just write to the session. Preserves theCLAUDE.mdconvention.needs_information" property falls out of session state, not from anyMAX_POSTSconstant.signupAuthtype-narrowed — the session field can only hold a success result, making thebin.tsread a single!== nullcheck.Testing
performDirectSignup,performSignupOrAuth, andSigningUpScreen; full router + flow-invariant coverage.wizard-mock-signup(new local dev tool at~/bin/wizard-mock-signup— not shipped as part of this PR, lives in Michael's toolbelt). Six scenarios exercising the response matrix without the real backend.Known follow-up (NOT fixed here)
MCP-195 — AuthScreen identity-resolution flash for new signups. Pre-existing behavior in
AuthScreen.tsx:415-429that renders a brief completion summary (Framework / Org / Workspace / Project) before the router advances. For a fresh signup — exactly 1 org / 1 workspace / 1 env, all auto-selected — the summary flashes for one React tick.Not introduced by this PR. The code has existed since the TUI-v2 redesign; neither this PR nor PR #165 touched
AuthScreen's completion summary,flows.ts,App.tsx, orrouter.tsin a way that affects it.The reason it's newly visible (and only newly visible — not newly broken): pre-#165,
--signupalways routed through a browser OAuth handoff. The user's attention was on the browser while the terminal silently resolved identity and advanced. By the time the user came back to the terminal, the flash had already happened. PR #165 removed that handoff. The user's attention now stays on the terminal, so every post-auth transition is visible — including this one.Fix is ~10 lines in
AuthScreen(minimum display time on the completion summary). Deliberately scope-carved into a separate ticket to keep this PR focused.References
~/repos/docs/snippets/direct-signup-needs-information-design.md~/repos/docs/snippets/direct-signup-needs-information-plan.mdamplitude/javascriptcommitc9b560e—needs_informationresponse arm to be restoredTest plan
--signup(no flags) in interactive TUI: email prompt → name prompt (after server asks) → success OR redirect fall-through--signup --email x(email flag only): no email prompt, name prompt appears if server asks--signup --email x --full-name "Y": no prompts at all, direct to auth / redirect--agent --signupand--ci --signupstill hard-fail with AUTH_REQUIRED (3) on missing flags — behavior unchangedNote
Medium Risk
Touches the onboarding/auth boundary (direct signup, OAuth handoff gating, and credential persistence) and adds new session state, so regressions could strand users in signup/auth flows or wipe project-scoped state unexpectedly.
Overview
Enables server-driven field collection for
--signupin the interactive TUI: the wizard now POSTs whatever signup data it has, handles aneeds_informationresponse by routing to new collection screens (email/full name), then retries signup or falls back to browser OAuth when required fields are unknown/unmet.This introduces new session fields (
signupRequiredFields,signupAuth,signupAbandoned, and unifiedaccountCreationFlow), updates flow/router predicates and auth-task gating so OAuth won’t start until the signup ceremony settles, and groups signup screens to suppress transition animations.Direct signup/OAuth plumbing is updated to return a discriminated union from
performSignupOrAuth, parseneeds_informationfrom the provisioning endpoint, support request abort viaAbortSignal, passinstallDirthrough auth/signup calls, and wipe per-installDir stale project state (API key, checkpoint, ampli.json auth fields) on successful signup or fresh OAuth. Extensive unit/property/BDD tests are added/updated to cover the new state machine.Reviewed by Cursor Bugbot for commit 087aef4. Bugbot is set up for automated code reviews on this repo. Configure here.