diff --git a/src/main/git/max-buffer-overflow.ts b/src/main/git/max-buffer-overflow.ts new file mode 100644 index 00000000000..ad67d154b20 --- /dev/null +++ b/src/main/git/max-buffer-overflow.ts @@ -0,0 +1,22 @@ +export function isMaxBufferOverflowError(error: unknown): boolean { + if (!error || typeof error !== 'object') { + return false + } + + const maybeError = error as { code?: unknown; message?: unknown } + if (maybeError.code === 'ENOBUFS') { + return true + } + + return typeof maybeError.message === 'string' && /\bmaxBuffer\b/i.test(maybeError.message) +} + +export function describeMaxBufferOverflowError(error: unknown): string { + if (error && typeof error === 'object') { + const message = (error as { message?: unknown }).message + if (typeof message === 'string' && message.length > 0) { + return message + } + } + return String(error) +} diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index b469eecb07f..5dbf9aa5ec4 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -832,6 +832,34 @@ describe('getStagedCommitContext', () => { } ) }) + + it('falls back to the file summary when the staged patch overflows the buffer', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature/ai\n' }) + .mockResolvedValueOnce({ stdout: 'A\thuge.jsonl\n' }) + .mockRejectedValueOnce( + Object.assign(new Error('stdout maxBuffer length exceeded'), { + code: 'ENOBUFS' + }) + ) + + const result = await getStagedCommitContext('/repo') + + expect(result).toEqual({ + branch: 'feature/ai', + stagedSummary: 'A\thuge.jsonl', + stagedPatch: '' + }) + }) + + it('rethrows staged patch failures that are not buffer overflows', async () => { + gitExecFileAsyncMock + .mockResolvedValueOnce({ stdout: 'feature/ai\n' }) + .mockResolvedValueOnce({ stdout: 'M\tREADME.md\n' }) + .mockRejectedValueOnce(new Error('fatal: bad revision')) + + await expect(getStagedCommitContext('/repo')).rejects.toThrow('fatal: bad revision') + }) }) describe('detectConflictOperation', () => { diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 38bd205ba98..6ffc44b9cd5 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -30,6 +30,7 @@ import { } from '../../shared/git-uncommitted-line-stats' import { decodeGitCQuotedPath } from '../../shared/git-cquoted-path' import { gitExecFileAsync, gitExecFileAsyncBuffer, gitOptionalLocksDisabledEnv } from './runner' +import { describeMaxBufferOverflowError, isMaxBufferOverflowError } from './max-buffer-overflow' import { removeSafeUntrackedDiscardTarget, removeSafeUntrackedDiscardTargets @@ -1212,16 +1213,28 @@ export async function getStagedCommitContext( return null } - const { stdout: stagedPatch } = await gitExecFileAsync( - ['diff', '--cached', '--patch', '--minimal', '--no-color', '--no-ext-diff'], - { - cwd: worktreePath, - // Why: the prompt builder truncates large staged patches later. Give git - // enough buffer room to reach that truncation step instead of failing at - // Node's default execFile limit first. - maxBuffer: MAX_STAGED_COMMIT_CONTEXT_BYTES + let stagedPatch = '' + try { + const patchResult = await gitExecFileAsync( + ['diff', '--cached', '--patch', '--minimal', '--no-color', '--no-ext-diff'], + { + cwd: worktreePath, + maxBuffer: MAX_STAGED_COMMIT_CONTEXT_BYTES + } + ) + stagedPatch = patchResult.stdout + } catch (error) { + if (!isMaxBufferOverflowError(error)) { + throw error } - ) + // Why: a very large staged diff overflows maxBuffer (ENOBUFS). The patch is + // optional context that gets truncated to STAGED_DIFF_BYTE_BUDGET anyway, so + // degrade to the file-name summary instead of failing commit-message generation. + console.warn( + '[git] Staged patch too large to read; using file summary only:', + describeMaxBufferOverflowError(error) + ) + } return { branch: branchResult.stdout.trim() || null, diff --git a/src/main/providers/ssh-git-provider.test.ts b/src/main/providers/ssh-git-provider.test.ts index 616faddde02..1b429b25454 100644 --- a/src/main/providers/ssh-git-provider.test.ts +++ b/src/main/providers/ssh-git-provider.test.ts @@ -286,6 +286,40 @@ describe('SshGitProvider', () => { expect(mux.request).toHaveBeenCalledTimes(2) }) + it('getStagedCommitContext falls back when the remote staged patch overflows', async () => { + mux.request.mockImplementation(async (_method, payload) => { + if (payload.args[1] === '--show-current') { + return { stdout: 'feature/ai-commit\n' } + } + if (payload.args[2] === '--name-status') { + return { stdout: 'A\thuge.jsonl\n' } + } + throw Object.assign(new Error('git stdout exceeded maxBuffer.'), { code: 'ENOBUFS' }) + }) + + await expect(provider.getStagedCommitContext('/home/user/repo')).resolves.toEqual({ + branch: 'feature/ai-commit', + stagedSummary: 'A\thuge.jsonl', + stagedPatch: '' + }) + }) + + it('getStagedCommitContext rethrows remote patch failures that are not buffer overflows', async () => { + mux.request.mockImplementation(async (_method, payload) => { + if (payload.args[1] === '--show-current') { + return { stdout: 'feature/ai-commit\n' } + } + if (payload.args[2] === '--name-status') { + return { stdout: 'M\tREADME.md\n' } + } + throw new Error('fatal: bad revision') + }) + + await expect(provider.getStagedCommitContext('/home/user/repo')).rejects.toThrow( + 'fatal: bad revision' + ) + }) + it('executeCommitMessagePlan delegates the prepared plan to the relay', async () => { const execResult = { stdout: 'Update docs', diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index cd8201677b9..4a1751f9a4d 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -22,6 +22,10 @@ import type { CommitMessageDraftContext } from '../../shared/commit-message-gene import type { CommitMessagePlan } from '../../shared/commit-message-plan' import type { RemoteCommitMessageExecResult } from '../text-generation/commit-message-text-generation' import type { RemoteHostPlatform } from '../ssh/ssh-remote-platform' +import { + describeMaxBufferOverflowError, + isMaxBufferOverflowError +} from '../git/max-buffer-overflow' type NonInteractiveExecQueueEntry = { started: boolean @@ -124,10 +128,25 @@ export class SshGitProvider implements IGitProvider { if (!stagedSummary) { return null } - const { stdout: stagedPatch } = await this.exec( - ['diff', '--cached', '--patch', '--minimal', '--no-color', '--no-ext-diff'], - worktreePath - ) + let stagedPatch = '' + try { + const patchResult = await this.exec( + ['diff', '--cached', '--patch', '--minimal', '--no-color', '--no-ext-diff'], + worktreePath + ) + stagedPatch = patchResult.stdout + } catch (error) { + if (!isMaxBufferOverflowError(error)) { + throw error + } + // Why: a very large staged diff can overflow the remote exec buffer. The + // patch is optional context (truncated later anyway), so degrade to the + // file-name summary instead of failing commit-message generation. + console.warn( + '[ssh-git] Staged patch too large to read; using file summary only:', + describeMaxBufferOverflowError(error) + ) + } return { branch: branchResult.stdout.trim() || null, stagedSummary, diff --git a/src/shared/commit-message-generation.test.ts b/src/shared/commit-message-generation.test.ts index 679d2d1d4ec..eb5884991e1 100644 --- a/src/shared/commit-message-generation.test.ts +++ b/src/shared/commit-message-generation.test.ts @@ -33,6 +33,20 @@ describe('buildCommitMessagePrompt', () => { expect(prompt).toContain('Branch: (detached)') expect(prompt).toContain('Additional user prompt:\nUse Conventional Commits.') }) + + it('notes when the diff was omitted so the agent relies on the file list', () => { + const prompt = buildCommitMessagePrompt( + { + branch: 'feature/big-diff', + stagedSummary: 'A\thuge.jsonl', + stagedPatch: '' + }, + '' + ) + + expect(prompt).toContain('Staged files:\nA\thuge.jsonl') + expect(prompt).toContain('diff omitted — too large to read') + }) }) describe('splitGeneratedCommitMessage', () => { diff --git a/src/shared/commit-message-generation.ts b/src/shared/commit-message-generation.ts index 5626f06f53e..a1b87f67801 100644 --- a/src/shared/commit-message-generation.ts +++ b/src/shared/commit-message-generation.ts @@ -35,7 +35,11 @@ export function buildCommitMessagePrompt( context: CommitMessageDraftContext, customPrompt: string ): string { - const patch = truncateDiffForPrompt(context.stagedPatch) + // Why: the staged patch is dropped when it's too large to read, so fall back to + // the file summary and tell the agent why the diff is missing. + const patch = context.stagedPatch.trim() + ? truncateDiffForPrompt(context.stagedPatch) + : '(diff omitted — too large to read; infer the change from the staged file list above)' const base = [ 'You are generating a single git commit message.', 'Return only the commit message text. Do not include a preamble, quotes, or code fences.', diff --git a/src/shared/commit-message-prompt.test.ts b/src/shared/commit-message-prompt.test.ts index c2d2d11d385..436075b397f 100644 --- a/src/shared/commit-message-prompt.test.ts +++ b/src/shared/commit-message-prompt.test.ts @@ -36,16 +36,42 @@ describe('truncateDiffForPrompt', () => { }) it('truncates and appends a marker when over budget', () => { - const oversized = 'A'.repeat(STAGED_DIFF_BYTE_BUDGET + 100) + const oversized = `${'line\n'.repeat(STAGED_DIFF_BYTE_BUDGET / 5 + 100)}` const result = truncateDiffForPrompt(oversized) expect(result.length).toBeLessThan(oversized.length) - expect(result).toMatch(/diff truncated, 100 bytes omitted/) + expect(result).toMatch(/diff truncated, \d+ bytes omitted/) }) - it('honors a custom budget', () => { - const result = truncateDiffForPrompt('abcdefghij', 5) - expect(result.startsWith('abcde')).toBe(true) - expect(result).toMatch(/diff truncated, 5 bytes omitted/) + it('clips on a line boundary so the diff is never cut mid-line', () => { + const diff = `${'keep this line\n'.repeat(40)}` + const result = truncateDiffForPrompt(diff, 95) + const body = result.split('\n...(diff truncated')[0] + // Every retained line is whole. + for (const line of body.split('\n').filter(Boolean)) { + expect(line).toBe('keep this line') + } + }) + + it('keeps clipped output within a tight custom budget', () => { + const files = Array.from( + { length: 20 }, + (_, i) => `diff --git a/file-${i}.txt b/file-${i}.txt\n${'+x\n'.repeat(200)}` + ).join('') + const result = truncateDiffForPrompt(files, 120) + + expect(result.length).toBeLessThanOrEqual(120) + }) + + it('shares the budget fairly so a huge file does not starve a small one', () => { + const hugeFile = `diff --git a/data.jsonl b/data.jsonl\n${'+x\n'.repeat(5000)}` + const smallFile = 'diff --git a/src/app.ts b/src/app.ts\n+const meaningful = true\n' + const result = truncateDiffForPrompt(`${hugeFile}${smallFile}`, 1_000) + + // The small, human-authored change survives instead of being cut off. + expect(result).toContain('a/src/app.ts') + expect(result).toContain('const meaningful = true') + // The huge file is clipped, not the small one. + expect(result).toMatch(/diff truncated, \d+ bytes omitted/) }) }) diff --git a/src/shared/commit-message-prompt.ts b/src/shared/commit-message-prompt.ts index b16aafd9fec..f8aeb495a1a 100644 --- a/src/shared/commit-message-prompt.ts +++ b/src/shared/commit-message-prompt.ts @@ -30,8 +30,82 @@ export function buildCommitPrompt(diff: string, customSuffix: string): string { export const STAGED_DIFF_BYTE_BUDGET = 200_000 -/** Truncates a diff that exceeds the byte budget; appends a marker so the agent - * knows the input was clipped. */ +/** Splits a unified diff into one section per file, keyed on the `diff --git` + * header. Each section keeps the leading newline that preceded its header so + * concatenating the sections reproduces the original byte-for-byte. */ +function splitDiffIntoFileSections(diff: string): string[] { + const boundary = '\ndiff --git ' + const sections: string[] = [] + let start = 0 + let next = diff.indexOf(boundary) + while (next !== -1) { + // Include the boundary newline in the current section; the next section + // starts at the `diff --git` header itself. + sections.push(diff.slice(start, next + 1)) + start = next + 1 + next = diff.indexOf(boundary, start) + } + sections.push(diff.slice(start)) + return sections +} + +/** Clips one section to `limit` bytes on a line boundary so the agent never sees + * a half-written diff line, and records how many bytes were dropped. */ +function clipSectionOnLineBoundary(section: string, limit: number): string { + if (section.length <= limit) { + return section + } + if (limit <= 0) { + return '' + } + + const markerFor = (omitted: number): string => `\n...(diff truncated, ${omitted} bytes omitted)\n` + let marker = markerFor(section.length) + if (marker.length >= limit) { + return marker.slice(0, limit) + } + + // Reserve headroom for the marker, then back up to the previous newline unless + // that would discard most of the budget (one very long line). + const target = limit - marker.length + const lineBreak = section.lastIndexOf('\n', target) + const cut = lineBreak > target / 2 ? lineBreak : target + const omitted = section.length - cut + marker = markerFor(omitted) + return `${section.slice(0, Math.min(cut, Math.max(0, limit - marker.length)))}${marker}` +} + +/** Distributes `budget` across `sizes` by water-filling: everyone starts with an + * equal share, and the slack from files that fit is handed back to the files + * that don't. Keeps one huge generated file from starving the human-authored + * changes elsewhere in the diff. */ +function allocateBudgetFairly(sizes: number[], budget: number): number[] { + const alloc: number[] = Array.from({ length: sizes.length }, () => 0) + let active = sizes.map((_, i) => i) + let remaining = budget + while (active.length > 0 && remaining > 0) { + const share = Math.floor(remaining / active.length) + if (share === 0) { + break + } + const stillActive: number[] = [] + for (const i of active) { + const need = sizes[i] - alloc[i] + const grant = Math.min(need, share) + alloc[i] += grant + remaining -= grant + if (grant < need) { + stillActive.push(i) + } + } + active = stillActive + } + return alloc +} + +/** Truncates a diff that exceeds the byte budget. Splits the budget fairly across + * files and clips on line boundaries, so a single oversized file can't crowd out + * the rest and the agent never receives a malformed diff. */ export function truncateDiffForPrompt( diff: string, budget: number = STAGED_DIFF_BYTE_BUDGET @@ -39,8 +113,15 @@ export function truncateDiffForPrompt( if (diff.length <= budget) { return diff } - const omitted = diff.length - budget - return `${diff.slice(0, budget)}\n...(diff truncated, ${omitted} bytes omitted)` + const sections = splitDiffIntoFileSections(diff) + if (sections.length <= 1) { + return clipSectionOnLineBoundary(diff, budget) + } + const allocations = allocateBudgetFairly( + sections.map((section) => section.length), + budget + ) + return sections.map((section, i) => clipSectionOnLineBoundary(section, allocations[i])).join('') } /** Strips noise around the agent's output: surrounding whitespace, a single