diff --git a/.changeset/bound-git-diff-output.md b/.changeset/bound-git-diff-output.md new file mode 100644 index 0000000000..1f69c50410 --- /dev/null +++ b/.changeset/bound-git-diff-output.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Limit file diff output by UTF-8 byte size while it is read so large changes no longer cause excessive memory use. diff --git a/packages/agent-core-v2/src/app/git/gitService.ts b/packages/agent-core-v2/src/app/git/gitService.ts index 3b38cb25e5..76c5104dad 100644 --- a/packages/agent-core-v2/src/app/git/gitService.ts +++ b/packages/agent-core-v2/src/app/git/gitService.ts @@ -87,18 +87,23 @@ export class GitService implements IGitService { const hasHead = headRes.exitCode === 0; let diffStdout: string; + let diffTruncated: boolean; if (untracked || !hasHead) { const res = await this.runCommand( 'git', ['diff', '--no-color', '--no-index', '--', '/dev/null', relPath], cwd, + { stdoutMaxBytes: DIFF_MAX_BYTES }, ); if (res.exitCode !== 0 && res.exitCode !== 1) { throw this.gitUnavailable(cwd, res.stderr.trim() || `git diff exit ${res.exitCode}`); } diffStdout = res.stdout; + diffTruncated = res.stdoutTruncated; } else { - const res = await this.runCommand('git', ['diff', '--no-color', 'HEAD', '--', relPath], cwd); + const res = await this.runCommand('git', ['diff', '--no-color', 'HEAD', '--', relPath], cwd, { + stdoutMaxBytes: DIFF_MAX_BYTES, + }); if (res.exitCode !== 0) { throw this.gitUnavailable(cwd, res.stderr.trim() || `git diff exit ${res.exitCode}`); } @@ -114,13 +119,13 @@ export class GitService implements IGitService { } } diffStdout = res.stdout; + diffTruncated = res.stdoutTruncated; } - const truncated = diffStdout.length > DIFF_MAX_BYTES; return { path: relPath, - diff: truncated ? diffStdout.slice(0, DIFF_MAX_BYTES) : diffStdout, - truncated, + diff: diffStdout, + truncated: diffTruncated, }; } @@ -158,12 +163,12 @@ export class GitService implements IGitService { () => ({ ok: false as const }), ); if (!spawned.ok) { - return { exitCode: -1, stdout: '', stderr: '' }; + return { exitCode: -1, stdout: '', stdoutTruncated: false, stderr: '' }; } const { proc } = spawned; const work = Promise.all([ - collect(proc.stdout), + collect(proc.stdout, options.stdoutMaxBytes), collect(proc.stderr), proc.wait().catch(() => -1), ] as const); @@ -173,7 +178,12 @@ export class GitService implements IGitService { try { if (options.timeoutMs === undefined) { const [stdout, stderr, exitCode] = await work; - return { exitCode, stdout, stderr }; + return { + exitCode, + stdout: stdout.value, + stdoutTruncated: stdout.truncated, + stderr: stderr.value, + }; } const timeout = new Promise<'timeout'>((resolve) => { timer = setTimeout(() => resolve('timeout'), options.timeoutMs); @@ -181,19 +191,28 @@ export class GitService implements IGitService { }); const result = await Promise.race([ work.then( - ([stdout, stderr, exitCode]) => - ({ kind: 'done' as const, stdout, stderr, exitCode }), + ([stdout, stderr, exitCode]) => ({ kind: 'done' as const, stdout, stderr, exitCode }), ), timeout.then((kind) => ({ kind })), ]); if (result.kind === 'done') { - return { exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; + return { + exitCode: result.exitCode, + stdout: result.stdout.value, + stdoutTruncated: result.stdout.truncated, + stderr: result.stderr.value, + }; } await proc.kill('SIGKILL').catch(() => {}); const [stdout, stderr] = await work .then(([so, se]) => [so, se] as const) - .catch(() => ['', ''] as const); - return { exitCode: -1, stdout, stderr }; + .catch(() => [createTextCapture(), createTextCapture()] as const); + return { + exitCode: -1, + stdout: stdout.value, + stdoutTruncated: stdout.truncated, + stderr: stderr.value, + }; } finally { if (timer !== undefined) clearTimeout(timer); proc.dispose(); @@ -210,22 +229,63 @@ export class GitService implements IGitService { interface RunResult { readonly exitCode: number; readonly stdout: string; + readonly stdoutTruncated: boolean; readonly stderr: string; } interface RunOptions { readonly timeoutMs?: number; readonly env?: Record; + readonly stdoutMaxBytes?: number; +} + +interface TextCapture { + value: string; + bytes: number; + truncated: boolean; } -async function collect(stream: AsyncIterable): Promise { +const UTF8_ENCODER = new TextEncoder(); + +function createTextCapture(): TextCapture { + return { value: '', bytes: 0, truncated: false }; +} + +function appendText(capture: TextCapture, chunk: string, maxBytes?: number): void { + if (maxBytes === undefined) { + capture.value += chunk; + return; + } + if (capture.truncated) return; + + const remaining = maxBytes - capture.bytes; + if (remaining <= 0) { + capture.truncated ||= chunk.length > 0; + return; + } + + const destination = new Uint8Array(Math.min(remaining, chunk.length * 3)); + const { read, written } = UTF8_ENCODER.encodeInto(chunk, destination); + capture.value += chunk.slice(0, read); + capture.bytes += written; + capture.truncated ||= read < chunk.length; +} + +async function collect( + stream: AsyncIterable, + maxBytes?: number, +): Promise { const decoder = new TextDecoder(); - let out = ''; + const capture = createTextCapture(); for await (const chunk of stream) { - out += typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); + appendText( + capture, + typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }), + maxBytes, + ); } - out += decoder.decode(); - return out; + appendText(capture, decoder.decode(), maxBytes); + return capture; } registerScopedService(LifecycleScope.App, IGitService, GitService, InstantiationType.Eager, 'git'); diff --git a/packages/agent-core-v2/test/app/git/gitService.test.ts b/packages/agent-core-v2/test/app/git/gitService.test.ts index ca7a569a03..18f85563a2 100644 --- a/packages/agent-core-v2/test/app/git/gitService.test.ts +++ b/packages/agent-core-v2/test/app/git/gitService.test.ts @@ -126,6 +126,15 @@ describe('GitService', () => { expect(result.diff).toContain('+brand new'); }); + it('limits a large UTF-8 diff to one MiB', async () => { + writeFileSync(join(repo, 'large.txt'), '界'.repeat(400_000)); + + const result = await service.diff(repo, 'large.txt', join(repo, 'large.txt')); + + expect(result.truncated).toBe(true); + expect(Buffer.byteLength(result.diff, 'utf8')).toBeLessThanOrEqual(1_048_576); + }); + it('throws FS_PATH_NOT_FOUND for a missing path', async () => { writeFileSync(join(repo, 'a.txt'), 'hello\n'); commitAll('init'); diff --git a/packages/agent-core/src/services/fs/fsGitService.ts b/packages/agent-core/src/services/fs/fsGitService.ts index d47b0f3fb9..b45af28aca 100644 --- a/packages/agent-core/src/services/fs/fsGitService.ts +++ b/packages/agent-core/src/services/fs/fsGitService.ts @@ -172,6 +172,7 @@ export class FsGitService extends Disposable implements IFsGitService { 'git', ['diff', '--no-color', '--no-index', '--', '/dev/null', rel], realCwd, + { stdoutMaxBytes: DIFF_MAX_BYTES }, ); if (diffRes.exitCode !== 0 && diffRes.exitCode !== 1) { throw new FsGitUnavailableError( @@ -184,6 +185,7 @@ export class FsGitService extends Disposable implements IFsGitService { 'git', ['diff', '--no-color', 'HEAD', '--', rel], realCwd, + { stdoutMaxBytes: DIFF_MAX_BYTES }, ); if (diffRes.exitCode !== 0) { throw new FsGitUnavailableError( @@ -202,12 +204,10 @@ export class FsGitService extends Disposable implements IFsGitService { } } - const full = diffRes.stdout; - const truncated = full.length > DIFF_MAX_BYTES; return { path: rel, - diff: truncated ? full.slice(0, DIFF_MAX_BYTES) : full, - truncated, + diff: diffRes.stdout, + truncated: diffRes.stdoutTruncated, }; } } @@ -215,12 +215,14 @@ export class FsGitService extends Disposable implements IFsGitService { interface RunResult { exitCode: number; stdout: string; + stdoutTruncated: boolean; stderr: string; } interface RunCommandOptions { readonly timeoutMs?: number; readonly env?: NodeJS.ProcessEnv; + readonly stdoutMaxBytes?: number; } async function runCommand( @@ -236,7 +238,7 @@ async function runCommand( env: options.env ? { ...process.env, ...options.env } : process.env, windowsHide: true, }); - let stdout = ''; + const stdout = createTextCapture(); let stderr = ''; let settled = false; let timer: ReturnType | undefined; @@ -249,27 +251,74 @@ async function runCommand( if (options.timeoutMs !== undefined) { timer = setTimeout(() => { killChild(child); - finish({ exitCode: -1, stdout, stderr }); + finish({ + exitCode: -1, + stdout: stdout.value, + stdoutTruncated: stdout.truncated, + stderr, + }); }, options.timeoutMs); timer.unref?.(); } child.stdout.setEncoding('utf-8'); child.stderr.setEncoding('utf-8'); child.stdout.on('data', (c: string) => { - stdout += c; + appendText(stdout, c, options.stdoutMaxBytes); }); child.stderr.on('data', (c: string) => { stderr += c; }); child.once('error', () => { - finish({ exitCode: -1, stdout, stderr }); + finish({ + exitCode: -1, + stdout: stdout.value, + stdoutTruncated: stdout.truncated, + stderr, + }); }); child.once('close', (code) => { - finish({ exitCode: code ?? -1, stdout, stderr }); + finish({ + exitCode: code ?? -1, + stdout: stdout.value, + stdoutTruncated: stdout.truncated, + stderr, + }); }); }); } +interface TextCapture { + value: string; + bytes: number; + truncated: boolean; +} + +const UTF8_ENCODER = new TextEncoder(); + +function createTextCapture(): TextCapture { + return { value: '', bytes: 0, truncated: false }; +} + +function appendText(capture: TextCapture, chunk: string, maxBytes?: number): void { + if (maxBytes === undefined) { + capture.value += chunk; + return; + } + if (capture.truncated) return; + + const remaining = maxBytes - capture.bytes; + if (remaining <= 0) { + capture.truncated ||= chunk.length > 0; + return; + } + + const destination = new Uint8Array(Math.min(remaining, chunk.length * 3)); + const { read, written } = UTF8_ENCODER.encodeInto(chunk, destination); + capture.value += chunk.slice(0, read); + capture.bytes += written; + capture.truncated ||= read < chunk.length; +} + function killChild(child: ChildProcess): void { // On Windows, `ChildProcess.kill()` only signals the direct child (e.g. the // `cmd.exe` wrapper when `shell` is involved, or the `git`/`gh` parent), diff --git a/packages/agent-core/test/services/fs-git-service.test.ts b/packages/agent-core/test/services/fs-git-service.test.ts index 918dad5250..3b22a23798 100644 --- a/packages/agent-core/test/services/fs-git-service.test.ts +++ b/packages/agent-core/test/services/fs-git-service.test.ts @@ -31,7 +31,7 @@ interface FakeChild extends EventEmitter { kill: ReturnType; cmd: string; args: readonly string[]; - finish(out: string, code: number): void; + finish(out: string | readonly string[], code: number): void; } const spawned: FakeChild[] = []; @@ -51,8 +51,11 @@ function createFakeChild(cmd: string, args: readonly string[]): FakeChild { child.kill = vi.fn(); child.cmd = cmd; child.args = args; - child.finish = (out: string, code: number) => { - if (out.length > 0) stdout.emit('data', out); + child.finish = (out: string | readonly string[], code: number) => { + const chunks = typeof out === 'string' ? [out] : out; + for (const chunk of chunks) { + if (chunk.length > 0) stdout.emit('data', chunk); + } child.emit('close', code); }; return child; @@ -93,7 +96,7 @@ async function waitForSpawn(n: number): Promise { } } -function finishLatest(out: string, code: number): void { +function finishLatest(out: string | readonly string[], code: number): void { const child = spawned[spawned.length - 1]; if (child === undefined) throw new Error('no child to finish'); child.finish(out, code); @@ -118,6 +121,19 @@ async function driveStatus(service: FsGitService) { return p; } +async function driveDiff(service: FsGitService, diff: string | readonly string[]) { + const p = service.diff('sid', { path: 'large.txt' }); + await waitForSpawn(1); + finishLatest('true\n', 0); + await waitForSpawn(2); + finishLatest('?? large.txt\n', 0); + await waitForSpawn(3); + finishLatest('', 1); + await waitForSpawn(4); + finishLatest(diff, 1); + return p; +} + describe('FsGitService pull request lookup', () => { it('returns a normalized pull request when gh pr view succeeds', async () => { ghResponse = { @@ -193,3 +209,27 @@ describe('FsGitService pull request lookup', () => { expect(spawned[2]?.kill).toHaveBeenCalled(); }); }); + +describe('FsGitService diff output', () => { + it('limits a large UTF-8 diff to one MiB', async () => { + const service = new FsGitService(sessions); + + const result = await driveDiff(service, '界'.repeat(400_000)); + + expect(result.truncated).toBe(true); + expect(Buffer.byteLength(result.diff, 'utf8')).toBeLessThanOrEqual(1_048_576); + }); + + it('stops after the first discarded character across stdout chunks', async () => { + const service = new FsGitService(sessions); + const prefix = 'a'.repeat(1_048_574); + + const result = await driveDiff(service, [`${prefix}界`, 'AB']); + + expect(result).toEqual({ + path: 'large.txt', + diff: prefix, + truncated: true, + }); + }); +});