Numbered fallback re-asks once on an answer naming no row (#634)

[philcunliffe]

In the non-TTY numbered prompt an answer that names no row (y, 0, an
out-of-range index) parses to [], which the wizard's sync menu reads as
"select nothing": a typo opts every candidate out of syncing. The opted-in
question now prints what did not match and asks once more, and "none"
is the word for a deliberate empty selection.

Why this one cannot loop or hang

The first attempt (991388d, reverted) wrapped the shared factory's Q&A in an
ungated while (true). Each of its four failures has a structural answer here:

Failure of 991388d	This PR
Unbounded loop	for (attempt = 1; attempt <= attempts; ...), attempts = 1 + 1. The loop cannot re-enter after the budget; the fall-through returns the historical [].
Applied to every caller	Gated on question.enterKeepsChecked, which only wizard/sync_scope.js sets. Everyone else computes attempts = 1: same bytes, same answers, same one ask.
EOF left rl.question unsettled	The interface's close resolves the pending ask as "no answer", and the prompt takes the same default a bare enter takes.
A correction in the same chunk was dropped	Answer lines are queued from the moment the interface opens, not per question() call, so both lines of a single y\n3\n write survive.

rl.question cannot do the last two jobs: it registers its line listener only
when called (readline emits a chunk's lines synchronously, a promise resolves a
microtask later) and at EOF its promise neither resolves nor rejects. So the
factory drives a small queued line reader over the same interface. With
terminal: false readline writes the query straight to output, so writing the
prompt directly is byte-identical.

Empirical evidence

Real spawned process, real piped stdin, timeout 20, no process.exit (a live
handle or an unsettled ask shows up as rc=124). flagged = the sync menu's
question, plain = every other caller.

991388d, the reverted implementation, on this harness:

endless-invalid   flagged  rc=124  20010ms  bytes=1268457  re-asks=10312  (never returned)
endless-invalid   plain    rc=124  20012ms  bytes=1031979  re-asks=9735   (never returned)
invalid-then-EOF  flagged  rc=13     112ms                                (unsettled top-level await)
one-chunk y+3     flagged  rc=13     101ms                                (correction lost, then unsettled)

This PR:

endless-invalid   flagged  rc=0  118ms  bytes=352  asks=2  picked=[]
endless-invalid   plain    rc=0  112ms  bytes=200  asks=1  picked=[]
invalid-then-EOF  flagged  rc=0  109ms  bytes=371  asks=2  picked=["claude","openclaw"]
invalid-then-EOF  plain    rc=0  107ms  bytes=200  asks=1  picked=[]
immediate-EOF     flagged  rc=0  106ms  bytes=248  asks=1  picked=["claude","openclaw"]
immediate-EOF     plain    rc=0  108ms  bytes=200  asks=1  picked=[]
one-chunk y+3     flagged  rc=0  102ms  bytes=360  asks=2  picked=["hermes"]
bare-enter        flagged  rc=0  106ms  bytes=248  asks=1  picked=["claude","openclaw"]
bare-enter        plain    rc=0  102ms  bytes=200  asks=1  picked=[]
all               plain    rc=0  107ms  bytes=228  asks=1  picked=["claude","openclaw","hermes"]
back              plain    rc=0  101ms  bytes=221  asks=1  threw=PromptBackRequestedError
valid-3           plain    rc=0  103ms  bytes=208  asks=1  picked=["hermes"]

Every plain row is byte-for-byte and answer-for-answer identical to master
(200/200, 228/228, 221/221, 208/208 bytes). The only master row this PR changes
outside the gate is immediate-EOF, which master exits 13 on
("Detected unsettled top-level await") and this PR resolves.

Through the packaged CLI on the reachable path from the issue's note (pty stdout,
fifo stdin), { printf '2\n'; yes; } | hyp init finishes in well under the
20s fuse: rc=0 bytes=1441 select_prompts=1 (master: identical; 991388d:
rc=124, ~1MB).

Regression tests

