AUTH-6732: fix workos install spinner/prompt race

src/lib/adapters/cli-adapter.spec.ts

@@ -364,6 +364,70 @@ describe('CLIAdapter', () => {
     });
   });
 
+  describe('spinner lifecycle (AUTH-6732)', () => {
+    type SpinnerMock = {
+      start: ReturnType<typeof vi.fn>;
+      stop: ReturnType<typeof vi.fn>;
+      message: ReturnType<typeof vi.fn>;
+      clear: ReturnType<typeof vi.fn>;
+    };
+    const makeSpinnerMock = (): SpinnerMock => ({
+      start: vi.fn(),
+      stop: vi.fn(),
+      message: vi.fn(),
+      clear: vi.fn(),
+    });
+
+    it('stops the agent spinner on agent:success so it cannot outlive the agent phase', async () => {
+      await adapter.start();
+      const ui = await import('../../utils/ui.js');
+      const spinnerMock = makeSpinnerMock();
+      vi.mocked(ui.default.spinner).mockImplementation(() => spinnerMock as never);
+
+      emitter.emit('agent:start', {});
+      emitter.emit('agent:success', { summary: 'done' });
+
+      expect(spinnerMock.stop).toHaveBeenCalledWith('Agent completed', 0);
+    });
+
+    it('starting a new phase spinner clears the previous handle instead of orphaning it', async () => {
+      await adapter.start();
+      const ui = await import('../../utils/ui.js');
+      // A fresh handle per ui.spinner() call, so each phase gets its own.
+      vi.mocked(ui.default.spinner).mockImplementation(() => makeSpinnerMock() as never);
+
+      emitter.emit('agent:start', {});
+      emitter.emit('postinstall:commit:generating', {});
+
+      const handles = vi.mocked(ui.default.spinner).mock.results.map((r) => r.value as SpinnerMock);
+      expect(handles).toHaveLength(2);
+      // The agent spinner was cleared (not just abandoned with a live interval)
+      // before the commit spinner started.
+      expect(handles[0].clear).toHaveBeenCalled();
+      expect(handles[0].start).not.toHaveBeenCalledWith('Generating commit message...');
+      expect(handles[1].start).toHaveBeenCalledWith('Generating commit message...');
+    });
+
+    it('a post-install prompt after agent:success has no stale spinner to resurrect', async () => {
+      await adapter.start();
+      const ui = await import('../../utils/ui.js');
+      const spinnerMock = makeSpinnerMock();
+      vi.mocked(ui.default.spinner).mockImplementation(() => spinnerMock as never);
+      vi.mocked(ui.default.confirm).mockResolvedValue(false);
+
+      emitter.emit('agent:start', {});
+      emitter.emit('agent:success', { summary: 'done' });
+      emitter.emit('postinstall:commit:prompt', {});
+      await new Promise((r) => setTimeout(r, 10));
+
+      // Exactly one stop (agent:success); the prompt did not restart or re-stop
+      // a stale agent spinner, and no new spinner was created for the prompt.
+      expect(spinnerMock.stop).toHaveBeenCalledTimes(1);
+      expect(spinnerMock.start).toHaveBeenCalledTimes(1);
+      expect(sendEvent).toHaveBeenCalledWith({ type: 'COMMIT_DECLINED' });
+    });
+  });
+
   describe('staging success copy', () => {
     it('device path announces a fresh environment without "retrieved"', async () => {
       await adapter.start();

src/lib/adapters/cli-adapter.ts

@@ -142,6 +142,7 @@ export class CLIAdapter implements InstallerAdapter {
     this.subscribe('config:complete', this.handleConfigComplete);
     this.subscribe('agent:start', this.handleAgentStart);
     this.subscribe('agent:progress', this.handleAgentProgress);
+    this.subscribe('agent:success', this.handleAgentSuccess);
     // Persistent, append-only log of file operations + tool calls above the spinner.
     this.subscribe('file:write', this.handleFileWrite);
     this.subscribe('file:edit', this.handleFileEdit);
@@ -211,6 +212,20 @@ export class CLIAdapter implements InstallerAdapter {
     }
   }
 
+  /**
+   * Start a fresh spinner for a new phase, clearing any previous handle first.
+   * ui.spinner() enforces the same single-spinner invariant globally (start()
+   * retires the active spinner), but clearing here additionally keeps
+   * this.spinner honest: it never points at a handle whose phase already ended
+   * (AUTH-6732 — an overwritten handle left its interval redrawing over the
+   * next prompt).
+   */
+  private startSpinner(message: string): void {
+    this.spinner?.clear();
+    this.spinner = ui.spinner();
+    this.spinner.start(message);
+  }
+
   /** Debug logging - only outputs when debug mode is enabled */
   private debugLog = (message: string): void => {
     if (this.debug) {
@@ -313,20 +328,16 @@ export class CLIAdapter implements InstallerAdapter {
     console.log(`  ${chalk.cyan(verificationUri)}`);
     console.log(`\nEnter code: ${chalk.bold(userCode)}\n`);
 
-    this.spinner = ui.spinner();
-    this.spinner.start('Waiting for authentication...');
+    this.startSpinner('Waiting for authentication...');
   };
 
   private handleDeviceSuccess = (): void => {
     // Spinner will be stopped by handleStagingFetching
   };
 
   private handleStagingFetching = (): void => {
-    if (this.spinner) {
-      this.spinner.stop('Authenticated');
-    }
-    this.spinner = ui.spinner();
-    this.spinner.start('Fetching your WorkOS credentials...');
+    this.stopSpinner('Authenticated');
+    this.startSpinner('Fetching your WorkOS credentials...');
   };
 
   private handleStagingSuccess = ({ source }: InstallerEvents['staging:success']): void => {
@@ -450,12 +461,22 @@ export class CLIAdapter implements InstallerAdapter {
   };
 
   private handleAgentStart = (): void => {
-    this.spinner = ui.spinner();
-    this.spinner.start(this.lastAgentMessage);
+    this.startSpinner(this.lastAgentMessage);
     // No setInterval: ui animates its own frames, and the old 2s reset
     // clobbered the current phase text set by handleAgentProgress.
   };
 
+  /**
+   * The agent phase is over — finalize its spinner. Integrations that run
+   * validation emit validation:start (which stops it first); this covers the
+   * ones that don't (Ruby, or any run with --no-validate), so the spinner
+   * never outlives its phase into the post-install commit/PR prompts
+   * (AUTH-6732). Failure paths are already finalized by handleError/handleComplete.
+   */
+  private handleAgentSuccess = (): void => {
+    this.stopSpinner('Agent completed');
+  };
+
   private handleAgentProgress = ({ step, detail }: InstallerEvents['agent:progress']): void => {
     const message = detail ? `${step}: ${detail}` : step;
     this.lastAgentMessage = message;
@@ -473,8 +494,7 @@ export class CLIAdapter implements InstallerAdapter {
     this.spinner = null;
     render();
     if (wasRunning) {
-      this.spinner = ui.spinner();
-      this.spinner.start(this.lastAgentMessage);
+      this.startSpinner(this.lastAgentMessage);
     }
   }
 
@@ -614,8 +634,7 @@ export class CLIAdapter implements InstallerAdapter {
 
   private handleScaffoldStart = ({ packageManager }: InstallerEvents['scaffold:start']): void => {
     this.scaffoldPackageManager = packageManager;
-    this.spinner = ui.spinner();
-    this.spinner.start(`Scaffolding a new Next.js app with ${packageManager} (this can take a minute)...`);
+    this.startSpinner(`Scaffolding a new Next.js app with ${packageManager} (this can take a minute)...`);
   };
 
   // create-next-app output is verbose; surface it only under --debug and keep
@@ -683,8 +702,7 @@ export class CLIAdapter implements InstallerAdapter {
   };
 
   private handleCommitGenerating = (): void => {
-    this.spinner = ui.spinner();
-    this.spinner.start('Generating commit message...');
+    this.startSpinner('Generating commit message...');
   };
 
   private handleCommitSuccess = ({ message }: InstallerEvents['postinstall:commit:success']): void => {
@@ -711,16 +729,14 @@ export class CLIAdapter implements InstallerAdapter {
   };
 
   private handlePrGenerating = (): void => {
-    this.spinner = ui.spinner();
-    this.spinner.start('Generating PR description...');
+    this.startSpinner('Generating PR description...');
   };
 
   private handlePrPushing = (): void => {
     if (this.spinner) {
       this.spinner.message('Pushing to remote...');
     } else {
-      this.spinner = ui.spinner();
-      this.spinner.start('Pushing to remote...');
+      this.startSpinner('Pushing to remote...');
     }
   };
 

src/utils/ui.spec.ts

@@ -196,6 +196,117 @@ describe('prompt coordination (withPrompt)', () => {
   });
 });
 
+describe('spinner coordination (AUTH-6732)', () => {
+  let stdoutTtyDesc: PropertyDescriptor | undefined;
+  let writeSpy: ReturnType<typeof vi.spyOn>;
+  let logSpy: ReturnType<typeof vi.spyOn>;
+
+  const writes = () => writeSpy.mock.calls.map((c) => String(c[0]));
+
+  beforeEach(() => {
+    vi.useFakeTimers();
+    // Spinners only animate on a TTY; stub it so the redraw interval runs.
+    stdoutTtyDesc = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY');
+    Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true });
+    writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
+    logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    vi.clearAllTimers();
+    vi.useRealTimers();
+    writeSpy.mockRestore();
+    logSpy.mockRestore();
+    if (stdoutTtyDesc) Object.defineProperty(process.stdout, 'isTTY', stdoutTtyDesc);
+    else delete (process.stdout as { isTTY?: boolean }).isTTY;
+  });
+
+  it('starting a new spinner retires the previous one (no orphaned interval)', () => {
+    const a = ui.spinner();
+    a.start('phase A');
+    vi.advanceTimersByTime(240);
+    expect(writes().some((w) => w.includes('phase A'))).toBe(true);
+
+    writeSpy.mockClear();
+    const b = ui.spinner();
+    b.start('phase B');
+    vi.advanceTimersByTime(240);
+
+    const after = writes();
+    expect(after.some((w) => w.includes('phase A'))).toBe(false);
+    expect(after.some((w) => w.includes('phase B'))).toBe(true);
+    b.stop('done');
+  });
+
+  it('a retired spinner handle goes inert — stop()/clear() print nothing', () => {
+    const a = ui.spinner();
+    a.start('phase A');
+    const b = ui.spinner();
+    b.start('phase B'); // retires A
+
+    writeSpy.mockClear();
+    logSpy.mockClear();
+    a.stop('should not print');
+    a.clear();
+    a.message('should not render');
+    vi.advanceTimersByTime(240);
+
+    expect(writes().every((w) => !w.includes('should not'))).toBe(true);
+    expect(logSpy).not.toHaveBeenCalled();
+    // The live spinner is untouched and keeps animating.
+    expect(writes().some((w) => w.includes('phase B'))).toBe(true);
+    b.stop('done');
+  });
+
+  it('a prompt pauses the active spinner and resumes it after the answer', async () => {
+    const s = ui.spinner();
+    s.start('Working');
+    vi.advanceTimersByTime(160);
+    expect(writes().some((w) => w.includes('Working'))).toBe(true);
+
+    let framesDuringPrompt = 0;
+    writeSpy.mockClear();
+    vi.mocked(inquirer.confirm).mockImplementationOnce(async () => {
+      // While the prompt awaits input, the spinner's 80ms redraw must not fire.
+      vi.advanceTimersByTime(500);
+      framesDuringPrompt = writes().filter((w) => w.includes('Working')).length;
+      return true;
+    });
+
+    await ui.confirm({ message: 'ok?' });
+    expect(framesDuringPrompt).toBe(0);
+
+    writeSpy.mockClear();
+    vi.advanceTimersByTime(240);
+    expect(writes().some((w) => w.includes('Working'))).toBe(true);
+    s.stop('done');
+  });
+
+  it('replacing the spinner mid-prompt does not erase the prompt, and the old spinner stays dead', async () => {
+    const a = ui.spinner();
+    a.start('Working');
+
+    let erasesAtReplace = 0;
+    vi.mocked(inquirer.confirm).mockImplementationOnce(async () => {
+      writeSpy.mockClear();
+      const b = ui.spinner();
+      b.start('Next phase'); // retires the paused "Working" spinner
+      erasesAtReplace = writes().filter((w) => w.includes('\x1b[2K')).length;
+      b.stop('done');
+      return true;
+    });
+
+    await ui.confirm({ message: 'ok?' });
+    // A paused spinner owns no line — retiring it must not wipe the prompt's.
+    expect(erasesAtReplace).toBe(0);
+
+    // The prompt's resume() must not resurrect the retired spinner.
+    writeSpy.mockClear();
+    vi.advanceTimersByTime(500);
+    expect(writes().every((w) => !w.includes('Working'))).toBe(true);
+  });
+});
+
 describe('dashboard mode suppresses output', () => {
   let logSpy: ReturnType<typeof vi.spyOn>;
   beforeEach(() => {