From e495d8845fa5512f3d9b89b8ccdfc729b3ae3d37 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 25 Jun 2026 12:51:54 -0500 Subject: [PATCH] fix(wizard): defer success summary until gateway ready --- .../B3-wizard-gateway-health-order.md | 36 +++++++ .../integration/unified-wizard.test.ts | 45 ++++++++- packages/mosaic/src/runtime/detector.ts | 2 +- packages/mosaic/src/stages/finalize.ts | 99 ++++++++++++------- packages/mosaic/src/stages/quick-start.ts | 30 +++--- packages/mosaic/src/wizard.ts | 52 +++++++--- 6 files changed, 198 insertions(+), 66 deletions(-) create mode 100644 docs/scratchpads/B3-wizard-gateway-health-order.md diff --git a/docs/scratchpads/B3-wizard-gateway-health-order.md b/docs/scratchpads/B3-wizard-gateway-health-order.md new file mode 100644 index 0000000..3345289 --- /dev/null +++ b/docs/scratchpads/B3-wizard-gateway-health-order.md @@ -0,0 +1,36 @@ +# B3 — Wizard completion ordering + +## Problem + +The wizard printed the success summary / `Mosaic is ready.` during `finalizeStage`, before the gateway configuration stage had completed its daemon health check. If the gateway health gate later failed, the user could see a success claim followed by a gateway failure. + +## Diagnosis + +`finalizeStage` handled both mutation work and terminal success messaging. Wizard paths then ran `gatewayConfigStage` and `gatewayBootstrapStage` afterward: + +1. finalize writes config, links runtime assets, syncs skills, runs doctor; +2. finalize prints `Installation Summary` + `Mosaic is ready.`; +3. gateway config starts/waits for daemon health; +4. gateway bootstrap runs. + +The summary needed to be deferred until after the gateway readiness gates. + +## Fix + +- `finalizeStage` now returns a `showSummary()` callback and supports `deferSummary`. +- Wizard/quick-start paths call finalize with `deferSummary: true`. +- `showSummary()` is called only after gateway config reports ready and bootstrap completes, or immediately when the caller explicitly skips gateway setup. +- If gateway health/config reports not ready, the wizard returns/aborts without printing the success summary. +- Folded in adjacent runtime install hint fix for Pi: `curl -fsSL https://pi.dev/install.sh | sh`. + +## Verification + +- Added unified-wizard coverage for summary-after-health and no-summary-on-health-failure. +- Targeted: `pnpm --filter @mosaicstack/mosaic test -- unified-wizard finalize-skills` +- `pnpm format:check` +- `pnpm typecheck` +- `pnpm lint` +- `pnpm build` +- `pnpm test` +- Codex code review: approve. +- Codex security review: one low finding on the requested Pi `curl | sh` install hint; no security finding in the wizard completion-ordering change. diff --git a/packages/mosaic/__tests__/integration/unified-wizard.test.ts b/packages/mosaic/__tests__/integration/unified-wizard.test.ts index 169f945..5becc83 100644 --- a/packages/mosaic/__tests__/integration/unified-wizard.test.ts +++ b/packages/mosaic/__tests__/integration/unified-wizard.test.ts @@ -98,8 +98,12 @@ describe('Unified wizard (runWizard with default skipGateway)', () => { expect(bootstrapCall[2]).toMatchObject({ host: 'localhost', port: 14242 }); }); - it('does not invoke bootstrap when config stage reports not ready', async () => { - gatewayConfigMock.mockResolvedValue({ ready: false }); + it('prints the success summary only after gateway health succeeds', async () => { + gatewayConfigMock.mockImplementation(async (p: HeadlessPrompter) => { + p.log('Gateway is healthy.'); + return { ready: true, host: 'localhost', port: 14242 }; + }); + gatewayBootstrapMock.mockResolvedValue({ completed: true }); const prompter = new HeadlessPrompter({ 'Installation mode': 'quick', @@ -118,6 +122,43 @@ describe('Unified wizard (runWizard with default skipGateway)', () => { skipGatewayNpmInstall: true, }); + const logs = prompter.getLogs(); + const healthIndex = logs.findIndex((line) => line.includes('Gateway is healthy.')); + const summaryIndex = logs.findIndex((line) => line.includes('Installation Summary')); + const readyIndex = logs.findIndex((line) => line.includes('Mosaic is ready.')); + + expect(healthIndex).toBeGreaterThanOrEqual(0); + expect(summaryIndex).toBeGreaterThan(healthIndex); + expect(readyIndex).toBeGreaterThan(summaryIndex); + }); + + it('does not claim success when gateway health reports not ready', async () => { + gatewayConfigMock.mockImplementation(async (p: HeadlessPrompter) => { + p.warn('Gateway did not become healthy within 30 seconds.'); + return { ready: false }; + }); + + const prompter = new HeadlessPrompter({ + 'Installation mode': 'quick', + 'What name should agents use?': 'TestBot', + 'Communication style': 'direct', + 'Your name': 'Tester', + 'Your pronouns': 'They/Them', + 'Your timezone': 'UTC', + }); + + await runWizard({ + mosaicHome: tmpDir, + sourceDir: tmpDir, + prompter, + configService: createConfigService(tmpDir, tmpDir), + skipGatewayNpmInstall: true, + }); + + const logs = prompter.getLogs(); + expect(logs.some((line) => line.includes('Gateway did not become healthy'))).toBe(true); + expect(logs.some((line) => line.includes('Installation Summary'))).toBe(false); + expect(logs.some((line) => line.includes('Mosaic is ready.'))).toBe(false); expect(gatewayConfigMock).toHaveBeenCalledTimes(1); expect(gatewayBootstrapMock).not.toHaveBeenCalled(); }); diff --git a/packages/mosaic/src/runtime/detector.ts b/packages/mosaic/src/runtime/detector.ts index c80167c..3f4d6a9 100644 --- a/packages/mosaic/src/runtime/detector.ts +++ b/packages/mosaic/src/runtime/detector.ts @@ -37,7 +37,7 @@ const RUNTIME_DEFS: Record< label: 'Pi', command: 'pi', versionFlag: '--version', - installHint: 'npm install -g @mariozechner/pi-coding-agent', + installHint: 'curl -fsSL https://pi.dev/install.sh | sh', }, }; diff --git a/packages/mosaic/src/stages/finalize.ts b/packages/mosaic/src/stages/finalize.ts index cde124b..eaaa92a 100644 --- a/packages/mosaic/src/stages/finalize.ts +++ b/packages/mosaic/src/stages/finalize.ts @@ -162,11 +162,24 @@ function setupPath(mosaicHome: string, _p: WizardPrompter): PathAction { } } +export interface FinalizeStageOptions { + /** + * Defer the success summary/outro so callers can run downstream readiness + * gates (gateway health/bootstrap) before claiming Mosaic is ready. + */ + deferSummary?: boolean; +} + +export interface FinalizeStageResult { + showSummary: () => void; +} + export async function finalizeStage( p: WizardPrompter, state: WizardState, config: ConfigService, -): Promise { + options: FinalizeStageOptions = {}, +): Promise { p.separator(); const spin = p.spinner(); @@ -213,44 +226,56 @@ export async function finalizeStage( // 6. PATH setup const pathAction = setupPath(state.mosaicHome, p); - // 7. Summary - const skillsSummary = skillsResult.success - ? skillsResult.installedCount > 0 - ? `${skillsResult.installedCount.toString()} installed` - : 'none selected' - : `install failed — ${skillsResult.failureReason ?? 'unknown error'}`; + let summaryShown = false; + const showSummary = () => { + if (summaryShown) return; + summaryShown = true; - const summary: string[] = [ - `Agent: ${state.soul.agentName ?? 'Assistant'}`, - `Style: ${state.soul.communicationStyle ?? 'direct'}`, - `Runtimes: ${state.runtimes.detected.join(', ') || 'none detected'}`, - `Skills: ${skillsSummary}`, - `Config: ${state.mosaicHome}`, - ]; + // 7. Summary + const skillsSummary = skillsResult.success + ? skillsResult.installedCount > 0 + ? `${skillsResult.installedCount.toString()} installed` + : 'none selected' + : `install failed — ${skillsResult.failureReason ?? 'unknown error'}`; - if (doctorResult.warnings > 0) { - summary.push( - `Health: ${doctorResult.warnings.toString()} warning(s) — run 'mosaic doctor' for details`, - ); - } else { - summary.push('Health: all checks passed'); + const summary: string[] = [ + `Agent: ${state.soul.agentName ?? 'Assistant'}`, + `Style: ${state.soul.communicationStyle ?? 'direct'}`, + `Runtimes: ${state.runtimes.detected.join(', ') || 'none detected'}`, + `Skills: ${skillsSummary}`, + `Config: ${state.mosaicHome}`, + ]; + + if (doctorResult.warnings > 0) { + summary.push( + `Health: ${doctorResult.warnings.toString()} warning(s) — run 'mosaic doctor' for details`, + ); + } else { + summary.push('Health: all checks passed'); + } + + p.note(summary.join('\n'), 'Installation Summary'); + + // 8. Next steps + const nextSteps: string[] = []; + if (pathAction === 'added') { + const profilePath = getShellProfilePath(); + nextSteps.push(`Reload shell: source ${profilePath ?? '~/.profile'}`); + } + if (state.runtimes.detected.length === 0) { + nextSteps.push('Install at least one runtime (claude, codex, or opencode)'); + } + nextSteps.push("Launch with 'mosaic claude' (or codex/opencode)"); + nextSteps.push('Edit identity files directly in ~/.config/mosaic/ for fine-tuning'); + + p.note(nextSteps.map((s, i) => `${(i + 1).toString()}. ${s}`).join('\n'), 'Next Steps'); + + p.outro('Mosaic is ready.'); + }; + + if (!options.deferSummary) { + showSummary(); } - p.note(summary.join('\n'), 'Installation Summary'); - - // 8. Next steps - const nextSteps: string[] = []; - if (pathAction === 'added') { - const profilePath = getShellProfilePath(); - nextSteps.push(`Reload shell: source ${profilePath ?? '~/.profile'}`); - } - if (state.runtimes.detected.length === 0) { - nextSteps.push('Install at least one runtime (claude, codex, or opencode)'); - } - nextSteps.push("Launch with 'mosaic claude' (or codex/opencode)"); - nextSteps.push('Edit identity files directly in ~/.config/mosaic/ for fine-tuning'); - - p.note(nextSteps.map((s, i) => `${(i + 1).toString()}. ${s}`).join('\n'), 'Next Steps'); - - p.outro('Mosaic is ready.'); + return { showSummary }; } diff --git a/packages/mosaic/src/stages/quick-start.ts b/packages/mosaic/src/stages/quick-start.ts index b97d1a4..f132cc4 100644 --- a/packages/mosaic/src/stages/quick-start.ts +++ b/packages/mosaic/src/stages/quick-start.ts @@ -58,8 +58,11 @@ export async function quickStartPath( // Skills (recommended set, no user input in quick mode) await skillsSelectStage(prompter, state); - // Finalize (writes configs, links runtime assets, syncs skills) - await finalizeStage(prompter, state, configService); + // Finalize writes configs/assets/skills, but defer the success summary until + // after the gateway health/bootstrap gates complete. + const finalizeResult = await finalizeStage(prompter, state, configService, { + deferSummary: true, + }); // Gateway config + bootstrap if (!options.skipGateway) { @@ -80,19 +83,24 @@ export async function quickStartPath( prompter.warn('Gateway configuration failed in headless mode — aborting wizard.'); process.exit(1); } - } else { - const bootstrapResult = await gatewayBootstrapStage(prompter, state, { - host: configResult.host, - port: configResult.port, - }); - if (!bootstrapResult.completed) { - prompter.warn('Admin bootstrap failed — aborting wizard.'); - process.exit(1); - } + return; } + + const bootstrapResult = await gatewayBootstrapStage(prompter, state, { + host: configResult.host, + port: configResult.port, + }); + if (!bootstrapResult.completed) { + prompter.warn('Admin bootstrap failed — aborting wizard.'); + process.exit(1); + return; + } + finalizeResult.showSummary(); } catch (err) { prompter.warn(`Gateway setup failed: ${err instanceof Error ? err.message : String(err)}`); throw err; } + } else { + finalizeResult.showSummary(); } } diff --git a/packages/mosaic/src/wizard.ts b/packages/mosaic/src/wizard.ts index 15b3188..757aeee 100644 --- a/packages/mosaic/src/wizard.ts +++ b/packages/mosaic/src/wizard.ts @@ -310,8 +310,11 @@ async function runFinishPath( await skillsSelectStage(prompter, state); } - // Finalize (writes configs, links runtime assets, syncs skills) - await finalizeStage(prompter, state, configService); + // Finalize writes configs/assets/skills, but defer the success summary until + // after the gateway health/bootstrap gates complete. + const finalizeResult = await finalizeStage(prompter, state, configService, { + deferSummary: true, + }); // Gateway stages if (!options.skipGateway) { @@ -333,12 +336,16 @@ async function runFinishPath( if (!bootstrapResult.completed) { prompter.warn('Admin bootstrap failed — aborting wizard.'); process.exit(1); + return; } + finalizeResult.showSummary(); } } catch (err) { prompter.warn(`Gateway setup failed: ${err instanceof Error ? err.message : String(err)}`); throw err; } + } else { + finalizeResult.showSummary(); } } @@ -374,8 +381,11 @@ async function runHeadlessPath( // Skills await skillsSelectStage(prompter, state); - // Finalize - await finalizeStage(prompter, state, configService); + // Finalize writes configs/assets/skills, but defer the success summary until + // after the gateway health/bootstrap gates complete. + const finalizeResult = await finalizeStage(prompter, state, configService, { + deferSummary: true, + }); // Gateway stages if (!options.skipGateway) { @@ -392,20 +402,25 @@ async function runHeadlessPath( if (!configResult.ready || !configResult.host || !configResult.port) { prompter.warn('Gateway configuration failed in headless mode — aborting wizard.'); process.exit(1); - } else { - const bootstrapResult = await gatewayBootstrapStage(prompter, state, { - host: configResult.host, - port: configResult.port, - }); - if (!bootstrapResult.completed) { - prompter.warn('Admin bootstrap failed — aborting wizard.'); - process.exit(1); - } + return; } + + const bootstrapResult = await gatewayBootstrapStage(prompter, state, { + host: configResult.host, + port: configResult.port, + }); + if (!bootstrapResult.completed) { + prompter.warn('Admin bootstrap failed — aborting wizard.'); + process.exit(1); + return; + } + finalizeResult.showSummary(); } catch (err) { prompter.warn(`Gateway setup failed: ${err instanceof Error ? err.message : String(err)}`); throw err; } + } else { + finalizeResult.showSummary(); } } @@ -426,8 +441,11 @@ async function runKeepPath( // Skills await skillsSelectStage(prompter, state); - // Finalize - await finalizeStage(prompter, state, configService); + // Finalize writes configs/assets/skills, but defer the success summary until + // after the gateway health/bootstrap gates complete. + const finalizeResult = await finalizeStage(prompter, state, configService, { + deferSummary: true, + }); // Gateway stages if (!options.skipGateway) { @@ -447,11 +465,15 @@ async function runKeepPath( if (!bootstrapResult.completed) { prompter.warn('Admin bootstrap failed — aborting wizard.'); process.exit(1); + return; } + finalizeResult.showSummary(); } } catch (err) { prompter.warn(`Gateway setup failed: ${err instanceof Error ? err.message : String(err)}`); throw err; } + } else { + finalizeResult.showSummary(); } } -- 2.49.1