From 23d928ad10acf807a0e7d5eb33cc86fbf2932514 Mon Sep 17 00:00:00 2001
From: Mohamed Boudra <boudra.moha@gmail.com>
Date: Mon, 2 Nov 2026 17:52:33 +0100
Subject: [PATCH] fix(app): ignore a late Finish once the user left the last
 setup step

Finish called complete(), which ignored the step visit, so pressing Back
while Orchestration applied its choices still closed setup and recorded it
as done. next(visit) now finishes setup from the last step, so a late
Finish is dropped the same way a late Continue already was.
---
 .../app/src/onboarding/internal/model.test.ts | 22 +++++++++++++++++--
 packages/app/src/onboarding/internal/model.ts | 21 +++++++++++-------
 .../onboarding/internal/onboarding-modal.tsx  |  7 ++----
 3 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/packages/app/src/onboarding/internal/model.test.ts b/packages/app/src/onboarding/internal/model.test.ts
index ee3a8b00c9..5838876e91 100644
--- a/packages/app/src/onboarding/internal/model.test.ts
+++ b/packages/app/src/onboarding/internal/model.test.ts
@@ -1,4 +1,4 @@
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 import {
   createOnboardingModel,
   type OnboardingCompletionStorage,
@@ -134,7 +134,6 @@ describe("onboarding model", () => {
     advance(model);
     advance(model);
     advance(model);
-    advance(model);
     expect(model.getState()).toMatchObject({ stepIndex: 3 });
     model.goTo("providers");
     expect(model.getState()).toMatchObject({ stepIndex: 1 });
@@ -162,6 +161,25 @@ describe("onboarding model", () => {
     expect(model.getState()).toMatchObject({ status: "open", stepIndex: 2 });
   });
 
+  it("moving on from the last step finishes setup, unless the user left that step first", async () => {
+    const storage = new MemoryCompletionStorage();
+    const model = await openModel(storage);
+    model.applySetupHost(FRESH_HOST, { includePhoneStep: false });
+    model.goTo("orchestration");
+
+    // Finish is applying choices when the user goes back to Projects.
+    const finishingFrom = currentVisit(model);
+    model.back();
+    model.next(finishingFrom);
+    expect(model.getState()).toMatchObject({ status: "open", stepIndex: 2 });
+    expect(storage.completed).toBe(false);
+
+    advance(model);
+    advance(model);
+    await vi.waitFor(() => expect(storage.completed).toBe(true));
+    expect(model.getState()).toEqual({ status: "hidden" });
+  });
+
   it("records completion when setup closes, so it does not open again", async () => {
     const storage = new MemoryCompletionStorage();
     const model = await openModel(storage);
diff --git a/packages/app/src/onboarding/internal/model.ts b/packages/app/src/onboarding/internal/model.ts
index 1ac2f01f4a..f8aaf46d4f 100644
--- a/packages/app/src/onboarding/internal/model.ts
+++ b/packages/app/src/onboarding/internal/model.ts
@@ -46,8 +46,9 @@ export interface OnboardingModel {
   applySetupHost(host: OnboardingSetupHost | null, options: OnboardingStepOptions): void;
   reopen(serverId: string, options: OnboardingStepOptions): void;
   /**
-   * Moves on from the step visit the caller saw. A no-op once that visit ended, so an action that
-   * finishes late never moves a step the user left or a setup they closed and reopened.
+   * Moves on from the step visit the caller saw; from the last step, that finishes setup. A no-op
+   * once that visit ended, so an action that finishes late never moves a step the user left or
+   * closes a setup they went back in, or closed and reopened.
    */
   next(visit: number): void;
   back(): void;
@@ -129,6 +130,12 @@ export function createOnboardingModel(input: {
 
   const ready = loadCompletion();
 
+  async function complete() {
+    completion = "completed";
+    setState({ status: "hidden" });
+    await input.storage.markCompleted();
+  }
+
   return {
     ready,
     getState: () => state,
@@ -146,7 +153,9 @@ export function createOnboardingModel(input: {
       setState(openState(serverId, options));
     },
     next(visit) {
-      if (state.status === "open" && state.visit === visit) moveTo(state.stepIndex + 1);
+      if (state.status !== "open" || state.visit !== visit) return;
+      if (state.stepIndex === state.steps.length - 1) void complete();
+      else moveTo(state.stepIndex + 1);
     },
     back() {
       if (state.status === "open") moveTo(state.stepIndex - 1);
@@ -156,10 +165,6 @@ export function createOnboardingModel(input: {
       const stepIndex = state.steps.indexOf(step);
       if (stepIndex >= 0) moveTo(stepIndex);
     },
-    async complete() {
-      completion = "completed";
-      setState({ status: "hidden" });
-      await input.storage.markCompleted();
-    },
+    complete,
   };
 }
diff --git a/packages/app/src/onboarding/internal/onboarding-modal.tsx b/packages/app/src/onboarding/internal/onboarding-modal.tsx
index c36c4855f1..0eb0418185 100644
--- a/packages/app/src/onboarding/internal/onboarding-modal.tsx
+++ b/packages/app/src/onboarding/internal/onboarding-modal.tsx
@@ -42,11 +42,8 @@ export function OnboardingModal({ model, state }: { model: OnboardingModel; stat
   }, [model]);
   const handleBack = useCallback(() => model.back(), [model]);
   const { visit } = state;
-  // Moving on from the last step finishes setup; any step's own work runs before that.
-  const handleAdvance = useCallback(() => {
-    if (isLast) void model.complete();
-    else model.next(visit);
-  }, [isLast, model, visit]);
+  // A step's own work runs before it moves on; from the last step, moving on finishes setup.
+  const handleAdvance = useCallback(() => model.next(visit), [model, visit]);
   const handleAddSuggestions = useCallback(() => {
     void (async () => {
       if (await addChecked()) handleAdvance();