Setup offers the first sync, and two wizard screens get shorter (LLP 0200)

[bgmcmullen]

Two wizard changes, smallest first.

Trim two screens

Overwrite confirm ran 44 words for a y/N. It still says the two things LLP 0183 #say-so requires (the file is rewritten from the picks; what carries over) in 27:

/Users/you/.hyp/hypaware-config.json will be rewritten from your picks. Retention,
exports, hand edits, and plugins the picker doesn't manage are carried over;
a backup is kept. Continue? [y/N]:

The finale's client-asset step printed one line per copy — a dozen destination paths on the last screen of setup. Now:

installed 7 skills and 1 agent for claude
installed 5 skills for codex

Counted per client, not summed: six skills copied to two clients is twelve copies and neither client got twelve. hyp skills install keeps the per-copy output, where the copies are the subject; the paths stay in the run summary and the skills.install span.

Setup offers the first sync (LLP 0200)

LLP 0101's amendment already conceded the case — the hold "blocks the demonstration that the product works at all, and the person running it has no way to say I have seen enough" — and minted hyp sync for it. It stopped one step short: the wizard names the verb in the fifth line of a six-line narration, then hands the terminal to a client. So an enrolled attended run now asks:

Send your recorded history to the server now, or wait?
> Wait until Aug 8, 2026, 11:59 PM PDT
Nothing leaves this machine before then
Send now
Runs `hyp sync`: it lists every destination and asks before sending

Waiting leads and is the default: a mis-keyed wait costs hours of latency, a mis-keyed send cannot be walked back.

The question is an offer, not the consent. "Send now" runs the real hyp sync, which prints the plan, escalates its warning because the window is open, and asks its own y/N — only that y clears the marker. LLP 0100 R2 and LLP 0101 #no-release hold verbatim; there is still exactly one release path in the codebase.

It spawns a child, and that is correctness rather than convenience.hyp init boots all-available, which withholds @hypaware/central (a CLI boot must not acquire a server identity), so this process has no central sink handle — an in-process sync would render a plan missing the very destination the release unblocks, the misleading artifact R2 exists to prevent.

Whether it sent is read back from the hold marker, never inferred from the exit code: hyp sync exits 0 both when it releases and when the user reads the destination list and says no. Any path that didn't send restates the deadline.

