feat(wizard): deliver progress over the RPC wizard protocol

bkudiess · Copilot · bkudiess · commit fb150b456f28 · 2026-06-18T13:31:28.000-07:00
WizardSessionPrompter.progress() was a no-op, so every spin.update/stop during installs, plugin/channel downloads, and auth waits was dropped for non-terminal clients (e.g. the Windows companion), which appeared frozen during the slowest parts of onboarding. Emit progress as non-interactive 'progress' steps (already in the wizard step schema). Latest-wins single slot, drained ahead of the pending prompt; next() loops so a progress wake never decides completion, which keeps concurrent next() callers from surfacing a spurious done while running. Clients re-poll without an answer to continue; progress steps are not answerable. Refs #94661. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
diff --git a/src/wizard/session.test.ts b/src/wizard/session.test.ts
@@ -131,4 +131,133 @@ describe("WizardSession", () => {
     }
     await session.answer(plainStep.id, "alice");
   });
+
+  test("delivers progress steps before the next prompt and advances on re-poll", async () => {
+    const session = new WizardSession(async (prompter) => {
+      const spin = prompter.progress("Working");
+      spin.update("Step 1");
+      spin.stop("Done step");
+      await prompter.text({ message: "Name" });
+    });
+
+    // progress()/update()/stop() collapse latest-wins to the final message.
+    const progress = await session.next();
+    expect(progress.done).toBe(false);
+    expect(progress.step?.type).toBe("progress");
+    expect(progress.step?.message).toBe("Done step");
+
+    // Re-poll WITHOUT an answer advances to the interactive prompt.
+    const prompt = await session.next();
+    expect(prompt.step?.type).toBe("text");
+    if (!prompt.step) {
+      throw new Error("expected text step");
+    }
+    await session.answer(prompt.step.id, "Peter");
+
+    const done = await session.next();
+    expect(done.done).toBe(true);
+    expect(done.status).toBe("done");
+  });
+
+  test("delivers progress emitted while a client is waiting on next()", async () => {
+    let releaseWork: () => void = () => {};
+    const work = new Promise<void>((resolve) => {
+      releaseWork = resolve;
+    });
+    const session = new WizardSession(async (prompter) => {
+      const spin = prompter.progress("Working");
+      await work;
+      spin.update("Halfway");
+      await prompter.text({ message: "Name" });
+    });
+
+    // Initial label emitted synchronously at construction.
+    const first = await session.next();
+    expect(first.step?.type).toBe("progress");
+    expect(first.step?.message).toBe("Working");
+
+    // No pending step yet: this poll blocks until progress wakes it.
+    const waiting = session.next();
+    releaseWork();
+    const woken = await waiting;
+    expect(woken.step?.type).toBe("progress");
+    expect(woken.step?.message).toBe("Halfway");
+
+    const prompt = await session.next();
+    expect(prompt.step?.type).toBe("text");
+    if (!prompt.step) {
+      throw new Error("expected text step");
+    }
+    await session.answer(prompt.step.id, "x");
+    const done = await session.next();
+    expect(done.done).toBe(true);
+  });
+
+  test("progress steps are not answerable", async () => {
+    const session = new WizardSession(async (prompter) => {
+      prompter.progress("Working");
+      await prompter.text({ message: "Name" });
+    });
+
+    const progress = await session.next();
+    if (!progress.step) {
+      throw new Error("expected progress step");
+    }
+    await expect(session.answer(progress.step.id, null)).rejects.toThrow(
+      /wizard: no pending step/i,
+    );
+  });
+
+  test("cancel clears pending progress", async () => {
+    const session = new WizardSession(async (prompter) => {
+      prompter.progress("Working");
+      await prompter.text({ message: "Name" });
+    });
+
+    session.cancel();
+
+    const done = await session.next();
+    expect(done.done).toBe(true);
+    expect(done.status).toBe("cancelled");
+  });
+
+  test("concurrent next() callers never get a spurious done while running", async () => {
+    let releaseGate: () => void = () => {};
+    const gate = new Promise<void>((resolve) => {
+      releaseGate = resolve;
+    });
+    const session = new WizardSession(async (prompter) => {
+      const spin = prompter.progress("start");
+      await gate;
+      spin.update("more");
+      await prompter.text({ message: "Name" });
+    });
+
+    // Consume the progress emitted synchronously at construction.
+    const firstStep = await session.next();
+    expect(firstStep.step?.message).toBe("start");
+
+    // Two concurrent waiters share the same internal step waiter — the race a
+    // progress wake (which resolves that waiter with null) used to mishandle.
+    const p1 = session.next();
+    const p2 = session.next();
+    releaseGate();
+    const [r1, r2] = await Promise.all([p1, p2]);
+
+    // Critical invariant: while running, no caller may surface a done result.
+    expect(r1.done).toBe(false);
+    expect(r2.done).toBe(false);
+    expect(
+      [r1.step?.type, r2.step?.type].toSorted((a, b) => String(a).localeCompare(String(b))),
+    ).toEqual(["progress", "text"]);
+
+    const textResult = [r1, r2].find((r) => r.step?.type === "text");
+    if (!textResult?.step) {
+      throw new Error("expected a text step");
+    }
+    await session.answer(textResult.step.id, "Peter");
+    const done = await session.next();
+    expect(done.done).toBe(true);
+    expect(done.status).toBe("done");
+  });
 });
