Combine collection and sync in remote setup

[platypii]

Remote setup currently asks what to collect, then asks again which selected sources to sync. Combine these into one "What do you want to collect and sync?" selection, with matching progress counts and Back navigation from folder handling.

Confirming a visible selected source enables both collection and sync, including clearing its existing client opt-out. Unselected and hidden source policies, locked fleet sources, directory policies, and first-sync holds retain their existing behavior. Unreadable policy stores remain intact and exports fail closed. LLP 0396 records the updated setup contract.

Validation: 355 affected tests pass, type checking passes, and the hermetic walkthrough_picker_to_first_query smoke passes. The full suite exposed an unrelated mcp-stdio-write-backstop JSON.stringify depth test failure, which also reproduces in isolation.

CPU and memory review: no concerns. Work is bounded by the existing picker and policy entry counts, with no new dependencies or background activity.

[philcunliffe]

Review round 1 — 07ed4f18 (findings; 7 fixed and pushed as 76e5bd54)

Verdict: approve with follow-ups. The mapping from the combined picker to
collection and sharing is correct on every path I could reach, the four
Extended-by: forward-refs are genuine extensions rather than contradictions,
and the express and interactive lanes agree about what was chosen. Seven
actionable defects were found and fixed on the branch; three remaining items
are design calls that belong to the author and the maintainer, not to me.

Reviewed in an isolated worktree at 07ed4f18. npm test 6506/6510 (the 3
test/core/hyparquet-floor-pin.test.js failures reproduce on origin/master
and are unrelated); npm run typecheck clean.

---

Findings fixed on the branch (76e5bd54)

1. src/core/cli/wizard/index.js:1284 — medium. The express gate could
promise "Record and sync everything" on a machine where nothing will export.

syncWithheldSafe kept its if (ids.length === 0) return false guard, but the
function's meaning changed underneath it. It used to answer "does the store
already withhold one of the named rows?", for which an empty non-locked row
list is a correct early false. It now answers only "is the store readable?" —
and ids is still the non-locked default rows, so on a fully fleet-managed
machine (every default row locked) the probe never opened the file. That is the
one machine where it matters: src/core/runtime/source_withhold.js:164 throws
ClientSyncListUnreadableError before it filters the central ids out, so a
corrupt store withholds the org's rows too and the entire export stops, while
the gate's accept row still reads "Record and sync everything".

Fixed: the probe now reads the store on every enrolled gate. expressRowsSafe
stops computing the optOutIds list nothing reads any more (a filter and a map
per attended run), and its return narrows to string[].

2. src/core/cli/wizard/sync_scope.js:194 — low. Fleet-managed rows lost
their · managed by your fleet suffix in the new narration.