Nine cases in test/core/walkthrough-prompt.test.js, including two new drivers:
askPiped (whole script in one chunk, then EOF) and askEndless (answers every
prompt forever, never closes, with a 10-ask fuse so a regression fails instead of
hanging the suite). Against master:

not ok - an answer naming no row re-asks once, and a correction in the same chunk still wins
not ok - endless invalid input stops after one re-ask instead of looping    (expected 2, actual 1)
not ok - an invalid answer then EOF resolves with the checked defaults instead of hanging
not ok - EOF with no answer at all resolves on both paths                   (cancelled: "Promise
         resolution is still pending but the event loop has already resolved" - the hang itself)

All pass after the fix. node scripts/run-tests.js: 3582 pass, 0 fail, 1 skipped.
npx tsc -p tsconfig.json --noEmit: clean. npm run smoke -- walkthrough_picker_to_first_query: ok.

LLP 0190 #sync-gate (Draft) amended in the same commit: it recorded this as a
deferred papercut, and now records the bounded, gated decision and the two
invariants that keep it terminating.

Fixes #634

🤖 Generated with Claude Code

[philcunliffe]

Neutral review round 1 - a251538

Reviewed adversarially, since this PR's predecessor (991388d, reverted) shipped a CLI hang. Two MEDIUM findings, both fixed in 1ee4e19.

The harness was validated before its results were trusted. Run against the reverted 991388d, it reproduced all three original failures exactly: endless yes y gave rc=124 with 1.2 MB in 20 s, y + EOF gave rc=13 "Detected unsettled top-level await", and a one-chunk y\n3\n lost the correction at rc=13. So the negative results below mean something.

Can it loop? No. Can it hang from this change? No. 16 hostile-input cases (endless invalid, immediate EOF, invalid-then-EOF, one-chunk multi-line, CRLF, no trailing newline, a 1 MB single line, 64 KB of /dev/urandom, NUL bytes, close mid-question) all returned rc=0 in ~100-120 ms. The one rc=124 is a stream that emits nothing and never closes, which behaves identically on master and is inherent to an open prompt. The non-flagged path is byte-identical to master across 12 inputs x 2 modes.

The queuedLineAsker audit came back clean on every hazard checked: no cross-question line consumption (asker and queue are per-ask and die with the interface), no listener leak, no lost resolution between the queue check and the await (askLine is synchronous through waiting = resolve), and no double-resolve (take() nulls waiting first).

Findings

1. MEDIUM - a spent budget silently re-created issue #634. After two answers naming no row the loop fell out to return [], and in the sync menu [] means opt every candidate out, the exact silent inversion this PR exists to prevent. :222's if (attempt < attempts) also suppressed the message on the final failure, so the user got silence. It was inconsistent with the EOF path two lines up, which returned defaulted().
Fixed: a spent budget now returns defaulted(), and the final attempt prints and names the fallback (nothing matched 'y' - keeping the checked rows: claude, openclaw). Gated, so the non-flagged path still returns [] byte-identically.

2. MEDIUM - EOF advanced the wizard into side-effecting steps instead of aborting. Measured under a real pty with the packaged CLI, same temp HYP_HOME each run:

build	rc	outcome
master 7116f95	13	unsettled await at the source picker, nothing further
a251538	1	resolves [], prints "Step 2 of 2 - Finish setup", attempts the daemon install with zero sources
1ee4e19	130	hyp init: cancelled, no step 2, no install

A dropped terminal was being read as "the user chose nothing" and the run then committed. Fixed by raising PromptCancelledError on a null line for questions without enterKeepsChecked, reusing the error the TUI already raises on ctrl-C; all three call sites already route it to WALKTHROUGH_CANCEL_EXIT_CODE. The flagged sync-menu path still keeps its defaults on EOF.

3. LOW - the EOF fix did not compose. closed is per-interface, and readline registers end at construction, so over an already-ended stream close never fires and askLine awaits forever (four sequential asks: Q1 and Q2 resolve, Q3 hangs). Not a regression, master hangs a prompt earlier, but the LLP text claimed more than it delivered. Fixed by seeding closed from input.readableEnded. The three sibling factories in this file still use bare rl.question and still hang at EOF; rather than change three behaviours in this PR, LLP 0190 now says so explicitly and names closing that class as a separate change. I agree with that call.

4. LOW - "none" was undiscoverable, advertised only in the gated re-ask message. Now in the prompt line for the flagged path, with the JSDoc scoped to match.

Verification of the fix

Five new regression tests, all confirmed failing on a251538, including a full runPickerWalkthrough case asserting rc 130 and no config written, and one that reproduces the sequential-ask hang. Hostile-input properties re-verified after the change with no rc=124. npm test 3587 pass / 0 fail / 1 skip, tsc clean, walkthrough_picker_to_first_query ok.

One honest note carried from the fix: walkthrough_to_first_query fails, but identically on origin/master and a251538, so it is pre-existing and not from this change.

[philcunliffe]

Neutral review round 2 - 1ee4e19 - CLEAN

All four round-1 findings genuinely fixed, verified by running rather than reading.

1. Spent budget (MEDIUM) - FIXED. Verified through the real runWizardSyncScope, where the bug actually bites. Feeding 2 then endless y:

	result
master	optedOut: ["claude","otel"] - #634 reproduced, every candidate silently opted out
branch	optedOut: [], with nothing matched 'y' - keeping the checked rows: claude, otel

2. EOF side effects (MEDIUM) - FIXED. Under a pty: master gives "Detected unsettled top-level await" at rc 13; the branch gives hyp init: cancelled at rc 130, no "Step 2 of 2", no daemon install, no config written. The flagged sync path still keeps its defaults on EOF.

3. readableEnded seeding (LOW) - FIXED. Two asks over one stdin: master resolves the first then hangs at rc 13; the branch settles both. Reachable for real, since the gate's readline swallows the whole buffer and the menu is then built over an already-ended stdin.

4. "none" (LOW) - FIXED, advertised on both flagged prompt variants with JSDoc scoped to match.

The cancel change does not break anything that used to work

This was the risk worth chasing, since throwing where the code previously returned a value changes control flow on a shared factory. All three call sites catch PromptCancelledError and the orchestrator honours the result; no uncaught rejection anywhere. Crucially, on master the non-flagged prompt at EOF never returned [] - rl.question left the promise unsettled at rc 13 - so there is no state master succeeded in that now fails. Scripted --yes/preset runs supply opts.picks and never construct the factory, and hyp init gates non-TTY stdin at rc 2 before any prompt.

Hostile input

15 cases. The only rc=124 is a stream that never closes, byte-for-byte identical to master and correct behaviour for an open prompt. Every direct-hang case master had (rc 13) now settles in ~110 ms. Non-flagged path byte-identical: 16/16 combinations of {allowBack} x {8 answers} match master in stdout bytes, return value and thrown error; all differences are on the flagged path, as intended.

npm test 3587 pass / 0 fail / 1 skip, tsc clean, walkthrough_picker_to_first_query ok. LLP 0190 is Draft, and its amended text does not overclaim: it says plainly that the three sibling factories still use rl.question and still hang at EOF.

Worth knowing, not blocking

The residual hang class fronts the flow. printf '' | hyp init still exits 13 on this branch, identical to master, because it hangs at fork.js's "Join a team, or set up HypAware locally?" prompt before ever reaching the picker. The class is also wider than the LLP sentence's scope: fork.js:360, cli/confirm.js:30 and plugin_install/confirm.js:150 are all in it. Not a regression and not this PR's job, but the user-visible win only materialises once past the fork prompt, which is narrower than "a dropped terminal now cancels" reads. Worth a follow-up that closes the class.

Two cosmetic nits recorded and not fixed: the fallback message names descriptor ids (claude, openclaw) rather than the labels or row numbers the menu just rendered, and walkthrough-prompt.test.js:210 and :278 assert the same thing.