Skip to content

LLP 0279: the wizard's question lanes record with the config, and an accepted default keeps a standing answer - #901

Draft
philcunliffe wants to merge 4 commits into
masterfrom
fix/issue-883
Draft

LLP 0279: the wizard's question lanes record with the config, and an accepted default keeps a standing answer#901
philcunliffe wants to merge 4 commits into
masterfrom
fix/issue-883

Conversation

@philcunliffe

@philcunliffe philcunliffe commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Two defects in how the hyp init wizard's question lanes persist state, both reported in #883, both asymmetries with a sibling lane that already gets it right.

1. deferWrite did not cover the lanes that write their own state

runInitWizard runs the pick lane with deferWrite: true so the config write lands after the question lanes (LLP 0190 #commit-point), on the stated grounds that "a cancel at the sync lane leaves the existing config untouched". The lanes themselves were not deferred: runWizardSyncScope wrote client-sync.json and runWizardFolderAsk wrote folder-ask.json the moment each was answered. Decline the overwrite confirm that follows (n at Continue? [Y/n], or a non-interactive commit without --force) and the run printed "keeping existing config at ..." and exited 1 with both policy stores already rewritten.

LLP 0190 carved out that exception in a single sentence: "a store entry for a source whose config never lands is inert and errs toward under-sync". It covers one direction of one lane, and three things fall outside it:

  • The store is an editor, not an append log. A re-run can check a source that was opted out before, which removes a local-only entry. That is the over-sync direction, and the "safe direction" argument does not reach it.
  • Absence is a signal. readClientSyncEntries returns null for a missing file and [] for an empty one, because absence is LLP 0188 #migration's marker. So even the "inert" write stamps the machine.
  • The new-folder lane was never considered. LLP 0200's lane landed after LLP 0190 and inherited the write-on-answer shape without the sentence that justified it.

Both lanes now take deferWrite and hand their write back as commit, which the orchestrator runs immediately after commitWizardPickedConfig succeeds and not at all when it does not. The statements stay with their lanes: they are the never-silent floor (LLP 0188), and on the express path LLP 0201 #narrate binds each one to its own indented block. A refusal therefore now names what else it dropped ("the sync and new-folder answers from this run were not recorded either"), listing only the lanes that actually asked. The held writes are re-assigned per pass and cleared at the top of every pick pass, so back navigation (LLP 0191) writes the last answer and a pass that never reaches the lanes carries nothing forward.

2. The express gate reset a standing ask preference

LLP 0201 #gate says accepting takes "every lane's stated default". The sync lane's auto-accept arm honors that (it returns the opt-outs already standing); the new-folder arm recorded DEFAULT_FOLDER_ASK_MODE. A user who set hyp policy folders ask, re-ran hyp init on the same enrolled machine, and accepted the express gate had the per-folder session-start question turned off - by the screen offering to accept the defaults, on a lane whose own asked screen deliberately uses default: before.

The arm now records before, which changes nothing on a first run (where before is the shipped default). The gate's one line of consequence follows the answer, since it is the one line the fast path is guaranteed to read: "new folders keep asking" on a machine set to ask, "new folders sync too" otherwise. The orchestrator reads the mode once, through the safe read, before the gate.

Design record

New decision LLP 0279, with Extended-by: forward-refs appended to LLP 0190 (whose #commit-point carve-out it withdraws) and LLP 0201.

Testing

Five new tests, each confirmed failing on master before the fix:

  • test/core/cli/wizard/index.test.js: a declined commit leaves client-sync.json unstamped and a standing ask untouched, and says so (failed on master: the store existed, the mode had flipped to sync); and a committed run persists both lanes right after the config lands.
  • test/core/cli/wizard/folder_ask.test.js: the express path keeps a standing ask; deferWrite states the answer and hands the write back; a deferred write that fails warns and reports the mode still in force.
  • test/core/cli/wizard/sync_scope.test.js: deferWrite states the split and hands the store write back.
  • test/core/cli/wizard/express.test.js: the accept row names the standing new-folder answer.

npm test: 4490 pass, 0 fail. npm run typecheck: clean. hyp smoke walkthrough_picker_to_first_query: ok.

Branch-disjoint from #874 (which touches the sync gate's locked-row filter and the pick-phase reconfigure classification); no overlapping hunks.

🤖 Generated with Claude Code

Fixes #883

test and others added 3 commits August 19, 2026 04:08
…accepted default keeps a standing answer (#883)

The sync lane wrote `client-sync.json` and the new-folder lane wrote
`folder-ask.json` the moment each was answered, while the config write
they belong to was deliberately deferred past them (LLP 0190
#commit-point). Decline the overwrite confirm that follows and the run
printed "keeping existing config at ..." and exited 1 with two policy
stores already rewritten.

LLP 0190 carved out that exception in one sentence, on the grounds that a
store entry for a source whose config never lands is inert and errs
toward under-sync. It covers one direction of one lane: the sync store is
an editor (a re-run can *remove* an opt-out, which over-syncs), its
absence is LLP 0188 #migration's own marker so even the empty write
stamps the machine, and the new-folder lane arrived later and inherited
the shape without the argument.

Both lanes now take `deferWrite` and hand their write back as `commit`,
which the orchestrator runs immediately after `commitWizardPickedConfig`
succeeds and not at all when it does not. The statements stay with their
lanes (LLP 0201 #narrate binds each to its own block), so a refusal now
names the answers it dropped alongside the config. The held writes are
re-assigned per pass, never accumulated, so back navigation writes only
the last answer.

Second defect, same pair of lanes: the express gate's folder arm recorded
DEFAULT_FOLDER_ASK_MODE rather than the standing mode, so a user with
`hyp policy folders ask` who re-ran `hyp init` and accepted the gate lost
the per-folder question. The sync lane's auto-accept arm already returns
the opt-outs already standing. The folder arm now records `before` - the
same value its asked screen puts on `default:` - and the gate's one line
of consequence follows it ("new folders keep asking" on a machine set to
`ask`).

Testing: `node --test test/core/cli/wizard/*.test.js` 268 pass; five new
tests, each failing on master first (the orchestrator-level abort test
stamped `client-sync.json` and flipped `ask` to `sync`; the express test
reset the standing answer). `npm test` 4490 pass / 0 fail;
`npm run typecheck` clean; `hyp smoke walkthrough_picker_to_first_query`
ok.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0268 is already claimed by fix/issue-838 (PR #847), which is older, so
this branch yields the number. 0279 is free across master and every
open branch.

Mechanical renumber only: no content change (LLP 0156).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…(review of #901)

Two follow-ups from review of the deferred-lane-write change:

- `sync_scope.js` handed back a `commit` that could throw. It now runs
  after the config is on disk, with the new-folder lane's held write, the
  configure phase, and the finale still owed, so a throw abandoned the run
  half-done with a stack trace. The deferred commit now warns and leaves
  the standing store alone, the same contract the new-folder lane's
  deferred write already had. The inline write, which still runs before
  anything is committed, keeps throwing.
- `index.js` cleared the held writes at the top of each pick pass but left
  `sourcesOptedOut` standing, so an abandoned pass's opt-outs were still
  reported in `wizard.finish` even though nothing was ever written.

LLP 0279 records the sync lane's deferred-failure contract alongside the
new-folder lane's.

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

Copy link
Copy Markdown
Contributor Author

neutral review round: 9a0541a0 -> findings (5 raised, 2 fixed)

Reviewed origin/master...9a0541a0 (12 files: the hyp init wizard's question lanes plus LLP 0279) in a detached worktree. Baseline on the reviewed head: npm test 4490 pass / 0 fail, npm run typecheck clean.

Verdict: the core change is correct. Deferring both lanes' policy writes to the config commit point genuinely fixes #883; the reset-per-pick-pass handling of the held closures is right across every back edge traced (continue atPick / atExpress / atFork all re-enter atPick, which clears them); and recording before instead of DEFAULT_FOLDER_ASK_MODE in the express arm is the right call. The findings below are behaviours the new tests do not cover.

Fixed in 7d5e00df

  • medium - src/core/cli/wizard/sync_scope.js:121 (pre-fix): the handed-back syncCommit() could throw after the config was already on disk. const commit = async () => { await writeClientSyncEntries(...) } had no error handling, and writeClientSyncEntries -> atomicWriteJson throws on a read-only state dir, ENOSPC, or a directory occupying client-sync.json (the exact failure the new folder-ask test simulates with fs.mkdir(folderAskPath(stateDir))). Before this PR that throw happened inside the lane, before commitWizardPickedConfig, so the run died with nothing written. After the PR it fires at index.js:674, leaving: config written, sync store not written, folderCommit never reached (the new-folder answer dropped without its warning), and runConfigurePhase + finale never run - no client attach, no daemon install. runInitWizard has no caller-side catch (src/core/commands/init.js:109 / :274 return result.exitCode directly), so the user sees a stack trace. It also contradicts LLP 0279's own "a run's answers are recorded together or not at all", and is asymmetric with the folder lane, whose deferred write already catches and warns.
    Fix: the deferred commit now catches, warns (could not record the sync answers (...); the previous sync scope stands - set it later with 'hyp policy client <name> sync|local-only'), and lets the run finish. The inline write (if (!opts.deferWrite) await write()) still throws, since it runs before anything has been committed. New regression test in test/core/cli/wizard/sync_scope.test.js; LLP 0279 #one-commit-point now records the sync lane's deferred-failure contract next to the folder lane's.

  • low - src/core/cli/wizard/index.js:448 (pre-fix): the per-pass reset cleared the held writes but not sourcesOptedOut, which is what the finish log reports. Reachable: enrolled run -> answer the sync lane (assigns sourcesOptedOut at index.js:577) -> escape at the new-folder lane -> escape back through sync -> pick -> express gate -> fork -> choose Local and confirm disconnect. The lanes are then skipped and nothing is written, but wizard.finish still logs sources_opted_out: N from the abandoned pass. Pre-PR the log was accurate because the write had actually happened; deferring it makes the log claim opt-outs that were never recorded anywhere. Telemetry-only, but it defeats CLAUDE.md's log-driven-development rule. Fix: sourcesOptedOut = [] joins the reset block. (folderAsk is not affected - the folder lane only assigns it on the path that exits the loop.)

Post-fix: npm test 4491 pass / 0 fail / 1 skip, npm run typecheck clean. No em dashes, no semicolons added.

Raised, not changed

  • low - src/core/cli/wizard/index.js:654: the "answers not recorded either" line also fires on the express path, where no lane prompted. autoAccept does not suppress the returned commit: promptSyncScopeSelection narrates and returns { optedOut: [...optedOutBefore] } (sync_scope.js:178) and folder_ask.js:88 returns recordAnswer(before, ...), so both syncCommit and folderCommit are set. Enrolled machine with an existing config -> hyp init -> accept the express gate -> decline the overwrite confirm prints "the sync and new-folder answers from this run were not recorded either" for two values that were the standing ones anyway. LLP 0279 says the list names "only the lanes that actually asked", and an express-accepted lane "rendered a statement rather than a question" by that same wording. Left alone because the express gate is itself an answer under LLP 0201 #gate and the message is not false; flagging it so a human can settle which reading LLP 0279 meant.

  • low - src/core/cli/wizard/folder_ask.js:166: under deferWrite the answer is announced before the write, and the lane's span reports status: 'ok'. The inline path deliberately suppresses the announcement when the write fails (if (written.skipped) return await finishSpan(written, opts) precedes announceAnswer); the deferred branch announces first and returns finishSpan({ mode, commit }) with no skipped. With folder-ask.json unwritable and the answer sync, stdout says "New folders will sync without asking." and stderr later says "it stays 'ask'". wizard.folder_ask.finish also records folder_ask: 'sync' for a write that never landed. Left alone: LLP 0279 #one-commit-point explicitly settles that the statement stays with the lane and only the write moves, and the run-level correction (folderAsk = await folderCommit()) is what the LLP's Consequences promise. Worth a follow-up only if the lane's own span is expected to be trustworthy in isolation.

  • low, non-code - the PR description still says "LLP 0268". The tip commit on the reviewed head renumbered the decision to llp/0279-lane-answers-commit-with-the-config.decision.md to clear a collision, so both in-body links to the new decision are dead and the "Design record" section names the wrong number. No stale 0268 reference survives anywhere in the tree (code, tests, or llp/) - it is the PR body only. Not edited here (this round touches nothing but its own commit and this comment); worth a one-line fix before merge so the PR record matches the doc.

…(review of #901)

Three findings from the review round on #901, all in the newly deferred
lane writes:

- The refusal message named both lanes on an express pass, where neither
  lane asked anything: it narrated the standing state and handed back a
  no-op write, so there was no answer to have dropped. The message now
  covers only a pass that actually asked.
- `sourcesOptedOut` still reported the answer rather than the write, so a
  deferred store write that failed left the finish log claiming an opt-out
  the export seam will never honour. The sync lane's `commit` now resolves
  to the scope in force, exactly as the new-folder lane's resolves to the
  mode in force, and the orchestrator reads it back.
- A pass that never reached the lanes (a back through the fork onto a solo
  local run) carried the previous pass's `folderAsk` into the finish log.
  It clears with the held writes it belongs to.

LLP 0279 updated in the same commit: the commit contract now says both
writes report what they left in force, and the consequences cover both
attributes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@philcunliffe philcunliffe changed the title LLP 0268: the wizard's question lanes record with the config, and an accepted default keeps a standing answer LLP 0279: the wizard's question lanes record with the config, and an accepted default keeps a standing answer Aug 19, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review round on 7d5e00d - verdict: findings (4 actionable, 3 fixed + verified, 1 left open)

Reviewed the full origin/master...7d5e00d diff (12 files) and read the post-change src/core/cli/wizard/{index,sync_scope,folder_ask,express}.js end to end, tracing the back-edge and commit-point control flow.

The core change is sound. Both question lanes hand their write back as commit, the orchestrator runs them right after commitWizardPickedConfig succeeds, the held writes are re-assigned and cleared at the top of atPick, and the express arm recording before instead of DEFAULT_FOLDER_ASK_MODE is the right fix for defect 2. Everything below is in the newly deferred write path.

Fixes pushed as 810ab3f4.


1. src/core/cli/wizard/index.js:658 - low - FIXED

The refusal's held list was derived from "did the lane hand back a commit", but under autoAccept (the express path) both lanes narrate rather than ask and still return a commit: runWizardSyncScope's auto-accept arm falls straight through to the commit construction (sync_scope.js:207), and runWizardFolderAsk's arm calls recordAnswer(before, ...) (folder_ask.js:90). So a user who answered one express gate and then declined the overwrite confirm was told hyp init: the sync and new-folder answers from this run were not recorded either - contradicting the comment directly above it ("Naming the lanes that actually asked, because a run where the sync lane only made a statement never had a sync answer to lose") and LLP 0279's own sentence. The message exists to name what the run threw away; naming something it never held sends the user to hyp policy to restore an answer they never gave.

Fixed: held is empty on an express pass. New test runInitWizard: a declined commit on the express path claims no dropped lane answers (test/core/cli/wizard/index.test.js), confirmed failing without the fix.

2. src/core/cli/wizard/index.js:837 - low/medium - FIXED

sources_opted_out on the wizard.finish log still reported the answer, not what was committed. LLP 0279 #consequences deliberately changed the sibling attribute (folder_ask comes from await folderCommit()) so "a failed write is visible in telemetry as the mode that stands", but sourcesOptedOut was left on the answer. That matters more here than for the folder lane, because a failing deferred store write is now a fail-open: the warning is emitted before runConfigurePhase and is buried under the configure output, the finale, the first-look block and the privacy narration, the run exits 0, and the daemon's withhold read finds no local-only entry - so the source the user explicitly opted out of starts forwarding. The finish log was the one signal that would let you detect it, and it was claiming N opt-outs were recorded when zero were.

Fixed: runWizardSyncScope's deferred commit now resolves to the opt-out set actually in force (the previously standing set on failure, mirroring the folder lane's contract), typed () => Promise<string[]> in types.d.ts, and the orchestrator reads it back into sourcesOptedOut. New test a failed deferred store write reports the sync scope still in force, not the answer (test/core/cli/wizard/sync_scope.test.js), confirmed failing without the fix. LLP 0279 updated in the same commit (commit contract + consequences).

3. src/core/cli/wizard/index.js:453 - low - FIXED

The new reset block cleared syncCommit, folderCommit and sourcesOptedOut at the top of atPick but not folderAsk. Team pathway, join, lanes answered (folderAsk = 'ask'), escape back through sync/pick/express to the fork, choose Local, confirm the disconnect - enrolled() is now false so the lane block is skipped, folderCommit is undefined so nothing is written, but folderAsk still held 'ask' and the finish log recorded folder_ask: 'ask' for a run that neither asked nor wrote it. Before this PR the value was truthful because the inline write had already happened.

Fixed: folderAsk = undefined joins the reset block. Not separately unit-tested (the attribute is only observable on the wizard.finish log and there is no log-capture seam in the wizard tests); verified by diff of the committed tree.

4. src/core/cli/wizard/sync_scope.js:129 - medium - LEFT OPEN (design call)

The deferred commit swallows a writeClientSyncEntries failure with a warning and the run continues to exit 0. Pre-PR the inline write failed closed: it threw before the config landed, so nothing was enabled. The new direction is the over-sync one that LLP 0279 #problem itself says the old carve-out did not cover. Fix 2 above makes the failure visible (truthful finish log, and the commit reports the scope that stands), but the user-facing outcome is still a successful-looking install on a machine syncing a source they opted out of.

Left open deliberately: 7d5e00d ("a failing deferred sync-store write must not abandon the run") settled this one round ago and LLP 0279 records the contract, so reversing it is a design decision, not a review fix. The question for a human: should a failed deferred policy write make hyp init exit non-zero (or restate itself in the finale), or is the mid-run warning plus the truthful finish log enough? Note that runInitWizard's exit code is load-bearing for the smoke battery, so an exit-code change is not free.


Non-blocking, also handled

  • The PR title and body said "LLP 0268" while the landed file is llp/0279-lane-answers-commit-with-the-config.decision.md (renumbered in 9a0541a0). All in-file cross-refs and @refs already said 0279; the PR text was the only stale copy. Title and body updated.
  • Checked the 0266-0278 gap the renumber left: 0279 is uniquely claimed by this branch across every ref in the repo, so the renumber did clear the collision. (0266 is separately double-claimed by two other branches - not this PR's problem.)

Checks

npm test: 4493 pass, 0 fail, 1 skipped. npm run typecheck: clean. No em dashes, no semicolons, @ref anchors (#one-commit-point, #standing-answer) both resolve in LLP 0279.

@philcunliffe philcunliffe added the neutral:stuck neutral attempted this but cannot complete it autonomously — needs a human label Aug 19, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor Author

neutral is stuck on this PR and needs a human decision

What neutral was doing: triage at the review-round cap on PR #901, head 810ab3f4. Two review rounds ran; of the seven findings raised across them, six are fixed and verified at this head (express-path refusal wording, truthful sources_opted_out and folder_ask on the finish log, the per-pass reset including folderAsk, the deferred sync commit catching instead of throwing mid-run, the stale "LLP 0268" PR text). One residual finding remains, and it is a production risk neutral cannot settle on its own, because resolving it either way is a design decision that LLP 0279 currently records in one direction.

Blocking finding: a failed deferred sync-store write fails open on a privacy opt-out

src/core/cli/wizard/sync_scope.js:136-146 (the deferred commit), run from src/core/cli/wizard/index.js:690.

Before this PR, the sync lane wrote client-sync.json inline, and a write failure threw before the config landed: the run died with nothing enabled (fail-closed). This PR defers the write to after commitWizardPickedConfig, and the deferred form deliberately catches, warns on stderr, and lets the run continue to exit 0 (settled in commit 7d5e00df and recorded in LLP 0279 #one-commit-point).

The consequence when that write fails (read-only or permission-skewed state dir, ENOSPC mid-run, a directory occupying client-sync.json): the config is committed, the configure phase attaches clients and installs the daemon, the run looks successful, and the local-only entry for a source the user explicitly opted out of is never recorded, so the export seam forwards that source's data to the fleet server. The warning is emitted before the configure phase and scrolls away under the configure output, finale, first-look, and privacy narration. Round 2's fixes made the failure visible (the commit resolves to the scope actually in force and the wizard.finish log reports it truthfully), but the user-facing outcome is unchanged: exit 0, and a machine syncing data its owner withheld. In a product whose release battery includes source_optout_export_withhold and local_only_export_withhold, a fail-open on exactly that control is a defect a human has to sign off, not a residual to defer.

Decision needed

How should hyp init behave when the deferred sync-store write fails after the config has committed? Options, none free:

  1. Exit non-zero after completing the run (warning stands, finish log truthful, but runInitWizard's exit code is load-bearing for the smoke battery, so this needs a look at every flow that asserts on it).
  2. Restate the failure in the finale (and/or as the run's last line), keeping exit 0, so it cannot scroll away unseen.
  3. Fail closed again: abort before the configure phase with a clean message (no attach, no daemon install), accepting that the config file is already on disk, or two-phase it (write the store first, roll it back on an overwrite refusal).
  4. Accept the current contract as shipped (warning + truthful telemetry is enough); LLP 0279 already records it, so this is a sign-off, not a code change.

Options 1-3 each want a small code change plus an LLP 0279 extension (Accepted docs change by extension, not edit).

Non-blocking residuals (for the whole picture)

  • src/core/cli/wizard/folder_ask.js:165-168: under deferWrite the lane announces the answer and its span records status: 'ok' with the answered mode before the write has run; a later failed commit is corrected at run level (the orchestrator reads folderCommit()'s result back into the finish log) but the lane-local span stays optimistic. Telemetry nicety only, fine as a follow-up.

To unstick: reply with a comment on this PR (or push to the branch); neutral monitors this thread and will re-engage with your guidance on its next tick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:stuck neutral attempted this but cannot complete it autonomously — needs a human

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wizard sync and folder lanes persist immediately despite deferWrite, and the express gate resets a standing folder-ask preference

1 participant