From 53888789063712539051e7eb1147704195bda9da Mon Sep 17 00:00:00 2001 From: davidson3556 Date: Wed, 24 Jun 2026 21:06:28 -0700 Subject: [PATCH] fix(cli): route interactive prompts and prelude to stderr (keep stdout pure) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interactive prompts (`prompt.ts` — the API-key prompt during `setup`/`auth configure`, the target prompt during `agent install`) wrote the question and masking to STDOUT, and the "Configuring profile …" prelude defaulted to stdout too. On the interactive path that mixes UI text into stdout — and under `--output json` it breaks the contract that stdout is a single JSON document, so a consumer doing `JSON.parse(stdout)` fails. Default both to stderr: prompts and informational preludes are interactive UI, not result data. stdout now carries only the command's result (the §8.1 stdout-purity principle the repo already enforces elsewhere). stderr is still the user's TTY, so prompts remain visible; the secret is still never echoed. Callers that inject explicit streams are unaffected. Adds regression tests: promptText writes the question to stderr by default, and the configure prelude lands on stderr (not the result stdout). --- src/commands/auth.test.ts | 29 +++++++++++++++++++++++++++++ src/commands/auth.ts | 5 ++++- src/lib/prompt.test.ts | 29 ++++++++++++++++++++++++++++- src/lib/prompt.ts | 10 ++++++++-- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/commands/auth.test.ts b/src/commands/auth.test.ts index d65e6cc..5a65027 100644 --- a/src/commands/auth.test.ts +++ b/src/commands/auth.test.ts @@ -140,6 +140,35 @@ describe('runConfigure', () => { expect(capture.prelude.join('')).toContain('Configuring profile "default"'); }); + it('routes the interactive prelude to stderr by default, keeping stdout for the result', async () => { + // Regression: the prelude used to default to process.stdout, polluting the + // result stream (and the JSON document under --output json). With no + // injected preludeWrite/stderr, the default must land on stderr, not stdout. + const stdout: string[] = []; + const errChunks: string[] = []; + const origErr = process.stderr.write.bind(process.stderr); + (process.stderr as unknown as { write: (c: string) => boolean }).write = c => { + errChunks.push(String(c)); + return true; + }; + try { + await runConfigure( + { profile: 'default', output: 'text', debug: false, fromEnv: false }, + { + stdout: line => stdout.push(line), + prompt: { secret: vi.fn(async () => 'sk-typed') }, + fetchImpl: meOkFetch, + credentialsPath, + env: {}, + }, + ); + } finally { + (process.stderr as unknown as { write: typeof origErr }).write = origErr; + } + expect(errChunks.join('')).toContain('Configuring profile "default"'); + expect(stdout.join('\n')).not.toContain('Configuring profile'); + }); + it('interactive path resolves the endpoint from TESTSPRITE_API_URL without prompting', async () => { const { capture, deps } = makeCapture(); const prompt = { secret: vi.fn(async () => 'sk-typed') }; diff --git a/src/commands/auth.ts b/src/commands/auth.ts index de1eda4..ef9cfee 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -73,7 +73,10 @@ export async function runConfigure(opts: ConfigureOptions, deps: AuthDeps = {}): const env = deps.env ?? process.env; const credentialsPath = deps.credentialsPath ?? defaultCredentialsPath(); const out = makeOutput(opts.output, deps); - const prelude = deps.preludeWrite ?? ((chunk: string) => process.stdout.write(chunk)); + // The "Configuring profile …" prelude is informational, not result data, so + // it defaults to stderr — stdout stays a pure result stream (the configured + // JSON/text), which matters under `--output json` (§8.1 stdout purity). + const prelude = deps.preludeWrite ?? ((chunk: string) => process.stderr.write(chunk)); const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`)); // Normalize the env endpoint: an empty / whitespace-only TESTSPRITE_API_URL is diff --git a/src/lib/prompt.test.ts b/src/lib/prompt.test.ts index a552882..8951b49 100644 --- a/src/lib/prompt.test.ts +++ b/src/lib/prompt.test.ts @@ -1,5 +1,5 @@ import { Readable, Writable } from 'node:stream'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { promptSecret, promptText } from './prompt.js'; class CaptureStream extends Writable { @@ -44,6 +44,33 @@ describe('promptText', () => { const output = new CaptureStream(); expect(await promptText('? ', { input, output })).toBe('eof-no-newline'); }); + + it('writes the question to stderr by default — keeps stdout pure for --output json', async () => { + // Regression: prompts used to default to stdout, which polluted the JSON + // result on the interactive setup/configure path. Interactive UI belongs + // on stderr. Manually swap the global stream writers (prompt reads + // process.stderr/stdout at call time, so this is reliably intercepted). + const errChunks: string[] = []; + const outChunks: string[] = []; + const origErr = process.stderr.write.bind(process.stderr); + const origOut = process.stdout.write.bind(process.stdout); + (process.stderr as unknown as { write: (c: string) => boolean }).write = c => { + errChunks.push(String(c)); + return true; + }; + (process.stdout as unknown as { write: (c: string) => boolean }).write = c => { + outChunks.push(String(c)); + return true; + }; + try { + await promptText('Q: ', { input: Readable.from(['x\n']) }); // no output → default + } finally { + (process.stderr as unknown as { write: typeof origErr }).write = origErr; + (process.stdout as unknown as { write: typeof origOut }).write = origOut; + } + expect(errChunks.join('')).toContain('Q: '); + expect(outChunks.join('')).not.toContain('Q: '); + }); }); describe('promptSecret (non-TTY behavior)', () => { diff --git a/src/lib/prompt.ts b/src/lib/prompt.ts index 96fca29..ee99f61 100644 --- a/src/lib/prompt.ts +++ b/src/lib/prompt.ts @@ -13,14 +13,20 @@ interface RawModeCapable { export async function promptText(question: string, streams: PromptStreams = {}): Promise { const input = streams.input ?? process.stdin; - const output = streams.output ?? process.stdout; + // Prompts are interactive UI, not data — write the question (and any echo) + // to stderr so stdout carries only the command's result. This keeps + // `--output json` stdout a single pure JSON document even on the interactive + // setup / configure path (§8.1 stdout purity). stderr is still the user's + // TTY, so the prompt remains visible. + const output = streams.output ?? process.stderr; output.write(question); return readLine(input, output, false); } export async function promptSecret(question: string, streams: PromptStreams = {}): Promise { const input = streams.input ?? process.stdin; - const output = streams.output ?? process.stdout; + // See promptText: interactive prompt + masking go to stderr, not stdout. + const output = streams.output ?? process.stderr; output.write(question); const inputAsTTY = input as Readable & RawModeCapable;