AUTH-6732: fix workos install spinner/prompt race - #216
Conversation
…6732) During npx workos install, a spinner's 80ms redraw interval could keep running while an interactive prompt was waiting for input, scribbling over the question so users never realized they were being asked (reported on a Rails run). Root cause — not clack (already removed in #200, which switched prompts to @inquirer/prompts and made withPrompt pause the *registered* active spinner). The remaining race was spinner lifecycle in the CLI adapter: 1. The agent spinner (started on agent:start) was only stopped by validation:start — which Ruby never emits and JS skips under --no-validate — so it kept animating through the whole post-install phase. 2. Handlers like handleCommitGenerating overwrote this.spinner with a fresh handle without stopping the old one. After the commit prompt resumed the agent spinner, its interval was orphaned: still firing every 80ms but no longer registered, so withPrompt could not pause it and it redrew over the next prompt (e.g. the PR prompt). Fix (minimal — no prompt-library swap needed; the race was lifecycle, not the prompt engine): - ui.spinner(): single-spinner invariant. start() retires the currently active spinner (halt interval + deregister; skips the line erase when the old spinner is paused mid-prompt so it can't wipe the question), and every write is guarded by an active flag so a stale handle goes inert instead of scribbling over its successor. - CLIAdapter: agent:success now finalizes the agent spinner ('Agent completed') so it never outlives the agent phase into the commit/PR prompts; failure paths were already finalized by handleError/handleComplete. - CLIAdapter: all spinner creation goes through startSpinner(), which clears the previous handle first, keeping this.spinner honest. Regression tests: ui.spec.ts covers retire-on-start, inert stale handles, prompt pause/resume, and no-resurrect-after-retire; cli-adapter.spec.ts covers agent:success stopping the spinner and phase-spinner replacement clearing the previous handle. The five lifecycle tests fail against the old code and pass with the fix. Refs AUTH-6732
Greptile SummaryThe PR coordinates spinner ownership across installer phases and prompts to prevent stale animation intervals from redrawing over interactive questions.
Confidence Score: 5/5The PR appears safe to merge with no actionable defects identified in the changed spinner lifecycle. The adapter finalizes the agent spinner before post-install work, while the UI layer consistently retires replaced handles and prevents paused or stale spinners from resuming unexpectedly. Important Files Changed
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| private startSpinner(message: string): void { | ||
| this.spinner?.clear(); | ||
| this.spinner = ui.spinner(); | ||
| this.spinner.start(message); | ||
| } |
There was a problem hiding this comment.
🔍 Adapter's startSpinner uses clear(), bypassing the new paused-aware retire()
startSpinner calls this.spinner?.clear() before creating the new handle, but clear() in src/utils/ui.ts always calls clearLine() (writes \r\x1b[2K) regardless of the paused flag, whereas the newly added retire() deliberately skips the erase while paused because the prompt owns the line. If a phase spinner is started while a prompt is open (spinner paused by withPrompt), this explicit clear() erases the prompt's line — exactly the class of damage retire() was added to avoid. start() already retires the previous spinner globally, so the clear() here is redundant for the interval-leak fix; consider dropping it or making clear() paused-aware. I could not construct a definitely-reachable adapter path (spinner starts appear not to overlap open prompts today), so this is flagged rather than reported as a bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
bosun task: AUTH-6732: fix workos install spinner/prompt race
Task id: task-msh06waw-xydw
Shape: ship
Project: workos/cli