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
5 changes: 5 additions & 0 deletions .changeset/archive-nontty-ansi.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fission-ai/openspec": patch
---

`openspec archive` no longer writes terminal escape codes to a redirected or captured stdout. Its confirmation prompts and the no-argument change picker drew their live UI with ANSI cursor-move sequences even when stdout was not a terminal — noise in a redirected log, and in some non-interactive hosts an unbounded render loop that could grow the captured output until the disk filled. When stdout (or stdin) is not a terminal, archive now reads the confirmations as plain text, and a no-argument run asks you to pass a change name up front instead of drawing a menu. Piped answers (`printf 'y\n' | openspec archive …`) and `--yes` behave as before, and interactive terminals are unchanged. Fixes #1526.
2 changes: 2 additions & 0 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ openspec archive <change-name> --yes

Keep any flags you were already passing — `--skip-specs` and `--no-validate` change what archive does, so a bare `--yes` rerun is not the same command. Current versions name the flag for you and print a `Fix:` line you can paste. If you meant to pick from a list, pass the change name explicitly: the picker needs an answer too.

If you instead ran archive with its output redirected to a file or captured by a tool and *did* pipe an answer (`printf 'y\n' | openspec archive …`), older versions wrote terminal escape codes into that capture while drawing the prompt — in some environments enough to bloat the file badly. Current versions read the confirmation prompts as plain text whenever stdout is not a terminal, and a no-argument `openspec archive` (which would otherwise draw an interactive change picker) asks you to pass a change name up front instead of rendering a menu into the capture. Either way, redirected and agent runs stay clean; passing `--yes` (with a change name) skips the prompts entirely.

## Configuration

### My `config.yaml` isn't being applied
Expand Down
18 changes: 15 additions & 3 deletions src/core/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
} from './specs-apply.js';
import { discoverSpecFiles, hasAnyFileUnder } from '../utils/spec-discovery.js';
import { METADATA_FILENAME, readRetireCapabilitiesMarker, readSkipSpecsMarker } from '../utils/change-metadata.js';
import { isNonInteractivePromptError } from '../utils/interactive.js';
import { confirmPrompt, isNonInteractivePromptError } from '../utils/interactive.js';
import { FileSystemUtils } from '../utils/file-system.js';
import { folderStyleNameProblem } from './id.js';

Expand Down Expand Up @@ -284,9 +284,8 @@ async function confirmOrBlock(
prompt: { message: string; default: boolean },
blocked: () => ArchiveBlockedError
): Promise<boolean> {
const { confirm } = await import('@inquirer/prompts');
try {
return await confirm(prompt);
return await confirmPrompt(prompt);
} catch (error) {
if (isNonInteractivePromptError(error)) {
throw blocked();
Expand Down Expand Up @@ -2011,6 +2010,19 @@ export class ArchiveCommand {
return null;
}

// A picker needs a real terminal, and @inquirer's `select` writes ANSI
// cursor escapes to stdout even when it is redirected — the same #1526
// mechanism the confirm prompts were fixed for. When either stream is not a
// TTY, refuse up front with the guidance the caught ExitPromptError would
// give, rather than render an escape-spewing menu into a pipe or file.
if (!process.stdin.isTTY || !process.stdout.isTTY) {
throw new ArchiveBlockedError(
'archive_change_name_required',
'A change name is required: no terminal is available to choose one from a list.',
withStoreFlag(root, `openspec archive <change-name> ${rerunFlags(options).join(' ')}`)
);
}

// Build choices with progress inline to avoid duplicate lists
let choices: Array<{ name: string; value: string }> = changeDirs.map(name => ({ name, value: name }));
try {
Expand Down
119 changes: 117 additions & 2 deletions src/utils/interactive.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { createInterface } from 'node:readline';
import type { Readable, Writable } from 'node:stream';

export type InteractiveOptions = {
/**
* Explicit "disable prompts" flag passed by internal callers.
Expand Down Expand Up @@ -47,7 +50,12 @@ export function isInteractive(value?: boolean | InteractiveOptions): boolean {
* 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.
* 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,
Expand All @@ -58,6 +66,113 @@ export function isNonInteractivePromptError(
error.name === 'ExitPromptError' || error.message.includes('force closed the prompt');
if (!failedPrompt) return false;
if (error.message.includes('SIGINT')) return false;
return !isInteractive(value);
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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

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();
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
}

Loading
Loading