From 604db6b3ca9259a037783a5a13f0a4c8a47d3504 Mon Sep 17 00:00:00 2001 From: M Waleed Kadous Date: Sat, 18 Apr 2026 06:31:14 -0700 Subject: [PATCH] [Bugfix #680] Fix: prevent V8 heap exhaustion in gemini consult on large PRs gemini-cli v0.37.x crashes on PR diffs >500KB due to V8 old-space exhaustion in the spawned subprocess. Two complementary mitigations: 1. Set NODE_OPTIONS=--max-old-space-size=8192 on the spawned env. This survives gemini-cli's internal relaunch and gives it enough heap to finish directory walks and buffer large responses. 2. Pipe the prompt via stdin instead of argv. Avoids ARG_MAX limits and prevents V8 from holding the prompt buffer twice (once in argv, once in the readStdin/prompt path). Adds 4 regression tests covering NODE_OPTIONS, stdio pipe mode, argv cleanliness, and preservation of a caller-supplied NODE_OPTIONS. --- packages/codev/src/__tests__/consult.test.ts | 138 +++++++++++++++++++ packages/codev/src/commands/consult/index.ts | 34 ++++- 2 files changed, 168 insertions(+), 4 deletions(-) diff --git a/packages/codev/src/__tests__/consult.test.ts b/packages/codev/src/__tests__/consult.test.ts index 4ec0238c..06634297 100644 --- a/packages/codev/src/__tests__/consult.test.ts +++ b/packages/codev/src/__tests__/consult.test.ts @@ -792,6 +792,144 @@ describe('consult command', () => { }); }); + describe('Gemini large-prompt crash mitigation (Bugfix #680)', () => { + // V8 old-space exhaustion crashed gemini-cli v0.37.x on PR diffs >500KB. + // Fix: bump heap via NODE_OPTIONS and pipe the prompt via stdin (no argv). + + it('should bump NODE_OPTIONS heap when spawning gemini', async () => { + vi.resetModules(); + const { spawn: spawnBefore } = await import('node:child_process'); + vi.mocked(spawnBefore).mockClear(); + + fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true }); + fs.writeFileSync( + path.join(testBaseDir, 'codev', 'roles', 'consultant.md'), + '# Consultant Role' + ); + process.chdir(testBaseDir); + + const { execSync } = await import('node:child_process'); + vi.mocked(execSync).mockImplementation((cmd: string) => { + if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini'); + return Buffer.from(''); + }); + + const { spawn } = await import('node:child_process'); + const { consult } = await import('../commands/consult/index.js'); + + await consult({ model: 'gemini', prompt: 'review this PR' }); + + const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini'); + expect(geminiCall).toBeDefined(); + const spawnOpts = geminiCall![2] as { env?: Record }; + expect(spawnOpts.env).toBeDefined(); + expect(spawnOpts.env!.NODE_OPTIONS).toContain('--max-old-space-size=8192'); + }); + + it('should NOT pass the query as a positional argv to gemini', async () => { + // Large queries on argv risk E2BIG and force V8 to hold the prompt twice. + // The query must flow through stdin, not argv. + vi.resetModules(); + const { spawn: spawnBefore } = await import('node:child_process'); + vi.mocked(spawnBefore).mockClear(); + + fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true }); + fs.writeFileSync( + path.join(testBaseDir, 'codev', 'roles', 'consultant.md'), + '# Consultant Role' + ); + process.chdir(testBaseDir); + + const { execSync } = await import('node:child_process'); + vi.mocked(execSync).mockImplementation((cmd: string) => { + if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini'); + return Buffer.from(''); + }); + + const { spawn } = await import('node:child_process'); + const { consult } = await import('../commands/consult/index.js'); + + const uniqueQuery = 'UNIQUE_BUGFIX_680_SENTINEL_' + Date.now(); + await consult({ model: 'gemini', prompt: uniqueQuery }); + + const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini'); + expect(geminiCall).toBeDefined(); + const args = geminiCall![1] as string[]; + expect(args.some(a => a.includes(uniqueQuery))).toBe(false); + }); + + it('should pipe the query to stdin instead of argv', async () => { + // stdio[0] must be 'pipe' for gemini (so we can write the prompt), not 'ignore'. + vi.resetModules(); + const { spawn: spawnBefore } = await import('node:child_process'); + vi.mocked(spawnBefore).mockClear(); + + fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true }); + fs.writeFileSync( + path.join(testBaseDir, 'codev', 'roles', 'consultant.md'), + '# Consultant Role' + ); + process.chdir(testBaseDir); + + const { execSync } = await import('node:child_process'); + vi.mocked(execSync).mockImplementation((cmd: string) => { + if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini'); + return Buffer.from(''); + }); + + const { spawn } = await import('node:child_process'); + const { consult } = await import('../commands/consult/index.js'); + + await consult({ model: 'gemini', prompt: 'small prompt' }); + + const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini'); + expect(geminiCall).toBeDefined(); + const spawnOpts = geminiCall![2] as { stdio?: Array }; + expect(spawnOpts.stdio).toBeDefined(); + expect(spawnOpts.stdio![0]).toBe('pipe'); + }); + + it('should preserve the caller NODE_OPTIONS when appending max-old-space-size', async () => { + vi.resetModules(); + const { spawn: spawnBefore } = await import('node:child_process'); + vi.mocked(spawnBefore).mockClear(); + + fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true }); + fs.writeFileSync( + path.join(testBaseDir, 'codev', 'roles', 'consultant.md'), + '# Consultant Role' + ); + process.chdir(testBaseDir); + + const { execSync } = await import('node:child_process'); + vi.mocked(execSync).mockImplementation((cmd: string) => { + if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini'); + return Buffer.from(''); + }); + + const priorNodeOptions = process.env.NODE_OPTIONS; + process.env.NODE_OPTIONS = '--enable-source-maps'; + try { + const { spawn } = await import('node:child_process'); + const { consult } = await import('../commands/consult/index.js'); + + await consult({ model: 'gemini', prompt: 'test' }); + + const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini'); + expect(geminiCall).toBeDefined(); + const spawnOpts = geminiCall![2] as { env?: Record }; + expect(spawnOpts.env!.NODE_OPTIONS).toContain('--enable-source-maps'); + expect(spawnOpts.env!.NODE_OPTIONS).toContain('--max-old-space-size=8192'); + } finally { + if (priorNodeOptions === undefined) { + delete process.env.NODE_OPTIONS; + } else { + process.env.NODE_OPTIONS = priorNodeOptions; + } + } + }); + }); + describe('diff stat approach (Bugfix #240)', () => { it('should export getDiffStat for file-based review', async () => { vi.resetModules(); diff --git a/packages/codev/src/commands/consult/index.ts b/packages/codev/src/commands/consult/index.ts index 6f434e8a..1f70ce36 100644 --- a/packages/codev/src/commands/consult/index.ts +++ b/packages/codev/src/commands/consult/index.ts @@ -610,6 +610,9 @@ async function runConsultation( let tempFile: string | null = null; const env: Record = {}; let cmd: string[]; + // When true, the query is written to the child's stdin instead of argv. + // Used for gemini to avoid V8 heap exhaustion on large prompts (#680). + let stdinPayload: string | null = null; if (model === 'gemini') { // Gemini uses GEMINI_SYSTEM_MD env var for role @@ -617,9 +620,20 @@ async function runConsultation( fs.writeFileSync(tempFile, role); env['GEMINI_SYSTEM_MD'] = tempFile; + // Bugfix #680: gemini-cli v0.37.x crashes on large PR diffs (>500KB) due to + // V8 old-space exhaustion in the spawned subprocess. Mitigations: + // 1. Bump heap via NODE_OPTIONS (survives gemini-cli's internal relaunch). + // 2. Pipe the prompt via stdin instead of argv — avoids ARG_MAX and keeps + // V8 from holding the full prompt buffer twice. + env['NODE_OPTIONS'] = [process.env.NODE_OPTIONS ?? '', '--max-old-space-size=8192'] + .join(' ') + .trim(); + stdinPayload = query; + // Use --output-format json to capture token usage/cost in structured output. // Never use --yolo — it allows Gemini to write files (#370). - cmd = [config.cli, '--output-format', 'json', ...config.args, query]; + // No positional query arg: prompt arrives on stdin (triggers non-interactive mode). + cmd = [config.cli, '--output-format', 'json', ...config.args]; } else if (model === 'hermes') { // Hermes does not have a dedicated system prompt flag for single-shot mode. // Include role context at the top of the prompt. @@ -643,18 +657,30 @@ async function runConsultation( throw new Error(`Unknown model: ${model}`); } - // Execute with passthrough stdio - // Use 'ignore' for stdin to prevent blocking when spawned as subprocess + // Execute with passthrough stdio. + // Use 'ignore' for stdin when no payload — prevents blocking when spawned as subprocess. + // Use 'pipe' when we need to stream the prompt in (e.g. gemini, see #680). const fullEnv = { ...process.env, ...env }; const startTime = Date.now(); + const stdinMode: 'ignore' | 'pipe' = stdinPayload !== null ? 'pipe' : 'ignore'; return new Promise((resolve, reject) => { const proc = spawn(cmd[0], cmd.slice(1), { cwd: workspaceRoot, env: fullEnv, - stdio: ['ignore', 'pipe', 'inherit'], + stdio: [stdinMode, 'pipe', 'inherit'], }); + if (stdinPayload !== null && proc.stdin) { + proc.stdin.on('error', (err) => { + // EPIPE can happen if the child exits before reading all input — not fatal. + if ((err as NodeJS.ErrnoException).code !== 'EPIPE') { + reject(err); + } + }); + proc.stdin.end(stdinPayload, 'utf-8'); + } + const chunks: Buffer[] = []; if (proc.stdout) {