mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
wave review-53cfb5bfe011: /ship pass 2 fixes
- lib/review-exclude.ts: binaryContent reads each blob's first 8,000 bytes (git cat-file blob, 8 KB buffer) instead of one --batch read capped at 256 MB, so head mode (receipt, carry, queue) and the plan agree past 256 MB of blobs; an unreadable blob is kept, as in live mode. Failing-first test with two 270 MB blobs. - lib/review-bots.ts: a file-level bot review comment (subject_type file, no line position) is triaged instead of read as outdated. Failing-first test. - test/ship-queue-land.test.ts: the merged-resume exclusion test runs its gate scan under the fixture state root, not the real home (CI free-suite 6 home containment).
This commit is contained in:
1 parent
c423539133
commit
e4043dc6af
5 files changed
+45
-15
No files matched your search
+2
-2
@@ -53,7 +53,7 @@ export interface BotComment {
|
||||
interface GhUser { login?: string; type?: string }
|
||||
interface GhItem {
|
||||
id: number; user?: GhUser | null; body?: string | null; html_url?: string; path?: string;
|
||||
line?: number | null; position?: number | null; in_reply_to_id?: number | null; submitted_at?: string | null;
|
||||
line?: number | null; position?: number | null; in_reply_to_id?: number | null; submitted_at?: string | null; subject_type?: string | null;
|
||||
}
|
||||
interface HistoryRow { repo: string; type: string; pattern: string; category: string; bot: string }
|
||||
|
||||
@@ -144,7 +144,7 @@ function fetch(env: ReviewBotsEnv, args: string[]): number {
|
||||
records.push({ id: item.id, bot, source, url: item.html_url ?? "", body: item.body ?? "", suppress_candidates: [], ...extra });
|
||||
for (const c of line as GhItem[]) {
|
||||
const bot = ownerOf(c, bots);
|
||||
if (bot && c.in_reply_to_id == null && c.position != null) add(c, bot, "line", { path: c.path, line: c.line ?? null });
|
||||
if (bot && c.in_reply_to_id == null && (c.position != null || c.subject_type === "file")) add(c, bot, "line", { path: c.path, line: c.line ?? null });
|
||||
}
|
||||
for (const c of top as GhItem[]) {
|
||||
const bot = ownerOf(c, bots);
|
||||
|
||||
+5
-12
@@ -141,18 +141,11 @@ function binaryContent(i: ExcludeInput, files: ReadonlyMap<string, Matched>, cla
|
||||
const sha = real(m.newSha) ? m.newSha : real(m.oldSha) ? m.oldSha : '';
|
||||
if (sha) bySha.set(sha, [...(bySha.get(sha) ?? []), p]);
|
||||
}
|
||||
if (!bySha.size) return out;
|
||||
const r = spawnSync('git', ['cat-file', '--batch'], { cwd: i.root, input: [...bySha.keys()].join('\n') + '\n', timeout: 120_000, maxBuffer: MAX_BUFFER });
|
||||
const buf = r.stdout ?? Buffer.alloc(0);
|
||||
let at = 0;
|
||||
while (at < buf.length) {
|
||||
const nl = buf.indexOf(10, at);
|
||||
if (nl < 0) break;
|
||||
const [sha, , size] = buf.subarray(at, nl).toString('utf8').split(' ');
|
||||
const n = Number(size);
|
||||
if (!Number.isFinite(n)) { at = nl + 1; continue; }
|
||||
if (buf.subarray(nl + 1, nl + 1 + Math.min(n, 8000)).includes(0)) for (const p of bySha.get(sha!) ?? []) out.add(p);
|
||||
at = nl + 1 + n + 1;
|
||||
for (const [sha, paths] of bySha) {
|
||||
const r = spawnSync('git', ['cat-file', 'blob', sha], { cwd: i.root, timeout: 30_000, maxBuffer: 8000 });
|
||||
const read = r.status === 0 || (r.error as NodeJS.ErrnoException | undefined)?.code === 'ENOBUFS';
|
||||
const binary = read && (r.stdout ?? Buffer.alloc(0)).subarray(0, 8000).includes(0);
|
||||
for (const p of paths) if (!read) unread.add(p); else if (binary) out.add(p);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -109,6 +109,16 @@ describe("fetch", () => {
|
||||
expect(r.calls.filter(c => c[0] === "gh" && c[1] === "api").every(c => c.includes("--paginate") && !c.includes("--slurp"))).toBe(true);
|
||||
});
|
||||
|
||||
test("a file-level bot comment (no line position) is triaged; an outdated line comment is not", () => {
|
||||
const r = run({ line: [[
|
||||
{ id: 1, user: bot("coderabbitai[bot]"), path: "auth.ts", line: null, position: null, subject_type: "file", html_url: "u1", body: "missing authorization check" },
|
||||
{ id: 2, user: bot("coderabbitai[bot]"), path: "a.ts", line: 4, position: null, subject_type: "line", html_url: "u2", body: "outdated" },
|
||||
]] }, "fetch");
|
||||
expect(r.code).toBe(0);
|
||||
expect(r.records.map(c => [c.id, c.source, c.path])).toEqual([[1, "line", "auth.ts"]]);
|
||||
expect(r.text).toContain("coderabbit=1");
|
||||
});
|
||||
|
||||
test("humans named like a bot and deleted accounts are never triaged", () => {
|
||||
const r = run({ top: [[
|
||||
{ id: 1, user: human("greptile-ci"), body: "fp please" },
|
||||
|
||||
@@ -359,5 +359,27 @@ describe('a branch cannot hide code under a base pattern', () => {
|
||||
expect(x.kept).toContainEqual({ path: 'receipts/a/shot.png', reason: 'consumer-scan-unavailable' });
|
||||
} finally { fs.chmodSync(gz, 0o644); }
|
||||
}, 120_000);
|
||||
|
||||
// Value: protects=the receipt (head blobs) and the plan (live files) agree on binary content past 256 MB of blobs; fails_when=
|
||||
// one git cat-file --batch read stops at its buffer and later blobs fall through to kept code, so the digests differ and
|
||||
// receipt write refuses on every rerun; why_new=/ship review of the GST-508 wave (Red Team, reproduced at 280 MB); seam=binaryContent
|
||||
test('head mode reads each blob\'s first 8,000 bytes, so blobs after 256 MB of others are still binary', () => {
|
||||
const repo = census();
|
||||
fs.mkdirSync(path.join(repo.work, 'receipts/big'), { recursive: true });
|
||||
const big = Buffer.alloc(270 * 1024 * 1024);
|
||||
fs.writeFileSync(path.join(repo.work, 'receipts/big/a.dat'), big);
|
||||
big[big.length - 1] = 1;
|
||||
fs.writeFileSync(path.join(repo.work, 'receipts/big/c.dat'), big);
|
||||
fs.writeFileSync(path.join(repo.work, 'receipts/big/b.dat'), GZ);
|
||||
commit(repo, {});
|
||||
const head = repo.git('rev-parse', 'HEAD');
|
||||
const baseSha = repo.git('rev-parse', 'origin/main');
|
||||
const c = collectCommit(repo.work, baseSha, head);
|
||||
const atHead = reviewExclude({ root: repo.work, baseSha, ref: 'origin/main', mergeBase: c.mergeBase!, head, perPath: c.perPath, added: c.added, classRoot: c.root ?? [], gateTagged: new Set(), context: 'test' });
|
||||
const l = collectLive(repo.work, 'origin/main');
|
||||
const live = reviewExclude({ root: repo.work, baseSha, ref: 'origin/main', mergeBase: l.mergeBase!, perPath: l.perPath, added: l.added, classRoot: repo.work, gateTagged: new Set(), context: 'test' });
|
||||
expect(atHead.state === 'active' && [...atHead.excluded].filter(p => p.startsWith('receipts/big/')).sort()).toEqual(['receipts/big/a.dat', 'receipts/big/b.dat', 'receipts/big/c.dat']);
|
||||
expect(atHead.state === 'active' && live.state === 'active' && atHead.record.digest).toBe(live.state === 'active' ? live.record.digest : 'none');
|
||||
}, 180_000);
|
||||
});
|
||||
|
||||
@@ -591,7 +591,12 @@ describe('land: one pass, base, lock and history', () => {
|
||||
const fx = setup({ ...POLICY, history: 'merge-only', review_exclude_paths: ['data/**'], min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION });
|
||||
const head = openPr(fx, '15', { edit: work => { fs.mkdirSync(path.join(work, 'data', 'a'), { recursive: true }); fs.writeFileSync(path.join(work, 'data', 'a', 'result.json'), '{"ok": true}\n'); } });
|
||||
fx.repo.git('checkout', '-q', '--detach', head);
|
||||
const { depth, exclude } = receiptReviewAtHead(fx.repo.work, 'main', path.join(ROOT, 'bin'), head);
|
||||
const prior = process.env.GSTACK_STATE_ROOT;
|
||||
process.env.GSTACK_STATE_ROOT = path.join(fx.repo.root, 'ready-state');
|
||||
let review: ReturnType<typeof receiptReviewAtHead>;
|
||||
try { review = receiptReviewAtHead(fx.repo.work, 'main', path.join(ROOT, 'bin'), head); }
|
||||
finally { if (prior === undefined) delete process.env.GSTACK_STATE_ROOT; else process.env.GSTACK_STATE_ROOT = prior; }
|
||||
const { depth, exclude } = review;
|
||||
fx.repo.git('checkout', '-q', '--detach', 'origin/main');
|
||||
const recorded = receiptExclude(exclude);
|
||||
expect(recorded?.excluded).toBe(1);
|
||||
|
||||
Reference in new issue
Block a user