diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 7a4f1dbb8..233c43ff5 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -765,7 +765,9 @@ function cmdCarry(rest: string[], json: boolean): number { const named = changedProduct.filter(p => !p.startsWith('(') && !p.includes(' (')); const inter = named.length ? spawnSync('git', ['diff', '--no-color', '--no-ext-diff', from, head, '--', ...named.map(p => `:(literal)${p}`)], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024 }) : null; const fullRange = rangeRaw.status === 0 && inter?.status === 0 ? `${rangeRaw.stdout.trim()}\n${inter.stdout.trim()}` : null; - const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length, legacy: !receipt.review_depth, walkCut, contextPaths: contextPaths.length }); + const numstat = named.length ? spawnSync('git', ['diff', '--numstat', from, head, '--', ...named.map(p => `:(literal)${p}`)], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 16 * 1024 * 1024 }) : null; + const binary = numstat?.status !== 0 || (numstat?.stdout ?? '').split('\n').some(l => l.startsWith('-\t-\t')); + const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length, legacy: !receipt.review_depth, walkCut, contextPaths: contextPaths.length, binary }); const record = flag(rest, '--recheck'); if (!r.ok) process.stderr.write(`RECHECK: not eligible (${r.reason}); run /ship on ${head.slice(0, 12)}\n`); else if (record) { diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 7822fd958..a2bb74aa9 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -423,7 +423,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: const rowKey = (r: Record) => JSON.stringify([r.skill, r.pass ?? null, r.commit_full ?? r.commit ?? '', (Array.isArray(r.findings) ? r.findings : []).map((f: any) => (f && typeof f === 'object' ? (typeof f.ledger_id === 'string' ? f.ledger_id : findingIdentity(f)) : '')).sort()]); const loopRows = [...new Map(rows.filter(r => inRun(r) && loopSkill(r)).map((r, i) => [r.commit_full || r.commit ? rowKey(r) : `#${i}`, r] as const)).values()]; const passes = loopRows.filter(r => r.skill === 'review'); - type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean; ledgerId?: string }; + type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean; ledgerId?: string; fingerprint?: string }; const seen = new Map(); const deferred = new Set(); const noted = new Set(); @@ -447,6 +447,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: candidate: !!prior?.candidate || f.severity === undefined || rank(f.severity) === 4 || rank(f.raw_severity) === 4 || f.blocking === true, disposed: !!prior?.disposed || DISPOSITIONS.includes(f.action), ...(ledgerId ?? prior?.ledgerId ? { ledgerId: ledgerId ?? prior?.ledgerId } : {}), + ...(prior?.fingerprint ?? findingIdentity(f) ? { fingerprint: prior?.fingerprint ?? findingIdentity(f) } : {}), }); if (f.action === 'deferred') deferred.add(id); if (f.action === 'noted') noted.add(id); @@ -479,7 +480,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: } for (const [id, n] of lastPaired) if (followed(n)) followedIds.add(id); for (const t of trailers) { - const hit = [...seen].find(([k, s]) => k === t.finding || s.ledgerId === t.finding); + const hit = [...seen].find(([k, s]) => k === t.finding || s.ledgerId === t.finding || s.fingerprint === t.finding); if (hit) { hit[1].disposed = true; fixed.add(hit[0]); continue; } const only = ledgerOnlyKeys.find(l => l.key === t.finding || l.id === t.finding); if (only) { only.fixed = true; continue; } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 1149bc647..83e72a7d7 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -99,7 +99,7 @@ export function bindReceipt(body: string, pr: string | undefined, accepted: read */ export const RUNS_FENCE = 'gstack-receipt-runs'; export const RECEIPT_RUNS_KEPT = 30; -const RUNS_RE = /```gstack-receipt-runs[ \t]*\r?\n([\s\S]*?)\r?\n?```\n?/; +const RUNS_RE = /```gstack-receipt-runs[ \t]*\r?\n([\s\S]*?)\r?\n```[ \t]*(?:\r?\n|$)/; export type ReceiptRun = Pick & { depth?: string; carried_from?: string }; export function receiptRuns(body: string): ReceiptRun[] { const m = RUNS_RE.exec(body); diff --git a/lib/ship-recheck.ts b/lib/ship-recheck.ts index 2253f295c..db1a85c43 100644 --- a/lib/ship-recheck.ts +++ b/lib/ship-recheck.ts @@ -8,19 +8,23 @@ * Qualifies: RECEIPT_CARRY_PRODUCT_CHANGED on linear history (the receipt's head is an ancestor of HEAD) against the same base. * Does not qualify: RECEIPT_CARRY_TREE_MISMATCH (rewritten history: the reviewed commits are gone, so there is no range to * diff), a retargeted stack (the reviewed range was against another base), a legacy receipt with no review_depth, more - * changed product paths than the carry walk names, a path it cannot name, base context moved next to reviewed hunks, - * an unreadable or empty range-diff, a range-diff over RECHECK_RANGE_DIFF_CAP lines, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs + * changed product paths than the carry walk names, a path it cannot name, a binary path, base context moved next to reviewed hunks, + * an unreadable or empty range-diff, a packet over RECHECK_RANGE_DIFF_CAP lines or RECHECK_PACKET_BYTES bytes, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs * repaired, not a review). */ import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; export const RECHECK_RANGE_DIFF_CAP = 400; +/** The packet's byte cap: 400 lines of unbounded width (a minified bundle) would not bound the one reader's input. */ +export const RECHECK_PACKET_BYTES = 64 * 1024; /** rangeDiff: \`git range-diff\` of the two ranges followed by the diff of the changed product paths between the heads (a range-diff names added commits without their content). */ export type RecheckInput = { code: string; ancestor: boolean; retarget: boolean; rangeDiff: string | null; unnamedPaths?: boolean; /** The receipt records no review_depth (a legacy receipt), so carry cannot compare HEAD's risk class with the reviewed one. */ legacy?: boolean; + /** A changed product path is binary, so the packet would show only "Binary files … differ" for it. */ + binary?: boolean; /** The changed-path walk stopped at CARRY_NAMED_PATHS, so later changed product paths would be missing from the packet. */ walkCut?: boolean; /** Paths whose only difference is base context next to reviewed hunks: they need RECEIPT_CARRY_SCOPED_REVIEW's packet, not this one. */ @@ -38,12 +42,15 @@ export function planRecheck(i: RecheckInput): RecheckPlan { if (i.legacy) return { ok: false, reason: 'the receipt records no review_depth, so a changed risk class could not be caught' }; if (i.walkCut) return { ok: false, reason: 'more changed product paths than the walk names, so the packet would miss some' }; if (i.unnamedPaths) return { ok: false, reason: 'a changed product path could not be named, so the packet would miss it' }; + if (i.binary) return { ok: false, reason: 'a changed product path is binary, so the packet cannot show its contents' }; if (i.contextPaths) return { ok: false, reason: 'base context also moved next to reviewed hunks, which needs the scoped review of those hunks' }; if (i.rangeDiff === null) return { ok: false, reason: 'git range-diff failed or timed out' }; const text = i.rangeDiff.trim(); if (!text) return { ok: false, reason: 'git range-diff is empty, so it shows nothing for a reader to check' }; const lines = text.split('\n').length; if (lines > RECHECK_RANGE_DIFF_CAP) return { ok: false, reason: `git range-diff is ${lines} lines, over the scoped re-check cap of ${RECHECK_RANGE_DIFF_CAP}` }; + const bytes = Buffer.byteLength(text); + if (bytes > RECHECK_PACKET_BYTES) return { ok: false, reason: `the packet is ${bytes} bytes, over the scoped re-check cap of ${RECHECK_PACKET_BYTES}` }; return { ok: true, lines, sha256: sha(text) }; } diff --git a/test/review-loop-binding.test.ts b/test/review-loop-binding.test.ts index 9e1a26eb8..a8c40e84b 100644 --- a/test/review-loop-binding.test.ts +++ b/test/review-loop-binding.test.ts @@ -68,6 +68,12 @@ describe('review loop counts bind by finding id (GST-1168)', () => { expect(s.unbound).toEqual(['pass 1 (ledger p1): no review-log row, so its fixes and dispositions cannot be counted']); }); + test('a fingerprint trailer binds a finding that also carries a ledger_id (outside voice #2)', () => { + const rows = [row('review', 1, [{ fingerprint: 'src/auth.ts:10:correctness', severity: 'CRITICAL', blocking: true, raised_by: 'red-team', ledger_id: 'p1/red-team#1' }])]; + const s = summarizeReviewLoop(rows, SINCE, [[blocker('p1', 'red-team#1', 'src/auth.ts:10')]], [pass(0, ['p1/red-team#1'])], parseFixTrailers('abc', ['src/auth.ts:10:correctness reader=red-team'])); + expect({ fixed: s.fixed, unrecorded: s.unrecorded, unbound: s.unbound }).toEqual({ fixed: 1, unrecorded: undefined, unbound: undefined }); + }); + test('rows without ledger_id keep pairing by path:line, as before', () => { const rows = [row('review', 1, [{ fingerprint: 'src/a.ts:10:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'specialist' }])]; const s = summarizeReviewLoop(rows, SINCE, [[blocker('p1', 'specialist/security#1', 'src/a.ts:10')]]); diff --git a/test/ship-receipt-runs.test.ts b/test/ship-receipt-runs.test.ts index f9957f900..9752b23c8 100644 --- a/test/ship-receipt-runs.test.ts +++ b/test/ship-receipt-runs.test.ts @@ -20,6 +20,15 @@ describe('receipt runs block', () => { expect(body.match(/## Summary/g)!.length).toBe(1); }); + test('a fence inside a recorded reason never closes the runs block (adversarial p-voices #1)', () => { + const withReason = (head: string) => orderReceipt({ ...receipt(head, { fix_checks: 1 }), review_loop_exception: { accepted_by: 'Garry', date: '2026-11-03', via: 'x', reason: 'the \x60\x60\x60retry\x60\x60\x60 block is fine', fix_check: { head, findings: 0 } } }); + let body = placeReceiptFirst('## Summary\nprose\n', withReason('b'.repeat(40))); + body = placeReceiptFirst(body, withReason('c'.repeat(40))); + body = placeReceiptFirst(body, withReason('d'.repeat(40))); + expect(receiptRuns(body).map(r => r.head[0])).toEqual(['b', 'c']); + expect(body.endsWith('## Summary\nprose\n')).toBe(true); + }); + test('rewriting the same receipt adds no run, and the block keeps the newest RECEIPT_RUNS_KEPT', () => { const same = receipt('d'.repeat(40)); expect(receiptRuns(placeReceiptFirst(placeReceiptFirst('', same), same))).toEqual([]); diff --git a/test/ship-recheck.test.ts b/test/ship-recheck.test.ts index 3d57f3279..1fcaab7eb 100644 --- a/test/ship-recheck.test.ts +++ b/test/ship-recheck.test.ts @@ -11,7 +11,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt'; -import { planRecheck, readRecheck, RECHECK_RANGE_DIFF_CAP } from '../lib/ship-recheck'; +import { planRecheck, readRecheck, RECHECK_PACKET_BYTES, RECHECK_RANGE_DIFF_CAP } from '../lib/ship-recheck'; import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; const repos: FixtureRepo[] = []; @@ -33,6 +33,8 @@ describe('planRecheck: which refusals qualify', () => { ['walk cut at the named-path limit', { ...OK, walkCut: true }, 'more changed product paths'], ['unnamed product path', { ...OK, unnamedPaths: true }, 'could not be named'], ['base context moved', { ...OK, contextPaths: 1 }, 'base context'], + ['binary product path', { ...OK, binary: true }, 'binary'], + ['wide packet over the byte cap', { ...OK, rangeDiff: 'x'.repeat(RECHECK_PACKET_BYTES + 1) }, 'bytes'], ['range-diff failed', { ...OK, rangeDiff: null }, 'failed or timed out'], ['empty range-diff', { ...OK, rangeDiff: '\n' }, 'empty'], ['over the cap', { ...OK, rangeDiff: Array.from({ length: RECHECK_RANGE_DIFF_CAP + 1 }, (_, i) => `l${i}`).join('\n') }, 'over the scoped re-check cap'], @@ -130,6 +132,10 @@ describe('carry with a scoped re-check (CLI)', () => { const wide = reviewed(many); wide.commit(Object.fromEntries(Object.keys(many).map(f => [f, 'export const changed = 2;\n'])), 'fix: all'); expect(runBin(wide.repo, 'gstack-ship-receipt', ['carry', '--body', wide.body], wide.env).err).toContain('RECHECK: not eligible (more changed product paths'); + + const bin = reviewed({ 'src/app.wasm': '\0asm\x01\0\0\0' }); + bin.commit({ 'src/app.wasm': '\0asm\x01\0\0\0' + 'x'.repeat(4096) + '\0' }, 'fix: new wasm'); + expect(runBin(bin.repo, 'gstack-ship-receipt', ['carry', '--body', bin.body], bin.env).err).toContain('RECHECK: not eligible (a changed product path is binary'); }); test('base context moved next to a reviewed hunk plus a product edit elsewhere is not eligible (red-team p1 repro)', () => {