fix(cli): route interactive prompts and prelude to stderr (keep stdout pure)#31
Open
Davidson3556 wants to merge 1 commit into
Open
fix(cli): route interactive prompts and prelude to stderr (keep stdout pure)#31Davidson3556 wants to merge 1 commit into
Davidson3556 wants to merge 1 commit into
Conversation
…t pure) 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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #30
Describe the changes you have made in this PR -
Interactive prompts wrote to stdout, polluting the result stream:
prompt.ts(promptText/promptSecret) defaulted the question + masking to stdout — used by the API-key prompt insetup/auth configureand the target prompt inagent install.Configuring profile …prelude inauth.tsdefaulted to stdout.On any interactive path this mixes UI text into stdout, and under
--output jsonit breaks the contract that stdout is a single JSON document (a consumer doingJSON.parse(stdout)fails on the prompt text). The repo already enforces stdout purity elsewhere (thestdoutPurityhelper, §8.1) — the interactive prompt path was the gap.Change: default both to stderr. Prompts and informational preludes are interactive UI, not result data; stderr is still the user's TTY so they stay visible, and the secret is still never echoed. Callers that inject explicit streams are unaffected.
Demo/Screenshot for feature changes and bug fixes -
The cleanest reproduction uses
agent installwith no--target(it prompts, and is fully local — no key/network). Redirect stdout to a file; the prompt should appear on your terminal (stderr) and the file should hold only the result.Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Problem: the repo treats stdout as a pure result stream (so
--output jsonyields one parseable document and text-mode stdout is script-clean), but interactive prompts and the configure prelude wrote to stdout, violating that on the interactive path.Approach: prompts are UI, not data, so the fix is to default their output stream to stderr.
prompt.tsalready accepts an injectableoutputstream; I only changed the default (process.stdout→process.stderr) inpromptText/promptSecret, and changed theauth.tsprelude's default writer the same way. stderr is the controlling terminal in an interactive session, so the prompt stays visible; nothing about masking or the "never echo the secret" behavior changes.Alternatives considered: (1) gate the stream on
--output jsononly — rejected, because text-mode stdout should be clean too (e.g.KEY=$(testsprite … )), and a mode-dependent stream is more surprising than "prompts always go to stderr," which is the conventional behavior for CLIs. (2) Strip the prompt from stdout after the fact — not possible/clean.Edge cases handled/tested: a
promptTextregression test confirms the question goes to stderr (not stdout) when no stream is injected; anauthregression test confirms theConfiguring profile …prelude lands on stderr while the result stays on stdout. Existing prompt tests (which inject an explicitoutputstream) are unaffected, and the secret-not-echoed test still passes.Note on the tests: I swap the global
process.stderr/process.stdoutwriters manually (save/restore) rather thanvi.spyOn, becausevi.spyOn(process.stderr, 'write')did not intercept the writes in this setup — the manual override is reliable since the code readsprocess.stderrat call time.Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.