mirror of
https://github.com/stablyai/orca.git
synced 2026-10-16 00:03:09 +00:00
Handle buffer overflows gracefully and truncate diffs fairly (#5083)
- Gracefully fall back to file-name summaries when staged diffs exceed node/ssh execution maxBuffer limits, preventing generation failures. - Split oversized diffs by file and allocate budget via water-filling, ensuring single huge files do not starve smaller human changes. - Clip truncated diff sections on line boundaries to avoid half-lines.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
@@ -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', () => {
|
||||
|
||||
+22
-9
@@ -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,
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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.',
|
||||
|
||||
@@ -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/)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user