Position: narration → offer → first ask. The narration stays the last thing HypAware says about privacy (LLP 0135 #privacy); the first ask stays last on screen because it may take the terminal for good (LLP 0198). Attended-only, TTY-only, and it can never fail a finished install.

Not offered on the aborted-but-enrolled path: an abort means "get me out", not "ask me differently".

Docs

New llp/0200-setup-offers-the-first-sync.decision.md (Draft), with Extended-by: forward-refs on LLP 0101 and LLP 0198.

Testing

• 7 unit tests for the new step (default is wait; declining spawns nothing; "send now" spawns hyp sync with inherited stdio; a 0-exit child that didn't release reports not-sent; spawn failure and cancellation both degrade to the wait).
• 2 orchestrator tests (offer sits between narration and first ask; no hold → never asked) and 1 for the per-client asset counts.
• npm test: 3757/3758. The one failure (resolve emits a hashed usage_policy.fold_tightened…) also fails on a stashed tree — pre-existing, unrelated. Same for the client_attach_idempotent smoke.
• npm run typecheck clean; walkthrough_picker_to_first_query smoke green.

🤖 Generated with Claude Code

[philcunliffe]

neutral review - 6449d54 - findings (1 fixed, pushed)

Reviewed the two commits (54c1717 wizard trims, 6449d54 the sync offer) against f0a793f. One correctness finding, fixed and pushed as d168b86. Everything else below is either sound or deliberately left for you.

Finding 1 (High, fixed): an unreadable hold re-read claimed the sync happened

src/core/cli/wizard/sync_now.js:208-216 (at the reviewed head; the return null is line 214).

readHold's own docstring, three lines above the bug, states the rule, and LLP 0200 #read-back settles it: "An unreadable re-read is treated as 'still held', because claiming a sync happened is the one wrong answer that cannot be corrected later." The implementation did the opposite. null is the caller's word for the marker is gone, it sent:

conststillHeld=awaitreadHold(opts)span.setAttribute('released',stillHeld===null)if(stillHeld!==null){ ...notsent... }return{asked: true,released: true}

so catch { return null } reported released: true, logged wizard.finish sync_now: 'released', and skipped writeStillHeld. A run whose state dir would not resolve ended with setup's last word on the subject being a release nobody observed, and with no restatement of the deadline that still applied. That is precisely the false claim this screen exists to be honest about.

The readDeadline seam was wrong in a second, separate way: it sat outside the try, so a throw from it escaped to the outer handler and returned { asked: false, reason: 'error' }after the child had already run - asked: false for a question that was asked, and again no restatement, so the run ended on silence.

Both paths now return the deadline the step started from, which reads as "still held" and prints the restatement.

Worth noting why the tests did not catch this: all 7 pass readDeadline, so the production readHold body has no coverage at all. The added regression test drives the seam to throw, which exercises both defects at once. Proven to fail without the fix: reverting only sync_now.js and re-running the file gives not ok 6 - a re-read that throws is reported as not sent, not as a release (# pass 7 / # fail 1); with the fix, 8/8.

Not fixed, deliberately

• readFirstSyncDeadline collapses "unreadable" into "absent" (Low).src/core/usage-policy/first_sync_hold.js:160-185 returns null for a genuinely missing marker and for an EACCES/EIO read or a malformed JSON parse. So the narrow case LLP 0200 #read-back names - the marker is there but cannot be read - still reads as a release, and my fix cannot reach it from the caller. Fixing it means changing the contract of a helper the LLP 0101 narration path also depends on, which is a design change owed a new LLP rather than something a review should slip in. Flagging it for a follow-up; the residual risk is small (the release path unlinks the marker rather than rewriting it, so a torn read is unlikely).
• LLP number 0200 is claimed by three open PRs (maintainer sequencing).Setup offers the first sync, and two wizard screens get shorter (LLP 0200) #677 (this one), A picker row can be hidden from the menu without ceasing to be a source (LLP 0200) #674, and New folders sync by default; one gate accepts the lot (LLP 0200/0201) #675 all add a different llp/0200-*.md. origin/master has nothing above 0199, so duplicate-numbers passes for each in isolation and will start failing for the second and third to merge. Renumbering is mechanical and allowed (LLP 0156), but which doc keeps 0200 is your call, not mine to guess.

Checked and sound

• LLP discipline. LLP 0200 is Draft, so it is the right place for this design. The only edits to Accepted docs are the Extended-by: forward-refs on LLP 0101 and 0198 - explicitly permitted editorial changes, and no accepted text is contradicted. Every anchor referenced from code resolves: 0200#offer, #child-process, #read-back, plus 0100#requirements, 0101#no-release, 0135#privacy, 0198#first-ask, 0190#abort-narration.
• The no-second-release-path claim holds. "Send now" spawns bin/hypaware.js sync and clears nothing itself; the marker is only cleared by the child's own y/N. LLP 0100 R2 and LLP 0101 #no-release are intact - I looked for a shortcut and there isn't one.
• The child-process argument is correct, not a rationalization.hyp init boots all-available, which withholds @hypaware/central, so an in-process commands.run('sync') really would render a plan missing the destination the release unblocks. The ../../../../bin/hypaware.js URL resolves to the repo root correctly.
• Gating matches the runWizardFirstAsk precedent exactly (attended, non-cancelled, non-dry-run), plus a real TTY check on both ends and a live hold. Ordering is narration to offer to first ask, as LLP 0200 requires, and nothing needing the terminal follows the first ask.
• Wait-first polarity is the defensible one and is tested.
• clientAssetCountLines is safe: ClientAssetKind is exactly 'skill' | 'agent', so the else branch cannot mis-bucket. stderr is still threaded, so the warnings that matter survive dropping stdout; only the per-copy lines go, and hyp skills install keeps them. The per-client-not-summed reasoning is right.
• Overwrite confirm still satisfies LLP 0183 #say-so in its shorter form: it states the file is rewritten from the picks and names what carries over.
• Conventions: no semicolon-terminated statements, no U+2014 anywhere in the diff, no smart quotes, no @typedef, no inline import('...'), type-import specifiers root-anchored. The { settled = true; resolve(r) } separator matches existing house style (src/core/dep_graph.js:80), so not raised.

Checks

npm test 3755 pass / 0 fail / 6 skipped (3761 tests, +1 from the regression test); npm run typecheck clean. The pre-existing usage_policy.fold_tightened failure the description mentions did not reproduce here. GitHub checks on 6449d54 were all green.

Fix verified by reading the committed remote blob at origin/wizard-offers-first-sync (d168b86), not by inference from a green suite.

Not merging or approving - this is a contributor PR and terminates in your decision.

[philcunliffe]

neutral review (round 2, final) - d168b86 - clean on correctness, one coverage gap closed

Round 2 caps the ladder, so this record is written to be disposed of by a human without re-deriving anything.

I reviewed the current head d168b86 (which contains round 1's own fix) with the specific brief of distrusting that fix. It holds. I found no new correctness defect. I did find that the code path the fix repaired has zero automated coverage, verified the gap empirically, and closed it. Pushed as 6827735.

Round 1's fix, re-checked

readHold now reads:

asyncfunctionreadHold(opts){try{if(opts.readDeadline)returnawaitopts.readDeadline()conststateDir=readObservabilityEnv(opts.env).stateDirreturnawaitreadFirstSyncDeadline({ stateDir })}catch{returnopts.deadline}}

• Does widening the try change any other throw site? No. The only statement pulled inside is the readDeadline seam, which production never passes (index.js supplies deadline/stdout/stderr/env/interactive/stdin and nothing else). Its previous escape to the outer handler returned asked: falseafter the child had run, which was wrong; nothing else routed through it.
• Is null still overloaded further along? The remaining overload is readFirstSyncDeadline's own fail-open contract, which is round 1's deferred Low and is restated below. Inside sync_now.js the caller's stillHeld === null now has exactly one meaning on every reachable path.
• Adjacent paths with the same shape? I walked all of them: the declined early return, the cancelled-prompt branch, spawn-failed, runSyncChild's error/close/throw seams, narratePrivacyIfTeamPath's null, and clientAssetCountLines. None turns an error into a success-shaped value. Two notes on the near-misses, both benign:
  • materializeClientAssetscontinues past a failed copy without pushing, so the new "installed N skills for X" counts cannot include a copy that did not land. The counts are honest.
  • narratePrivacyIfTeamPath returning null for an unreadable state dir now also suppresses the offer. That errs toward silence, never toward a claimed release, and the catch predates this PR.

Finding 1 (Medium, fixed): the read-back's production body had never been executed by a test

src/core/cli/wizard/sync_now.js:214-222, covered from test/core/cli/wizard/sync_now.test.js.

All eight tests on d168b86 - including round 1's own regression test - pass readDeadline, which short-circuits the first line of readHold. So the two lines that actually decide the release claim in production (resolve the state dir from the environment, read the marker there) had no coverage at all. That is the one place in this feature where a wrong answer cannot be walked back, and the absence of coverage is precisely what let the original catch { return null } sit there unnoticed.

This is not theoretical. I proved the blind spot by mutating the production line to a plausible drift - dropping the threaded env, readObservabilityEnv(opts.env) to readObservabilityEnv() - which makes the re-read look in the caller's real home, find no marker, and report released: true for every run:

ok 1..7 (all pre-existing tests, including "a re-read that throws is reported as not sent")
not ok 8 - the read-back resolves a real hold marker from the environment: left in place
# pass 9 / # fail 1

Every pre-existing test stayed green against a wizard that falsely claims it sent your history. Only the added tests see it.

The fix is test-only: two cases drive runWizardSyncNow end to end with no seam, against a real first-sync-hold.json under a temp HYP_HOME, with a spawn stub standing in for hyp sync that either clears the marker (a release) or leaves it (a declined plan). Both assert the result and the presence/absence of the restatement line. Reverted mutation, 10/10 green.

Before landing it I also confirmed by direct execution that the production body is correct today: with a real marker present it returns { asked: true, released: false, reason: 'sync-declined' } and prints Nothing was sent. Your history stays on this machine until ...; with the marker cleared it returns { asked: true, released: true } and prints nothing. So this commit locks in behaviour that already works, rather than repairing anything.

Not fixed, deliberately

• A failed re-read is reported as sync-declined (Low).src/core/cli/wizard/sync_now.js:102-107. Round 1's fix makes an unreadable re-read return a deadline, which is the correct user-facing answer (still held, deadline restated). But the caller then labels it sync-declined, and LLP 0200 #consequences defines that value as the user reading the destination list and backing out - it names the declined/sync-declined split as the measurement that says whether the window is sized right. The span also keeps status: 'ok' with no error_kind, against the Log-Driven Development rule in CLAUDE.md. I left it: the path is very close to unreachable in production (see the next item - readFirstSyncDeadline is documented never to throw, and readObservabilityEnv is pure path.join over env strings), and distinguishing it means widening the public WizardSyncNowResult union and amending LLP 0200's enumerated sync_now values. That belongs with the follow-up below, not in a review round.
• readFirstSyncDeadline collapses "unreadable" into "absent" (Low, round 1's deferral, unchanged).src/core/usage-policy/first_sync_hold.js:159-184. Its fail-open polarity is deliberate and documented (LLP 0093 #bounded), so the narrow case LLP 0200 #read-back names still reads as a release at that layer, and sync_now.js cannot reach it from the caller. Changing that contract touches the LLP 0101 narration path and is owed a new LLP. Confirmed still true at d168b86; not touched.
• readHold's catch { return opts.deadline } is type-safe only by a distant invariant (Low, latent).opts.deadline is number | null, so the catch can type-check while returning null - the exact High defect round 1 fixed. It is safe today only because the function is unreachable unless the typeof opts.deadline !== 'number' early return at line 57 has already fired. tsc will not catch it if that guard ever moves. Noted rather than churned, since narrowing the helper's signature is a refactor with no present bug behind it.
• LLP number 0200 is claimed by three open PRs (maintainer sequencing).Setup offers the first sync, and two wizard screens get shorter (LLP 0200) #677 (this one), New folders sync by default; one gate accepts the lot (LLP 0200/0201) #675 (0200/0201) and A picker row can be hidden from the menu without ceasing to be a source (LLP 0200) #674 each add a different llp/0200-*.md; origin/master still tops out at 0199, so duplicate-numbers passes for each in isolation and starts failing for the second and third to merge. Renumbering is mechanical and permitted (LLP 0156), but which doc keeps 0200 is a maintainer call.
• Ctrl-C at the child's y/N kills setup (observation, not a finding).stdio: 'inherit' puts the child in the caller's foreground process group, and no SIGINT handler exists on the CLI path, so ctrl-C at the hyp sync prompt terminates hyp init too - no restatement line, no first ask, exit 130. This is inherent to the stdio: 'inherit' pattern LLP 0198 #real-launch already established and which LLP 0200 #child-process explicitly inherits; typing n (the ordinary way to decline) behaves correctly. Raised only so it is on the record.

Checked and sound

• The child-process argument, re-verified.bin/hypaware.js resolves correctly from ../../../../ at src/core/cli/wizard/, is spawned unscoped (['sync']) so LLP 0100 R2 permits the release, and clears nothing itself. There is still exactly one release path in the codebase.
• Gating.holdDeadline !== null && interactive && !cancelled && dryRun !== true, plus a real TTY check on both ends inside the step. ctx.stdin is process.stdin in production (dispatch.js:205) and the guided-wizard entry already gates on isTty(ctx.stdout), so the offer genuinely appears on a real attended run and genuinely does not on a piped one. Ordering is narration to offer to first ask, and nothing needing the terminal follows the ask.
• clientAssetCountLines.ClientAssetKind is exactly 'skill' | 'agent' so the else cannot mis-bucket; parts can never be empty because the map is built from the items; failed copies are excluded upstream; stderr warnings still survive; hyp attach and hyp skills install keep their per-copy lines (clients.js:989).
• Overwrite confirm still satisfies LLP 0183 #say-so in 27 words: it states the file is rewritten from the picks, names what carries over, and names the backup.
• LLP discipline. LLP 0200 is Draft. The only edits to Accepted docs are the Extended-by: forward-refs on 0101 and 0198, which are the permitted editorial change and contradict no accepted text. Every anchor referenced from code resolves: 0200#offer, #child-process, #read-back, #no-new-consent, plus 0100#requirements, 0101#no-release, 0135#privacy, 0198#first-ask.
• Conventions. No statement-terminating semicolons, no U+2014, no smart quotes, no @typedef, no inline import('...'), type-import specifiers root-anchored - checked over the full diff including my commit.

Checks

npm test3757 pass / 0 fail / 6 skipped (3763 tests, +2 from this round; baseline on d168b86 was 3755/0/6). npm run typecheck clean. The usage_policy.fold_tightened failure the PR description calls pre-existing did not reproduce in either run.

The pushed commit was verified by reading the committed remote blob at origin/wizard-offers-first-sync (6827735), not inferred from a green suite.

No approval, no verdict label, no merge - this is a contributor PR and terminates in a maintainer decision.

[philcunliffe]

Review capacity for this PR is spent (2 rounds, the ladder's cap), and the current head 6827735 carries neutral's own round-2 work, which has not itself been reviewed. Handing the remainder back rather than deferring it - this is your PR, so the residual calls are yours.

Round 1 (head 6449d54) found one High and fixed it.src/core/cli/wizard/sync_now.jsreadHold had a catch { return null }, where null is the caller's word for "marker gone, it sent". An unreadable re-read therefore ended setup reporting the sync as released, logging it released, and skipping the deadline restatement - a false claim about the one thing that screen exists to be honest about. The function's own docstring and LLP 0200 #read-back both settle the opposite. A second defect in the same function had the readDeadline seam outside the try, so a throw returned asked: false after the child had already run.

Round 2 (head d168b86) re-examined that fix and it holds. Checked three ways: widening the try pulls in only the test seam production never passes; null is no longer overloaded on any reachable path in the file; and every adjacent seam (declined, cancelled-prompt, spawn-failed, runSyncChild's three seams, narratePrivacyIfTeamPath, clientAssetCountLines) was walked for the same "error collapses into a success-shaped value" defect, with none found.

What round 2 did find is a coverage gap, and it is worth your attention. The production body of readHold (sync_now.js:214-222) had zero test coverage - all eight tests, including round 1's own regression test, inject the readDeadline seam that short-circuits it. Demonstrated by mutation: changing readObservabilityEnv(opts.env) to readObservabilityEnv() (a plausible drift) makes the wizard report released: true for every run - a false "your history is on its way" - and all eight pre-existing tests stayed green. Two seam-free tests now drive a real hold marker under a temp HYP_HOME; only they catch it. The production body is correct today, so this locks in working behaviour rather than repairing a live bug.

Suite at the current head: 3757 passing, 0 failing; typecheck clean.

What needs your decision:

1. A failed re-read is labelled sync-declined, which corrupts the measurement LLP 0200 #consequences names. Low and near-unreachable, but fixing it widens a public union and amends the LLP, so it is not a review-time edit.
2. first_sync_hold.js:160-185 collapses "unreadable marker" into "absent" - the exact case LLP 0200 #read-back names still reads as a release at that layer. Unreachable from the caller-side fix; changing that helper's contract touches the LLP 0101 narration path and is owed a new LLP.
3. readHold's catch { return opts.deadline } is type-safe only via a distant guard - Low, latent, worth a local assertion if you touch this again.
4. ctrl-C at the child's y/N kills setup - inherent to the LLP 0198 stdio: 'inherit' pattern. Observation only, no action proposed.
5. The current head is unreviewed.6827735 is neutral's own round-2 work.

Cross-PR conflict that no single PR's checks can see.origin/master stops at LLP 0199, and three in-flight PRs each add a differentllp/0200-*.decision.md:

• #6740200-hidden-picker-rows.decision.md
• #6750200-folder-ask-is-a-preference.decision.md
• #677 (this one) 0200-setup-offers-the-first-sync.decision.md

duplicate-numbers passes for each in isolation and will fail for the second and third to merge. Renumbering is mechanical under LLP 0156, but which doc keeps 0200 is your call, so neutral has not renumbered anyone's branch.

Both review records are in this thread with full file:line evidence. Nothing here is a known production defect - this is an exhausted-ladder handback.

Push to the branch or reply here and neutral will re-engage on its next tick.