Skip to content

Integrate the three wizard PRs (#675, #674, #677): conflicts resolved, LLP 0200 collision renumbered, cross-PR seams closed - #679

Merged
bgmcmullen merged 18 commits into
masterfrom
integrate-wizard-prs
Aug 8, 2026
Merged

Integrate the three wizard PRs (#675, #674, #677): conflicts resolved, LLP 0200 collision renumbered, cross-PR seams closed#679
bgmcmullen merged 18 commits into
masterfrom
integrate-wizard-prs

Conversation

@bgmcmullen

@bgmcmullenbgmcmullen commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Integrates #675, #674, and #677, which all touch the setup wizard and each mint an LLP numbered 0200. Merged in that order, with the conflicts resolved and the cross-PR seams closed. If this lands, the three source PRs close with it.

Why an integration branch

Resolutions

LLP numbers.#675 merged first and keeps 0200 (folder-ask) and 0201 (express gate). #674's hidden-picker-rows doc is renumbered to 0202, #677's setup-offers-the-first-sync doc to 0203. All @refs and Extended-by: forward-refs retargeted (mechanical renumbering per LLP 0156).

pick.js (#674 x #675).#675's resolvePickSeeding hoist is kept, and #674's hidden-row logic moves into it:

  • defaultRows is drawn from visiblePickerDescriptors(...), so a hidden row can never be stated by the pick gate or the express gate. A picker row can be hidden from the menu without ceasing to be a source (LLP 0200) #674 wrote "absent from the interactive menu and the defaults gate" before LLP 0201's second gate existed; computing the filter once in the seeding step is what extends that contract to both.
  • The carry-through decision (seedOrigin, carried) is computed in resolvePickSeeding and applied at every confirming screen, including the express auto-accept branch. Neither PR could have written that arm: without it, an express-accepted run on a fleet-locked machine whose local config is raw-only would silently strip the raw upstream the carry exists to preserve. A new test in pick.test.js pins this exact composition.

Overwrite confirm (#675 x #677). Both PRs rewrote the same string with opposite shapes: #677 trimmed it to one 27-word paragraph, #675 laid it out with the path and the question on their own lines. This branch keeps #675's layout: it carries the same facts at similar length, and #677's paragraph form re-buries the question at the end, which is what #675's change was for. Flagging as the one judgment call to review; reversing it is a one-string change in defaultOverwriteConfirmFactory.

Orchestrator (#675 x #677). Union merge. #677's sync-now offer sits after the privacy narration and before the first ask, post-finale, so #675's express fast path does not skip it - deliberately. The express gate accepts setup defaults; the sync offer is a consent step, and "Send now" still runs the real hyp sync with its own plan and y/N. The aborted-but-enrolled path still never sees the offer.

Asset counts (#677). Restored in d6c427a after the confirm-copy resolution initially clobbered them (checkout --ours takes the whole file, not the hunk); #677's own test caught it.

Verification

  • npm test: 3852/3855, 1 fail: usage_policy.fold_tightened, which is environment-specific on this machine and fails identically on a clean master checkout; all three source PRs report the same.
  • npm run typecheck clean.
  • Smokes green: walkthrough_picker_to_first_query, cli_bundled_plugins_activated, status_diagnostics, daemon_foreground_start_stop.
  • New coverage: express auto-accept + hidden-row carry-through (the cross-PR seam).

Note for the source PRs

All three carry changes-requested neutral verdicts. If revision rounds land on any of them, re-integrate here rather than re-merging: the renumbering (0202/0203) and the two seam resolutions above are the reference.

🤖 Generated with Claude Code

bgmcmullenand others added 18 commits August 7, 2026 14:25
…ce (LLP 0200)
The `raw-anthropic` / `raw-openai` wizard rows cost every first-run user a
question and earn it back for almost nobody. Beside a client row they compose
byte-identical config (claude already contributes the `anthropic` upstream,
codex `openai` + `chatgpt`, and the fold dedupes by name), so their checked
state is cosmetic. Alone they capture nothing at all: projection is
adapter-owned, `@hypaware/ai-gateway` registers no projector of its own, and
the default `config` boot profile activates only what the config names - so a
raw-only install proxies traffic correctly and writes zero rows.
Add `hidden` to the `contributes.picker` contract rather than a row-id list in
core, which would undo what LLP 0130 settled about who owns the picker table.
`visiblePickerDescriptors` is the one display filter, used by the legacy
prompt and by the wizard's gate and menu.
Hiding is a display filter, never a catalog deletion. The owner map arming
LLP 0192's fail-closed withholding is folded from picker descriptors, so
deleting the two `contributes.picker` blocks would give `ai_gateway_messages`
an empty owner list - which both withhold rules read as "never withhold",
turning off a privacy guard under cover of a UI cleanup. The descriptors stay.
Carry-through is narrower than it first looks. A hidden row rides through the
selection only when the config on disk collects nothing the menu can show (a
`--source raw-anthropic` install being reconfigured). Carrying on seed
membership was tried and is wrong: seeding is derivative for these rows -
`raw-openai` reads as configured whenever codex's `openai` upstream is present
- so it resurrected that upstream the moment someone unchecked codex. The
existing "unchecking a row still removes its gateway upstream" test caught it.
What this does not fix: raw traffic still needs an adapter's projector to be
recorded, and generic Anthropic-dialect traffic is still stamped
`client_name: 'claude'` by the LLP 0115 fallback. Both are projector
behaviour, independent of any picker row, and belong to LLP 0192's deferred
attribution decision. Filed as #673.
npm test: 3728 pass, 1 fail (`usage_policy.fold_tightened`, pre-existing on
master). typecheck clean. Smokes green: walkthrough_picker_to_first_query,
cli_bundled_plugins_activated, status_diagnostics.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two changes to the same complaint: setup asks too much, and one of its
questions comes back every session.
LLP 0200 makes the per-folder classification ask (LLP 0106) opt-in. A
machine-local preference decides whether an unclassified folder is asked
about at session start, and its default is `sync` - the folder keeps the
implicit default it already had and nobody is interrupted. The ask still
exists for people who want it, chosen in the wizard's new-folder step or
with `hyp policy folders ask`. This gates the question only: no class
moves, `.hypignore` is untouched, and nothing new leaves a machine that
was not already syncing by default. `hyp status` names the mode on every
enrolled run so it is never a silent state, and the classification prompt
itself now teaches its own off switch.
The new-folder question is its own wizard step, after the per-adapter
sync lane rather than a row on it: that lane answers "which adapters
ship" and this one answers "what happens the next time I work somewhere
new", and folding the second into the first let a checklist decide a
policy silently.
LLP 0201 adds an express gate before the question lanes: it lists what
was found and offers "Record and sync all of these" or "Let me choose".
Accepting skips the prompts, never the statements - each lane still
narrates the block its gate would have shown, so the fast path prints the
same rows in the same order, minus the keypresses. The rows come from the
pick lane's own computation (hoisted to `resolvePickSeeding`), with
detection run once per wizard run, so the two screens cannot disagree
about what "all of these" means. With nothing detected and nothing locked
there is nothing to accept, so no gate is shown.
Also: the overwrite confirm is laid out instead of run together as one
paragraph, same facts and order.
npm test: 3765 pass, 1 fail (usage_policy.fold_tightened, identical on a
clean checkout). npm run typecheck: clean. Smokes
walkthrough_picker_to_first_query, status_diagnostics,
cli_bundled_plugins_activated, daemon_foreground_start_stop: green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…stop offering the hidden rows
Three review findings on the `hidden` picker field.
1. Carry-through could fire off a DETECTION seed. `promptPickSelection`
read `carried` from `seed`, but `seed` is `initialSelection ?? config
?? detected`. LLP 0200 #carry-through scopes carry-through to "the
config on disk collects nothing the menu can show"; off a detection
seed it would instead compose a hidden row the user was never shown
and cannot uncheck, which is what LLP 0011 #autodetect-vs-default
forbids and what `pick.js`'s own @ref to it promises. Unreachable
today only because no bundled hidden row declares a `detect` probe,
and `hidden` is a kernel-contract field any plugin can set beside
one. `seedIsChosen` now gates it, and a test drives detection
directly to pin it.
2. README and CONTEXT.md still listed `raw-anthropic` / `raw-openai`
among the rows the interactive walkthrough offers. Both now say the
rows are `--source`-only and why.
3. `picker-disclosure.test.js` justified its raw-row assertions with
"these two rows carry no adapter to disclose it anywhere else",
which stopped being true when the menu (the only consumer of
`summary`) stopped rendering them. Comment corrected; the
assertions stay, because hiding is a display filter, not a deletion.
Co-Authored-By: Claude <noreply@anthropic.com>
…shows
CI failed on `runInitWizard end-to-end`: it read the real machine for
detection, and the express gate (LLP 0201) is shown only when there is
something to accept. A developer laptop with Claude installed gets the
gate; a CI runner with no AI clients does not, so every scripted answer
after the fork landed on the wrong prompt and the run finished on the
team pathway it was supposed to step back out of.
The rows themselves were never what the test was about ('all' picks
whatever the menu offers); that there *are* rows is. So detection is
injected like every other seam in that call, and the script is stable on
any host.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The overwrite confirm ran 44 words for a y/N. It still says the two
things LLP 0183 #say-so requires - that the file is rewritten from the
picks, and what carries over - in 27: the path already reads as "the
config", and the enumeration compresses without losing its reassurance.
The finale's client-asset step printed one line per copy, which on a
two-client install is a dozen destination paths burying the step's one
fact. It now reports counts per client, and counts them per client
rather than summed: six skills copied to two clients is twelve copies,
and neither client got twelve. The materializer keeps its per-copy
output for `hyp skills install`, where the copies are the subject; the
paths stay in the run summary and the skills.install span.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LLP 0101's amendment already conceded the case: on an attended
onboarding the hold blocks the demonstration that the product works,
and the user had no way to say "I have seen enough". It minted `hyp
sync` for that and 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. A user with no privacy concern has to notice a sentence,
remember a command, and find a terminal setup is giving away.
So an enrolled attended run now asks: wait until the deadline, or send
now. Waiting leads and is the default; the unwanted outcome of a
mis-keyed wait is a few hours' latency, and of a mis-keyed send is
permanent.
The question is an offer, not the consent. "Send now" runs the real
`hyp sync`, which prints the destination 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, and this adds no second one.
It spawns a child rather than running in process, and that is
correctness, not convenience: `hyp init` boots all-available, which
withholds @hypaware/central because 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 destination the
release unblocks - the misleading artifact R2 requires the plan to
prevent. Whether it sent is read back from the hold marker rather than
from the exit code, which is 0 both for a release and for a user who
read the list and said no.
Position: narration, offer, first ask. The narration stays the last
thing HypAware says about privacy; the first ask stays last on screen
because it may take the terminal for good.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o pick
Carry-through re-tested its own precondition on every pass. A re-entry
seeds with the selection the previous pass confirmed (LLP 0191
#re-entry-seeding), which for a raw-only install holds the carried hidden
row beside whatever visible rows the user just added. Asking "does the
seed collect nothing the menu can show?" of that seed answers no, so the
carried row was dropped: `back` then `enter` deleted the gateway upstream
the install runs on, silently.
Replaces the boolean `seedIsChosen` with a `SeedOrigin` the seed carries
alongside it. `detected` never carries (unchanged), `config` carries only
when the menu can show nothing the config collects (unchanged), and
`selection` carries unconditionally: read-back never reaches that tier,
so a hidden id is in a previous answer only because this rule put it
there. LLP 0200 #carry-through records the re-entry case.
Also pins the `hidden` manifest validation branch, which shipped
untested.
Co-Authored-By: Claude <noreply@anthropic.com>
Escape at the pick lane reached past the new gate to the fork, and escape
at the sync lane landed on the gate instead of the picker: the question was
inserted between the fork and the pick lane without being given a loop
level, so both of its neighbours overshot by one screen. LLP 0191
#back-edges is that escape steps exactly one screen back.
The gate now owns `atExpress` between `atFork` and `atPick`. Pick backs to
it when the pass showed one and to the fork when it did not (LLP 0201
#no-default-no-accept), and a sync back re-runs the picker without
re-asking it. Three tests pin the chain; the end-to-end walk gains the
keypress the corrected edge needs.
Also corrects three doc blocks left over from a draft where `ask` was the
default. `src/core/usage-policy/types.d.ts` and `src/core/daemon/types.d.ts`
ship in the published declaration build, so they were telling consumers the
opposite of both the code and LLP 0200 #default about a privacy-relevant
default; `usage-policy/index.js` said the same and attributed the write to
the sync lane, which LLP 0200 #wizard explicitly rejects. Its export block
had also been spliced between the client-sync comment and the export that
comment describes.
`hyp policy list` no longer drops "no machine-local entries (policy store:
...)" on a store that is empty while the folder mode is non-default; the
mode is an addition to that line, not a replacement for it.
Co-Authored-By: Claude <noreply@anthropic.com>
LLP 0200 #read-back settles the direction: "An unreadable re-read is
treated as 'still held', because claiming a sync happened is the one
wrong answer that cannot be corrected later." `readHold` said the same
thing in its own docstring and then did the opposite - it returned
`null` on a failed read, and `null` is the caller's word for "the marker
is gone, it sent". So a state dir that would not resolve ended the run
reporting `released: true`, logging `sync_now: released`, and skipping
the line that restates the deadline: setup's last word on the subject
was a release nobody observed.
A throw from the `readDeadline` seam was worse in a second way - it
escaped to the outer catch and returned `{ asked: false, reason:
'error' }` after the child had already run, so the run ended on silence
with no statement of what still held.
Both now return the deadline the step started from, which reads as
"still held" and prints the restatement.
Co-Authored-By: Claude <noreply@anthropic.com>
…/0201)
The sync lane asks nothing when everything picked is fleet-locked, and
nothing when the client store is corrupt: it states its outcome and
returns. The new-folder lane behind it backed unconditionally into it, so
escape re-ran the statement and re-asked the same question. On a fully
managed machine that is a trap with no exit but ctrl+c.
The lane now reports `noQuestion` on both non-prompting returns and the
orchestrator steps past it to the picker, which is the last screen the user
could answer.
Same class one step earlier: the express gate lists the picker's own
confirmed defaults, so a confirmed empty selection leaves it nothing to
show. A back out of the re-entered picker landed on a gate that could not
render and fell forward into the picker again. It now reaches the fork.
Also: the pick lane re-derived `defaultRows` instead of taking the
`resolvePickSeeding` value the express gate reads, which is the second
derivation of "the defaults" LLP 0201 #gate says must not exist.
Co-Authored-By: Claude <noreply@anthropic.com>
Every test of `runWizardSyncNow` replaced the hold re-read with the
`readDeadline` seam, so the body that actually decides the release claim -
resolve the state dir from the environment, read the marker there - had no
coverage at all. That is the body a wrong answer cannot be walked back in
(LLP 0200 #read-back), and the blind spot is what let the earlier
`catch { return null }` report a release nobody observed.
Two tests drive it end to end against a real marker on disk with no seam:
a spawn stub that clears the marker (a release) or leaves it (a declined
plan). Proven to bite: dropping `opts.env` from the state-dir resolution -
the drift that would make every run claim a release - fails only these two
and leaves all eight pre-existing tests green.
Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts:
#	src/core/cli/wizard/pick.js
# Conflicts:
#	src/core/cli/walkthrough.js
#	src/core/cli/wizard/index.js
@bgmcmullenbgmcmullen added the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Aug 8, 2026
@philcunliffephilcunliffe added the neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) label Aug 8, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

neutral review - 1540e86 - clean

Verdict: clean. No findings, nothing pushed.

The brief for this round was the one that matters for an integration branch: not "does the suite pass" but "did a conflict resolution silently drop a fix that a source PR's review round put there". The PR body admits that failure mode already fired once (d6c427a, the asset counts). I checked for others systematically rather than by reading the diff, and found none.

Reviewed in a fresh detached worktree at origin/integrate-wizard-prs, npm install (no lockfile, so npm ci is not usable here).


Per-fix survival table

Every row marked survived was proved by mutation: revert only the fix in the integrated tree, run its test file, restore. A green suite proves nothing here, which is the whole point.

#FixStatusProof
674SeedOrigin union replaces seedIsChosen (pick.js:136, :178; types.d.ts:55)survivedcarry rule reverted to seedOrigin !== 'detected' && !seededVisible -> not ok 47 - a carried hidden row survives a re-entry that adds a visible row
674non-boolean hidden manifest validation (manifest.js:214)survivedbranch deleted -> not ok 13 - validateManifest rejects a non-boolean hidden
675atExpress: loop level (index.js:351, :463-468)survivedback edge collapsed to continue atFork -> not ok 15 - a back from pick re-presents the express gate, not the fork + not ok 29 (end-to-end walk)
675noQuestion on the sync lane's two non-prompting returns + continue atPick (sync_scope.js:71, :87; index.js:551)survivedcontinue atPick deleted -> not ok 18 - a back from folders skips a sync lane that asked nothing and reaches pick
675backFromPick routes a back-out to the fork when the gate cannot render (index.js:350, :396-400, :465)survivedarm deleted -> not ok 20 - a back from pick reaches the fork when the confirmed picks leave the gate empty
675three doc blocks stating the folder-ask default (usage-policy/types.d.ts:112/:123, daemon/types.d.ts:275, usage-policy/index.js:73)survived, no test (docs)read directly; all four statements say sync, matching DEFAULT_FOLDER_ASK_MODE = 'sync' (folder_ask.js:30). Grepped the whole tree for a residual "ask is the default" / "absence means ask": zero hits. These ship through build:types, so this was checked by reading, not inferred
677readHold catch returns opts.deadline, never null (sync_now.js:220-222)survivedreturn null restored -> not ok 6 - a re-read that throws is reported as not sent, not as a release
677readDeadline seam inside the try (sync_now.js:216)survivedseam hoisted back out -> same not ok 6
677two seam-free tests over a real hold marker under a temp HYP_HOME (sync_now.test.js:208, scenario loop, 10 tests total)survivedproduction body mutated readObservabilityEnv(opts.env) -> readObservabilityEnv() -> not ok 8 - the read-back resolves a real hold marker from the environment: left in place. The other nine stayed green, which is exactly the blind spot round 2 closed
677per-client asset counts (walkthrough.js:1583, clientAssetCountLines at :2044)survived (restored in d6c427a)finale line deleted -> not ok 1 - the finale reports asset counts per client, never a line per copy

Sweep for other dropped hunks

Rather than trust the ten above, I diffed each source branch against its merge base and checked every added line for presence in the integrated tree, normalising away the 0200 -> 0202/0203 renumbering. Total unexplained lines: zero. The complete set of source-branch lines absent from the integration is:

No file a source branch touched is missing from the integration. No conflict markers anywhere.

The cross-PR seam

Verified as claimed, not assumed.

  • The carry-through arm exists on the express auto-accept branch: pick.js:553-557 returns withCarried(defaultRows.map(...)), the same widening as the pick gate (:588) and the menu (:605).
  • test/core/cli/wizard/pick.test.js:1080 pins it as a real composition, not a stub: real bundled catalog, a raw-only config on disk, locked: ['claude'], both prompt seams throwing if touched, and assertions on the writtenai-gateway upstreams. Deleting the arm -> not ok 43.
  • defaultRows is drawn from visiblePickerDescriptors(...) (pick.js:193, via visibleList at :158), and expressRowsSafe (index.js:944) labels that same value, so a hidden row can be stated by neither gate. Pointing defaultRows at the unfiltered descriptorList -> not ok 41 (pick gate) andnot ok 43 (express gate): both halves are pinned, independently.

The renumbering

  • duplicate-numbers (the workflow's own shell): PASS, no duplicate across llp/.
  • 0200-0203 all exist; 0202 is A picker row can be hidden from the menu without ceasing to be a source (LLP 0200) #674's hidden-rows doc, 0203 is Setup offers the first sync, and two wizard screens get shorter (LLP 0200) #677's first-sync doc. Both are byte-identical to their source-branch originals modulo the number substitution, as are 0200/0201. Purely mechanical, per LLP 0156.
  • Zero stale self-references: 020[0-3] appears in 0202 and 0203 only on their own title lines.
  • Every @ref anchor on 0200/0201/0202/0203 resolves in the right doc, with no cross-contamination: #hidden-rows/#carry-through only ever hit 0202, #offer/#read-back/#child-process only ever hit 0203, the folder-ask anchors only 0200, the gate anchors only 0201. This is the failure the workflow comment warns about (a retargeted number still resolving, to the wrong doc), so I checked it at anchor level rather than doc level.
  • Extended-by: forward-refs all land correctly: 0012 and 0130 -> 0202, 0101 and 0198 -> 0203, 0106 -> 0200, 0190 -> 0201, 0191 -> 0200 and 0201.
  • I ran a full-repo anchor check. The 39 broken anchors it reports (0042, 0074, 0044, 0045, 0186, 0006, 0002, 0060, 0172, 0001, 0007) are all pre-existing and in files this branch does not touch. test/core/llp-ref-hygiene.test.js is green.

usage_policy.fold_tightened - verified independently, and the body is not quite right

The body says it fails, and fails identically on a clean master checkout. It does not fail in either place here.

  • On this branch: ok 2030 - resolve emits a hashed usage_policy.fold_tightened only when folding changed the verdict. Full suite 3849 pass / 0 fail / 6 skipped (3855 tests).
  • On a separate clean origin/master worktree, test/core/usage-policy-fold.test.js: 23 pass / 0 fail.

So the load-bearing half of the claim holds - the failure is not caused by this branch - but it is not reproducible on master either. It is specific to the author's machine, which matches both #677 review rounds ("did not reproduce here"). No finding; recorded so nobody re-derives it.

The judgment call: the overwrite confirm

I agree with the choice, and I agree it is a preference rather than a defect. Both forms satisfy LLP 0183 #say-so: each states the file is rewritten from the picks, names what carries over, and names the backup. The deciding factor for me is not word count but terminal reality - #677's single paragraph hard-wraps at whatever width the user's terminal happens to be, so Continue? [y/N]: lands mid-line at an unpredictable place; #675's layout puts the path on its own line and the question alone on the last one, which is where a reader's eye goes. That is what #675's change was for, and #677's form undoes it. Reversing remains a one-string edit at walkthrough.js:311-320; the test (pick.test.js:1010) asserts substance (/rewritten from your picks/i, /carried over/i) and passes either way, so no coverage is tied to the choice.

Also checked, clean

  • Orchestrator ordering. finale -> first look -> privacy narration -> sync offer (index.js:666) -> first ask (index.js:687). The sync offer is gated on interactive && !cancelled && !dryRun and not on express, so an express-accepted run genuinely still sees it, as the body claims. The aborted-but-enrolled path (narrateEnrolledAbort) never reaches it.
  • Express composes with the new lanes: autoAccept is threaded to pick, syncScope and folderAsk alike (index.js:454, :516, :538), and express suppresses every downstream position line (:417-418, :574).
  • Kernel contract.hidden?: boolean added to PluginPickerContribution and PickerDescriptor with real doc blocks, read in plugin_catalog.js:120 behind a typeof guard, validated in manifest.js:214, set on both raw rows in the ai-gateway manifest.
  • Conventions over the full 60-file diff: no U+2014, no statement semicolons, no @typedef, no inline import('...') types, type-import specifiers root-anchored .js (the one new bare specifier is node:child_process, correct).
  • npm run typecheck: clean. Smokes: walkthrough_picker_to_first_query, cli_bundled_plugins_activated, status_diagnostics all ok.

Noted, not raised as findings

Nothing fixed, nothing left

No actionable finding, so nothing was pushed and 1540e86 is the head this record describes.

No verdict label, no approval, no merge, no ready-flip - this is a maintainer's PR and terminates in their decision.

@philcunliffephilcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 8, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Approved — mergeable, green, reviewed. Held for you to merge; neutral does not merge a maintainer's PR.

The review came back clean: no findings, nothing pushed, head unchanged at 1540e86.

Because this branch's whole job is to carry three PRs' worth of already-reviewed fixes through a set of conflict resolutions, the review was run as a survival audit rather than a fresh read. The three source PRs went through six neutral review rounds that found real defects, and a merge resolution can drop any of them without a single test going red - as in fact happened once already on this branch (checkout --ours took a whole file and clobbered the asset counts, restored in d6c427a).

Every row below was proved by mutation: revert only that fix in the integrated tree, run its test, confirm it fails, restore.

FixStatusProof
#674SeedOrigin union replacing seedIsChosensurvivednot ok 47 - carried hidden row survives re-entry
#674 non-boolean hidden manifest validationsurvivednot ok 13
#675atExpress: loop levelsurvivednot ok 15, not ok 29
#675noQuestion + continue atPick (the inescapable trap)survivednot ok 18
#675backFromPicksurvivednot ok 20
#675 three privacy-default doc blockssurvived (docs, no test)read directly; all say sync; zero residual "ask is the default"
#677readHold catch returns opts.deadline not nullsurvivednot ok 6
#677readDeadline seam inside the trysurvivednot ok 6
#677 two seam-free tests (real marker, temp HYP_HOME)survivednot ok 8; the other nine stayed green - exactly the blind spot they exist for
#677 per-client asset countssurvived (restored in d6c427a)not ok 1

Systematic sweep for other dropped hunks. Each source branch was diffed against its merge base and every added line checked for presence in the integrated tree, normalising away the renumbering. Zero unexplained absences. The only missing lines are the two deliberately superseded promptPickSelection signatures / defaultRows derivations (the cross-PR seam, correct direction), the retargeted @refs, and the overwrite-confirm string you disclosed. No missing files, no conflict markers.

The cross-PR seam is real and pinned.pick.js:553-557 applies withCarried on the express auto-accept arm, and pick.test.js:1080 is a genuine composition test (real catalog, real config on disk, both prompt seams throw if touched) that fails when the arm is removed. defaultRows and expressRowsSafe label the same visiblePickerDescriptors(...) value; pointing it at the unfiltered list fails not ok 41andnot ok 43, so the pick gate and the express gate are independently pinned.

Renumbering verified at anchor level, not just by number: duplicate-numbers passes, 0202/0203 are byte-identical to their originals modulo the number, zero stale self-references, and every anchor on 0200-0203 resolves in the right doc with no cross-contamination (a mis-retargeted number still resolves, so number-level checking would not have caught a swap). All Extended-by: forward-refs land correctly. A full-repo check reports 39 broken anchors; all are pre-existing in files this branch does not touch.

On usage_policy.fold_tightened - the body is half right. It does not fail on this branch (ok 2030, 3849 pass / 0 fail / 6 skipped), so it is not caused by the integration. But it also does not reproduce on a clean origin/master worktree (23/23), so "fails identically on a clean master checkout" is not right either. It appears specific to your machine, matching both #677 review rounds. Recorded so nobody re-derives it.

On the overwrite-confirm judgment call - agreed, keep #675's layout, and agreed it is a preference rather than a defect; both forms satisfy LLP 0183 #say-so. The reason is not word count: #677's single paragraph hard-wraps at the user's terminal width, so Continue? [y/N]: lands mid-line unpredictably, whereas #675's puts the path on its own line and the question alone on the last. The test asserts substance and passes either way, so no coverage is tied to the choice.

Two things noted but deliberately not raised as findings: a carried hidden row appears as an opt-out candidate on the sync-scope lane (a property of #674 standing alone, and arguably what LLP 0188 #never-silent requires), and expressRowsSafe's any cast (#675's own standing deferral).

npm test 3849/0/6 · npm run typecheck clean · smokes walkthrough_picker_to_first_query, cli_bundled_plugins_activated, status_diagnostics green · conventions clean.

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

Labels

neutral:adoptForeign PR adopted into neutral's reconcile scopeneutral:adoptedAdoption completion record: merged while carrying neutral:adopt (LLP 0031)neutral:approvedneutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@bgmcmullen@philcunliffe