feat(cli): allow -i/--prompt-interactive with piped stdin#23414
feat(cli): allow -i/--prompt-interactive with piped stdin#23414GoBeromsu wants to merge 4 commits intogoogle-gemini:mainfrom
Conversation
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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.
gemini-cli
bot
added
priority/p2
|
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, |
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
Replace string.length (UTF-16 code units) with Buffer.byteLength for accurate UTF-8 byte tracking in size limits. Add truncateUtf8Bytes helper that walks back past continuation bytes to find the last complete character boundary, preventing multi-byte character corruption. Also fixes totalSize to consistently track post-truncation bytes across all code paths (newline and flush), resolving an inconsistency where a 9MB line with a newline would be dropped while the same content without a newline would be truncated and yielded. Add comprehensive unit tests covering multi-byte truncation, cumulative limits, flush path, and CJK/emoji edge cases. Constraint: readStdin.ts has the same string.length pattern but is not modified here to keep scope narrow — tracked in google-gemini#23537 Rejected: Incremental bufferBytes counter | adds complexity for a path bounded by 8MB cap, not a practical perf concern Confidence: high Scope-risk: narrow
GoBeromsu
force-pushed
the
feat/headless-interactive
branch
from
f3121e0 to
0b1d608
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request extends the -i/--prompt-interactive flag to support multi-turn sessions over piped stdin, which was previously not allowed. The changes involve removing the guard that prevented this, and introducing a new readStdinLines utility to process stdin line-by-line in this mode. The overall implementation is well-structured, but I found a critical issue in the new readStdinLines utility that can lead to data loss when processing the final chunk of data from stdin. My review includes a suggested fix for this issue, with an added note on handling Unicode characters during truncation.
Note: Security Review did not run due to the size of the PR.
| const remaining = buffer.trim(); | ||
| if (remaining) { | ||
| const remainingBytes = Buffer.byteLength(remaining, 'utf8'); | ||
| if (totalSize + remainingBytes <= MAX_TOTAL_SIZE) { | ||
| yield remainingBytes > MAX_LINE_SIZE | ||
| ? truncateUtf8Bytes(remaining, MAX_LINE_SIZE) | ||
| : remaining; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for flushing the remaining buffer at the end of the stream is incorrect and can lead to data loss. The check totalSize + remainingBytes <= MAX_TOTAL_SIZE uses the size of the untruncated remaining buffer. If this buffer is larger than the remaining capacity but could be truncated to fit, it is incorrectly dropped entirely.
For example, if totalSize is 0 and remaining is 9MB, the check 0 + 9MB <= 8MB fails, and nothing is yielded. The expected behavior is to yield a truncated 8MB line.
The logic should first determine the string to be yielded (after potential truncation) and then check if its size fits within the MAX_TOTAL_SIZE limit. When implementing truncateUtf8Bytes, ensure it handles multi-byte Unicode characters (like emojis) correctly by operating on grapheme clusters to prevent character splitting.
const remaining = buffer.trim();
if (remaining) {
const lineToYield =
Buffer.byteLength(remaining, 'utf8') > MAX_LINE_SIZE
? truncateUtf8Bytes(remaining, MAX_LINE_SIZE)
: remaining;
const yieldBytes = Buffer.byteLength(lineToYield, 'utf8');
if (totalSize + yieldBytes <= MAX_TOTAL_SIZE) {
yield lineToYield;
}
}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.
The EOF flush path checked totalSize against the untruncated remaining buffer size. A 9MB buffer with totalSize=0 would fail the 8MB check and yield nothing, even though truncating to 8MB would fit. Now truncates first, then checks if the truncated result fits within the cumulative limit — consistent with the newline and buffer-flush paths. Constraint: Must match truncate-first pattern used in other paths Confidence: high Scope-risk: narrow
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request extends the -i/--prompt-interactive flag to support non-TTY environments, enabling multi-turn interactive sessions over piped stdin. This is achieved by introducing a new readStdinLines utility for safe, line-by-line input processing with size limits, and refactoring the main application logic in gemini.tsx to use an async generator that handles different input modes. The changes appear to be well-implemented and correctly address the described problem, with new tests for the added utility.
Note: Security Review is unavailable for this PR.
| // Unified prompt loop: yields once for single-shot, multiple times for | ||
| // multi-turn pipe sessions. RESULT events signal each response end. | ||
| let promptCount = 0; | ||
| for await (const prompt of prompts(input, config.isInteractive())) { |
There was a problem hiding this comment.
I think, the new piped stdin path has no error handling for broken pipes
Since this PR specifically targets long-running programmatic sessions, the pipe can break at any point - e.g., the parent process crashes, or the network drops in a remote session.
The existing readStdin.ts already handles this with an explicit onError callback (lines 53-56) and a safety net for late errors (lines 70-72). But the new readStdinLines.ts uses a bare for await...of at line 49 with no error handling:
// readStdinLines.ts, line 49 — throws if the stream emits an error
for await (const chunk of stream) {The consumer in gemini.tsx also has no try-catch around the loop at line 659:
// gemini.tsx, line 659 — no try-catch
for await (const prompt of prompts(input, config.isInteractive())) {So when the pipe breaks, the error flies past the cleanup code at lines 690-692 (runExitCleanup() + process.exit), which means telemetry doesn't flush, SessionEnd hooks don't fire, and temp files aren't cleaned up.
A lightweight fix would be wrapping the loop in gemini.tsx:
try {
for await (const prompt of prompts(input, config.isInteractive())) {
promptCount++;
// ... existing loop body ...
}
} catch (err) {
debugLogger.error('Piped stdin stream error:', err);
}
// Cleanup now runs normally regardless of pipe errors
if (promptCount === 0) { ... }
await runExitCleanup();
process.exit(ExitCodes.SUCCESS);This way, a broken pipe logs the error and falls through to the existing cleanup path - same graceful behavior that readStdin.ts already provides.
3 participants
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