[...(opts.locked ?? []), ...opts.candidates].map((d) => \ ${d.label}`)printed org rows and user-chosen rows identically. The auto-accept arm 60 lines below appendsLOCKED_LABEL_SUFFIX` to the locked half, and LLP 0188 #locked is
explicit that the sync picture shows the org's rows fleet-labelled — otherwise
"These will sync to your server:" reads as though every row on it were the
user's to change here. Fixed, and pinned by an assertion in the existing
combined test (removing the suffix again fails both loop iterations).

3. src/core/cli/wizard/sync_scope.js:196 — low. The combined path dropped
the hidden_picks_syncing span attribute.

Every other noQuestion return passes { hidden_picks_syncing } to
finishSpan; the new arm passed nothing, so the run where a hidden pick (a
carried raw-anthropic, say) still ships emitted no signal — the exact question
LLP 0289 #ask-the-store added the attribute for. hiddenCandidateSyncs is
already computed above and was otherwise unused on that path. Fixed.

4. src/core/cli/wizard/pick.js:696 — low. A docstring that now describes a
screen this PR retires. buildPickOption still said the retired
· stays on this machine suffix is absent because "the sync-scope step after
this prompt is where local-only is offered". On an enrolled run this menu is
the sharing choice. Reworded to say so and to name hyp privacy client as the
standing control.

5. src/core/cli/wizard/sync_scope.js:26 — low. "The standalone editor below
retains its existing semantics" has no standalone caller. runWizardSyncScope
is called from exactly one production site (index.js:645) which always passes
collectAndSync: true, so the menu is not an editor anyone reaches. Reworded to
say the menu stands unchanged but setup no longer reaches it. (See open item C.)

6. docs/TEAM_SETUP.md:53-68 — low. The published enrolled walkthrough still
had three screens including "Choose what syncs". Rewritten to two, and the
combined screen now states the consequence the picker itself cannot: confirming
it clears any standing hyp privacy client <name> local-only for the tools on
it. This is the user-facing half of LLP 0396 and it seemed wrong to leave the
docs contradicting the code.

7. test/core/cli/wizard/progress.test.js:147 — low. A test that no longer
bites. 'the sync lane states its position even when it has nothing to ask'
hand-fed progress: 'Step 3 of 5 · Choose what syncs', a string
wizardStepProgress can no longer produce — the same file asserts
wizardStepProgress('team', 'sync') === undefined 40 lines later. It therefore
asserted only that the lane echoes whatever string it was handed, under a
@ref LLP 0338#counts-anyway [tests] describing behaviour this PR retires. It
now pins the real frame: the fleet statement with no position line above it,
under an LLP 0396 ref. Also corrected a stale assertion message in
back_navigation.test.js:629 ("still asked what syncs" — it is applied, not
asked).

---

Open items — author's and maintainer's call, not fixed

A. The store write still precedes the commit point (medium).
sync_scope.js:190 writes the cleared policy immediately, while the config
commit is deliberately last (LLP 0190 #commit-point). So a user who confirms
the picker and then hits ctrl+C at "Choose how new folders are handled" exits
with narrateEnrolledAbort — whose own comment reads "past this line a cancel
is a cancel over a machine that changed" — having already lost their standing
opt-outs. The same is true across Back: pass 2's existing.filter(...) has
nothing left to preserve, so a source unpicked on the second pass no longer
finds its opt-out standing when it is re-picked later (the behaviour
sync_scope.js's own docstring promises).

I did not change this. Master wrote the store at the identical point, so the
write position is not a regression — what changed is that the write is now
derived from the picker rather than from a dedicated answer, which makes the
un-undone clearing newly visible. Moving it to the commit point, or restoring
the prior entries on cancel and back, is a design change to the wizard's
transaction shape and should be the author's.

B. The picker gives no sign that a checked row is currently local-only
(medium). buildPickOption renders a seeded row as a plain checked box.
Master's sync menu rendered such a row unchecked, so re-running setup
round-tripped the store; now a user who set three clients local-only, later
reruns hyp setup to add one tool, and presses enter through a pre-checked
picker clears all three. It is disclosed — the "These will sync to your server:"
list names them — but only after the decision, and there is no way to answer
differently inside setup. LLP 0396 #combined-selection states this outcome
plainly, so it is a documented choice rather than an accident; a per-row marker
for rows the store withholds would let the one screen carry the whole question.

C. The sync menu is now unreachable in production (low).
promptSyncScopeSelection, SYNC_SCOPE_MENU_TITLE, optedOutBefore, the
back/cancel arms and the "Keeping local-only:" write are dead outside their
tests, as are WIZARD_STEP_LABELS.sync, the 'sync' member of the step()
union at index.js:574, and continue atSync at index.js:754. Meanwhile
runWizardSyncScope still carries @ref LLP 0188#never-silent [implements] and
@ref LLP 0190#sync-gate [implements]. The 0188 ref survives — the combined arm
narrates before anything ships — but 0190 #sync-gate describes a menu nobody can
open. Deleting it would take ~25 tests with it, which is a maintainer's call, so
I left it and only corrected the surrounding prose.

---

Verdicts on the requested scrutiny items

1. Picker-to-scope mapping, including the unchecked, none-selected and
back-navigation cases: correct.

• Checked → in picked.descriptors → a visible candidate → its opt-out entry
  is removed (existing.filter((e) => !candidateIds.has(e.source))), so it is
  collected and shared. Disclosed by the narration before the run ends.
• Unchecked → not a candidate → its entry is preserved verbatim, and the pick
  lane stops collecting it, so nothing can forward. Both halves right.
• Hidden (display-filtered, e.g. a carried raw-anthropic) → candidateIds
  is built from the visible candidates only, so hidden policies survive.
  This matches LLP 0396's "hidden source policies remain intact" exactly.
• None selected → opts.candidates.length === 0 returns before the combined
  arm, so nothing is written and the "You picked nothing to record, so nothing
  syncs to your server." line still discriminates the five cases it did before.
• Back from folders → syncScope.noQuestion is true on the combined path, so
  index.js:754 takes continue atPick and lands on the combined picker, which
  is what LLP 0396 specifies. Verified by the reworked end-to-end test in
  back_navigation.test.js, which now also asserts doesNotMatch(/Choose what syncs/) — a real negative. The residual is open item A.
• One thing I specifically checked and found safe: the sync lane's guard is
  pathway === 'team' || enrolled() while the picker's flag is
  interactive && enrolled(). A team run that was not enrolled would get a
  collection-only question and a sharing write. It is unreachable:
  pathway = 'team' is only assigned at index.js:475 (immediately after
  joined is set) and at index.js:349 (guarded on joined), and the
  disconnect arm clears joined before setting pathway = 'local'. So
  pathway === 'team' implies enrolled(), and the two flags always agree.

2. LLP 0396 vs. the code: matches, with one wording gap. Every clause holds
— combined ask, express parity, hidden and unselected policies intact, fleet
locked, unenrolled local unchanged, scripted unchanged (the lane is behind
interactive &&), no prompt or progress position, Back to the picker,
unreadable store preserved and exports fail closed, hyp privacy still
standing. The gap: 0396 says opt-out clearing follows "confirming the picker",
which is true, but the doc does not say the write lands before the run's
commit point (open item A). Worth a sentence before the doc moves off Draft.

3. The four extended LLPs: genuine extensions, no contradiction. No
supersession needed.

• 0188 #never-silent required the enrolled wizard to name what syncs before
  anything ships. It bound the statement, not the keypress — 0188's own
  express-accept arm already discharged it with a narration rather than a
  prompt. The combined arm narrates "These will sync to your server:" over
  locked plus candidates. The obligation is met by a different screen, which is
  what an extension is. #locked (org rows always sync, shown labelled) is
  preserved, and finding 2 above was the one place it had slipped.
• 0190 #sync-gate settled the shape of the menu (checked means syncs,
  locked rows lead read-only). 0396 moves the consent, it does not re-decide the
  menu — and the menu is still in the tree implementing exactly what 0190 said.
  Extension, though see open item C on the now-unreachable code.
• 0201 #gate / #decline settled that the accept row claims sync only where
  the install can keep the promise, and that the decline gloss names every
  question a decline opens. Both still hold, and are now easier to keep: the
  accept genuinely does sync everything named (it clears the opt-outs rather
  than preserving them), and the gloss names two lanes because a decline opens
  two. express-gate-disclosure.test.js still asserts the rendered frame
  forwards the whole gloss. Extension.
• 0338 #counts-anyway decided that a question lane counts on the machine
  where it has nothing to ask, its shipped instance being the sync lane on a
  fully fleet-managed machine. 0396 does not overturn that rule; it removes the
  lane the rule was about, because the lane no longer asks anything on any
  machine. That lands it under rule 1 of steps.js ("only lanes that exist to
  ask something are counted"), which 0338 left standing, and it leaves no hole
  in the numbering — pick is 2, folders is 3, with no blank position between
  them, which is the failure mode 0338 existed to prevent. Extension, correctly.
  Deleting rule 4 from the steps.js docstring is consistent with that.

4. Express parity: the two lanes agree. Express implies interactive
(interactive = !opts.picks, and the gate only renders when interactive), so
an express run gets collectAndSync on both the picker (which narrates
"HypAware will record and sync:") and the sync lane. The combined arm is
evaluated before promptSyncScopeSelection's autoAccept branch, so express
accept clears opt-outs rather than preserving them — which is precisely what
makes the gate's unqualified "Record and sync everything" true again, and why
retiring the syncWithheld narrowing was right. The disclosure test still pins
the rendered gloss, now at two clauses. Finding 1 was the one hole left in that
claim, and it is closed.

5. Test quality: good, with the one vacuity now fixed. The reworked tests
mostly bite. back_navigation.test.js:809's assert.doesNotMatch(out, /Choose what syncs/) is a real negative; the new sync_scope.test.js cases run both
autoAccept arms, seed three entries and assert exactly which two survive
(catching both an over-broad and an under-broad filter), and guard the second
question with a throwing prompt. progress.test.js asserts the sync positions
are now undefined rather than deleting the assertions. The one exception was
finding 7, now fixed and negative-checked. express-gate-disclosure.test.js:123
still builds a fixture around the old summary string — harmless, since that test
exercises the renderer with arbitrary text, but it is a stale literal.

6. Conventions: clean. No semicolons, no em dashes (U+2014), no NUL bytes,
no @typedef, no inline import('...') types, no TypeScript types outside
.d.ts, no new runtime dependencies. The two new types.d.ts fields are
optional booleans on existing interfaces, not new config keys or schema fields.
@ref annotations are honest and their anchors resolve, with the 0190
#sync-gate caveat under open item C.

CPU and memory pass

No concerns; the change is a small net win. Per enrolled attended run the
new work is one Array.filter over the policy entries plus one atomic JSON
write, both bounded by picker and policy-entry counts, and it replaces a
rendered multiselect over the same rows. Retiring the sync prompt removes a
screen's worth of option construction and a Set build. My finding-1 fix
removes a filter + map over defaultRows per attended run and drops the
optOutIds array allocation; it adds a store read on the fully-fleet-managed
enrolled gate that previously skipped one, which is a single small file read
once per run, not a per-record or hot-path cost. No unbounded growth, no busy
loop, no background work, no behaviour that worsens with data volume or uptime.
source_withhold.js's TTL cache is untouched.

[philcunliffe]

Important

Section A below is corrected by the follow-up comment.
A parallel reviewer refuted the premise that master wrote at the same point to the same effect, and I reproduced their probe:
master preserved a standing opt-out through this cancel, this PR destroys it. The item is high and blocking, not a deferred medium.
Six further findings are in that comment. Marker word is unchanged.

Review round 2 (of 2) at 76e5bd54

Verdict: approve the mechanism, escalate two consent questions. The combined
selection maps correctly in every case I could construct, round 1's fixes hold,
and I found one incomplete claim which I fixed. Two items are design calls I
deliberately did not touch, because they are yours and not defects; they are
written up below for the maintainer rather than changed under you.

Pushed as 3ff830d8. Full suite green against baseline (6508 pass; the 3
hyparquet-floor-pin failures reproduce on origin/master and are not yours).
npm run typecheck clean.

---

The mapping

Re-derived from scratch after round 1's changes. It holds:

the row	reaches the store as	result
checked, visible, not locked	in opts.candidates, so candidateIds	opt-out entry dropped, syncs
unchecked	never picked, never a candidate	existing entry preserved untouched
picked but display-filtered (hidden)	candidatesHiddenIds, not candidateIds	entry preserved, and hidden_picks_syncing still computed off the pre-write read
org/locked	never a candidate	always syncs, entries inert at the seam
nothing selected	candidates.length === 0, returns before the new branch	no write at all, same as master

The existing.filter((entry) => !candidateIds.has(entry.source)) in
sync_scope.js:189 is the right predicate: it clears exactly the visible
selected rows and nothing else, which is what LLP 0396 says. Placing the new
branch after the candidates.length === 0 block is also right, and the
writeClientSyncEntries on an empty entries is load-bearing, not redundant:
store absence is the LLP 0188 migration marker, so materializing it is what
stops a later boot migration from deriving a withheld set for rows the user
just confirmed. Good catch on your part; the one-line comment there earns its
place.

Back navigation: the combined branch returns noQuestion: true, so
index.js's if (folders.back) { if (syncScope.noQuestion) continue atPick }
lands on the picker, as LLP 0396 requires. Verified end to end by the
back_navigation.test.js walk you updated.

Round 1's seven fixes: verified

All seven landed and are correct. On the important one (syncWithheldSafe):
the probe does now run on every enrolled gate, it is not over-eager (still
short-circuited behind enrolled(), so a solo run pays nothing), and it
cannot return a false "withheld" -- a missing file returns null without
throwing, so only a present-but-unparseable file trips it, which is exactly
the file the export seam fails closed on.

But it was incomplete in two ways, which is finding 1.

---

Findings

1. medium -- the sync claim was dropped from the gate but not from the picker behind it. Fixed.

src/core/cli/wizard/index.js:600, pick.js:637, pick.js:656

Round 1 made the express accept row degrade to Record everything on an
unreadable policy store, on the correct grounds that a screen may not promise
sharing the confirm cannot enable. But collectAndSync was gated only on
interactive && enrolled(), so on that same machine:

• the express accept row said Record everything (claim withheld), and then
  the pick lane's own narration immediately said HypAware will record and sync: (claim made) -- two screens of one run contradicting each other;
• a Customize run, whose picker is the only screen it decides on, was
  titled What do you want to collect and sync? with no claim withheld
  anywhere before the write.

This is a claim the PR introduces (master's title was What do you want to collect?, which promised nothing). Fixed by hoisting the probe one scope so
both claims read it:

let syncWithheld = false
...
if (interactive) {
  syncWithheld = enrolled() && (await syncWithheldSafe({ opts }))
...
  ...(interactive && enrolled() && !syncWithheld ? { collectAndSync: true } : {}),

collectAndSync only ever selects between two string literals in pick.js,
so this changes wording and nothing else; the sync lane still skips-with-warning
on that store as before. The probe now also runs on an enrolled pass that shows
no gate, because the picker makes the claim on those passes too. Cost is one
extra small-file read on an attended enrolled run.

2. medium -- round 1's central fix had no test. Fixed.

Every orchestrator-level assertion on syncWithheld asserted undefined.
Nothing pinned the true arm at all, so restoring the stale
if (ids.length === 0) return false guard left the suite green -- including on
the fully fleet-managed machine that was the whole reason for the change.

Two regression tests added in test/core/cli/wizard/index.test.js, both
mutation-checked:

• unguarding the picker's claim fails both new tests;
• re-introducing the "skip the probe when every default row is locked" behaviour
  fails a fully fleet-managed machine still probes the store for its sync claim
  and nothing else.

3. low -- stale @ref on wizardStepProgress. Fixed.

src/core/cli/wizard/steps.js:89

The PR correctly deletes rule 4 from the itinerary docstring, but the
@ref LLP 0338#counts-anyway below it still glossed "a lane with nothing to
ask on this machine keeps both its place in the total and its position line".
The sync lane was that rule's only shipped instance and this PR retires it;
progress.test.js now asserts the opposite for it. Re-glossed to what the
function still implements, naming LLP 0396 as what retired the instance.
(@ref LLP 0338#consequences at line 70 still holds and is untouched.)

4. low -- two test names and a ref describing the retired trigger. Fixed.

• index.test.js:445 was still titled "a standing opt-out ... narrows the
  express gate's sync claim" over an assertion that it does not, with a lead
  comment stating the retired rule as fact.
• express.test.js:119 was titled and glossed as "the store already withholds
  one of the named rows", which round 1 replaced with "the store is unreadable",
  and carried @ref LLP 0201#gate where express.js now carries LLP 0396.

Both retitled and re-glossed. The assertions were already correct; only the
prose lied.

5. low -- sync_scope.js:28 says the menu "stands unchanged for direct

callers and its own tests". Not fixed, reported. There are no direct callers
outside tests: src/core/cli/wizard/index.js is the only production caller and
it passes collectAndSync: true unconditionally. See item C.

---

The three items round 1 left open

A. The policy write precedes the commit point. medium. Left, and now recorded in the LLP.

Correct that the write position is master's and not a regression. But the
substance that is new is the direction of the leftover. On master an
abandoned run left extra opt-outs standing: it failed closed, more withheld
than the user settled on. Here an abandoned run leaves opt-outs cleared: it
fails open, more shared than the user settled on, and neither Back nor Ctrl+C
restores them. A Back-then-uncheck also cannot put back an opt-out the first
pass destroyed.

I did not fix it, on purpose. Making the clearing recoverable means the lane
returning "clear these" and the orchestrator writing after
commitWizardPickedConfig, which changes the lane's contract for its ~30
direct-calling tests. That is a redesign of your feature at the review cap, not
the smallest change, and the fail-open direction is a call for the maintainer,
not for me.

What I did do: added a paragraph to llp/0396 (still Draft, so still
editable) recording the ordering and naming the direction change explicitly, so
the next reader meets it in the spec instead of deriving it from index.js.
Please read that paragraph and push back if I have characterised your intent
wrongly.

Maintainer: this is the one thing in the PR I would want a human decision on
before merge.

B. No per-row sign that a checked source is currently local-only. medium. Left; it is the design.

I agree with round 1 that this is your call, and I want to say why more
strongly than "it is documented". One checkbox carrying two axes is the point
of the PR; a picker able to round-trip "collect but do not sync" would need the
second axis back, which is the thing being retired. LLP 0396 states the clearing,
docs/TEAM_SETUP.md now warns about it on exactly the path where it bites
(Customize), and the lane narrates the resulting list.

One thing worth having on the record: sync_scope.js's own docstring still
describes the property being given up, that "a re-entry renders the sources
already opted out unchecked so re-running the wizard round-trips the store
instead of resetting it". That was an explicit invariant, and the combined
picker deliberately drops it. buildPickOption's docstring names the mechanism
a future change would revive (the retired · stays on this machine suffix), so
the way back is documented if the maintainer wants it. Not churning it at the cap.

C. The sync menu is unreachable in production. low. Left.

Confirmed by grep: index.js:659 is the only production caller and it passes
collectAndSync: true unconditionally, so both promptSyncScopeSelection and
the autoAccept narration arm above it are dead in production. Deleting takes
~25 tests, SYNC_SCOPE_MENU_TITLE, WIZARD_STEP_LABELS.sync, the 'sync'
member of WizardStepName, and the enterKeepsChecked coverage with it. That
is a separate change; CLAUDE.md says land the small one and defer the rest, and
the file's own docstring already discloses the situation. Not filing an issue
for it either: it is maintenance drag with no user-facing consequence, which is
below the bar for autonomous issue-filing. Recorded here so it is not lost.

---

CPU and memory pass

No CPU or memory concern. Nothing here is a hot path, a per-record path, or
a long-running process: this is the interactive setup wizard, bounded by the
picker row count and the policy entry count (single digits each), running once
per install or reconfigure.

• syncWithheldSafe is one readObservabilityEnv plus one small-file read and
  parse. Round 1 made it strictly cheaper than master's version, which also
  built a Set from optedOutClientSourceIds and scanned ids against it;
  the entries are now read and discarded. My change adds at most one such read
  per atExpress pass (so one more per back-navigation), still behind
  enrolled(). I annotated the read-and-discard so it does not read as an
  accidental unused result.
• expressRowsSafe dropped a second filter().map() over defaultRows and its
  wrapper object. Less work and less allocation than master.
• The collectAndSync branch in sync_scope.js is one O(entries) filter plus
  one writeClientSyncEntries (a Map plus a sort over single digits) and one
  atomic write, replacing master's prompt plus the same write. It writes even
  when the content is unchanged, which is once per run and load-bearing for the
  migration marker.
• wizardItinerary still allocates one array copy per call, unchanged, and the
  itineraries got shorter (5 to 4, 4 to 3), so indexOf scans marginally
  shorter. wizardStepProgress added one string comparison.
• pick.js added two ternaries over string literals: no allocation change.

No new dependencies, no unbounded growth, no busy loops, nothing that worsens
with data volume or uptime.

Conventions

No semicolons, no U+2014, no NUL bytes, no @typedef, no inline import('...')
types, root-anchored .js type-import specifiers in the touched types.d.ts
imports, no new runtime dependencies. All clean, in your changes and in mine.

---

Marked findings rather than clean: items A and B are real and unresolved by
design, and A in particular is a privacy-direction change that needs a human's
yes before this merges. Nothing I consider a defect remains.

[philcunliffe]

Correction to my review above, and six further findings

A second reviewer ran the round in parallel and refuted a premise I relied on.
I reproduced their probe before writing this. Section A of my review above is
wrong on the point that matters, and the item is high, not a deferred
medium. New head a4b93746.

The correction: the write ordering is a new regression

I wrote that "the write position is master's and not a regression". The position
is master's; the consequence at that position is not, because master's write
there was a no-op round-trip and this one is destructive. Same probe, both
branches: enrolled reconfigure, store holds {claude: local-only}, accept
Record and sync everything, then cancel at the new-folder question.

master (d378a01d)

result : {"exitCode":130,"cancelled":true}
store  : {"version":1,"entries":[{"source":"claude","class":"local-only"}]}
stdout : Staying local-only:
           Claude Code
         Keeping local-only: claude. Change later with 'hyp privacy client <name> sync|local-only'.

this PR (76e5bd54)

result : {"exitCode":130,"cancelled":true}
store  : {"version":1,"entries":[]}
stdout : These will sync to your server:
           Claude Code

The opt-out is gone, no config was committed, and nothing was printed on
stderr (narrateEnrolledAbort only fires when joined, and this is a
reconfigure). On a reconfigure the daemon is already installed and running,
so the revocation takes effect immediately on a run the user cancelled.
index.js:762's comment on that branch still reads "nothing new was written".

Master's old auto-accept arm returned optedOutBefore verbatim, so a user who
changed nothing had their setting preserved. The combined branch clears by
design. I had reasoned about the case where a user adds opt-outs and cancels
(master failed closed there); I missed that the common case, changing nothing,
went from preserved to destroyed. My apologies for the wrong steer.

src/core/cli/wizard/sync_scope.js:189, index.js:761. high.

I still have not fixed the ordering, and I still think that is right at the
review cap: the idiomatic fix is to mirror the pick lane's existing
deferWrite + commitWizardPickedConfig pattern for the policy write, which
changes the lane's contract for its direct-calling tests. That is a design
change to your feature and it wants your hand, not mine. But it should now be
read as a blocking item rather than a deferred one.

What I did fix (a4b93746)

The lane that revokes a standing opt-out said nothing about it. The menu it
replaced printed the mirror line whenever it kept one, and hyp privacy client <name> sync prints the same two qualifiers for the identical store
write (commands/policy.js:544-548, under @ref LLP 0188#no-retroactive-ship
and LLP 0345#command). The "These will sync to your server:" list cannot
carry it: a row reads the same there whether it was already syncing or was
local-only until this keypress. So:

These will sync to your server:
  Claude Code
No longer local-only: claude. Future rows sync to your server; rows already
recorded are not sent. Change back with 'hyp privacy client <name> local-only'.

Printed only when something was actually revoked. Two tests, and your existing
whole-screen assertion updated to include it. Also added sources_cleared to
wizard.sync_scope.finish: sources_opted_out is 0 on every combined run by
construction, so a "setup turned my sync back on" report currently has no
signal behind it at all (LDD rule in CLAUDE.md).

This does not make the ordering safe. It does mean the user is told, which
turns a silent revocation into a disclosed one.

The other findings, credit to the parallel reviewer

Relayed as reported; I have not fixed these.

• medium sync_scope.js:194 - on the express fast path the same list now
  prints twice back to back (HypAware will record and sync: then These will sync to your server:). On master the two blocks carried different facts;
  now that the picker narration absorbed "and sync", the second is a
  restatement. My revocation line gives it something of its own to say, but
  the duplicated list itself is still there.
• medium index.js:642 vs :586 vs steps.js:100 - three independent
  derivations of "this run is the combined picker"
  (interactive && (pathway === 'team' || enrolled()), interactive && enrolled(), pathway === 'team' || opts?.managed). They agree only because
  pathway = 'team' is set in the same synchronous block as joined. Set
  pathway='team' without a join and the picker asks "What do you want to
  collect?" while the lane silently clears opt-outs. Note my own fix added a
  fourth condition (&& !syncWithheld) to the second of these, deliberately
  narrower, which makes collapsing them onto one predicate more attractive,
  not less.
• low steps.js:100 - opts?.managed truthiness against
  wizardItinerary's opts.managed === true at :62. Also the enrolled pick
  label is now a literal rather than a WIZARD_STEP_LABELS entry, so
  express.test.js's subject(step) cross-check no longer covers pick.
• low types.d.ts:64 / steps.js:15 - 'sync' survives in
  WizardStepName and WIZARD_STEP_LABELS while wizardStepProgress(_, 'sync') silently returns undefined.

Standing

My marker comment above stays findings, which is still the honest word and
now more clearly so. Verification unchanged in kind: full suite green against
baseline (6510 pass, only the 3 pre-existing hyparquet-floor-pin failures),
npm run typecheck clean. CPU/memory pass unchanged: the added line is one
conditional write over an already-computed set, and one integer on an
existing span. No concern.

The blocking item for a human is the write ordering. Everything else can ride.

[philcunliffe]

Neutral is stuck on this PR and needs a human decision

What neutral was doing: triage at head a4b93746, after two review rounds (the cap). The job at this rung is to classify every residual finding as blocking or deferrable, and either defer them all as issues or stop. One finding blocks, so neutral stops here rather than merging or redesigning a contributor's feature on its own authority.

First: this is good work. The combined picker's mapping is correct on every path two review rounds could construct, the LLP extensions are genuine, and both rounds' fixes landed cleanly. One ordering issue is the only thing standing between this PR and merge.

The blocker: a cancelled run destroys a standing privacy opt-out

Round 2's correction claimed it; neutral re-derived it independently before acting on it. Probe: enrolled reconfigure, policy store holding {claude: local-only}, accept "Record and sync everything", then ctrl+C at the new-folder question. Real orchestrator, real sync lane, real store; only the gate, fork, express accept, pick confirm, and the folder-lane cancel are scripted.

	result	store after	stdout
master d378a01d	{"exitCode":130,"cancelled":true}	entries: [{claude: local-only}] intact	Keeping local-only: claude. Change later with 'hyp privacy client ...'
PR a4b93746	{"exitCode":130,"cancelled":true}	entries: [] destroyed	No longer local-only: claude. Future rows sync ...

Mechanism: the combined arm in src/core/cli/wizard/sync_scope.js:191 calls writeClientSyncEntries the moment the lane runs, but the config commit is deliberately last (index.js:797, LLP 0190 #commit-point), and the folder question sits between them. A cancel there returns at index.js:775 under a comment that still says "nothing new was written", which was true of master and is no longer true here. Master's auto-accept arm returned optedOutBefore verbatim, so the identical cancel was a no-op round trip: the user who changed nothing kept what they had. On the PR the write is destructive by design, so the common case went from preserved to destroyed. On a reconfigure the daemon is already installed and running, so the revocation takes effect immediately, on a run the user cancelled, with no config committed and no way inside the wizard to undo it. Round 2's a4b93746 fix means the revocation is at least disclosed on stdout, but disclosure of a write the user then cancels is not consent to keep it.

That is a user losing a privacy setting they never chose to change, failing open (more shared than they settled on). Neutral classifies it as blocking and will not resolve a fail-open privacy direction on its own.

The decision needed

One of:

1. Fix the ordering (the reviewers' suggested shape): have the sync lane return "clear these" instead of writing, and let the orchestrator apply it at or after commitWizardPickedConfig, mirroring the pick lane's existing deferWrite pattern. This also repairs the Back-then-uncheck case (a source unpicked on pass 2 currently cannot find its opt-out standing when re-picked, because pass 1 already destroyed it). Cost: the lane's contract changes for its ~30 direct-calling tests, which is why neither review round did it under a contributor's feature at the cap.
2. Accept the behaviour as intended, and say so: extend LLP 0396 to state explicitly that the policy write is pre-commit and survives a cancel, and adjust the stale "nothing new was written" comment at index.js:775 and the cancel narration to match.
3. Something else you prefer (e.g. restore the prior entries on cancel/back without moving the write).

Neutral's own read is that option 1 matches the repo's stated transaction shape (LLP 0190 #commit-point: every question lane runs, then the run commits), but the call belongs to the author and maintainer.

Non-blocking residuals, so you see the whole picture

None of these block; they are recorded in the review thread and can ride or be deferred once the blocker is decided:

• Express fast path prints the same source list twice back to back (HypAware will record and sync: then These will sync to your server:), sync_scope.js:214.
• Three independent derivations of "this run is the combined picker" (index.js:656, index.js:600, steps.js:100) that agree only because pathway = 'team' and joined are set in the same synchronous block; round 2's own fix added a deliberately narrower fourth condition, which strengthens the case for one shared predicate.
• opts?.managed truthiness at steps.js:100 vs opts.managed === true at steps.js:62, and the enrolled pick label now a literal outside the WIZARD_STEP_LABELS cross-check.
• 'sync' survives in WizardStepName (types.d.ts) and WIZARD_STEP_LABELS while wizardStepProgress(_, 'sync') returns undefined.
• Design call B (no per-row sign in the picker that a checked source is currently local-only): documented in LLP 0396 and disclosed after confirm, so a choice, not a defect; worth revisiting alongside whatever you decide above, since it is the completed-run face of the same clearing.
• Design call C (the sync menu and its ~25 tests are unreachable in production; @ref LLP 0190#sync-gate describes a screen nobody can open): maintenance drag only.

How to unstick

Reply on this PR with your decision, or push the fix to fix/combined-collection-sync. Neutral monitors the thread and re-engages with your guidance on its next tick.

[platypii]

Fixed the blocking write-ordering issue in 698d09b.

The combined sync lane now returns pending source ids without changing the policy store. The orchestrator applies only the final selection after the config commit succeeds. Back discards the pending selection; Cancel, overwrite refusal, and config-write failure preserve existing opt-outs.

At commit, the policy store is re-read so unrelated changes made while setup was open are preserved. An unreadable store or failed policy save stops setup without clearing it. Revocation narration and the sources_cleared commit signal follow the successful write. The config may already have landed if the policy save fails; the two saves are ordered, not an atomic transaction.

Regression coverage exercises Cancel, Back-then-uncheck, overwrite refusal, config-save failure, policy-save failure, newly corrupted policy data, and successful commit preserving an unrelated opt-out. LLP 0396 and the team setup guide now describe the deferred behavior.

Validation: all 366 affected tests, type checking, and the onboarding smoke pass. Full suite: 6517 passed, 3 skipped, and the previously identified unrelated mcp-stdio-write-backstop JSON.stringify depth test failed. CPU and memory review found no concerns: processing remains bounded by the selected sources and existing policy entries.