Skip to content

Commit 7efeeba

Browse files
committed
Bound cumulative Git blob memory per review history
1 parent c88b510 commit 7efeeba

4 files changed

Lines changed: 18 additions & 2 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ Inputs such as `planText` and the ledger must come from the caller. The future `
4343

4444
## Current limits and safety
4545

46-
- History must be linear and descend from the requested base. Merge histories are rejected with a rebase instruction; repositories using object alternates are rejected. Reads are bounded to 500 commits, 32 MiB per Git response, and a 2-second budget per line diff; oversized work fails explicitly.
46+
- History must be linear and descend from the requested base. Merge histories are rejected with a rebase instruction; repositories using object alternates are rejected. Reads are bounded to 500 commits, 32 MiB per Git response, 64 MiB of unique blob bytes across the history (callers may lower `maxBlobBytes`), and a 2-second budget per line diff; oversized work fails explicitly.
4747
- Ownership uses line diffs, not semantic inference. Within one replacement block, new lines inherit all affected owners conservatively. Function context comes from Git hunk headers, not an AST.
4848
- The importer requires an accurate base-file list. It rejects path traversal, Git metadata paths, and traversal through a listed file/symlink/submodule. Runtime symlink and write-scope enforcement belong to the future container/runner; plan validation alone is not a sandbox.
4949
- Allowed commands restrict accidents, not hostile programs or changed scripts. Parsing returns argv and never executes it. An unlisted valid command is a warning and must not run until allowed.

‎docs/implementation/build-step-1.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ Round 3 reviewed the pre-fix commit and repeated the environment finding; the ex
4747

4848
Declined adding rename-only ownership to later text edits. The approved design represents a no-content rename as a separate file-change segment. A real-Git test confirms that a foreign rename remains an Unplanned file card while the later P2 text edit belongs to P2; the rename has not disappeared from review. Text edits before a rename still retain their line ancestry. Scope tests now explicitly require nonempty P2 rows.
4949

50+
## Review round 5
51+
52+
Fixed unbounded accumulation of unique blobs across a history. The adapter checks object size before loading it and enforces a cumulative 64 MiB byte budget; callers can choose a smaller positive limit. A small-budget fixture failed before the fix, then passed with explicit rejection below the required total and success at the exact total. Repeated references to the same blob do not count twice.
53+
5054
## Remaining gates
5155

5256
This is a working foundation, not a completed application or a claim that all implementation tasks are done. T18's pure validation/edit core is present; its agent adapters, import UI, and persistence are pending. Ledger storage, rebase mappings, and the read-only review screen remain next. The already-fixed GitHub check belongs to the later GitHub/runner integration.

‎git/history.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import { resolve as resolvePath } from 'node:path';
44
import type { FileDelta, FileVersion, History } from '../core/linking.ts';
55

66
/** Read-only Git adapter. Never follows working-tree symlinks or runs diff helpers. */
7-
export function readHistory(repo: string, baseRef: string, headRef = 'HEAD'): History {
7+
export function readHistory(repo: string, baseRef: string, headRef = 'HEAD', limits: { maxBlobBytes?: number } = {}): History {
8+
const maxBlobBytes = limits.maxBlobBytes ?? 64 * 1024 * 1024;
9+
if (!Number.isSafeInteger(maxBlobBytes) || maxBlobBytes < 1 || maxBlobBytes > 64 * 1024 * 1024) throw new Error('Blob byte budget must be a positive integer no larger than 64 MiB.');
10+
let blobBytes = 0;
811
// Inherited Git variables can redirect repository, index, config, and object lookup.
912
const environment = Object.fromEntries(Object.entries(process.env).filter(([key]) => !/^GIT_/i.test(key)));
1013
const run = (...args: string[]) => execFileSync('git', ['--no-pager', '--no-replace-objects', '-c', 'core.hooksPath=/dev/null', '-c', 'protocol.allow=never', ...args], {
@@ -32,7 +35,10 @@ export function readHistory(repo: string, baseRef: string, headRef = 'HEAD'): Hi
3235
if (/^0+$/.test(oid)) return null;
3336
if (mode === '160000') return { oid, mode, text: null }; // gitlink is not a local blob
3437
if (!blobs.has(oid)) {
38+
const size = Number(run('cat-file', '-s', oid).toString().trim());
39+
if (!Number.isSafeInteger(size) || size < 0 || size > maxBlobBytes - blobBytes) throw new Error('Review history exceeds the cumulative blob byte budget; choose a narrower base.');
3540
const data = run('cat-file', 'blob', oid);
41+
blobBytes += data.length;
3642
let text: string | null = null;
3743
if (!data.includes(0)) { try { text = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true }).decode(data); } catch { /* binary */ } }
3844
blobs.set(oid, text);

‎test/history.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,9 @@ it('keeps a foreign rename on its file card while attributing later text edits',
193193
expect(text.length).toBeGreaterThan(0);
194194
expect(text.every(s => s.row === 'P2' && s.scope === 'in-scope')).toBe(true);
195195
});
196+
197+
it('fails explicitly when cumulative unique blob bytes exceed the history budget', () => {
198+
const f = fixture(); f.write('a.txt', 'changed text\n'); f.commit('P1');
199+
expect(() => readHistory(f.dir, f.base, 'HEAD', { maxBlobBytes: 20 })).toThrow(/blob byte budget/i);
200+
expect(readHistory(f.dir, f.base, 'HEAD', { maxBlobBytes: 27 }).final).toHaveLength(1);
201+
});

0 commit comments

Comments
 (0)