feat(signup): bounded + abortable post-signup user fetch#613
Draft
bird-m wants to merge 1 commit into
Draft
Conversation
Captures the reviewer-staged work for split-out into a focused PR.
Five items at this preservation point:
1. Bounded + abortable user fetch: api.ts adds {timeoutMs, signal}
options; signup-or-auth.ts adds UserFetchTimeoutError,
abortError(), isTimeoutOrAbort(), abortableDelay(),
fetchAmplitudeUserBounded(). Retry loop bails immediately on
timeout/abort.
2. In-memory signupAuth tokens (default.ts): TUI auth task uses
session.signupAuth instead of getStoredToken from disk.
3. Fail-closed on premature success (SigningUpScreen.tsx): refuses
tokens if ToS not accepted or required fields not satisfied.
4. Abort classification (direct-signup.ts isCallerAbort + wrapper
short-circuit): caller aborts surface as code:'aborted' and skip
signup_error telemetry.
5. Tests: two abort tests in direct-signup.test.ts, hang + abort
tests in signup-or-auth.test.ts, cli.test.ts in-memory tokens
test, new SigningUpScreen.test.tsx component test.
Items 2-4 will move to feat/signup-server-driven-fields. Item 1
stays here for its own follow-up PR after that branch merges. This
WIP commit will be reorganized into proper commits at rebase time.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
Plumb a timeout and abort signal into the post-signup user fetch loop so the TUI auth gate can't hang after tokens are issued, and so user-initiated cancels (Esc,
/exit, screen unmount) propagate end-to-end through the retry loop instead of running silently in the background.This branch was carved out of the larger server-driven signup ceremony work (#539) so the bounded-fetch infrastructure can be reviewed in isolation. The persistence-side abort guard from #539 already prevents data integrity issues; this branch is a UX/observability quality pass that closes the "background work continues after Esc" wart.
What changed
src/lib/api.ts—fetchAmplitudeUseraccepts anoptions: { timeoutMs?, signal? }parameter and threads both into axios. Backwards-compatible (defaults to current behavior).src/utils/signup-or-auth.tsUserFetchTimeoutErrorclass for distinguishing user-fetch timeouts from generic errors.abortError(),isTimeoutOrAbort()helpers.abortableDelay(delayMs, signal)— replaces baresetTimeoutfor the retry sleep loop, with properclearTimeout+removeEventListenercleanup infinally.fetchAmplitudeUserBounded(idToken, zone, signal)— wrapsfetchAmplitudeUserinPromise.racewith both an explicit 5s timer and a signal-abort listener (defense-in-depth in case axios's internal handling drops one).fetchUserWithProvisioningRetrynow accepts a signal and bails immediately on timeout/abort instead of continuing through the retry loop.src/utils/__tests__/signup-or-auth.test.ts— adds a "fetchAmplitudeUser hangs → falls back to pending sentinel" test that exercises the bounded-fetch path under fake timers.Why this isn't on the parent PR
#539 is already a sizable surgery on the signup ceremony (server-driven multi-step field collection, ceremony state reset across four entry points, etc.). The bounded-fetch work introduces ~140 lines of new abstraction (
UserFetchTimeoutError, four helpers, retry-loop refactor, public-API change tofetchAmplitudeUser) that's worth its own focused review. Splitting it out keeps #539's diff scoped to "what changes about the user-facing ceremony" and keeps this PR scoped to "how do we cancel work cleanly when the user backs out."Status
🚧 Draft, parked for now.
Base is set to
feat/signup-server-driven-fieldsso the diff renders cleanly against the parent. Once #539 merges, this branch needs:git rebase main(base will switch fromfeat/signup-server-driven-fieldstomain).The WIP commit currently includes the union of items 2–5 from the same review pass that produced this work — those landed on #539 in their own commits, so the rebase will resolve those hunks as no-ops. The remaining diff at rebase time will be exactly the bounded-fetch infrastructure.
Test plan
src/utils/__tests__/signup-or-auth.test.tsfetchAmplitudeUser(e.g. point at unreachable Data API) — confirm the wrapper times out at 5s and falls back to the pending sentinel rather than stranding the auth gate🤖 Generated with Claude Code