feat(adopt): values confirm-or-override flow (#104) by breferrari · Pull Request #130 · breferrari/shardmind

breferrari · 2026-05-31T12:25:37Z

Closes #104.

Summary

shardmind adopt ran the full step-by-step InstallWizard (header → one prompt per value → module review → confirm) before it could classify the user's vault — the planner renders the shard's .njk against those values, so values must come first. But adopt's user already has a populated vault; being interrogated value-by-value reads as a fresh-install flow shoehorned onto a retrofit.

Adopt now opens on a new AdoptValuesGate confirm page that surfaces the exact values that will drive classification — resolved defaults, computed defaults, and any --values prefill, each labelled with its provenance (default / computed / from --values) — plus the module default, with Use these values / Override individually / Cancel. "Override individually" drops into the existing InstallWizard verbatim (per-value + module editing). Required values with no default and no prefill open straight into the wizard. The --yes / --values non-interactive paths never reach the gate and are byte-identical.

Design notes

Reuse over fork. The gate reuses mergePrefill / missingValueKeys / resolveComputedDefaults / defaultModuleSelections / isComputedDefault / formatValue; override delegates to InstallWizard. No value-logic was duplicated.
Use ≡ --yes parity. "Use these values" feeds the machine exactly what runNonInteractive (the --yes path) does: resolveComputedDefaults(mergePrefill(prefill)) + all-default selections. Both re-validate identically in runPlanning. Asserted by a test.
Machine unchanged except a doc comment — wizard phase still means "collect values"; onWizardComplete/Cancel/Error are unchanged.

Quality gate

npm run typecheck — clean.
npm test — 1092 passed | 30 skipped (Layer 2 PTY skipped on Windows per E2E: bridge SIGINT delivery reliably on GH Actions Windows runner #57). Was 1090 pre-PR.
npm run build — clean (dist/cli.js + runtime).
New behavior has new tests — tests/component/AdoptValuesGate.test.tsx (9), Layer 1 adopt scenarios 19/20/21 re-driven + new 25/26, Layer 2 PTY scenario 20.
Adversarial cases enumerated + covered (see audit below).
Copilot review requested + addressed (pending — will reply per comment).
Invariant 1 E2E still green (tests/e2e/cli.test.ts, obsidian-mind-contract.test.ts).
Issue acceptance criteria checked (below).
ROADMAP checkbox updated in this PR.

#104 acceptance criteria

Adopt's interactive flow is "confirm + maybe override" rather than "answer step-by-step".
Values used for classification are surfaced before classification runs (no hidden defaults) — scenario 26 asserts this.
--yes continues to work (auto-confirm path unchanged).
No regression in AdoptDiffView.test.tsx, AdoptSummary.test.tsx, or the contract acceptance suite.

Invariant 1 proxy

Adopt's --yes --values path is untouched (the gate is interactive-only), so the byte-equivalence suites (obsidian-mind-contract.test.ts, cli.test.ts adopt cases) exercise the unchanged classification/executor path and stay green. No engine-level behavior changed.

Harden Audit

Reference bar: #101 (multiselect value type) — component + Layer-1 flow + Layer-2 PTY + adversarial + CHANGELOG/docs.

Rounds

Round 1 — /simplify (reuse + simplification + efficiency + altitude): source clean; applied 2 in-scope test-readability fixes (gateProps() helper, comment trim). Skipped (out of scope, noted): move formatValue to a shared format.ts, add hideFileCounts to InstallWizard.
Round 2 — /harden (adversarial + correctness + docs): 2 real correctness issues + 1 stale doc.
Round 3 — re-audit of the fix: dry.

Real issues found + fixed

Stale-closure / pre-resolution submit race — AdoptValuesGate.tsx — computed defaults resolved in a mount effect left a window where a fast ENTER submitted unresolved values (machine validator would reject) and the setResolved re-render re-rendered the live Select. Fixed: resolve synchronously in useMemo; errors surface via a separate effect (off the render path).
Select double-fire — AdoptValuesGate.tsx — Select can re-fire onChange on Ink re-focus; a double-fire would advance the adopt machine twice. Fixed with a firedRef guard, matching the existing CollisionReview / DiffView defense.
Stale doc — docs/SHARD-LAYOUT.md §Adopt "Reuses: … values wizard" → AdoptValuesGate gate + InstallWizard-on-override.

Verified clean (no action)

Provenance precedence (prefill > computed > default), prototype-pollution (hasOwnProperty.call), hasMissingRequired predicate (fed the merged map), empty-schema.values, override→Cancel semantics, computed-coercion error → onError, module boundaries, ESM .js imports, zero any/@ts-ignore.

Tests added

Component (9): provenance labels, Use ≡ --yes parity, no-submit-before-ENTER (race guard), double-ENTER-fires-once (firedRef), Override→wizard, Cancel, computed-resolve, computed-throw→onError, required-no-default→override, zero-values shard.
Flow: Layer 1 scenarios 25 (Override→wizard→adopt) + 26 (values surfaced); 19/20/21 re-driven through the gate; Layer 2 PTY scenario 20 drives the gate.
Tests-before → after: 1090 → 1092 (net; several scenarios rewritten in place).

Deferrals

Value-inference prefill (the issue's "optional second pass" — regex values out of existing vault content) — not implemented; genuinely separate fuzzy feature with its own design questions. Can file as a follow-up if wanted.
formatValue → shared util / InstallWizard hideFileCounts prop — cosmetic altitude follow-ups.

Adopt collects values before classification (the planner renders the shard's .njk against them), but its user already has a populated vault — the full step-by-step InstallWizard reads as a fresh-install flow shoehorned onto a retrofit. AdoptValuesGate opens on a single page that surfaces the exact values that will drive classification (resolved defaults, computed defaults, --values prefill) each labelled with provenance, plus the module default, and offers Use these values / Override individually / Cancel. "Override" reuses InstallWizard verbatim rather than forking its value-iteration / computed-preview / module-review logic. Required values with no default and no prefill open straight into the wizard (no confident set to confirm) — the predicate feeds missingValueKeys the *merged* map, exactly as the --yes path does, so all-defaults shards land on the confirm page. formatValue is exported from InstallWizard for reuse. The --yes/--values non-interactive paths never reach this component, so they are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Render AdoptValuesGate for the wizard phase instead of InstallWizard. Adopt's user already has a populated vault, so a single confirm page ("here are the values that will drive classification: Use / Override / Cancel") beats a fresh-install-style per-value interview. "Override individually" still drops into the full InstallWizard. The --yes/--values non-interactive paths never reached the wizard component, so they are unchanged. Test sweep (folded in to keep the commit self-consistent — the wire-up is what changes the interactive flow): - Layer 1 (adopt-flow): replace driveAdoptWizard with driveAdoptConfirm; scenarios 19/20/21 drive the gate; add 25 (Override → wizard → adopt) and 26 (values surfaced before classification). - Layer 2 (adopt-diff PTY, macOS/Linux): drive the confirm gate instead of driveMinimalWizard before the diff loop. The shared driveMinimalWizard helper is untouched — install Layer 1/2 still use it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- ARCHITECTURE §10.5: rewrite adopt step 2 (values gate, not full wizard); refresh the --yes/--values/--dry-run flag descriptions and the implementation-modules line with AdoptValuesGate. - IMPLEMENTATION §3.5: adopt data-flow node now shows the confirm / override-into-wizard shape. - CLAUDE.md: add AdoptValuesGate.tsx to the component source tree. - CHANGELOG: [Unreleased] Changed entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

/simplify findings (reuse/efficiency/altitude agents found the source clean; these are the two in-scope test-readability fixes): - AdoptValuesGate.test.tsx: add a gateProps() default-props helper so each test spells out only the prop it asserts on, not all six. - adopt-flow scenario 25: tighten two narrative comments to the decision- critical line. No behavior change. Skipped (noted in PR): moving formatValue to a shared format util and a hideFileCounts prop on InstallWizard — out-of-scope follow-ups, not blockers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…#104) /harden round 1 (adversarial + correctness agents, confirmed against repo precedent CollisionReview/DiffView): - Resolve computed defaults synchronously in useMemo instead of a mount effect. The effect-based setResolved left a window where a fast ENTER on "Use these values" submitted pre-resolution values (computed keys absent), which the machine's validator would then reject; the setResolved re-render also re-rendered the live Select. The memo resolves before first paint; errors still surface via an effect to keep the onError parent-setState off the render path. - Guard the Select onChange with firedRef — Select can re-fire on Ink re-focus, and a double-fire would advance the adopt machine twice. Same defense CollisionReview and DiffView already carry. Tests: parity assertion (Use ≡ resolveComputedDefaults(mergePrefill) + default selections, the --yes contract); "no submit before ENTER" for a computed-default shard (race guard); double-ENTER fires onComplete once. Docs: SHARD-LAYOUT.md §Adopt — "values wizard" → AdoptValuesGate gate + InstallWizard-on-override (stale reuse reference). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR updates the interactive shardmind adopt UX to a confirm-or-override values flow, so users adopting an existing vault can review the exact value set used for classification before the planner runs.

Changes:

Add a new AdoptValuesGate screen that lists resolved values + provenance and offers Use / Override individually / Cancel.
Rewire adopt’s wizard phase to render AdoptValuesGate (delegating to InstallWizard only when overriding).
Update/extend component + flow + PTY tests and refresh docs/changelog/roadmap to reflect the new flow.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/e2e/tui/adopt-diff.test.ts	Updates PTY scenario to drive the new values gate before diff iteration.
tests/component/flows/adopt-flow.test.tsx	Re-drives Layer 1 adopt scenarios through the new confirm gate; adds new gate-specific scenarios.
tests/component/AdoptValuesGate.test.tsx	New component test suite covering provenance, parity, override/cancel, and error paths.
source/components/InstallWizard.tsx	Exports `formatValue` for reuse by the new gate.
source/components/AdoptValuesGate.tsx	Introduces the new confirm-or-override gate component.
source/commands/hooks/use-adopt-machine.ts	Updates doc comment to describe the new wizard-phase UI surface.
source/commands/adopt.tsx	Switches adopt wizard rendering from `InstallWizard` to `AdoptValuesGate`.
ROADMAP.md	Marks #104 as completed.
docs/SHARD-LAYOUT.md	Updates adopt “reuses” description to mention the values gate + wizard override.
docs/IMPLEMENTATION.md	Updates adopt flow diagram node to `AdoptValuesGate` + override path.
docs/ARCHITECTURE.md	Updates adopt phase ordering + flags to reflect the values gate.
CLAUDE.md	Adds `AdoptValuesGate.tsx` to the component tree list.
CHANGELOG.md	Adds an entry describing the new adopt confirm-or-override behavior and tests/docs updates.

breferrari · 2026-05-31T12:45:34Z

+  // Required values with no default and not supplied via `--values`: there
+  // is no confident "use these" set to confirm, so fall straight through to
+  // the per-value wizard (adopt's pre-#104 behaviour for this shard shape).
+  // `missingValueKeys` is fed the *merged* map (literal defaults filled),
+  // exactly as the `--yes` path does (`use-adopt-machine.ts::runNonInteractive`)
+  // — feeding it the raw prefill would flag every default-bearing key as
+  // "missing" and wrongly force override for the common all-defaults shard.
+  const hasMissingRequired = useMemo(
+    () => missingValueKeys(schema, merged).length > 0,
+    [schema, merged],
+  );


Fixed in 8d878d0. Replaced the missingValueKeys(merged) check with a dedicated predicate: a value now blocks the confirm page only if it is required === true, has no default (literal or computed), and isn't in --values. Optional unset values stay on the confirm page (they render as (unset) and "Use these values" leaves them unset, which the validator accepts). Added a regression test (optional value with no default → stays on the confirm page).

breferrari · 2026-05-31T12:45:43Z

+          valueKeys.map((key) => (
+            <Text key={key}>
+              <Text>  {key}: </Text>
+              <Text color="cyan">{formatValue(resolved[key])}</Text>
+              <Text dimColor>  ({provenanceOf(key, schema, prefillValues)})</Text>
+            </Text>
+          ))


Fixed in 8d878d0. provenanceOf now returns null for a value with no default and no --values prefill, and the JSX omits the label in that case — so an unset value renders as (unset) with no trailing (default). Covered by the new optional-no-default test (expect(frame).not.toMatch(/nickname:[^\n]*\(default\)/)).

breferrari · 2026-05-31T12:45:54Z

+        onChange={(v) => {
+          if (firedRef.current) return;
+          firedRef.current = true;
+          if (v === 'use') {


Fixed in 8d878d0. When resolveError is set the gate now early-returns a non-interactive Preparing values… placeholder (no Select) instead of the confirm page, so there is no submittable control in the window before the error effect drives the machine to its error phase. The existing computed-throw test (computed default that fails to evaluate → onError) still asserts onError fires with no crash.

…error guard (#104) Three Copilot review comments on AdoptValuesGate, all valid: 1. The confirm/override decision used `missingValueKeys(merged)`, which (by design, for the install wizard's prompt list) returns optional unset keys too — so a shard with any optional no-default value wrongly forced the wizard. Replace with a dedicated predicate: a value blocks the confirm page only if it is *required*, has no default (literal or computed), and isn't in --values. Optional unset values confirm fine (validator accepts). 2. Provenance label was always printed, so a no-default/no-prefill value rendered as "(unset) (default)" — a lie. provenanceOf now returns null in that case and the label is omitted. 3. On a computed-default resolution failure, the confirm Select still rendered until the error effect fired, leaving a window to submit the unresolved set. Render a non-interactive "Preparing values…" placeholder when resolveError is set, so there is no submittable Select in that window. Test: optional-no-default value stays on the confirm page with no misleading provenance label. Existing required-no-default → override and computed-throw → onError cases still hold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Scenario 24 (the #121 version-mismatch refusal) waited on one regex then re-read r.lastFrame() for a different regex, which races the 100 ms exit() that clears the testing-library buffer — the second read can return '\n'. A slow windows-latest/node-22 CI cell (718s suite) surfaced it on this PR. Capture the matched frame from waitFor and assert both the message and the SHARDMIND_VERSION_MISMATCH code on it — the same capture pattern scenarios 19 and 22 already use. The error phase renders message + code in one frame. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

breferrari and others added 6 commits May 31, 2026 14:00

docs(roadmap): check #104 — adopt confirm-or-override values flow

8f4867b

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings May 31, 2026 12:25

Copilot started reviewing on behalf of breferrari May 31, 2026 12:25 View session

Copilot AI reviewed May 31, 2026

View reviewed changes

breferrari and others added 2 commits May 31, 2026 14:45

breferrari merged commit aadee46 into main May 31, 2026
6 checks passed

breferrari deleted the feat/104-adopt-confirm-override branch May 31, 2026 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(adopt): values confirm-or-override flow (#104)#130

feat(adopt): values confirm-or-override flow (#104)#130
breferrari merged 8 commits into
mainfrom
feat/104-adopt-confirm-override

breferrari commented May 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

breferrari May 31, 2026

Uh oh!

breferrari May 31, 2026

Uh oh!

breferrari May 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

breferrari commented May 31, 2026

Summary

Design notes

Quality gate

#104 acceptance criteria

Invariant 1 proxy

Harden Audit

Rounds

Real issues found + fixed

Verified clean (no action)

Tests added

Deferrals

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

breferrari May 31, 2026

Choose a reason for hiding this comment

Uh oh!

breferrari May 31, 2026

Choose a reason for hiding this comment

Uh oh!

breferrari May 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants