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

Merged
jason.woltje merged 1 commits from fix/wizard-step-dedup into next 2026-06-25 18:44:36 +00:00
Owner

Summary

  • skip already-completed menu sections instead of re-running Providers/Skills after they show [done]
  • mark Quick Start Providers and Skills sections complete after they run
  • distinguish explicit provider setup with no key from an unrun provider step so Finish does not ask for a second API key

Verification

  • 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

Fixes #16

## Summary - skip already-completed menu sections instead of re-running Providers/Skills after they show [done] - mark Quick Start Providers and Skills sections complete after they run - distinguish explicit provider setup with no key from an unrun provider step so Finish does not ask for a second API key ## Verification - 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 Fixes #16
jason.woltje added 1 commit 2026-06-25 18:33:59 +00:00
fix(wizard): avoid rerunning completed setup steps
Some checks failed
ci/woodpecker/push/ci Pipeline was canceled
ci/woodpecker/pr/ci Pipeline was successful
0113d89ab8
Author
Owner

REVIEW-OF-RECORD — APPROVE (#692, head 0113d89ab8). I reviewed the wizard step deduplication fix and found no blockers.

Deduplication: runMenuLoop now maps menu choices to section keys and skips completed sections before dispatching their stages. Providers, Identity, Skills, Gateway, and Advanced no longer rerun after they are marked [done]; the regression test explicitly re-selects Providers and Skills and verifies each stage executes exactly once, with clear skip logs.

Provider/no-key handling: providerSetupStage already records an explicit no-key provider setup as providerType='none'. This PR preserves that state into gateway config instead of defaulting unrun/undefined provider setup to none. gatewayConfigStage now treats providerType='none' as an explicit completed provider step and skips the second ANTHROPIC_API_KEY prompt while writing no API key env var. If provider setup was never run, providerType remains undefined and the gateway prompt path remains available.

B1-B3 regression check: Quick Start marks Providers and Skills complete after running them. The B2 finalize/skills-sync path stays intact (full mosaic tests include finalize-skills), and the B3 deferred success-summary ordering remains covered by unified-wizard tests. I did not see unrelated 0.0.50/provider behavior mixed into this diff.

Reviewer verification on head 0113d89a: targeted unified-wizard gateway-config tests pass (28/28); full mosaic test suite pass (629/629); mosaic typecheck pass; mosaic lint pass; repo format:check pass; repo build pass (23/23). PR CI was still running when reviewed, so merge should still wait for terminal-green CI. APPROVE.

REVIEW-OF-RECORD — APPROVE (#692, head 0113d89ab870a40a81690faffaf69e42313a5509). I reviewed the wizard step deduplication fix and found no blockers. Deduplication: `runMenuLoop` now maps menu choices to section keys and skips completed sections before dispatching their stages. Providers, Identity, Skills, Gateway, and Advanced no longer rerun after they are marked `[done]`; the regression test explicitly re-selects Providers and Skills and verifies each stage executes exactly once, with clear skip logs. Provider/no-key handling: `providerSetupStage` already records an explicit no-key provider setup as `providerType='none'`. This PR preserves that state into gateway config instead of defaulting unrun/undefined provider setup to `none`. `gatewayConfigStage` now treats `providerType='none'` as an explicit completed provider step and skips the second `ANTHROPIC_API_KEY` prompt while writing no API key env var. If provider setup was never run, `providerType` remains undefined and the gateway prompt path remains available. B1-B3 regression check: Quick Start marks Providers and Skills complete after running them. The B2 finalize/skills-sync path stays intact (full mosaic tests include finalize-skills), and the B3 deferred success-summary ordering remains covered by unified-wizard tests. I did not see unrelated 0.0.50/provider behavior mixed into this diff. Reviewer verification on head 0113d89a: targeted `unified-wizard gateway-config` tests pass (28/28); full mosaic test suite pass (629/629); mosaic typecheck pass; mosaic lint pass; repo format:check pass; repo build pass (23/23). PR CI was still running when reviewed, so merge should still wait for terminal-green CI. APPROVE.
Author
Owner

REVIEW-OF-RECORD — APPROVE

Independent review (reviewer ≠ author). Blocker B4 (greenfield wizard: Providers + Skills menu steps re-ran after [done] — API key asked twice, skills block shown twice).

Fix:

  • New skipCompletedMenuChoice() short-circuits any menu section already in the completed set ("…is already complete; skipping") instead of re-running its stage — applied to providers/identity/skills/gateway-config/advanced. menuSectionKey() centralizes the MenuChoice→MenuSection mapping (gateway-config→gateway; null for quick-start/finish).
  • quickStartPath now records providers + skills in completedSections after running them.
  • Double API-key prompt fixed: gateway-config handles providerType === "none" (empty key, skip the gateway key prompt) and all callers now pass the real state.providerType (no ?? "none" coercion) so a "none" selection from provider setup propagates and the gateway stage does not re-ask.

Tests: +76 lines unified-wizard integration coverage + new gateway-config.spec (+39) for the providerType="none" / no-reprompt path.

CI: PR-event pipeline 1649 fully green (ci-postgres, typecheck, lint, format, test all success).

Approving for squash-merge to next. This is the 4th and final 0.0.49 RC blocker.

REVIEW-OF-RECORD — APPROVE Independent review (reviewer ≠ author). Blocker **B4** (greenfield wizard: Providers + Skills menu steps re-ran after `[done]` — API key asked twice, skills block shown twice). **Fix:** - New `skipCompletedMenuChoice()` short-circuits any menu section already in the `completed` set ("…is already complete; skipping") instead of re-running its stage — applied to providers/identity/skills/gateway-config/advanced. `menuSectionKey()` centralizes the MenuChoice→MenuSection mapping (gateway-config→gateway; null for quick-start/finish). - `quickStartPath` now records `providers` + `skills` in `completedSections` after running them. - Double API-key prompt fixed: `gateway-config` handles `providerType === "none"` (empty key, skip the gateway key prompt) and all callers now pass the real `state.providerType` (no `?? "none"` coercion) so a "none" selection from provider setup propagates and the gateway stage does not re-ask. **Tests:** +76 lines unified-wizard integration coverage + new gateway-config.spec (+39) for the providerType="none" / no-reprompt path. **CI:** PR-event pipeline 1649 fully green (ci-postgres, typecheck, lint, format, test all success). Approving for squash-merge to `next`. This is the 4th and final 0.0.49 RC blocker.
jason.woltje merged commit 495f73bfdb into next 2026-06-25 18:44:36 +00:00
jason.woltje deleted branch fix/wizard-step-dedup 2026-06-25 18:44:37 +00:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#692