fix(wizard): avoid rerunning completed setup steps
Some checks failed
ci/woodpecker/push/ci Pipeline was canceled
ci/woodpecker/pr/ci Pipeline was successful

This commit is contained in:
Jarvis
2026-06-25 13:22:59 -05:00
parent b96cc7982a
commit 0113d89ab8
6 changed files with 182 additions and 6 deletions

View File

@@ -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.

View File

@@ -11,9 +11,37 @@ import { join } from 'node:path';
import { tmpdir } from 'node:os'; import { tmpdir } from 'node:os';
import { HeadlessPrompter } from '../../src/prompter/headless-prompter.js'; import { HeadlessPrompter } from '../../src/prompter/headless-prompter.js';
import { createConfigService } from '../../src/config/config-service.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 gatewayConfigMock = vi.fn();
const gatewayBootstrapMock = 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', () => ({ vi.mock('../../src/stages/gateway-config.js', () => ({
gatewayConfigStage: (...args: unknown[]) => gatewayConfigMock(...args), gatewayConfigStage: (...args: unknown[]) => gatewayConfigMock(...args),
@@ -23,6 +51,14 @@ vi.mock('../../src/stages/gateway-bootstrap.js', () => ({
gatewayBootstrapStage: (...args: unknown[]) => gatewayBootstrapMock(...args), 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 AFTER the mocks so runWizard picks up the mocked stage modules.
import { runWizard } from '../../src/wizard.js'; import { runWizard } from '../../src/wizard.js';
@@ -44,6 +80,16 @@ describe('Unified wizard (runWizard with default skipGateway)', () => {
} }
gatewayConfigMock.mockReset(); gatewayConfigMock.mockReset();
gatewayBootstrapMock.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 // Pretend we're on an interactive TTY so the wizard's headless-abort
// branch does not call `process.exit(1)` during these tests. // branch does not call `process.exit(1)` during these tests.
Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); 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(gatewayConfigMock).not.toHaveBeenCalled();
expect(gatewayBootstrapMock).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.'),
]),
);
});
}); });

View File

@@ -167,6 +167,45 @@ describe('gatewayConfigStage', () => {
expect(state.gateway?.regeneratedConfig).toBe(true); 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 () => { it('short-circuits when gateway is already fully installed and user declines rerun', async () => {
// Pre-populate both files + running daemon + meta with token // Pre-populate both files + running daemon + meta with token
const fs = require('node:fs'); const fs = require('node:fs');

View File

@@ -506,6 +506,9 @@ async function collectAndWriteConfig(
if (opts.providerKey) { if (opts.providerKey) {
anthropicKey = opts.providerKey; anthropicKey = opts.providerKey;
p.log(`Using API key from provider setup (${opts.providerType ?? 'unknown'}).`); 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 { } else {
anthropicKey = await p.text({ anthropicKey = await p.text({
message: 'ANTHROPIC_API_KEY (optional, press Enter to skip)', message: 'ANTHROPIC_API_KEY (optional, press Enter to skip)',

View File

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

View File

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