Pin the pick-gate disclosure at the render layer, not only the question spec

[philcunliffe]

The pick gate's accept option carries the wizard's only side-effect disclosure on the happy path ("Configures these tools to record through HypAware; the menu rows say how."), but the only test holding it was at the question-spec layer (test/core/cli/wizard/pick.test.js). Nothing asserted it reached the screen, so a renderer that stopped forwarding summary would leave the suite green and the disclosure gone: the same failure shape that lost the picker copy the first time.

Test-only change. No production code was modified.

What was added

test/core/cli/pick-gate-disclosure.test.js (4 tests):

• renderSelect: an option summary lands on its own indented line under its row - pure frame assertion against src/core/cli/tui/render.js, covering the cursor row and a non-cursor row.
• renderSelect: the summary line is dim, not the row colour - pins the styling in the coloured frame.
• TTY gate: the accept disclosure reaches the screen through the real select prompt - drives defaultConfirmSelectPromptFactory over fake-TTY streams, so the assertion runs the whole chain: question spec, tuiConfirmSelectPromptFactory, select() into SelectState, renderSelect, bytes.
• non-TTY gate: the numbered fallback prints the accept disclosure under its row - drives the same factory over non-TTY streams into legacyConfirmSelectPromptFactory.

The two end-to-end tests replay the gate question captured off runWizardPick rather than a hand-written fixture, so they assert the shipped copy in the rendered bytes. A copy edit in pick.js travels here for free; the wording itself stays pinned at the spec layer by pick.test.js, so this is not a duplicate of that assertion.

Landed in a dedicated file rather than test/core/walkthrough-prompt.test.js, which keeps it clear of open PR #648.

Mutation table

Each forwarding link was broken in turn and the full suite re-run (node scripts/run-tests.js), then reverted. Baseline: 3578 pass / 0 fail.

#	Broken link	Result	pass / fail	Tests that fired
a	renderSelect stops printing o.summary	caught	3575 / 3	both renderSelect tests + TTY gate
b	tuiConfirmSelectPromptFactory stops forwarding summary	caught	3577 / 1	TTY gate
c	tui/index.js select() stops copying summary into SelectState	caught	3577 / 1	TTY gate
d	legacyConfirmSelectPromptFactory stops printing opt.summary	caught	3577 / 1	non-TTY gate

No link survived. The failure counts also show no pre-existing test covers any of these four links: every failure under every mutation is one of the new tests.

Checks

• node scripts/run-tests.js: 3578 pass, 0 fail, 1 skipped
• npx tsc -p tsconfig.json --noEmit: clean

Fixes #636

🤖 Generated with Claude Code

[philcunliffe]

Neutral review round 1 - 134da67

Test-only, and the mutation table was re-measured independently rather than trusted. It is accurate as published.

broken link	claimed	measured
renderSelect stops printing o.summary	3575 / 3	3575 / 3
tuiConfirmSelectPromptFactory stops forwarding	3577 / 1	3577 / 1
select() stops copying into SelectState	3577 / 1	3577 / 1
legacy factory stops printing	3577 / 1	3577 / 1

Each edit was verified with git diff before its run. Worth recording: the reviewer's first attempt used a python3 heredoc, python3 is absent here, the edit silently no-opped and the suite came back green at 3578 - the exact trap flagged on the last PR. It was caught because git diff --stat printed nothing. An unapplied mutation is indistinguishable from a test that misses the bug.

In every case the only failures were the new tests, which confirms nothing pre-existing covered any of the four links.

Three extra mutations probed assertion quality, all diff-verified: concatenating the summary onto its row (3 fail), painting it cyan instead of dim (1 fail), and dropping summary from pick.js entirely (3 fail). So the assertions pin position and styling, not mere presence.

The tests genuinely replay the shipped question. captureGateQuestion() runs the real runWizardPick over real discoverBundledPlugins() + buildPluginCatalog() with detect seeded so the gate opens, and captures the object handed to confirm; pick.js:420 is the only confirm( call site in that module, so there is no ambiguity. The wording is read off the live option rather than hardcoded, which is why the "drop summary from pick.js" mutation kills these tests.

No meaningful duplication. render.test.js covered only the multiselect summary branch (six-space indent); the select branch (four-space) was uncovered, which mutation 1 proves. pick.test.js pins the wording at the spec layer; this file deliberately reads it live instead of re-pinning it.

Deterministic. Colour comes solely from the passed env, never ambient process.env or FORCE_COLOR; width falls back to 80 since PassThrough has no columns, and render.js does no wrapping. Verified 4/4 across 5 consecutive runs plus NO_COLOR=1, FORCE_COLOR=3, COLUMNS=40, HYP_NO_TUI=1, and CI=true TERM=dumb.

Finding, fixed in 4e0273f

LOW - the helper leaked a temp directory per run. captureGateQuestion() called fs.mkdtemp with no cleanup, and runWizardPick genuinely writes a .hyp/hypaware-config.json into it, so 26 orphaned dirs accumulated across the review's runs. Contained inside the temp HOME so nothing escaped, and a fair number of existing test files leak the same way, but cheap to close.

Fixed by registering test.after(() => fs.rm(tmp, { recursive: true, force: true })) at the creation site, following usage-policy-symlink.test.js's tempRoot() pattern (cleanup travels with creation, which is what this file's memoized shared helper needs, since a per-test t.after would require the mkdtemp inside each test body). Verified 0 leftovers across 6 runs, including a deliberately failed run to confirm cleanup survives failure.

Noted, not fixed

Test 2 pins dim (\x1b[2m) but does not assert the absence of the row colour, so bold+cyan+dim would still pass. Cosmetic. Test 4's stdout stub replies only to chunks starting with select; changing that prefix does not hang, it surfaces as cancelledByParent with a non-zero exit, so CI still goes red, just less legibly.

Scope is one test file, zero production lines. npm test 3578 pass / 0 fail / 1 skip, tsc clean, @ref LLP 0190#pick-gate resolves.

[philcunliffe]

Neutral review round 2 - 4e0273f - CLEAN

The round-1 finding is fixed and the four original tests are intact.

Cleanup works and survives failure. 0 leftover /tmp/hypaware-pick-gate-disclosure-* directories across 3 consecutive runs, and 0 after a deliberately failed run (# fail 2, exit 1). The forced-failure edit was confirmed applied via git diff before the run was trusted.

The four tests did not weaken. git diff 134da67..4e0273f is exactly one added line. Spot-checked by re-running a round-1 mutation: stripping the o.summary forwarding from renderSelect still fails the file (# pass 1 / # fail 3), so the render-layer pin is still live after the cleanup change.

The cleanup is safe and registers exactly once. tmp is a fresh mkdtemp path captured only in that closure, so fs.rm can only remove the tree it created. captureGateQuestion() is reached solely through the memoized gateQuestionPromise ??= ... guard, and with no await between the check and the assignment, concurrent callers all await the same promise, so there is no double registration.

One thing the reviewer checked rather than assumed, and worth recording because it is not obvious: a top-level test.after() invoked from inside a running test's async body attaches to that running test's context (Node backs the test context with AsyncLocalStorage) and fires right after that test, not at end-of-file. So the directory is removed after whichever test happens to run first. That is safe here only because the temp HOME is needed transiently during runWizardPick, and later tests reuse the already-resolved question object rather than the directory. Correct as written, but it depends on that property.

Scope is one test file, 222 insertions, zero production lines. npm test 3578 pass / 0 fail / 1 skip, tsc clean. No new findings.