Skip to content

[1601 by Claude Opus 4.8] Implement the setup wizard engine that sequences checks and safe actions - #1613

Merged
propr-dev[bot] merged 2 commits into
1599-epic-build-comprehensive-b5cfrom
1601/claude-opus-4-8-implement-the-setup-wizar-20260622-0950-0em
Jun 22, 2026
Merged

[1601 by Claude Opus 4.8] Implement the setup wizard engine that sequences checks and safe actions#1613
propr-dev[bot] merged 2 commits into
1599-epic-build-comprehensive-b5cfrom
1601/claude-opus-4-8-implement-the-setup-wizar-20260622-0950-0em

Conversation

@propr-dev

@propr-dev propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

AI Implementation Summary

Closes #1601

Branch: 1601/claude-opus-4-8-implement-the-setup-wizar-20260622-0950-0em
Commits: ✅ Changes committed (0c98d83)


AI Processing Completed

Execution Details:

Summary:
The implementation is complete and verified. Here's a summary of what I implemented for issue #1601.

Summary

I created the UI-agnostic setup wizard engine that sequences check + init stack behavior and the remaining one-time setup tasks in a safe order.

Files

  1. packages/cli/src/commands/setup/engine.ts (new) — the engine. It builds on the existing setup/types.ts and setup/state.ts (step model + non-destructive .env helpers) already present in the base branch.

  2. packages/cli/src/commands/setup/engine.test.ts (new) — 9 tests, all passing, run via npx tsx --test.

  3. packages/cli/tsconfig.json (modified) — excludes *.test.ts from the build so the test lives next to its source without leaking into dist/.

Design

The engine separates sequencing/decision logic from I/O via two injectable seams, so it runs fully mocked in tests (no Docker/network/TTY):

  • SetupPrompts — optional callback hooks a renderer (Ink or readline, in later issues) supplies for user decisions (root, agents, GitHub auth, start, whitelist, repo, UI). Every hook is optional with a safe non-interactive default.
  • SetupActions — the side-effecting operations, with createDefaultActions() binding to the real runChecks, scaffoldStack, orchestrator (pull/start/health), and repo API. Heavy modules are lazy-imported so importing the engine stays cheap and Docker-free.
  • SetupReporter — optional progress hooks (onState/onStepStart/onStepSettled/onLog).

Order & safety contract (matches all requirements)

check → init-stack → pull-images → configure-agents → github-auth → start-stack → whitelist → repo → launch-ui

  • Checks run first and steer the rest; a missing/unreachable Docker daemon blocks the flow with a clear next action, while addressable failures (e.g. GitHub auth) continue.
  • Init only when .env is missing or the user picks a new root; scaffoldStack is always called without force, so an existing .env is never overwritten wholesale.
  • Core images pull by default; agent images only for selected agents (default = agents detected on the host).
  • Stack start validates backend health afterward; a running stack is reused, not restarted (user data untouched).
  • All .env edits go through the non-destructive per-key applyEnvSelection.
  • Optional repo addition can be skipped or fail without aborting the run (becomes a warning).

Verification

  • npm run typecheck
  • npm run build ✅ (declarations emit cleanly; test excluded from dist/)
  • npm run lint ✅ (only a pre-existing warning in a generated file)
  • 9/9 engine tests pass, covering: re-run leaves install intact, selective image pulls, optional-repo skip/failure tolerance, Docker-blocked check, unresolved auth warning, unhealthy-backend warning, running-stack reuse, and a full unattended run hitting every step in order.

Detailed Logs:

  • Conversation: 139 messages
  • Session: 22028230-932c-4e73-8fb8-36d4feaa9a9d

Log files stored at:

  • conversation: /tmp/claude-logs/issue-1601-2026-06-22T10-02-25-217Z-conversation.json
  • output: /tmp/claude-logs/issue-1601-2026-06-22T10-02-25-217Z-output.txt
Latest Conversation Messages
ASSISTANT: [content unavailable]

ASSISTANT: The implementation is complete and verified. Here's a summary of what I implemented for issue #1601.

## Summary

I created the **UI-agnostic setup wizard engine** that sequences `check` + `init stack...


This PR was created automatically by ProPR after processing issue #1601.


💡 Need changes?

Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!

… I implemented for issue #1601.

Summary

I created the UI-agnostic setup wizard engine that sequences `check` + `init stack` behavior and the remaining one-time setup tasks in a safe order.

Files

1. `packages/cli/src/commands/setup/engine.ts` (new) — the engine. It builds on the existing `setup/types.ts` and `setup/state.ts` (step model + non-destructive `.env` helpers) already present in the base branch.

2. `packages/cli/src/commands/setup/engine.test.ts` (new) — 9 tests, all passing, run via `npx tsx --test`.

3. `packages/cli/tsconfig.json` (modified) — excludes `*.test.ts` from the build so the test lives next to its source without leaking into `dist/`.

Design

The engine separates *sequencing/decision logic* from *I/O* via two injectable seams, so it runs fully mocked in tests (no Docker/network/TTY):

- `SetupPrompts` — optional callback hooks a renderer (Ink or readline, in later issues) supplies for user decisions (root, agents, GitHub auth, start, whitelist, repo, UI). Every hook is optional with a safe non-interactive default.
- `SetupActions` — the side-effecting operations, with `createDefaultActions()` binding to the real `runChecks`, `scaffoldStack`, orchestrator (pull/start/health), and repo API. Heavy modules are lazy-imported so importing the engine stays cheap and Docker-free.
- `SetupReporter` — optional progress hooks (`onState`/`onStepStart`/`onStepSettled`/`onLog`).

Order & safety contract (matches all requirements)

`check → init-stack → pull-images → configure-agents → github-auth → start-stack → whitelist → repo → launch-ui`

- Checks run first and steer the rest; a missing/unreachable Docker daemon blocks the flow with a clear next action, while addressable failures (e.g. GitHub auth) continue.
- Init only when `.env` is missing or the user picks a new root; `scaffoldStack` is always called without `force`, so an existing `.env` is never overwritten wholesale.
- Core images pull by default; agent images only for selected agents (default = agents detected on the host).
- Stack start validates backend health afterward; a running stack is reused, not restarted (user data untouched).
- All `.env` edits go through the non-destructive per-key `applyEnvSelection`.
- Optional repo addition can be skipped or fail without aborting the run (becomes a warning).

Verification
- `npm run typecheck` ✅
- `npm run build` ✅ (declarations emit cleanly; test excluded from `dist/`)
- `npm run lint` ✅ (only a pre-existing warning in a generated file)
- 9/9 engine tests pass, covering: re-run leaves install intact, selective image pulls, optional-repo skip/failure tolerance, Docker-blocked check, unresolved auth warning, unhealthy-backend warning, running-stack reuse, and a full unattended run hitting every step in order.
@propr-dev

propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

/ultrafix
Triggered automatically by Planner execution settings.

@propr-dev

propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

🔄 Ultrafix loop started (goal: 8/10, max cycles: 10)

First action: /review

💡 Tip: Remove the ultrafix label from this PR to stop further ultrafix cycles.

@propr-dev

propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @propr-dev[bot]

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.5

Overall Evaluation

This PR adds a clean, UI-agnostic setup wizard engine with injectable actions/prompts and focused tests around the main happy paths and several failure modes. The structure is generally solid, but I would not merge it as-is because a few required setup scenarios can still fail silently or escape the engine’s state model.

Overall: needs minor-to-moderate changes before merge.

Findings

🔴 Incomplete stack roots are treated as initializedpackages/cli/src/commands/setup/engine.ts:369 uses !init.envExists to decide whether to scaffold. If .env exists but required directories are missing, inspectStackInit() can report an incomplete stack, but the engine skips scaffoldStack(). This can leave data, logs, or repos missing and cause later startup failures. Use !init.initialized or otherwise account for missing dirs.

🔴 Several setup action failures can throw out of runSetup() instead of becoming step failurespackages/cli/src/commands/setup/engine.ts:498, :530, :555, :581, and nearby prompt calls are not consistently guarded. For example, isStackRunning(), checkBackendHealth(), readEnvVars(), applyEnvSelection(), and prompt callbacks can throw and bypass the returned SetupRunResult, despite the documented contract that expected setup failures are reported in state.

🟡 Prompt-selected agent types are not validatedpackages/cli/src/commands/setup/engine.ts:404. A renderer can return unknown or duplicate agent names; unknown names are passed into pullImages() and duplicates affect later credential handling. Filtering against catalog and de-duping would make the engine boundary safer.

🟡 RootDecision.reinitialize is misleading for existing .env filespackages/cli/src/commands/setup/engine.ts:104 and :374. The interface says “Scaffold .env/data/logs/repos here,” but the implementation calls scaffoldStack({ root }) without force, so an existing .env is preserved. That may be intentional, but the API name and comment imply stronger behavior than is actually possible.

🟡 skipRemoteImageCheck is accepted by pullImages() but ignored by the default implementationpackages/cli/src/commands/setup/engine.ts:166 and :264. If this flag is intentionally only for runChecks, it should not be part of PullImagesParams; otherwise the default action should honor it.

🟡 Hard-coded Docker check names are brittlepackages/cli/src/commands/setup/engine.ts:637. blockingDockerFailure() depends on exact result names "Docker installed" and "Docker daemon". If checkCommands.ts wording changes, setup may continue after a blocking Docker failure. A structured check identifier would be safer.

🟡 Tests are excluded from tsc compilationpackages/cli/tsconfig.json:18. Excluding src/**/*.test.ts avoids declaration/build noise, but it also means type errors in tests are not caught by the normal package typecheck unless another test-specific tsconfig or command covers them.

🟢 Consider sharing the agent catalog instead of duplicating itpackages/cli/src/commands/setup/engine.ts:56. The comment notes this mirrors other modules. Duplication is manageable now, but new agents or changed credential paths could drift across setup, check, and init behavior.

🟢 Consider reporting GitHub auth warnings from detectGithubAuthMode()packages/cli/src/commands/setup/engine.ts:477. The engine only reports the mode. If the shared detector returns warnings for partially configured auth, surfacing them in step detail would make setup more actionable.

Good separation of orchestration from UIpackages/cli/src/commands/setup/engine.ts:128 and :207. SetupPrompts, SetupReporter, and SetupActions make the flow testable and ready for multiple renderers.

Non-destructive setup behavior is a strong defaultpackages/cli/src/commands/setup/engine.ts:374, :452, and :481. Avoiding wholesale .env overwrites and reusing a running stack are the right safety choices for a setup wizard.

Tests cover important behavior without Docker/network dependenciespackages/cli/src/commands/setup/engine.test.ts:58. The test suite verifies re-run safety, optional repo handling, selected image pulls, Docker blocking, and already-running stack reuse.

Score

The implementation is well-structured and close, but the incomplete-stack handling and uncaught action failures are important correctness gaps for a setup wizard.
Score: 7/10


🤖 Review Details

  • Model: gpt-5.5
  • Time: 39s
  • Tokens: 36,349 (34,817 in / 1,532 out)
  • Cost: $0.22

View Task


💡 Next step: Comment /fix on this PR to have the AI automatically implement the suggestions above.
The /fix command gathers all unprocessed AI review comments and applies fixes in a single pass.
You can edit or delete review comments before running /fix to control which suggestions are applied.
Add extra instructions if needed, e.g. /fix only address the critical findings.

@propr-dev

propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit d5ef628

Addressed 1 AI review comment (IDs: 4767120385)

Summary of Changes

All findings are addressed. skipRemoteImageCheck now appears only in the check-step path (its legitimate use), confirming it was cleanly removed from the image-pull boundary.

Here's a summary of the changes I made to address the AI review findings on PR #1613:

🔴 Critical fixes

  1. Incomplete stack roots no longer treated as initialized (engine.ts) — the scaffold decision now keys off !init.initialized instead of !init.envExists. A root with a stray .env but missing data//logs//repos/ now gets re-scaffolded (non-destructively, .env preserved) instead of being skipped and failing later at startup.

  2. Setup action failures become step failures, never escaped exceptions — wrapped the previously-unguarded bodies of pull-images (incl. selectAgents), configure-agents, github-auth, start-stack (incl. isStackRunning/checkBackendHealth), whitelist (incl. readEnvVars), repo, and launch-ui prompts in try/catch. Required steps settle as failed and stop the flow; optional steps degrade to warning. Honors the documented "expected failures are reported in state" contract.

🟡 Moderate fixes

  1. Agent selections validated — renderer-returned agent names are now de-duped (first wins) and filtered against the catalog, so unknown names never reach pullImages() and duplicates can't double-apply credentials.
  2. RootDecision.reinitialize comment clarified to reflect the actual non-destructive behavior.
  3. skipRemoteImageCheck removed from PullImagesParams since the default action ignored it; it remains only on the check step where it's used.
  4. blockingDockerFailure made structural — keys off group === "Docker" && status === "fail" instead of exact check-name strings (the socket check tops out at warn, so this is precise).
  5. Tests are now type-checked — added tsconfig.test.json (includes *.test.ts) and wired it into npm run typecheck.

🟢 Minor

  1. GitHub auth warnings surfaced — when the shared detector resolves a mode but flags a partial config, the step now reports a warning with those warnings instead of silently showing done.

I left the 🟢 "share the agent catalog" suggestion as-is — deduplicating it would require restructuring exports across the check/init modules, which is broader than this focused fix.

Verification

  • npm run typecheck (now incl. tests) — clean
  • npm run build — clean
  • Engine tests — 12 passing (added 3 new: incomplete-stack scaffolding, agent de-dup/filtering, action-throw-becomes-step-failure)
  • npm run lint — only a pre-existing warning in a generated dist/ file I didn't touch

🤖 Implementation Details

  • Model: Claude Opus 4.8
  • Requested By: @propr-ultrafix
  • Time: 6m 37s
  • Tokens: 2,391,612 (2,366,256 in / 25,356 out)
  • Cost: $2.21
  • Subscription usage: Session +5%, Weekly +1%

View Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

…ars only in the check-step path (its legitimate use), confirming it was cleanly removed from the image-pull boundary.

All findings are addressed. `skipRemoteImageCheck` now appears only in the check-step path (its legitimate use), confirming it was cleanly removed from the image-pull boundary.

Here's a summary of the changes I made to address the AI review findings on PR #1613:

## 🔴 Critical fixes

1. **Incomplete stack roots no longer treated as initialized** (`engine.ts`) — the scaffold decision now keys off `!init.initialized` instead of `!init.envExists`. A root with a stray `.env` but missing `data/`/`logs/`/`repos/` now gets re-scaffolded (non-destructively, `.env` preserved) instead of being skipped and failing later at startup.

2. **Setup action failures become step failures, never escaped exceptions** — wrapped the previously-unguarded bodies of `pull-images` (incl. `selectAgents`), `configure-agents`, `github-auth`, `start-stack` (incl. `isStackRunning`/`checkBackendHealth`), `whitelist` (incl. `readEnvVars`), `repo`, and `launch-ui` prompts in try/catch. Required steps settle as `failed` and stop the flow; optional steps degrade to `warning`. Honors the documented "expected failures are reported in state" contract.

## 🟡 Moderate fixes

3. **Agent selections validated** — renderer-returned agent names are now de-duped (first wins) and filtered against the catalog, so unknown names never reach `pullImages()` and duplicates can't double-apply credentials.
4. **`RootDecision.reinitialize` comment clarified** to reflect the actual non-destructive behavior.
5. **`skipRemoteImageCheck` removed from `PullImagesParams`** since the default action ignored it; it remains only on the check step where it's used.
6. **`blockingDockerFailure` made structural** — keys off `group === "Docker" && status === "fail"` instead of exact check-name strings (the socket check tops out at `warn`, so this is precise).
7. **Tests are now type-checked** — added `tsconfig.test.json` (includes `*.test.ts`) and wired it into `npm run typecheck`.

## 🟢 Minor

8. **GitHub auth warnings surfaced** — when the shared detector resolves a mode but flags a partial config, the step now reports a `warning` with those warnings instead of silently showing `done`.

I left the 🟢 "share the agent catalog" suggestion as-is — deduplicating it would require restructuring exports across the check/init modules, which is broader than this focused fix.

## Verification
- `npm run typecheck` (now incl. tests) — clean
- `npm run build` — clean
- Engine tests — **12 passing** (added 3 new: incomplete-stack scaffolding, agent de-dup/filtering, action-throw-becomes-step-failure)
- `npm run lint` — only a pre-existing warning in a generated `dist/` file I didn't touch

PR: #1613
Comment by: @propr-ultrafix (ID: 0)
Model: claude-opus-4-8
@propr-dev

propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.5

Overall Evaluation

This PR adds a UI-agnostic setup wizard engine with good separation between prompts, reporting, and side effects. The flow is mostly well-structured, heavily mockable, and covered by focused node:test tests for re-runs, partial stack scaffolding, optional steps, and failure handling.

I would rate this as close to mergeable, but it needs minor changes before merging due to a couple of behavior and maintainability concerns in the engine defaults.

Findings

🟡 Warning — Start skip still allows later backend-dependent steps: In packages/cli/src/commands/setup/engine.ts around the start-stack step, if confirmStartStack returns false, the engine marks the step as skipped but continues into whitelist, repo, and UI launch steps. Whitelist is local .env work, but repo addition and UI launch imply a usable running backend/UI. Consider short-circuiting optional backend/UI work when startup is skipped, or at least explicitly skip repo/UI with details.

🟡 Warning — Duplicated agent catalog can drift from command behavior: packages/cli/src/commands/setup/engine.ts defines agentCatalog() separately while noting it mirrors checkCommands.ts and initStack.ts. This is a correctness risk as new agents, env keys, or credential paths can diverge. A shared catalog/helper would reduce the chance that setup pulls/configures a different set of agents than check/init.

🟡 Warning — imageKey is currently unused: In packages/cli/src/commands/setup/engine.ts, AgentDescriptor.imageKey is declared and populated but not used. That increases the drift risk above and may hide mismatches between agent type and manifest image key. Either use it when filtering images or remove it until needed.

🟢 Suggestion — Sanitize empty whitelist prompt results before deciding status: In packages/cli/src/commands/setup/engine.ts, if (whitelist && whitelist.length > 0) treats [" "] as a configured whitelist, then writes an empty string and reports 0 user(s) allowed. Clean the array before the branch and warn/skip if the cleaned list is empty.

🟢 Suggestion — GithubAuthDecision.keep and mode are currently informational only: In packages/cli/src/commands/setup/engine.ts, only authDecision.vars affects behavior. That is documented, but future renderer authors may reasonably expect mode or keep to be authoritative. Consider narrowing the interface or adding explicit validation/comments near the call site.

Positive — Good side-effect isolation: packages/cli/src/commands/setup/engine.ts cleanly separates SetupPrompts, SetupReporter, and SetupActions, which makes the engine testable without Docker, network, or TTY dependencies.

Positive — Re-run safety is explicitly handled: The initialization logic preserves existing .env files and only scaffolds incomplete roots, which matches the setup wizard’s safety requirements well.

Positive — Tests cover the important flow boundaries: packages/cli/src/commands/setup/engine.test.ts covers initialized reruns, incomplete stack roots, selected agent filtering, fatal versus optional failures, and step ordering.

Positive — Build/test TypeScript separation is sensible: Excluding tests from packages/cli/tsconfig.json while adding tsconfig.test.json to npm run typecheck keeps emitted package output clean without losing test type coverage.

Score

Score: 8/10

Strong implementation with clear architecture and useful tests. I would address the skipped-start follow-on behavior and agent catalog drift before merging.


🤖 Review Details

  • Model: gpt-5.5
  • Time: 37s
  • Tokens: 39,556 (38,265 in / 1,291 out)
  • Cost: $0.23

View Task


💡 Next step: Comment /fix on this PR to have the AI automatically implement the suggestions above.
The /fix command gathers all unprocessed AI review comments and applies fixes in a single pass.
You can edit or delete review comments before running /fix to control which suggestions are applied.
Add extra instructions if needed, e.g. /fix only address the critical findings.

@propr-dev propr-dev Bot removed the ultrafix label Jun 22, 2026
@propr-dev
propr-dev Bot merged commit 348560e into 1599-epic-build-comprehensive-b5c Jun 22, 2026
2 checks passed
@propr-dev
propr-dev Bot deleted the 1601/claude-opus-4-8-implement-the-setup-wizar-20260622-0950-0em branch June 22, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants