Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions packages/codev/src/__tests__/consult.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> };
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<string> };
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<string, string> };
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();
Expand Down
34 changes: 30 additions & 4 deletions packages/codev/src/commands/consult/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -610,16 +610,30 @@ async function runConsultation(
let tempFile: string | null = null;
const env: Record<string, string> = {};
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
tempFile = path.join(tmpdir(), `codev-role-${Date.now()}.md`);
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.
Expand All @@ -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) {
Expand Down
Loading