mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-18 05:59:56 +08:00
* fix(archive): stop non-TTY confirm prompts from writing ANSI escapes to stdout `openspec archive` asks up to three yes/no questions through @inquirer's `confirm`, which renders by writing ANSI cursor-movement escape sequences — and emits them even when stdout is not a TTY. When archive runs with its output captured to a file or pipe (an agent's background task, CI), those escapes are noise, and in some non-TTY hosts the render loop never settles and repeats `ESC[NNG` moves until the disk fills (reporter hit 19.8 GB). Add `confirmPrompt` in interactive.ts: a real terminal (stdin AND stdout TTY) still gets @inquirer's rich prompt; every other case reads one plain line via node:readline with `terminal:false`, emitting no escapes. Parsing mirrors @inquirer/confirm exactly (prefix match on y/yes and n/no, else the default), and an unreadable stdin rejects with an ExitPromptError-shaped error so the existing #1479 "rerun with --yes" guidance is unchanged. archive's confirmOrBlock now calls confirmPrompt. Closes #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(interactive): cover Windows CRLF and drained-stdin paths; doc note Adds two regression tests surfaced by adversarial review of the #1526 fix: - Windows CRLF piped input (`y\r\n`) parses as a clean yes with no ANSI — the reporter's platform, previously untested (all inputs used `\n`). - A second prompt after stdin was already drained blocks with an ExitPromptError instead of hanging, exercising the readableEnded guard. Also documents in troubleshooting.md that a redirected/agent archive run that pipes an answer no longer writes terminal escape codes into the capture. Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(interactive): align non-interactive classification and handle readline errors Addresses two review findings on the #1526 confirm-prompt fix: - confirmPrompt drops to the plain reader whenever either stream is not a TTY, but isNonInteractivePromptError only checked stdin. A stdin-TTY / stdout-redirected run that hit EOF leaked the raw ExitPromptError instead of the #1479 "rerun with --yes" guidance. Classification now also counts a redirected stdout, matching how the prompt mode is chosen. (isInteractive, used broadly elsewhere, is left untouched.) - readYesNo never listened for the readline/input 'error' event, so a stdin error would hang the promise (and go unhandled). It now settles with the underlying fault, guarded so the promise resolves or rejects exactly once. Tests: TTY-stdin/redirected-stdout EOF is classified non-interactive; an erroring input stream rejects instead of hanging; the archive usable-terminal test now models a full terminal (both streams TTY). Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(archive): gate the change picker on a TTY and tidy the reader Follow-ups from a second review round: - selectChange (the no-argument change picker) called @inquirer's `select` unconditionally. `select` writes ANSI escapes to stdout even when redirected — the same #1526 mechanism the confirm prompts were fixed for — so `openspec archive > log.txt` with no change name still spewed cursor moves into the capture before blocking. Refuse before rendering when either stream is not a TTY, with the same "pass a change name / --yes" guidance the caught ExitPromptError already gives. A new test asserts the picker is never reached in a non-terminal run. - readYesNo now removes its input-stream 'error' listener on every settle path (it lives on the long-lived process.stdin) and closes the readline interface on error too, so nothing accumulates across archive's sequential prompts. - troubleshooting.md now notes the picker also stays clean. Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): add patch changeset for the archive non-TTY fix (#1526) User-facing patch note for the archive ANSI/disk-fill fix. Also drops an unnecessary optional-chain on the non-nullable readline handle in readYesNo (the listener is only attached after the interface exists). Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
179 lines
7.0 KiB
TypeScript
179 lines
7.0 KiB
TypeScript
import { createInterface } from 'node:readline';
|
|
import type { Readable, Writable } from 'node:stream';
|
|
|
|
export type InteractiveOptions = {
|
|
/**
|
|
* Explicit "disable prompts" flag passed by internal callers.
|
|
*/
|
|
noInteractive?: boolean;
|
|
/**
|
|
* Commander-style negated option: `--no-interactive` sets this to false.
|
|
*/
|
|
interactive?: boolean;
|
|
};
|
|
|
|
/**
|
|
* Resolves whether non-interactive mode is requested.
|
|
* Handles both explicit `noInteractive: true` and Commander.js style `interactive: false`.
|
|
* Use this helper instead of manually checking options.noInteractive to avoid bugs.
|
|
*/
|
|
export function resolveNoInteractive(value?: boolean | InteractiveOptions): boolean {
|
|
if (typeof value === 'boolean') return value;
|
|
return value?.noInteractive === true || value?.interactive === false;
|
|
}
|
|
|
|
export function isInteractive(value?: boolean | InteractiveOptions): boolean {
|
|
if (resolveNoInteractive(value)) return false;
|
|
if (process.env.OPEN_SPEC_INTERACTIVE === '0') return false;
|
|
// Respect the standard CI environment variable (set by GitHub Actions, GitLab CI, Travis, etc.)
|
|
if ('CI' in process.env) return false;
|
|
return !!process.stdin.isTTY;
|
|
}
|
|
|
|
/**
|
|
* True when a prompt failed because no answer could be read — an agent or a
|
|
* script that ran the command with stdin closed, a CI job, or a shell whose
|
|
* stdin is not a terminal. @inquirer rejects those with `User force closed
|
|
* the prompt with 0 null`, which is accurate and useless: it names no flag
|
|
* and no next step (#1479).
|
|
*
|
|
* Two things it deliberately is not:
|
|
*
|
|
* - It is not a substitute for `isInteractive()`. This classifies a prompt
|
|
* that has *already failed*, so piped answers are unaffected: an answer
|
|
* that arrives resolves the prompt and never reaches this check. Refusing
|
|
* to prompt up front would break `printf 'y\n' | openspec archive ...`,
|
|
* which works today.
|
|
* - It is not a cancellation check. Ctrl-C raises the same error class, and
|
|
* it reaches a process whose stdin is a pipe just as easily as one at a
|
|
* terminal, so the SIGINT signal - not the terminal - is what proves
|
|
* somebody was there and chose to quit.
|
|
*
|
|
* Beyond that it defers to `isInteractive()`, so `CI`, `OPEN_SPEC_INTERACTIVE=0`
|
|
* and `--no-interactive` count even when a runner allocated a pty. It also
|
|
* counts a redirected stdout: `confirmPrompt` drops to the plain reader whenever
|
|
* *either* stream is not a TTY, so a stdin-TTY-but-stdout-redirected run
|
|
* (`openspec archive x > log.txt` from a terminal) that hits EOF must classify
|
|
* the same way the prompt was selected — otherwise it would leak the raw
|
|
* `ExitPromptError` instead of the `--yes` guidance.
|
|
*/
|
|
export function isNonInteractivePromptError(
|
|
error: unknown,
|
|
value?: boolean | InteractiveOptions
|
|
): boolean {
|
|
if (!(error instanceof Error)) return false;
|
|
const failedPrompt =
|
|
error.name === 'ExitPromptError' || error.message.includes('force closed the prompt');
|
|
if (!failedPrompt) return false;
|
|
if (error.message.includes('SIGINT')) return false;
|
|
return !isInteractive(value) || !process.stdout.isTTY;
|
|
}
|
|
|
|
export type ConfirmPrompt = {
|
|
message: string;
|
|
default: boolean;
|
|
};
|
|
|
|
/**
|
|
* Ask a yes/no question. A real terminal gets @inquirer's rich prompt;
|
|
* everything else — a pipe, a file redirect, an agent that captures stdout —
|
|
* reads one plain line instead.
|
|
*
|
|
* @inquirer renders `confirm` by writing ANSI cursor-movement escape sequences,
|
|
* and it emits them even when stdout is not a TTY. Redirected to a file those
|
|
* sequences are noise, and in some non-TTY hosts the render loop never settles
|
|
* and repeats `ESC[NNG` cursor moves until the disk fills (#1526). Reading
|
|
* the answer ourselves keeps the single piped answer @inquirer ever supported
|
|
* working (`printf 'y\n' | openspec archive ...`) without emitting any escapes.
|
|
*
|
|
* `io` overrides the streams; it exists for tests and mirrors @inquirer's own
|
|
* `{ input, output }` context. Production callers pass only the prompt.
|
|
*/
|
|
export async function confirmPrompt(
|
|
prompt: ConfirmPrompt,
|
|
io: { input?: Readable; output?: Writable } = {}
|
|
): Promise<boolean> {
|
|
const input = io.input ?? process.stdin;
|
|
const output = io.output ?? process.stdout;
|
|
const isTerminal =
|
|
Boolean((input as { isTTY?: boolean }).isTTY) &&
|
|
Boolean((output as { isTTY?: boolean }).isTTY);
|
|
if (isTerminal) {
|
|
const { confirm } = await import('@inquirer/prompts');
|
|
return confirm(prompt);
|
|
}
|
|
return readYesNo(prompt, input, output);
|
|
}
|
|
|
|
function readYesNo(
|
|
prompt: ConfirmPrompt,
|
|
input: Readable,
|
|
output: Writable
|
|
): Promise<boolean> {
|
|
return new Promise((resolve, reject) => {
|
|
let settled = false;
|
|
// Detach the input-stream error listener on settle: `input` is the
|
|
// long-lived process.stdin, so a leftover listener would accumulate across
|
|
// archive's successive prompts and swallow a later, unrelated stdin error.
|
|
const cleanup = () => {
|
|
input.removeListener('error', onError);
|
|
};
|
|
const blockOnNoAnswer = () => {
|
|
if (settled) return;
|
|
settled = true;
|
|
cleanup();
|
|
// No line could be read (stdin closed / EOF). Mirror @inquirer's failure
|
|
// so callers that classify it — isNonInteractivePromptError, the #1479
|
|
// "rerun with --yes" guidance — keep working unchanged.
|
|
const error = new Error('User force closed the prompt');
|
|
error.name = 'ExitPromptError';
|
|
reject(error);
|
|
};
|
|
// A stdin error surfaces on the interface (readline forwards input-stream
|
|
// errors since Node 16). Without a handler the promise would hang and the
|
|
// 'error' would go unhandled; settle it with the real fault instead.
|
|
const onError = (err: unknown) => {
|
|
if (settled) return;
|
|
settled = true;
|
|
cleanup();
|
|
rl.close();
|
|
reject(err instanceof Error ? err : new Error(String(err)));
|
|
};
|
|
// An earlier prompt may have already drained stdin (only one piped answer
|
|
// was ever supported). A fresh readline over an ended stream never emits
|
|
// 'close', so guard here rather than hang and exit as a no-op.
|
|
if (input.readableEnded) {
|
|
blockOnNoAnswer();
|
|
return;
|
|
}
|
|
output.write(`${prompt.message} ${prompt.default ? '(Y/n)' : '(y/N)'} `);
|
|
// terminal:false guarantees readline never emits its own line-editing
|
|
// escapes — an escape-free read is the whole point here.
|
|
const rl = createInterface({ input, terminal: false });
|
|
input.once('error', onError);
|
|
rl.once('error', onError);
|
|
rl.once('line', (line) => {
|
|
if (settled) return;
|
|
settled = true;
|
|
cleanup();
|
|
rl.close();
|
|
output.write('\n');
|
|
// Mirror @inquirer/confirm's parser (prefix match on y/yes and n/no,
|
|
// otherwise the default) so a piped answer resolves identically to the
|
|
// interactive prompt it replaces.
|
|
const answer = line.trim();
|
|
if (/^(y|yes)/i.test(answer)) {
|
|
resolve(true);
|
|
} else if (/^(n|no)/i.test(answer)) {
|
|
resolve(false);
|
|
} else {
|
|
resolve(prompt.default);
|
|
}
|
|
});
|
|
rl.once('close', () => {
|
|
blockOnNoAnswer();
|
|
});
|
|
});
|
|
}
|
|
|