feat(desktop,web_ui): E2 first-run validation wizard — RAM gate, sha256 manifest verification, pack activation, license gate (issue #85) - #116
Conversation
…56 manifest verification, pack activation, license gate (issue #85) Six-state wizard (detect-hardware -> select-profile -> verify-manifest -> activate-packs -> licensing-notices -> complete) wired from the A3 RAM-gate formula, the #84-pinned integrity-manifest contract, and the C3 pack lifecycle. firstRun.* state persists in an atomic profile-dir sidecar; drift on any manifest-covered file re-triggers the wizard; the license acknowledgment is unskippable and every failure names the file + expected/ actual. The Electron bootstrap now honors TRAININGAPP_DESKTOP_STORE_PATH (dev-server parity), giving the e2e suite per-test store isolation. Acceptance: 7 frozen NEW-SURFACE checks RED->GREEN; desktop vitest 425, desktop e2e 7 (incl. renderer-smoke + production-mode training-player), web_ui vitest 1278 + typecheck(app+test) clean.
Swarm PR review (run pr116-review-1)Bound head: Verified findings
Rejected candidates (transparency)
Obligation checkCloses #85 — all six Required-scope checkboxes map to implemented, tested code; the four out-of-scope items (E3, E5-reuse note, A3, E1) are respected. E2E fixture-manifest boundary disclosed in the PR body. Micro-lane attestationMATCHED+evaluated: untrusted-input-boundaries (PRR-001), concurrency-state (PRR-002), ui-accessibility (PRR-003), test-infrastructure (PRR-004), api-schema-migrations (clean — ambient renderer type only, preload+main ship in one artifact), unclassified-risk (clean — full change-path pass). NOT_TRIGGERED with absence evidence: auth-identity-secrets (no authn/z/secret code in diff), subprocess-platform (no subprocess/fs-exec changes outside test-only taskkill, pre-existing pattern), dependencies-build-release (no manifest/lockfile/CI changes in the PR), privacy-observability (no telemetry/log-surface changes beyond local console.error of local paths), generated-provenance (dist/renderer artifacts not committed). VerdictAPPROVE with findings (no CRITICAL; PRR-001 MEDIUM is being fixed in a follow-up commit on this branch before merge). |
There was a problem hiding this comment.
🟡 Changes recommended
FirstRunGate unmounts on status.needed flipping false after completion, making the "Setup complete"/Finish screen unreachable in production (masked in e2e by the force seam), plus redundant multi-GB manifest hashing.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR implements Workstream E2 (issue #85): a desktop-only first-run validation wizard that composes previously-landed primitives (A3 RAM-gate math, B4 profile selection, B9 model presence, C3 PackManager, E1 manifest) into a guided six-state sequence: detect-hardware → select-profile → verify-manifest → activate-packs → licensing-notices → complete. It adds four Electron-free main-process modules under desktop/main/first-run/, IPC wiring, a preload bridge extension, and a React modal stepper mounted through FirstRunGate. The wizard's defining behavior is failure fidelity: manifest verification names the specific file with expected/actual sha256 (a closed union, never a generic message), the license acknowledgment is unskippable, and drift re-triggers setup on relaunch.
Changes:
- New RAM gate, sha256 manifest verifier, atomic first-run sidecar store, and pure completion-guard state machine (
desktop/main/first-run/*), wired via fourdesktop:first-run:*IPC channels + a boot push. - Renderer wizard (
FirstRunWizard.tsx/FirstRunGate), a typedfirst-run.tsclient, a Settings "Re-run setup" card, and an extendedDesktopApiBridgetype. - Electron bootstrap now honors
TRAININGAPP_DESKTOP_STORE_PATHfor per-test store isolation;CONTEXT_SIZEexported from the engine; new vitest + Playwright specs; docs/CHANGELOG updates.
File summaries
| File | Description |
|---|---|
desktop/main/first-run/ram-gate.ts |
A3 RAM-gate port with declared-size fallback and free-RAM dev seam |
desktop/main/first-run/manifest-verifier.ts |
Manifest loader + sha256/size verifier with closed-union failures |
desktop/main/first-run/first-run-store.ts |
Atomic sidecar persistence + drift evaluation |
desktop/main/first-run/wizard.ts |
Pure state machine + completion guard |
desktop/main/index.ts |
IPC handlers, engine reuse, store-path seam, boot push |
desktop/main/backend/index.ts |
getFirstRunPackTools() own-property exposure |
desktop/main/backend/inference/llama-engine.ts |
Exports CONTEXT_SIZE for the gate |
desktop/preload/index.ts |
Bridges the four first-run IPC channels + subscription |
web_ui/src/components/FirstRunWizard.tsx |
Modal stepper + FirstRunGate |
web_ui/src/lib/first-run.ts |
Typed bridge client + reopen bus |
web_ui/src/types/desktop.d.ts |
FirstRunStatus/bridge types |
web_ui/src/pages/SettingsPage.tsx |
"First-run setup" / Re-run card |
web_ui/src/App.tsx |
Mounts FirstRunGate |
web_ui/src/lib/electron-detect.test.ts, DocumentsPage.electron.test.tsx |
Type-complete bridge stubs |
desktop/src/__tests__/e2-first-run-wizard.test.ts, desktop/e2e/first-run-wizard.spec.ts |
Unit + e2e coverage |
desktop/README.md, CHANGELOG.md |
Feature documentation |
Review details
- Files reviewed: 18/19 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function sha256File(absolutePath: string): string { | ||
| return createHash('sha256').update(readFileSync(absolutePath)).digest('hex'); | ||
| } |
| const wmForGate = wizardManifest(); | ||
| const gateSizes = { | ||
| qualityFileBytes: | ||
| qualityBytes?.bytes ?? | ||
| (wmForGate.manifest !== null | ||
| ? manifestGroupBytes(wmForGate.manifest, 'llm-quality') | ||
| : undefined), | ||
| fastFileBytes: | ||
| fastBytes?.bytes ?? | ||
| (wmForGate.manifest !== null ? manifestGroupBytes(wmForGate.manifest, 'llm-fast') : undefined), | ||
| }; | ||
| const recommendation = autoSelectProfile({ | ||
| freeBytes, | ||
| nCtx: CONTEXT_SIZE, | ||
| ...gateSizes, | ||
| }); | ||
| const wm = wizardManifest(); |
| }; | ||
| }, []); | ||
|
|
||
| if (!open || status === null || !status.needed) return null; |
…containment, single verify pass, wizard Esc/focus trap, typed stubs PRR-001 (MEDIUM): manifest-controlled files[].path / packs[].dir now resolve through containedJoin — traversal attempts are a named 'traversal' failure (path + expected/actual) and pack dirs outside the manifest packs/ dir are refused at activation, never read. PRR-002 (LOW): buildFirstRunStatus loads+verifies the manifest ONCE per invocation (was two full sha256 passes). PRR-003 (LOW): wizard modal gains Esc-to-dismiss (same as Skip for now — never completes) and a Tab focus trap; covered by FirstRunWizard.test.tsx. PRR-004 (INFO): inert bridge stubs in two web_ui tests are type-honest (minimal FirstRunStatus literal) instead of as-unknown-as casts. Regression tests: traversal rejection + containedJoin unit tests (desktop, 19 passed), FirstRunWizard.test.tsx (2 passed); full first-run e2e 4/4, desktop compile + web_ui typechecks (app+test) clean.
Swarm PR Review — PR #116 (issue #85, first-run validation wizard)Scope: 98d3cef...2a8c824 (19 files, +2296/-4), tier L. Profile B (Claude Code native subagents), 6 base dimension lanes + 1 risk-family micro-lane (ui-accessibility-i18n; other 10 risk families evaluated NOT_TRIGGERED with evidence: no subprocess/shell calls, no dependency/lockfile/CI changes, no generated/vendored files, no new telemetry in the diff) → 3 independent reviewer passes → 1 critic pass on the highest-severity findings. Verdict: APPROVE_WITH_NOTESNo unresolved CRITICAL or HIGH findings survive critic review — the two candidates initially rated HIGH (RP-01, ui-a11y-1) were both downgraded to MEDIUM after adversarial re-verification. Several MEDIUM findings are real, confirmed defects worth fixing, but none block merge. Confirmed findings (reviewer + critic validated)F-01 (MEDIUM) — Synchronous full-file sha256 hashing on the Electron main process, multiple redundant passes
F-02 (MEDIUM) — RAM-gate fallback silently defeated for files missing
F-03 (MEDIUM) — Accessible-but-unmanaged modal:
F-04 (MEDIUM) — F-05 (LOW, critic-downgraded from MEDIUM) — Manifest-file deletion (as opposed to a covered file) after completion silently defeats drift re-detection
F-06 (MEDIUM) — Existing/upgraded installs are gated into the wizard with no docs disclosure Lower-severity confirmed findings (LOW, not critic-escalated):
Disproved / not-reachable-as-claimedNone of the 30 base-lane + micro-lane candidates were disproved outright; all held up under independent re-reading, though several were downgraded in severity (detailed above) once critic-verified mitigating context (dismissibility, scope docs, missing-manifest latency, equally-easy alternate bypasses, Node's Test/coverage gaps worth closing
Obligation mapping (issue #85 AC1-AC7)All seven acceptance criteria have corresponding passing tests per the PR's own evidence. One caveat: AC2's "low RAM auto-selects Fast + warning + override" primary path is correctly tested and works — but its unknown-size fallback path (F-02) breaks silently, and the "operator override always wins" claim (F-04) may not be functionally wired to the runtime model loader beyond being recorded and displayed. Recommended actions before/after merge
Review methodology: 6 parallel base-dimension explorer lanes + 1 targeted risk-family micro-lane (accessibility) → 3 independent reviewer passes (fresh context, re-read all cited evidence) → 1 adversarial critic pass on the 4 highest-severity findings. 30 total candidates generated, 30 reviewer-validated (0 disproved, several severity-adjusted), 4 critic-challenged (2 downgraded HIGH→MEDIUM, 1 upheld MEDIUM, 1 downgraded MEDIUM→LOW). 🤖 Generated with Claude Code |
Post-fix re-verification — commit 5771e4c (PRR-001..004)Re-checked out the PR at its new head ( [REVERIFIED] | sec-01 (PRR-001) | FIXED | [REVERIFIED] | F-01 / RP-02 redundant-pass half (PRR-002) | FIXED | [REVERIFIED] | F-03 / ui-a11y-1 (PRR-003) | FIXED | [REVERIFIED] | CD/TF typed-stub note (PRR-004) | FIXED | Findings from the original review that remain open (not claimed fixed, correctly untouched)
Updated verdict: APPROVE_WITH_NOTES (unchanged)The fixes in this commit are real, correctly scoped, and don't introduce new regressions. No CRITICAL/HIGH findings existed before or after. F-02 remains the single most valuable outstanding fix (it's a one-line change with an already-execution-proven repro), followed by tracking F-01's remaining half against the E1/#84 manifest landing. 🤖 Generated with Claude Code |
Issue #85 — PR Body
Closes #85
PR head: 5771e4c
Root Cause
Not a defect — a planned feature slot (Workstream E2, epic #50) whose prerequisite primitives had just landed: the A3 RAM-gate formula (
estimate_required_memory, llm_interface.py:42-44, Python-only), B4 profile auto-selection (selectProfile, 6 GiB inclusive threshold), B9 model-presence status (modelStatus()—existsSync-only at desktop/main/backend/inference/llama-engine.ts:375-385, which ships corruption silently), and the C3 PackManager lifecycle. Nothing composed them into a first-run experience, and no desktop surface verified the sha256 of any packaged file or required license acknowledgment. Additionally, the Electron bootstrap ignoredTRAININGAPP_DESKTOP_STORE_PATH(consumed only by the headless dev-server, dev-server.ts:101), so the Playwright-under-Electron suite could not isolate per-test stores — the very seam the existing e2e specs already assumed.Fix
desktop/main/first-run/ram-gate.ts(new): TS port of A3'sfile_size + kv_estimate(n_ctx) + GGUF_LOAD_OVERHEAD_BYTES(1 GiB each;n_ctx = CONTEXT_SIZE8192, now exported from the engine so the gate estimates the context it will actually create),autoSelectProfilewith a number-naming downgrade warning, a declared-manifest-size fallback for models not yet staged, and theTRAININGAPP_DESKTOP_FREE_RAM_BYTESdev/test seam.desktop/main/first-run/manifest-verifier.ts(new): loads the [Workstream E] PR 1 of 6: Package models and default packs into the Electron installer with an integrity manifest and a <=7 GB size budget #84-pinnedresources/manifest.jsoncontract and verifies every required file's sha256 (sizeBytes check included). Failures are a CLOSED UNION (missing | hash-mismatch | size-mismatch | sha256-required | manifest-unreadable) each carrying path + expected/actual — the generic "configure an LLM backend" failure shape is unrepresentable. Packaged install without a manifest fails closed; dev/CI degrades explicitly (staged:false).desktop/main/first-run/first-run-store.ts(new): atomic<profileDir>/first-run.jsonsidecar persistingfirstRun.completed/selectedProfile/completedAt+ license acknowledgment + a per-file digest anchor (the engine settings API validates a closed rag_* set and cannot carry these).evaluateStatusyields not-completed/drift/reset/complete; drift re-derives from disk and outranks the force seam.desktop/main/first-run/wizard.ts(new): the six-state machine (detect-hardware → select-profile → verify-manifest → activate-packs → licensing-notices → complete) withassertCanComplete(license acknowledgment unskippable; every refusal names the unmet gate) andmanifestCompletionState(packaged fail-closed rule).getFirstRunPackTools()(backend/index.ts:230 — b3 prototype pin respected), fourdesktop:first-run:*IPC channels +first-run:requiredboot push + stub-engine exemption withTRAININGAPP_FIRST_RUN_FORCE=1seam (desktop/main/index.ts),desktopApipreload extension with typed surface (web_ui/src/types/desktop.d.ts).FirstRunWizard.tsxmodal stepper (failure table with path/expected/actual, profile warning + explicit Quality override, pack activation with results, license notice rendered fromdocs/licenses.mdwhen present with a required acknowledgment checkbox) mounted viaFirstRunGatein App; Settings "First-run setup" section with "Re-run setup" (reset + reopen bus).TRAININGAPP_DESKTOP_STORE_PATH(dev-server parity), giving the e2e suite true per-test store isolation.desktop/README.md"First-run wizard (E2)" section;CHANGELOG.mdentry.Recurrence Prevention (defect class)
existsSyncuses — all absence-signals or hash-prefetches, none an integrity claim; P3 five catches — all named degradations or cleanup-with-rethrow), each dispositioned in 08a-recurrence-sweep.md.Tests
repro-check.sh run, base 98d3cef → head 2a8c824): C1-C4, C5-C6 (Playwright-under-Electron), C3/C4/C7 (vitest) — verdicts PASS, logs in the issue trace.desktop/src/__tests__/e2-first-run-wizard.test.ts(17 tests) anddesktop/e2e/first-run-wizard.spec.ts(4 tests) -> PASS.desktop && npm test-> 425 passed (67 files);desktop && npx playwright test-> 7 passed (incl. renderer-smoke restart leg and production-mode training-player);web_ui && npx vitest run-> 1278 passed | 2 skipped.desktop && npm run compile-> clean;web_ui && npx tsc --noEmitand-p tsconfig.test.json-> clean;web_ui && npm run build-> ok.repro-check.sh verify-checkpoint-> 7/7 OK; per-driver last-manifest-row blob audit -> 7/7 MATCH (7 CHECK_WRONG AMEND rows were driver-infrastructure fixes only: always-rebuild guard against a stale renderer dist, and a vitest cwd-relative path; discriminating assertions untouched).Regression Protection
electron-detect.test.ts,DocumentsPage.electron.test.tsx) gained inert type-complete bridge stubs — required by the extendedDesktopApiBridge; no assertion weakened.Acceptance Criteria -> Evidence
C1:GREEN (first-run-wizard.spec.ts) — all six step sections traversed; activation resultinstalled bundled-min@1.0.0; Finish closes a wizard whose status.needed is false. Fixture manifest stands in for E1's installer manifest (E1/#84 owns the real one) — disclosed boundary, real-weight validation is E3/#86.C2:GREEN — TRAININGAPP_DESKTOP_FREE_RAM_BYTES=4 GiB, declared 3.85 GiB quality model → Fast preselected, warning names required/free bytes, Quality radio enabled ("override"); backed by unit parity tests vs llm_interface.py:42-44.C3:GREEN — path +sha256 <expected>+ actual "missing" + reason 'missing'.C4:GREEN — 'hash-mismatch' with distinct expected/actual sha256 strings.C5:GREEN (with no required packs, so the license gate is the only varying condition) — Complete disabled before, enabled after the checkbox; IPC arm pinned by assertCanComplete unit refusals.C6:GREEN — complete → mutate covered file → kill → relaunch → wizard re-opens with drift reason; unit pins evaluateStatus drift (incl. drift-outranks-force); Settings "Re-run setup" wired (reset → reason 'reset', reopen bus).C7:GREEN — atomic sidecar round-trip (completed/selectedProfile/completedAt + ack + digest anchor), no tmp residue, corrupt sidecar degrades to defaults.Invariant Audit
Repository invariant/architecture-contract sources checked: ADR-0003 (backend host), ADR-0004 (pack format), ADR-0006 (profile model), ADR-0008 (memory budget), the b3 duck-type pin (backend/index.ts comments), and the store SCHEMA-BUMP-CONTRACT.
getFirstRunPackToolsis an own property (backend/index.ts:230), mirroringcreateStoreBackup.ipcMain.handle+contextBridgepattern; no new remote surface; token still never in web storage/URL.test:secure-defaults.DEFAULT_PROFILE_THRESHOLD_GBimported by memory/budget.ts unchanged.Risk and Rollback
FirstRunGaterenders nothing without the bridge). One behavioral seam change outside the wizard: the Electron bootstrap now honorsTRAININGAPP_DESKTOP_STORE_PATH(previously dev-server-only) — this is additive (no env set → identical ADR-0006 behavior) and is what the existing e2e specs already assumed; the full desktop e2e suite (incl. renderer-smoke and production-mode training-player) passes with it.first-run.jsonin a profile dir is inert on older builds.Review round (swarm-pr-review pr116-review-1)
A structured swarm-pr-review pass (base lanes + all 11 micro risk-families) validated four findings, ALL FIXED in 5771e4c with regression tests: PRR-001 (MEDIUM, manifest-path containment — traversal now a named failure, unit-pinned), PRR-002 (single manifest verify per status call), PRR-003 (wizard Esc-to-dismiss + Tab focus trap, component-tested), PRR-004 (type-honest bridge stubs). Closure ledger: 4/4 FIXED, 0 rejected, 0 deferred.
Waivers (or none)
No waivers of the Full-Resolution Contract. One disclosed gate limitation requiring user ratification rather than a waiver: subagent dispatch was unavailable for this entire session (harness-side "No reasoning level selected" error on 9 dispatch attempts across 4 agent types, including two minimal OK-probes), so the plan-critic, implementation-review, and final-critic gates ran in the orchestrating context under the issue-tracer's documented fallback rule instead of pinned cross-model independent contexts. Every gate artifact (06-critic-review.md, 08b-implementation-review.md, 09-final-critic.md) records this disclosure verbatim, and all mechanical gate work (frozen-check replays, checkpoint blob audits, typechecks, freshness proofs) was executed as fresh commands in a pristine detached worktree whose tree-id equals the commit tree. If the user prefers, a post-hoc cross-model re-review can be run before merge.
Merge status
AWAITING_USER_APPROVAL