fix(setup): quick wizard — require key on first run, drop SOUL/knowledge/tools steps#276
Conversation
…dge/tools steps The wizard let you "finish" with a blank API key (the placeholder said "leave blank to preserve current key" — wrong on first run), so setup_complete stayed false and the modal couldn't close: an inescapable loop with no key configured. - Require the API key on first run: Finish is disabled until it's entered, and the placeholder reads "Enter your API key" (only a reconfigure may leave it blank to keep the existing key). - Slim to a genuine quick setup — Welcome -> Identity -> Model -> Finish. Drop the SOUL/persona editor (don't let setup rewrite protoPen's identity), the knowledge workspace step, and the middleware-toggle step — all keep their defaults server-side (run_setup only overrides what's sent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesQuick Setup Wizard Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
QA Audit — PR #276 | fix(setup): quick wizard — require key on first run, drop SOUL/knowledge/tools steps
VERDICT: WARN (non-terminal CI — comment only; re-review once checks settle)
CI Status
- Verify workspace config: queued
- Lint: queued
Diff Review
- Single-file change to
apps/web/src/setup/SetupWizard.tsx(~328 lines). Reduces wizard from 7 steps to 4 (welcome → identity → model → finish) and enforces API key on first run. - Key enforcement logic is sound:
needsKeyderived from!status.setup_complete,keyOkgates bothcanGoNext(model step) andfinishSetup— two-layer guard against the blank-key loop. - All removed state fields, imports, functions (
setMiddleware,loadPreset), and props (projectPath,onProjectPathChange) are cleaned up consistently. No dangling references. run_setuppayload now sends empty string for soul and omits middleware/subagents/knowledge config — relies on server-side defaults per PR description.
Observations
- LOW: Diff truncated at 200/328 lines — the JSX render block for the simplified steps and tail of
finishSetupnot reviewable. No red flags in the portion visible. - LOW: Clawpatch structural review unavailable —
protoLabsAI/protoPennot in project registry. - LOW:
projectPath/onProjectPathChangeprops removed from signature but no caller changes in diff. Author reports cleantsc+vite build; CI will confirm.
— Quinn, QA Engineer
|
Submitted COMMENT review on #276. |
Why
Two problems, found live on the Deck during first real bring-up:
setup_completestayedfalse, and the modal — gated onsetup_complete === false— could never close. An inescapable loop with no key configured.What
run_setuponly overrides what's sent, so protoPen's SOUL and knowledge config are untouched.Testing
Deployed live to the Deck and validated: clean
tsc+vite build, wizard renders the 4 steps, Finish stays disabled until a key is entered, and completing it flipssetup_completetrue → modal closes → console.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes