feat(cli): allow -i/--prompt-interactive with piped stdin#23414
feat(cli): allow -i/--prompt-interactive with piped stdin#23414GoBeromsu wants to merge 2 commits intogoogle-gemini:mainfrom
Conversation
Reads piped stdin line-by-line as an async generator with safety limits matching readStdin(): - Per-line: 8MB cap (truncates lines exceeding this) - Total session: 8MB cumulative (stops reading when exceeded) Reads raw chunks instead of using readline.createInterface to enforce size caps during buffering and prevent OOM from input without newline delimiters.
When stdin is not a TTY, -i now falls through to a chunk-based stdin reader instead of exiting with FATAL_INPUT_ERROR. This enables multi-turn interactive sessions over pipes: - TTY + -i: Ink TUI (unchanged) - Pipe + -i: readStdinLines() with session persistence - Pipe without -i: readStdin drain as single prompt (unchanged) - ACP mode: unaffected Session state persists via shared GeminiClient instance. The pre-sandbox readStdin() is also guarded for -i to avoid draining stdin before the interactive path. Closes google-gemini#13924
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the gemini CLI's flexibility by allowing the -i/--prompt-interactive flag to function with piped stdin. Previously, this combination would result in an error due to TTY requirements. The changes introduce a robust line-by-line stdin reader with built-in safety limits, enabling programmatic consumers to engage in multi-turn interactive sessions with session persistence, thereby broadening the CLI's integration capabilities. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively extends the -i/--prompt-interactive flag to support piped stdin, which is a great enhancement for programmatic use. The introduction of the prompts async generator in gemini.tsx is a clean way to handle different input modes. However, I've identified a significant issue in the new readStdinLines.ts utility concerning the handling of byte limits and multi-byte characters, which could lead to incorrect behavior and data corruption. Please see the detailed comment.
| export async function* readStdinLines( | ||
| stream: NodeJS.ReadableStream = process.stdin, | ||
| ): AsyncGenerator<string> { | ||
| const MAX_LINE_SIZE = 8 * 1024 * 1024; // 8MB per line | ||
| const MAX_TOTAL_SIZE = 8 * 1024 * 1024; // 8MB cumulative | ||
| let buffer = ''; | ||
| let totalSize = 0; | ||
| stream.setEncoding('utf8'); | ||
| for await (const chunk of stream) { | ||
| buffer += chunk; | ||
| let newlineIdx: number; | ||
| while ((newlineIdx = buffer.indexOf('\n')) !== -1) { | ||
| const line = buffer.slice(0, newlineIdx).trim(); | ||
| buffer = buffer.slice(newlineIdx + 1); | ||
| if (!line) continue; | ||
| totalSize += line.length; | ||
| if (totalSize > MAX_TOTAL_SIZE) { | ||
| debugLogger.warn( | ||
| `Total piped input exceeds ${MAX_TOTAL_SIZE} bytes, stopping.`, | ||
| ); | ||
| return; | ||
| } | ||
| yield line.length > MAX_LINE_SIZE ? line.slice(0, MAX_LINE_SIZE) : line; | ||
| } | ||
| // Flush buffer if it exceeds per-line limit without a newline | ||
| if (buffer.length > MAX_LINE_SIZE) { | ||
| debugLogger.warn( | ||
| `Stdin line exceeds ${MAX_LINE_SIZE} bytes, truncating.`, | ||
| ); | ||
| const line = buffer.slice(0, MAX_LINE_SIZE).trim(); | ||
| buffer = ''; | ||
| if (line) { | ||
| totalSize += line.length; | ||
| if (totalSize > MAX_TOTAL_SIZE) return; | ||
| yield line; | ||
| } | ||
| } | ||
| } | ||
| // Flush remaining buffer after EOF | ||
| const remaining = buffer.trim(); | ||
| if (remaining && totalSize + remaining.length <= MAX_TOTAL_SIZE) { | ||
| yield remaining.length > MAX_LINE_SIZE | ||
| ? remaining.slice(0, MAX_LINE_SIZE) | ||
| : remaining; | ||
| } | ||
| } |
There was a problem hiding this comment.
This function has two related issues regarding the handling of byte limits and multi-byte Unicode characters:
-
Incorrect Byte Counting: The variables
MAX_LINE_SIZEandMAX_TOTAL_SIZEare defined in bytes (8MB), but their usage is based onstring.length, which counts UTF-16 code units, not bytes. For multi-byte characters (like many emojis or non-latin scripts),string.lengthwill not equal the number of bytes, leading to incorrect enforcement of the size limits. You should useBuffer.byteLength(line, 'utf8')to get the actual byte size of the string. -
Unsafe String Truncation: The use of
string.slice()to truncate lines can corrupt multi-byte characters by splitting them in the middle, which can lead to malformed data. The repository's general rules advise against this. A safer, though not perfect, approach for truncation is to work with Buffers:Buffer.from(line).slice(0, MAX_LINE_SIZE).toString('utf8').
Given these issues, I recommend refactoring this function to correctly handle byte lengths and perform safer truncation.
References
- When truncating strings that may contain multi-byte Unicode characters (e.g., emojis), use methods that operate on grapheme clusters (like
Intl.SegmenterorArray.from()) instead of UTF-16 code units (string.length,string.slice()) to prevent character splitting.
There was a problem hiding this comment.
This follows the same pattern as the existing readStdin() (packages/cli/src/utils/readStdin.ts), which also uses setEncoding('utf8') + string.length for size tracking and string.slice() for truncation (lines 14, 33-35). The byte-counting concern is valid in theory, but changing it here would diverge from the established pattern — fixing it would be better suited as a separate PR that addresses both readStdin() and readStdinLines() together.
|
Hello @GoBeromsu, Nice improvement 🙂 Supporting One small observation: the limits in Not blocking for this PR since
Overall, the unified |
|
Hi @sairohithpasupuleti Thanks. I agree — I’d like to fix the points you mentioned. While looking into it, I also found similar patterns in other parts of the codebase. The string.length vs UTF-8 byte limit issue makes sense. Since readStdin() currently follows the same approach, I think it would be better to update both utilities together and switch to Buffer.byteLength(..., 'utf8') for consistency and more accurate limits. |
|
Hello @GoBeromsu, |
Summary
Extends
-i/--prompt-interactiveto work whenstdin.isTTYis false, enabling multi-turn interactive sessions over pipes for programmatic consumers.Problem: When the CLI is launched programmatically (e.g., from Node.js, Java, or a backend service),
stdinis not a TTY. Using-iwith piped stdin immediately exited withFATAL_INPUT_ERRORbecause Ink TUI requires TTY. This forced programmatic consumers to either use single-shot-pwith no session persistence, or adopt the full ACP protocol.Solution: When
-iis used with piped stdin, skip Ink TUI and fall through to a chunk-based stdin reader that processes each line as a separate prompt. Session state persists via sharedGeminiClientinstance.Changes
readStdinLines.ts(new utility): Reads piped stdin line-by-line with safety limits matchingreadStdin():readline.createInterface)gemini.tsx:-i+ non-TTY error guardstartInteractiveUI()onisInteractive() && isTTYreadStdin()drain when-iis active (stdin consumed byreadStdinLinesinstead)readStdin()for-ito prevent early stdin consumptionprompts()async generatorBehavior matrix
-i+ TTY-i+ piped stdinFATAL_INPUT_ERROR-p+ piped stdin--acpDesign decisions
-ialready means "execute prompt and continue interactive." Extending it to non-TTY is the natural reading of Issue Allow interactive mode to stay alive even when process.stdin.isTTY is false #13924.readStdinLinesvsreadline.createInterface:readlinebuffers entire lines unbounded until\narrives, risking OOM. Our chunk-based reader enforces the 8MB cap during buffering, matchingreadStdin()'s defense-in-depth pattern.readStdin()'s safety limit.Known limitations
--promptargs. Multi-turn not supported in sandbox (follow-up).--output-format stream-jsonfor explicitRESULTevents.Test plan
-i+ non-TTY no longer exits withFATAL_INPUT_ERRORprintf "what is 1+1?\n" | gemini -i "hello"— two responses-istill launches Ink TUI-punchanged--acpunchangedCloses #13924