diff --git a/src/wizard/session.ts b/src/wizard/session.ts
@@ -155,10 +155,18 @@ class WizardSessionPrompter implements WizardPrompter {
     return Boolean(res);
   }
 
-  progress(_label: string): WizardProgress {
+  progress(label: string): WizardProgress {
+    // Emit progress as non-interactive steps so remote clients see live status
+    // during slow operations. Clients render the message and re-poll wizard.next
+    // WITHOUT an answer to continue. Local CLI keeps using the clack prompter.
+    this.session.emitProgress(label);
     return {
-      update: (_message) => {},
-      stop: (_message) => {},
+      update: (message) => this.session.emitProgress(message),
+      stop: (message) => {
+        if (message !== undefined) {
+          this.session.emitProgress(message);
+        }
+      },
     };
   }
 
@@ -176,6 +184,9 @@ export class WizardSession {
   private currentStep: WizardStep | null = null;
   private stepDeferred: Deferred<WizardStep | null> | null = null;
   private pendingTerminalResolution = false;
+  // Latest undelivered progress step. Progress is non-interactive and "latest
+  // wins", so it is held as a single slot rather than an unbounded queue.
+  private pendingProgress: WizardStep | null = null;
   private answerDeferred = new Map<string, Deferred<unknown>>();
   private status: WizardSessionStatus = "running";
   private error: string | undefined;
@@ -186,24 +197,39 @@ export class WizardSession {
   }
 
   async next(): Promise<WizardNextResult> {
-    if (this.currentStep) {
-      return { done: false, step: this.currentStep, status: this.status };
-    }
-    if (this.pendingTerminalResolution) {
-      this.pendingTerminalResolution = false;
-      return { done: true, status: this.status, error: this.error };
-    }
-    if (this.status !== "running") {
-      return { done: true, status: this.status, error: this.error };
-    }
-    if (!this.stepDeferred) {
-      this.stepDeferred = createDeferred();
-    }
-    const step = await this.stepDeferred.promise;
-    if (step) {
-      return { done: false, step, status: this.status };
+    // Loop so a progress wake never decides completion. A progress emit resolves
+    // the shared step waiter with null while the session is still running; with
+    // concurrent next() callers, one consumes the progress and the rest must
+    // re-classify state (and re-wait) rather than surface a spurious done.
+    for (;;) {
+      // Deliver any in-flight progress before the pending prompt so remote
+      // clients see live status during slow operations (installs, auth waits).
+      // This cannot starve an interactive prompt: the runner is sequential, so
+      // while it awaits a prompt it is not emitting progress, and latest-wins
+      // keeps at most one progress step pending.
+      const progress = this.takePendingProgress();
+      if (progress) {
+        return { done: false, step: progress, status: this.status };
+      }
+      if (this.currentStep) {
+        return { done: false, step: this.currentStep, status: this.status };
+      }
+      if (this.pendingTerminalResolution) {
+        this.pendingTerminalResolution = false;
+        return { done: true, status: this.status, error: this.error };
+      }
+      if (this.status !== "running") {
+        return { done: true, status: this.status, error: this.error };
+      }
+      if (!this.stepDeferred) {
+        this.stepDeferred = createDeferred();
+      }
+      // Wait for the next event (prompt pushed, progress emitted, or terminal),
+      // then re-loop to classify it via the checks above. The resolved value is
+      // intentionally ignored: a real step is read back from currentStep, and a
+      // null resolution falls through to progress/terminal/status handling.
+      await this.stepDeferred.promise;
     }
-    return { done: true, status: this.status, error: this.error };
   }
 
   async answer(stepId: string, value: unknown): Promise<void> {
@@ -223,6 +249,7 @@ export class WizardSession {
     this.status = "cancelled";
     this.error = "cancelled";
     this.currentStep = null;
+    this.pendingProgress = null;
     for (const [, deferred] of this.answerDeferred) {
       // Reject all pending prompt promises so the runner can unwind through its
       // normal cancellation path.
@@ -264,6 +291,41 @@ export class WizardSession {
     return await deferred.promise;
   }
 
+  /**
+   * Emits a non-interactive progress step for remote clients. Fire-and-forget:
+   * the runner does not block, and clients re-poll wizard.next without an answer
+   * to continue. Latest-wins, so an undelivered progress step is replaced.
+   */
+  emitProgress(message: string): void {
+    if (this.status !== "running") {
+      return;
+    }
+    this.pendingProgress = {
+      id: randomUUID(),
+      type: "progress",
+      message,
+      executor: "client",
+    };
+    this.wakePendingProgress();
+  }
+
+  private takePendingProgress(): WizardStep | null {
+    const step = this.pendingProgress;
+    this.pendingProgress = null;
+    return step;
+  }
+
+  private wakePendingProgress(): void {
+    // Wake a long-polling next() so live progress is delivered mid-operation.
+    // Resolves with null; next() re-checks the pending progress slot on wake.
+    if (!this.stepDeferred) {
+      return;
+    }
+    const deferred = this.stepDeferred;
+    this.stepDeferred = null;
+    deferred.resolve(null);
+  }
+
   private resolveStep(step: WizardStep | null) {
     if (!this.stepDeferred) {
       if (step === null) {
