From d89f7ea9b5e8e851f5529138ada60c466d174cc8 Mon Sep 17 00:00:00 2001 From: Sudheer Tripathi <31629433+dumbbellcode@users.noreply.github.com> Date: Sat, 23 Aug 2025 05:49:20 +0530 Subject: [PATCH] fix(cli): gemini command stuck in git bash (#6397) Co-authored-by: Arya Gummadi --- integration-tests/stdin-context.test.ts | 27 +++++ integration-tests/test-helper.ts | 12 ++- packages/cli/src/gemini.tsx | 4 +- packages/cli/src/utils/readStdin.test.ts | 112 +++++++++++++++++++++ packages/cli/src/utils/readStdin.ts | 121 +++++++++-------------- 5 files changed, 198 insertions(+), 78 deletions(-) create mode 100644 packages/cli/src/utils/readStdin.test.ts diff --git a/integration-tests/stdin-context.test.ts b/integration-tests/stdin-context.test.ts index c7c89a91..912fa0c2 100644 --- a/integration-tests/stdin-context.test.ts +++ b/integration-tests/stdin-context.test.ts @@ -67,4 +67,31 @@ describe('stdin context', () => { 'Expected the model to identify the secret word from stdin', ).toBeTruthy(); }); + + it('should exit quickly if stdin stream does not end', async () => { + /* + This simulates scenario where gemini gets stuck waiting for stdin. + This happens in situations where process.stdin.isTTY is false + even though gemini is intended to run interactively. + */ + + const rig = new TestRig(); + await rig.setup('should exit quickly if stdin stream does not end'); + + try { + await rig.run({ stdinDoesNotEnd: true }); + throw new Error('Expected rig.run to throw an error'); + } catch (error: unknown) { + expect(error).toBeInstanceOf(Error); + const err = error as Error; + + expect(err.message).toContain('Process exited with code 1'); + expect(err.message).toContain('No input provided via stdin.'); + console.log('Error message:', err.message); + } + const lastRequest = rig.readLastApiRequest(); + expect(lastRequest).toBeNull(); + + // If this test times out, runs indefinitely, it's a regression. + }, 3000); }); diff --git a/integration-tests/test-helper.ts b/integration-tests/test-helper.ts index 6ad90ae0..760eff73 100644 --- a/integration-tests/test-helper.ts +++ b/integration-tests/test-helper.ts @@ -177,7 +177,9 @@ export class TestRig { } run( - promptOrOptions: string | { prompt?: string; stdin?: string }, + promptOrOptions: + | string + | { prompt?: string; stdin?: string; stdinDoesNotEnd?: boolean }, ...args: string[] ): Promise { let command = `node ${this.bundlePath} --yolo`; @@ -221,7 +223,13 @@ export class TestRig { if (execOptions.input) { child.stdin!.write(execOptions.input); } - child.stdin!.end(); + + if ( + typeof promptOrOptions === 'object' && + !promptOrOptions.stdinDoesNotEnd + ) { + child.stdin!.end(); + } child.stdout!.on('data', (data: Buffer) => { stdout += data; diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index b285a5af..f9cacfb7 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -342,7 +342,9 @@ export async function main() { } } if (!input) { - console.error('No input provided via stdin.'); + console.error( + `No input provided via stdin. Input can be provided by piping data into gemini or using the --prompt option.`, + ); process.exit(1); } diff --git a/packages/cli/src/utils/readStdin.test.ts b/packages/cli/src/utils/readStdin.test.ts new file mode 100644 index 00000000..6ff5bd6d --- /dev/null +++ b/packages/cli/src/utils/readStdin.test.ts @@ -0,0 +1,112 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi, describe, expect, it, beforeEach, afterEach } from 'vitest'; +import { readStdin } from './readStdin.js'; + +// Mock process.stdin +const mockStdin = { + setEncoding: vi.fn(), + read: vi.fn(), + on: vi.fn(), + removeListener: vi.fn(), + destroy: vi.fn(), +}; + +describe('readStdin', () => { + let originalStdin: typeof process.stdin; + let onReadableHandler: () => void; + let onEndHandler: () => void; + + beforeEach(() => { + vi.clearAllMocks(); + originalStdin = process.stdin; + + // Replace process.stdin with our mock + Object.defineProperty(process, 'stdin', { + value: mockStdin, + writable: true, + configurable: true, + }); + + // Capture event handlers + mockStdin.on.mockImplementation((event: string, handler: () => void) => { + if (event === 'readable') onReadableHandler = handler; + if (event === 'end') onEndHandler = handler; + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + Object.defineProperty(process, 'stdin', { + value: originalStdin, + writable: true, + configurable: true, + }); + }); + + it('should read and accumulate data from stdin', async () => { + mockStdin.read + .mockReturnValueOnce('I love ') + .mockReturnValueOnce('Gemini!') + .mockReturnValueOnce(null); + + const promise = readStdin(); + + // Trigger readable event + onReadableHandler(); + + // Trigger end to resolve + onEndHandler(); + + await expect(promise).resolves.toBe('I love Gemini!'); + }); + + it('should handle empty stdin input', async () => { + mockStdin.read.mockReturnValue(null); + + const promise = readStdin(); + + // Trigger end immediately + onEndHandler(); + + await expect(promise).resolves.toBe(''); + }); + + // Emulate terminals where stdin is not TTY (eg: git bash) + it('should timeout and resolve with empty string when no input is available', async () => { + vi.useFakeTimers(); + + const promise = readStdin(); + + // Fast-forward past the timeout (to run test faster) + vi.advanceTimersByTime(500); + + await expect(promise).resolves.toBe(''); + + vi.useRealTimers(); + }); + + it('should clear timeout once when data is received and resolve with data', async () => { + const clearTimeoutSpy = vi.spyOn(global, 'clearTimeout'); + mockStdin.read + .mockReturnValueOnce('chunk1') + .mockReturnValueOnce('chunk2') + .mockReturnValueOnce(null); + + const promise = readStdin(); + + // Trigger readable event + onReadableHandler(); + + expect(clearTimeoutSpy).toHaveBeenCalledOnce(); + + // Trigger end to resolve + onEndHandler(); + + await expect(promise).resolves.toBe('chunk1chunk2'); + }); +}); diff --git a/packages/cli/src/utils/readStdin.ts b/packages/cli/src/utils/readStdin.ts index 26ebeb3a..3ccdaee7 100644 --- a/packages/cli/src/utils/readStdin.ts +++ b/packages/cli/src/utils/readStdin.ts @@ -9,86 +9,57 @@ export async function readStdin(): Promise { return new Promise((resolve, reject) => { let data = ''; let totalSize = 0; - let hasReceivedData = false; - process.stdin.setEncoding('utf8'); - function cleanup() { - clearTimeout(timeout); + const pipedInputShouldBeAvailableInMs = 500; + let pipedInputTimerId: null | NodeJS.Timeout = setTimeout(() => { + // stop reading if input is not available yet, this is needed + // in terminals where stdin is never TTY and nothing's piped + // which causes the program to get stuck expecting data from stdin + onEnd(); + }, pipedInputShouldBeAvailableInMs); + + const onReadable = () => { + let chunk; + while ((chunk = process.stdin.read()) !== null) { + if (pipedInputTimerId) { + clearTimeout(pipedInputTimerId); + pipedInputTimerId = null; + } + + if (totalSize + chunk.length > MAX_STDIN_SIZE) { + const remainingSize = MAX_STDIN_SIZE - totalSize; + data += chunk.slice(0, remainingSize); + console.warn( + `Warning: stdin input truncated to ${MAX_STDIN_SIZE} bytes.`, + ); + process.stdin.destroy(); // Stop reading further + break; + } + data += chunk; + totalSize += chunk.length; + } + }; + + const onEnd = () => { + cleanup(); + resolve(data); + }; + + const onError = (err: Error) => { + cleanup(); + reject(err); + }; + + const cleanup = () => { + if (pipedInputTimerId) { + clearTimeout(pipedInputTimerId); + pipedInputTimerId = null; + } process.stdin.removeListener('readable', onReadable); process.stdin.removeListener('end', onEnd); process.stdin.removeListener('error', onError); - } - - function processChunk(chunk: string): boolean { - hasReceivedData = true; - if (totalSize + chunk.length > MAX_STDIN_SIZE) { - const remainingSize = MAX_STDIN_SIZE - totalSize; - data += chunk.slice(0, remainingSize); - console.warn( - `Warning: stdin input truncated to ${MAX_STDIN_SIZE} bytes.`, - ); - process.stdin.destroy(); - return true; // Indicates truncation occurred - } else { - data += chunk; - totalSize += chunk.length; - return false; - } - } - - function checkInitialState(): boolean { - if (process.stdin.destroyed || process.stdin.readableEnded) { - cleanup(); - resolve(''); - return true; - } - - const chunk = process.stdin.read(); - if (chunk !== null) { - processChunk(chunk); - return false; - } - - if (!process.stdin.readable) { - cleanup(); - resolve(''); - return true; - } - - return false; - } - - if (checkInitialState()) { - return; - } - - function onReadable() { - let chunk; - while ((chunk = process.stdin.read()) !== null) { - const truncated = processChunk(chunk); - if (truncated) { - break; - } - } - } - - function onEnd() { - cleanup(); - resolve(data); - } - - function onError(err: Error) { - cleanup(); - reject(err); - } - - const timeout = setTimeout(() => { - if (!hasReceivedData) { - cleanup(); - resolve(''); - } - }, 50); + }; process.stdin.on('readable', onReadable); process.stdin.on('end', onEnd);