Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions llp/0190-wizard-defaults-gate.decision.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,14 @@ an already-ended stream never emits `close` at all, so the asker seeds
its spent flag from the stream's own `readableEnded` and a second
question on a spent stdin settles like the first. This covers the
numbered prompt. The file's other readline prompts (the overwrite
confirm, the gate's numbered fallback, the backfill consent) still call
`rl.question` directly and still hang at EOF; closing that class is a
separate change.
confirm, the gate's numbered fallback, the backfill consent) called
`rl.question` directly and hung at EOF; they now read through the same
asker, coalescing its EOF `null` into the empty line rather than
branching on it, so each lands on the default its own question prints
and no EOF answer can drift from the advertised one. The wizard's fork
screen (`src/core/cli/wizard/fork.js`) is the one prompt outside this
file still asking through `rl.question`, and closing that is a separate
change.
This flips the polarity of the
prompt LLP 0188 #never-silent quoted ("check any to keep local-only");
everything behind the prompt - the store schema, editor semantics over
Expand Down
37 changes: 29 additions & 8 deletions src/core/cli/walkthrough.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,17 +303,27 @@ function legacyNumberedPromptFactory(opts) {
* list indented under it, and `Continue?` alone on the last line where a
* reader's eye lands. Same facts, same order, scannable.
*
* A stdin that ends without a line (a terminal that dropped, a scripted
* run whose input runs out before the commit point) is read through
* `queuedLineAsker` rather than `rl.question`, whose promise is left
* permanently unsettled at EOF. The unanswerable question falls to the
* default it prints, which is the same answer a bare Enter gives, so the
* on-screen `[Y/n]` stays the whole contract: EOF completes the run the
* same way that Enter does, and the backup is taken either way.
*
* @param {{ stdin?: NodeJS.ReadableStream, stdout: { write(chunk: string): unknown } }} opts
* @returns {(targetPath: string) => Promise<boolean>}
* @ref LLP 0183#say-so [implements]: the overwrite confirm states that the config is regenerated and what is carried over
* @ref LLP 0190#sync-gate [implements]: a spent stdin lands on the prompt's stated default instead of waiting on an answer that can never come
*/
export function defaultOverwriteConfirmFactory(opts) {
const input = /** @type {NodeJS.ReadableStream} */ (opts.stdin ?? process.stdin)
const output = /** @type {NodeJS.WritableStream} */ (opts.stdout)
return async function (targetPath) {
const rl = readline.createInterface({ input, output, terminal: false })
const askLine = queuedLineAsker(rl, input, output)
try {
const answer = await rl.question(
const answer = await askLine(
'\n' +
`This config will be rewritten from your picks:\n` +
` ${targetPath}\n` +
Expand All @@ -324,8 +334,10 @@ export function defaultOverwriteConfirmFactory(opts) {
'Continue? [Y/n]: '
)
// Only an explicit no declines; a bare enter (and any stray answer)
// proceeds, matching the stated default.
return !/^n(o)?$/i.test(answer.trim())
// proceeds, matching the stated default. `null` is EOF, read as that
// same empty line so one parse serves both: the answer a spent stdin
// takes cannot drift from the default the printed question advertises.
return !/^n(o)?$/i.test((answer ?? '').trim())
} finally {
rl.close()
}
Expand Down Expand Up @@ -423,6 +435,7 @@ function legacyConfirmSelectPromptFactory(opts) {
const output = /** @type {NodeJS.WritableStream} */ (opts.stdout)
return async function ask(question) {
const rl = readline.createInterface({ input, output, terminal: false })
const askLine = queuedLineAsker(rl, input, output)
try {
// @ref LLP 0135#progress [implements]: the non-TUI fallback prints the position too
if (question.progress) output.write(`\n${question.progress}`)
Expand All @@ -436,9 +449,13 @@ function legacyConfirmSelectPromptFactory(opts) {
})
const fallback = question.default ?? question.options[0].value
const fallbackIdx = question.options.findIndex((o) => o.value === fallback)
const answer = await rl.question(
// `askLine` rather than `rl.question`, which never settles at EOF. The
// gate prints its default in the prompt (`select [2]`), so a stdin that
// can no longer answer takes that default instead of hanging the wizard.
// @ref LLP 0190#sync-gate [implements]: a spent stdin lands on the prompt's stated default
const answer = (await askLine(
question.allowBack ? `select [${fallbackIdx + 1}, b back]: ` : `select [${fallbackIdx + 1}]: `
)
)) ?? ''
// The readline form of the TUI's escape (LLP 0191).
if (question.allowBack && answer.trim().toLowerCase() === 'b') throw new PromptBackRequestedError()
const n = Number.parseInt(answer.trim(), 10)
Expand Down Expand Up @@ -503,10 +520,14 @@ function legacyBackfillConsentPromptFactory(opts) {
const output = /** @type {NodeJS.WritableStream} */ (opts.stdout)
return async function ({ providers, retentionDays }) {
const rl = readline.createInterface({ input, output, terminal: false })
const askLine = queuedLineAsker(rl, input, output)
try {
const answer = await rl.question(`${backfillConsentTitle(providers, retentionDays)} [Y/n]: `)
const trimmed = answer.trim().toLowerCase()
// Default yes: only an explicit no opts out.
// `askLine` rather than `rl.question`, which never settles at EOF.
// @ref LLP 0190#sync-gate [implements]: a spent stdin lands on the prompt's stated default
const answer = await askLine(`${backfillConsentTitle(providers, retentionDays)} [Y/n]: `)
const trimmed = (answer ?? '').trim().toLowerCase()
// Default yes: only an explicit no opts out, and EOF is read as the
// bare Enter the `[Y/n]` advertises rather than as a hang.
return !(trimmed === 'n' || trimmed === 'no')
} finally {
rl.close()
Expand Down
181 changes: 181 additions & 0 deletions test/core/walkthrough-prompt-eof.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
// @ts-check

// The wizard's three legacy readline prompts on a stdin that can no longer
// answer. `rl.question()` leaves its promise permanently unsettled at EOF,
// so a scripted run whose input runs out (or any run whose terminal drops)
// hung on the prompt forever instead of taking the default it had just
// printed. The run that reaches all three is a partially scripted wizard:
// answer the fork, let stdin dry up, and the express gate, the commit-point
// confirm and the backfill consent each read the spent stream. A fully
// unanswered `hyp init` never gets this far - `wizard/fork.js` still asks
// through `rl.question` on the wizard's first screen and hangs there.
// `hyp attach <client>`'s own backfill consent is the other caller, and it
// is reached only from a terminal (`maybeInteractiveEnableAttach` refuses a
// non-TTY stdin) with the TUI off, so what lands on it there is a dropped
// terminal rather than a pipe. Each case below is raced against a timer,
// because the pre-fix failure mode is a hang rather than a wrong value and
// an unraced assertion would never run at all.
//
// @ref LLP 0190#sync-gate [tests]: a spent stdin lands on the prompt's stated default rather than waiting on an answer that can never come

import test from 'node:test'
import assert from 'node:assert/strict'
import { PassThrough } from 'node:stream'

import {
defaultBackfillConsentPromptFactory,
defaultConfirmSelectPromptFactory,
defaultOverwriteConfirmFactory,
} from '../../src/core/cli/walkthrough.js'

/** Long enough to be unambiguous, short enough that a hang fails fast. */
const SETTLE_MS = 500

/** Sentinel the race resolves to when the prompt never answers. */
const HUNG = Symbol('hung')

/**
* Resolve `promise` or fail the test with the hang it is guarding against.
*
* @template T
* @param {Promise<T>} promise
* @param {string} what
* @returns {Promise<T>}
*/
async function settles(promise, what) {
/** @type {NodeJS.Timeout | undefined} */
let timer
const timeout = new Promise((resolve) => {
timer = setTimeout(() => resolve(HUNG), SETTLE_MS)
})
try {
const result = await Promise.race([promise, timeout])
assert.notEqual(result, HUNG, `${what} never settled within ${SETTLE_MS}ms - the prompt hung on EOF`)
return /** @type {T} */ (result)
} finally {
if (timer) clearTimeout(timer)
}
}

function makeBuf() {
/** @type {string[]} */
const chunks = []
return {
write(/** @type {string} */ chunk) {
chunks.push(chunk)
return true
},
text() {
return chunks.join('')
},
}
}

test('overwrite confirm takes its printed default on a stdin that ends without a line', async () => {
const stdin = new PassThrough()
const stdout = makeBuf()
const confirm = defaultOverwriteConfirmFactory({ stdin: /** @type {any} */ (stdin), stdout })
const answer = confirm('/tmp/hypaware.toml')
stdin.end()

assert.equal(await settles(answer, 'overwrite confirm at EOF'), true)
// The question is still printed, byte for byte: the default is taken
// because it was advertised, not instead of advertising it. That default
// is a yes, so a spent stdin completes the run (over a backup) rather
// than discarding the answers the walk just collected.
assert.match(stdout.text(), /Continue\? \[Y\/n\]: $/)
})

test('overwrite confirm takes its printed default on a stdin that was already spent', async () => {
// Readline registers its `end` listener at construction, so an interface
// built over an already-ended stream never emits `close`. This is the case
// a `close`-only guard still hangs on.
const stdin = new PassThrough()
stdin.resume()
stdin.end()
await new Promise((resolve) => setImmediate(resolve))

const confirm = defaultOverwriteConfirmFactory({ stdin: /** @type {any} */ (stdin), stdout: makeBuf() })
assert.equal(await settles(confirm('/tmp/hypaware.toml'), 'overwrite confirm on a spent stdin'), true)
})

// An explicit `n` rather than an explicit `y`: the default is a yes, so only
// the decline distinguishes an answered prompt from a defaulted one.
test('overwrite confirm still honours an explicit no', async () => {
const stdin = new PassThrough()
const confirm = defaultOverwriteConfirmFactory({ stdin: /** @type {any} */ (stdin), stdout: makeBuf() })
const answer = confirm('/tmp/hypaware.toml')
stdin.write('n\n')

assert.equal(await settles(answer, 'overwrite confirm with an answer'), false)
})

test('defaults gate takes its stated default on a stdin that ends without a line', async () => {
const stdin = new PassThrough()
const stdout = makeBuf()
const ask = defaultConfirmSelectPromptFactory({
stdin: /** @type {any} */ (stdin),
stdout,
env: { HYP_NO_TUI: '1' },
})
const answer = ask(/** @type {any} */ ({
title: 'Record these sources?',
options: [
{ value: 'accept', label: 'Yes, record these' },
{ value: 'choose', label: 'Let me choose' },
],
default: 'choose',
}))
stdin.end()

assert.equal(await settles(answer, 'defaults gate at EOF'), 'choose')
assert.match(stdout.text(), /select \[2\]: $/)
})

test('defaults gate still honours an explicit pick', async () => {
const stdin = new PassThrough()
const ask = defaultConfirmSelectPromptFactory({
stdin: /** @type {any} */ (stdin),
stdout: makeBuf(),
env: { HYP_NO_TUI: '1' },
})
const answer = ask(/** @type {any} */ ({
title: 'Record these sources?',
options: [
{ value: 'accept', label: 'Yes, record these' },
{ value: 'choose', label: 'Let me choose' },
],
default: 'choose',
}))
stdin.write('1\n')

assert.equal(await settles(answer, 'defaults gate with an answer'), 'accept')
})

test('backfill consent takes its printed default on a stdin that ends without a line', async () => {
const stdin = new PassThrough()
const stdout = makeBuf()
const ask = defaultBackfillConsentPromptFactory({
stdin: /** @type {any} */ (stdin),
stdout,
env: { HYP_NO_TUI: '1' },
})
const answer = ask({ providers: ['claude'], retentionDays: 30 })
stdin.end()

assert.equal(await settles(answer, 'backfill consent at EOF'), true)
assert.match(stdout.text(), /\[Y\/n\]: $/)
})

test('backfill consent still honours an explicit no', async () => {
const stdin = new PassThrough()
const ask = defaultBackfillConsentPromptFactory({
stdin: /** @type {any} */ (stdin),
stdout: makeBuf(),
env: { HYP_NO_TUI: '1' },
})
const answer = ask({ providers: ['claude'], retentionDays: 30 })
stdin.write('n\n')

assert.equal(await settles(answer, 'backfill consent with an answer'), false)
})
Loading