fix(wizard): avoid rerunning completed setup steps (#692)

docs/scratchpads/B4-wizard-step-dedup.md

@@ -0,0 +1,36 @@
+# B4 — Wizard step deduplication
+
+## Problem
+
+Greenfield wizard testing showed completed wizard steps could be executed again after the menu marked them `[done]`. In practice this made the Providers/API-key flow and Skills flow appear twice in one wizard run.
+
+There was a second related API-key duplication path: when the Providers step was completed with no key, `gatewayConfigStage` still prompted for `ANTHROPIC_API_KEY` during Finish because it only skipped the gateway API-key prompt when `providerKey` was non-empty.
+
+## Diagnosis
+
+- `runMenuLoop` labeled completed sections with `[done]`, but still dispatched the selected step again if the user selected that row.
+- Quick Start ran Providers and Skills but did not mark those sections complete in `completedSections`.
+- `runFinishPath`/`quickStartPath` defaulted `providerType` to `none` for gateway config, which made it impossible for `gatewayConfigStage` to distinguish:
+  - provider step completed and user intentionally skipped the key, vs.
+  - provider step was never run.
+
+## Fix
+
+- Added a shared menu section key helper and a completed-step guard in `runMenuLoop`.
+- Completed menu steps now log a skip message instead of re-running their stage.
+- Quick Start marks Providers and Skills complete after running them.
+- Finish/Quick Start now pass `state.providerType` as-is to gateway config instead of defaulting to `none`.
+- `gatewayConfigStage` treats `providerType: 'none'` as an explicit completed provider setup with no key and skips the second gateway API-key prompt.
+
+## Verification
+
+- Added unified wizard regression coverage asserting repeated Providers/Skills menu selections only execute each stage once.
+- Added gateway config coverage asserting `providerType: 'none'` does not prompt for a gateway API key and writes no API key env var.
+- Targeted: `pnpm --filter @mosaicstack/mosaic test -- unified-wizard gateway-config`
+- `pnpm format:check`
+- `pnpm typecheck`
+- `pnpm lint`
+- `pnpm build`
+- `pnpm test`
+- Codex code review: approve.
+- Codex security review: no findings.

packages/mosaic/__tests__/integration/unified-wizard.test.ts

@@ -11,9 +11,37 @@ import { join } from 'node:path';
 import { tmpdir } from 'node:os';
 import { HeadlessPrompter } from '../../src/prompter/headless-prompter.js';
 import { createConfigService } from '../../src/config/config-service.js';
+import type { SelectOption } from '../../src/prompter/interface.js';
+import type { MenuSection, WizardState } from '../../src/types.js';
 
 const gatewayConfigMock = vi.fn();
 const gatewayBootstrapMock = vi.fn();
+const providerSetupMock = vi.fn();
+const skillsSelectMock = vi.fn();
+
+class SequencedMenuPrompter extends HeadlessPrompter {
+  constructor(
+    answers: Record<string, string | boolean | string[]>,
+    private readonly menuChoices: string[],
+  ) {
+    super(answers);
+  }
+
+  override async select<T>(opts: {
+    message: string;
+    options: SelectOption<T>[];
+    initialValue?: T;
+  }): Promise<T> {
+    if (opts.message === 'What would you like to configure?') {
+      const next = this.menuChoices.shift();
+      if (!next) throw new Error('No queued menu choice left');
+      const match = opts.options.find((o) => String(o.value) === next);
+      if (!match) throw new Error(`Queued menu choice not available: ${next}`);
+      return match.value;
+    }
+    return super.select(opts);
+  }
+}
 
 vi.mock('../../src/stages/gateway-config.js', () => ({
   gatewayConfigStage: (...args: unknown[]) => gatewayConfigMock(...args),
@@ -23,6 +51,14 @@ vi.mock('../../src/stages/gateway-bootstrap.js', () => ({
   gatewayBootstrapStage: (...args: unknown[]) => gatewayBootstrapMock(...args),
 }));
 
+vi.mock('../../src/stages/provider-setup.js', () => ({
+  providerSetupStage: (...args: unknown[]) => providerSetupMock(...args),
+}));
+
+vi.mock('../../src/stages/skills-select.js', () => ({
+  skillsSelectStage: (...args: unknown[]) => skillsSelectMock(...args),
+}));
+
 // Import AFTER the mocks so runWizard picks up the mocked stage modules.
 import { runWizard } from '../../src/wizard.js';
 
@@ -44,6 +80,16 @@ describe('Unified wizard (runWizard with default skipGateway)', () => {
     }
     gatewayConfigMock.mockReset();
     gatewayBootstrapMock.mockReset();
+    providerSetupMock.mockReset();
+    skillsSelectMock.mockReset();
+    providerSetupMock.mockImplementation(async (_p: HeadlessPrompter, state: WizardState) => {
+      state.providerType = 'none';
+      state.completedSections?.add('providers' satisfies MenuSection);
+    });
+    skillsSelectMock.mockImplementation(async (_p: HeadlessPrompter, state: WizardState) => {
+      state.selectedSkills = [];
+      state.completedSections?.add('skills' satisfies MenuSection);
+    });
     // Pretend we're on an interactive TTY so the wizard's headless-abort
     // branch does not call `process.exit(1)` during these tests.
     Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true });
@@ -184,4 +230,34 @@ describe('Unified wizard (runWizard with default skipGateway)', () => {
     expect(gatewayConfigMock).not.toHaveBeenCalled();
     expect(gatewayBootstrapMock).not.toHaveBeenCalled();
   });
+
+  it('does not re-run completed provider or skills menu steps', async () => {
+    const prompter = new SequencedMenuPrompter(
+      {
+        'What name should agents use?': 'TestBot',
+        'Communication style': 'direct',
+        'Your name': 'Tester',
+        'Your pronouns': 'They/Them',
+        'Your timezone': 'UTC',
+      },
+      ['providers', 'providers', 'skills', 'skills', 'finish'],
+    );
+
+    await runWizard({
+      mosaicHome: tmpDir,
+      sourceDir: tmpDir,
+      prompter,
+      configService: createConfigService(tmpDir, tmpDir),
+      skipGateway: true,
+    });
+
+    expect(providerSetupMock).toHaveBeenCalledTimes(1);
+    expect(skillsSelectMock).toHaveBeenCalledTimes(1);
+    expect(prompter.getLogs()).toEqual(
+      expect.arrayContaining([
+        expect.stringContaining('Providers [done] is already complete; skipping.'),
+        expect.stringContaining('Skills [done] is already complete; skipping.'),
+      ]),
+    );
+  });
 });

packages/mosaic/src/stages/gateway-config.spec.ts

@@ -167,6 +167,45 @@ describe('gatewayConfigStage', () => {
     expect(state.gateway?.regeneratedConfig).toBe(true);
   });
 
+  it('does not ask for a gateway API key when provider setup was completed with no key', async () => {
+    delete process.env['MOSAIC_ASSUME_YES'];
+    const originalIsTTY = process.stdin.isTTY;
+    Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true });
+
+    try {
+      const textFn = vi.fn(async (opts: { message: string; initialValue?: string }) => {
+        if (opts.message === 'Gateway port') return opts.initialValue ?? '14242';
+        if (opts.message === 'Web UI hostname (for browser access)') return 'localhost';
+        if (opts.message.includes('API_KEY')) {
+          throw new Error('gateway API key prompt should be skipped');
+        }
+        return '';
+      });
+      const p = buildPrompter({ text: textFn, select: vi.fn().mockResolvedValue('local') });
+      const state = makeState('/home/user/.config/mosaic');
+
+      const result = await gatewayConfigStage(p, state, {
+        host: 'localhost',
+        defaultPort: 14242,
+        skipInstall: true,
+        providerType: 'none',
+      });
+
+      expect(result.ready).toBe(true);
+      expect(textFn).not.toHaveBeenCalledWith(
+        expect.objectContaining({ message: expect.stringContaining('API_KEY') }),
+      );
+      const envContents = readFileSync(daemonState.envFile, 'utf-8');
+      expect(envContents).not.toContain('ANTHROPIC_API_KEY=');
+      expect(envContents).not.toContain('OPENAI_API_KEY=');
+    } finally {
+      Object.defineProperty(process.stdin, 'isTTY', {
+        value: originalIsTTY,
+        configurable: true,
+      });
+    }
+  });
+
   it('short-circuits when gateway is already fully installed and user declines rerun', async () => {
     // Pre-populate both files + running daemon + meta with token
     const fs = require('node:fs');

packages/mosaic/src/stages/gateway-config.ts

@@ -506,6 +506,9 @@ async function collectAndWriteConfig(
     if (opts.providerKey) {
       anthropicKey = opts.providerKey;
       p.log(`Using API key from provider setup (${opts.providerType ?? 'unknown'}).`);
+    } else if (opts.providerType === 'none') {
+      anthropicKey = '';
+      p.log('No API key provided during provider setup; skipping gateway API key prompt.');
     } else {
       anthropicKey = await p.text({
         message: 'ANTHROPIC_API_KEY (optional, press Enter to skip)',

packages/mosaic/src/stages/quick-start.ts

@@ -37,6 +37,7 @@ export async function quickStartPath(
 
   // 1. Provider setup (first question)
   await providerSetupStage(prompter, state);
+  state.completedSections?.add('providers');
 
   // Apply sensible defaults for everything else
   state.soul.agentName ??= 'Mosaic';
@@ -57,6 +58,7 @@ export async function quickStartPath(
 
   // Skills (recommended set, no user input in quick mode)
   await skillsSelectStage(prompter, state);
+  state.completedSections?.add('skills');
 
   // Finalize writes configs/assets/skills, but defer the success summary until
   // after the gateway health/bootstrap gates complete.
@@ -75,7 +77,7 @@ export async function quickStartPath(
         portOverride: options.gatewayPortOverride,
         skipInstall: options.skipGatewayNpmInstall,
         providerKey: state.providerKey,
-        providerType: state.providerType ?? 'none',
+        providerType: state.providerType,
       });
 
       if (!configResult.ready || !configResult.host || !configResult.port) {

packages/mosaic/src/wizard.ts

@@ -126,6 +126,11 @@ type MenuChoice =
   | 'advanced'
   | 'finish';
 
+function menuSectionKey(section: MenuChoice): MenuSection | null {
+  if (section === 'quick-start' || section === 'finish') return null;
+  return section === 'gateway-config' ? 'gateway' : section;
+}
+
 function menuLabel(section: MenuChoice, completed: Set<MenuSection>): string {
   const labels: Record<MenuChoice, string> = {
     'quick-start': 'Quick Start',
@@ -137,14 +142,24 @@ function menuLabel(section: MenuChoice, completed: Set<MenuSection>): string {
     finish: 'Finish & Apply',
   };
   const base = labels[section];
-  const sectionKey: MenuSection =
+  const sectionKey = menuSectionKey(section);
-    section === 'gateway-config' ? 'gateway' : (section as MenuSection);
+  if (sectionKey && completed.has(sectionKey)) {
-  if (completed.has(sectionKey)) {
     return `${base} [done]`;
   }
   return base;
 }
 
+function skipCompletedMenuChoice(
+  prompter: WizardPrompter,
+  completed: Set<MenuSection>,
+  choice: MenuChoice,
+): boolean {
+  const sectionKey = menuSectionKey(choice);
+  if (!sectionKey || !completed.has(sectionKey)) return false;
+  prompter.log(`${menuLabel(choice, completed)} is already complete; skipping.`);
+  return true;
+}
+
 async function runMenuLoop(
   prompter: WizardPrompter,
   state: WizardState,
@@ -201,21 +216,25 @@ async function runMenuLoop(
         return; // Quick start is a complete flow — exit menu
 
       case 'providers':
+        if (skipCompletedMenuChoice(prompter, completed, choice)) break;
         await providerSetupStage(prompter, state);
         completed.add('providers');
         break;
 
       case 'identity':
+        if (skipCompletedMenuChoice(prompter, completed, choice)) break;
         await agentIntentStage(prompter, state);
         completed.add('identity');
         break;
 
       case 'skills':
+        if (skipCompletedMenuChoice(prompter, completed, choice)) break;
         await skillsSelectStage(prompter, state);
         completed.add('skills');
         break;
 
       case 'gateway-config':
+        if (skipCompletedMenuChoice(prompter, completed, choice)) break;
         // Gateway config is handled during Finish — mark as "configured"
         // after user reviews settings.
         await runGatewaySubMenu(prompter, state, options);
@@ -223,6 +242,7 @@ async function runMenuLoop(
         break;
 
       case 'advanced':
+        if (skipCompletedMenuChoice(prompter, completed, choice)) break;
         await runAdvancedSubMenu(prompter, state);
         completed.add('advanced');
         break;
@@ -325,7 +345,7 @@ async function runFinishPath(
         portOverride: options.gatewayPortOverride,
         skipInstall: options.skipGatewayNpmInstall,
         providerKey: state.providerKey,
-        providerType: state.providerType ?? 'none',
+        providerType: state.providerType,
       });
 
       if (configResult.ready && configResult.host && configResult.port) {
@@ -396,7 +416,7 @@ async function runHeadlessPath(
         portOverride: options.gatewayPortOverride,
         skipInstall: options.skipGatewayNpmInstall,
         providerKey: state.providerKey,
-        providerType: state.providerType ?? 'none',
+        providerType: state.providerType,
       });
 
       if (!configResult.ready || !configResult.host || !configResult.port) {