Skip to content

Commit 604db6b

Browse files
committed
[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.
1 parent eaefd35 commit 604db6b

2 files changed

Lines changed: 168 additions & 4 deletions

File tree

packages/codev/src/__tests__/consult.test.ts

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,144 @@ describe('consult command', () => {
792792
});
793793
});
794794

795+
describe('Gemini large-prompt crash mitigation (Bugfix #680)', () => {
796+
// V8 old-space exhaustion crashed gemini-cli v0.37.x on PR diffs >500KB.
797+
// Fix: bump heap via NODE_OPTIONS and pipe the prompt via stdin (no argv).
798+
799+
it('should bump NODE_OPTIONS heap when spawning gemini', async () => {
800+
vi.resetModules();
801+
const { spawn: spawnBefore } = await import('node:child_process');
802+
vi.mocked(spawnBefore).mockClear();
803+
804+
fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true });
805+
fs.writeFileSync(
806+
path.join(testBaseDir, 'codev', 'roles', 'consultant.md'),
807+
'# Consultant Role'
808+
);
809+
process.chdir(testBaseDir);
810+
811+
const { execSync } = await import('node:child_process');
812+
vi.mocked(execSync).mockImplementation((cmd: string) => {
813+
if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini');
814+
return Buffer.from('');
815+
});
816+
817+
const { spawn } = await import('node:child_process');
818+
const { consult } = await import('../commands/consult/index.js');
819+
820+
await consult({ model: 'gemini', prompt: 'review this PR' });
821+
822+
const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini');
823+
expect(geminiCall).toBeDefined();
824+
const spawnOpts = geminiCall![2] as { env?: Record<string, string> };
825+
expect(spawnOpts.env).toBeDefined();
826+
expect(spawnOpts.env!.NODE_OPTIONS).toContain('--max-old-space-size=8192');
827+
});
828+
829+
it('should NOT pass the query as a positional argv to gemini', async () => {
830+
// Large queries on argv risk E2BIG and force V8 to hold the prompt twice.
831+
// The query must flow through stdin, not argv.
832+
vi.resetModules();
833+
const { spawn: spawnBefore } = await import('node:child_process');
834+
vi.mocked(spawnBefore).mockClear();
835+
836+
fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true });
837+
fs.writeFileSync(
838+
path.join(testBaseDir, 'codev', 'roles', 'consultant.md'),
839+
'# Consultant Role'
840+
);
841+
process.chdir(testBaseDir);
842+
843+
const { execSync } = await import('node:child_process');
844+
vi.mocked(execSync).mockImplementation((cmd: string) => {
845+
if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini');
846+
return Buffer.from('');
847+
});
848+
849+
const { spawn } = await import('node:child_process');
850+
const { consult } = await import('../commands/consult/index.js');
851+
852+
const uniqueQuery = 'UNIQUE_BUGFIX_680_SENTINEL_' + Date.now();
853+
await consult({ model: 'gemini', prompt: uniqueQuery });
854+
855+
const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini');
856+
expect(geminiCall).toBeDefined();
857+
const args = geminiCall![1] as string[];
858+
expect(args.some(a => a.includes(uniqueQuery))).toBe(false);
859+
});
860+
861+
it('should pipe the query to stdin instead of argv', async () => {
862+
// stdio[0] must be 'pipe' for gemini (so we can write the prompt), not 'ignore'.
863+
vi.resetModules();
864+
const { spawn: spawnBefore } = await import('node:child_process');
865+
vi.mocked(spawnBefore).mockClear();
866+
867+
fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true });
868+
fs.writeFileSync(
869+
path.join(testBaseDir, 'codev', 'roles', 'consultant.md'),
870+
'# Consultant Role'
871+
);
872+
process.chdir(testBaseDir);
873+
874+
const { execSync } = await import('node:child_process');
875+
vi.mocked(execSync).mockImplementation((cmd: string) => {
876+
if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini');
877+
return Buffer.from('');
878+
});
879+
880+
const { spawn } = await import('node:child_process');
881+
const { consult } = await import('../commands/consult/index.js');
882+
883+
await consult({ model: 'gemini', prompt: 'small prompt' });
884+
885+
const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini');
886+
expect(geminiCall).toBeDefined();
887+
const spawnOpts = geminiCall![2] as { stdio?: Array<string> };
888+
expect(spawnOpts.stdio).toBeDefined();
889+
expect(spawnOpts.stdio![0]).toBe('pipe');
890+
});
891+
892+
it('should preserve the caller NODE_OPTIONS when appending max-old-space-size', async () => {
893+
vi.resetModules();
894+
const { spawn: spawnBefore } = await import('node:child_process');
895+
vi.mocked(spawnBefore).mockClear();
896+
897+
fs.mkdirSync(path.join(testBaseDir, 'codev', 'roles'), { recursive: true });
898+
fs.writeFileSync(
899+
path.join(testBaseDir, 'codev', 'roles', 'consultant.md'),
900+
'# Consultant Role'
901+
);
902+
process.chdir(testBaseDir);
903+
904+
const { execSync } = await import('node:child_process');
905+
vi.mocked(execSync).mockImplementation((cmd: string) => {
906+
if (cmd.includes('which')) return Buffer.from('/usr/bin/gemini');
907+
return Buffer.from('');
908+
});
909+
910+
const priorNodeOptions = process.env.NODE_OPTIONS;
911+
process.env.NODE_OPTIONS = '--enable-source-maps';
912+
try {
913+
const { spawn } = await import('node:child_process');
914+
const { consult } = await import('../commands/consult/index.js');
915+
916+
await consult({ model: 'gemini', prompt: 'test' });
917+
918+
const geminiCall = vi.mocked(spawn).mock.calls.find(call => call[0] === 'gemini');
919+
expect(geminiCall).toBeDefined();
920+
const spawnOpts = geminiCall![2] as { env?: Record<string, string> };
921+
expect(spawnOpts.env!.NODE_OPTIONS).toContain('--enable-source-maps');
922+
expect(spawnOpts.env!.NODE_OPTIONS).toContain('--max-old-space-size=8192');
923+
} finally {
924+
if (priorNodeOptions === undefined) {
925+
delete process.env.NODE_OPTIONS;
926+
} else {
927+
process.env.NODE_OPTIONS = priorNodeOptions;
928+
}
929+
}
930+
});
931+
});
932+
795933
describe('diff stat approach (Bugfix #240)', () => {
796934
it('should export getDiffStat for file-based review', async () => {
797935
vi.resetModules();

packages/codev/src/commands/consult/index.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -610,16 +610,30 @@ async function runConsultation(
610610
let tempFile: string | null = null;
611611
const env: Record<string, string> = {};
612612
let cmd: string[];
613+
// When true, the query is written to the child's stdin instead of argv.
614+
// Used for gemini to avoid V8 heap exhaustion on large prompts (#680).
615+
let stdinPayload: string | null = null;
613616

614617
if (model === 'gemini') {
615618
// Gemini uses GEMINI_SYSTEM_MD env var for role
616619
tempFile = path.join(tmpdir(), `codev-role-${Date.now()}.md`);
617620
fs.writeFileSync(tempFile, role);
618621
env['GEMINI_SYSTEM_MD'] = tempFile;
619622

623+
// Bugfix #680: gemini-cli v0.37.x crashes on large PR diffs (>500KB) due to
624+
// V8 old-space exhaustion in the spawned subprocess. Mitigations:
625+
// 1. Bump heap via NODE_OPTIONS (survives gemini-cli's internal relaunch).
626+
// 2. Pipe the prompt via stdin instead of argv — avoids ARG_MAX and keeps
627+
// V8 from holding the full prompt buffer twice.
628+
env['NODE_OPTIONS'] = [process.env.NODE_OPTIONS ?? '', '--max-old-space-size=8192']
629+
.join(' ')
630+
.trim();
631+
stdinPayload = query;
632+
620633
// Use --output-format json to capture token usage/cost in structured output.
621634
// Never use --yolo — it allows Gemini to write files (#370).
622-
cmd = [config.cli, '--output-format', 'json', ...config.args, query];
635+
// No positional query arg: prompt arrives on stdin (triggers non-interactive mode).
636+
cmd = [config.cli, '--output-format', 'json', ...config.args];
623637
} else if (model === 'hermes') {
624638
// Hermes does not have a dedicated system prompt flag for single-shot mode.
625639
// Include role context at the top of the prompt.
@@ -643,18 +657,30 @@ async function runConsultation(
643657
throw new Error(`Unknown model: ${model}`);
644658
}
645659

646-
// Execute with passthrough stdio
647-
// Use 'ignore' for stdin to prevent blocking when spawned as subprocess
660+
// Execute with passthrough stdio.
661+
// Use 'ignore' for stdin when no payload — prevents blocking when spawned as subprocess.
662+
// Use 'pipe' when we need to stream the prompt in (e.g. gemini, see #680).
648663
const fullEnv = { ...process.env, ...env };
649664
const startTime = Date.now();
665+
const stdinMode: 'ignore' | 'pipe' = stdinPayload !== null ? 'pipe' : 'ignore';
650666

651667
return new Promise((resolve, reject) => {
652668
const proc = spawn(cmd[0], cmd.slice(1), {
653669
cwd: workspaceRoot,
654670
env: fullEnv,
655-
stdio: ['ignore', 'pipe', 'inherit'],
671+
stdio: [stdinMode, 'pipe', 'inherit'],
656672
});
657673

674+
if (stdinPayload !== null && proc.stdin) {
675+
proc.stdin.on('error', (err) => {
676+
// EPIPE can happen if the child exits before reading all input — not fatal.
677+
if ((err as NodeJS.ErrnoException).code !== 'EPIPE') {
678+
reject(err);
679+
}
680+
});
681+
proc.stdin.end(stdinPayload, 'utf-8');
682+
}
683+
658684
const chunks: Buffer[] = [];
659685

660686
if (proc.stdout) {

0 commit comments

Comments
 (0)