fix(receipt): carry and write handle PRs with thousands of files; tree receipt fails closed on a failed diff (GST-448)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-ship-receipt

@@ -31,6 +31,7 @@
  */
 import { spawnSync } from 'node:child_process';
 import * as fs from 'node:fs';
+import * as os from 'node:os';
 import * as path from 'node:path';
 import { EXIT, exitTableHelp, type ShipReceipt } from '../lib/headless-artifacts';
 import { resultLine, resultLineWithDocs } from '../lib/result-codes';
@@ -116,7 +117,7 @@ function jsonArg(raw: string | undefined): Record<string, unknown> | undefined {
   return parsed;
 }
 function git(args: string[], cwd?: string): string | undefined {
-  const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 30_000 });
+  const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 30_000, maxBuffer: 256 * 1024 * 1024 });
   return r.status === 0 ? r.stdout.trim() : undefined;
 }
 function gh(args: string[]): { ok: boolean; out: string; err: string } {
@@ -390,12 +391,20 @@ function writeRunRecord(a: { receipt: ShipReceipt; head: string; pr: string | un
  */
 function patchId(repoRoot: string, from: string, to: string, pathspec: string[]): string | { error: string } {
   const firstLine = (s: string | null | undefined, fallback: string) => (s ?? '').trim().split('\n')[0]?.slice(0, 200) || fallback;
-  const diff = spawnSync('git', ['diff', '--no-color', '--no-ext-diff', '--full-index', from, to, '--', ...pathspec], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 256 * 1024 * 1024 });
-  if (diff.status !== 0) return { error: `git diff ${from.slice(0, 12)} ${to.slice(0, 12)}: ${firstLine(diff.stderr, diff.signal ? `killed by ${diff.signal}` : `exit ${diff.status}`)}` };
-  if (!diff.stdout.trim()) return '';
-  const id = spawnSync('git', ['patch-id', '--verbatim'], { cwd: repoRoot, input: diff.stdout, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 });
-  if (id.status !== 0) return { error: `git patch-id --verbatim: ${firstLine(id.stderr, id.signal ? `killed by ${id.signal}` : `exit ${id.status}`)}` };
-  return id.stdout.trim().split(/\s+/)[0] ?? '';
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-patch-id-'));
+  const file = path.join(dir, 'diff.patch');
+  let fd: number | undefined;
+  try {
+    const diff = spawnSync('git', ['diff', '--no-color', '--no-ext-diff', '--full-index', `--output=${file}`, from, to, '--', ...pathspec], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000 });
+    if (diff.status !== 0) return { error: `git diff ${from.slice(0, 12)} ${to.slice(0, 12)}: ${firstLine(diff.stderr, diff.signal ? `killed by ${diff.signal}` : `exit ${diff.status}`)}` };
+    fd = fs.openSync(file, 'r');
+    const id = spawnSync('git', ['patch-id', '--verbatim'], { cwd: repoRoot, stdio: [fd, 'pipe', 'pipe'], encoding: 'utf8', timeout: 120_000 });
+    if (id.status !== 0) return { error: `git patch-id --verbatim: ${firstLine(id.stderr, id.signal ? `killed by ${id.signal}` : `exit ${id.status}`)}` };
+    return id.stdout.trim().split(/\s+/)[0] ?? '';
+  } finally {
+    if (fd !== undefined) fs.closeSync(fd);
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
 }
 
 /** `git patch-id --verbatim` exists from git 2.39 (docs/ship-policy.md#receipt-carry); an older git is a named refusal, never a silent fallback to the whitespace-stripping default. */
@@ -597,13 +606,15 @@ function cmdCarry(rest: string[], json: boolean): number {
     else if (productFrom !== productHead && !retarget) {
       // planCarry names at most CARRY_NAMED_PATHS differing paths, so the walk stops there, and it reads only the paths the
       // branch itself touched against the base at either head: a base merge brings in every path the base changed, none of
-      // which can differ between the two base diffs. A path whose only difference is base context next to unchanged hunks
+      // which can differ between the two base diffs. A touched path with the same blob at both heads and at both bases has
+      // the same patch, so only paths that moved on one side are compared (GST-448: 6000 touched paths). A path whose only difference is base context next to unchanged hunks
       // (lib/ship-patch.ts) needs a scoped review instead of the loop.
       const sides = { reviewedBase: fromBase, reviewed: from, currentBase: headBase, current: head };
-      const listed = [touchedPaths(repoRoot, fromBase, from), touchedPaths(repoRoot, headBase, head)];
-      if (!listed[0] || !listed[1]) return refused('RECEIPT_CARRY_REFUSED', `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`);
-      const touched = new Set([...listed[0], ...listed[1]]);
-      for (const p of [...touched].sort()) {
+      const listed = [touchedPaths(repoRoot, fromBase, from), touchedPaths(repoRoot, headBase, head), touchedPaths(repoRoot, from, head), touchedPaths(repoRoot, fromBase, headBase)];
+      if (listed.some(l => !l)) return refused('RECEIPT_CARRY_REFUSED', `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`);
+      const [touchedFrom, touchedHead, movedHead, movedBase] = listed as Set<string>[];
+      const touched = new Set([...touchedFrom, ...touchedHead]);
+      for (const p of [...touched].filter(p => movedHead.has(p) || movedBase.has(p)).sort()) {
         if (changedProduct.length >= CARRY_NAMED_PATHS) break;
         const cls = changed.find(c => c.path === p)?.class ?? classifyCarryPath(p, rules.extra);
         if (cls !== 'product') continue;

changelog.d/gst-430-notes-only-pass.md

@@ -1,9 +1,14 @@
 **A /ship review pass that only left notes now binds at Step 11.5.** A pass whose findings were all fixed, deferred or noted has no open defect, but a driver could still log it as `issues_found`. Step 11.5 then refused it, and the only way out was an extra rebind pass (CAPY-2, qm#2837). When the tree changes during a pass, the refusal now names the start and end trees and the cheapest repair (GST-430).
 
+**Receipt carry and write handle results PRs with thousands of files (GST-448).** A PR that adds about 8,300 run receipts (harness-evals#147) refused every carry with `git could not compute HEAD's product patch-id: git diff … failed`, and every base move cost it about 3 hours of /ship. The name-only listing ran past a 1 MB output buffer.
+
 ### Fixed
+- **`gstack-ship-receipt carry` and `write`** read git's path listings with a 256 MB buffer instead of the 1 MB default, so a large PR's `product_patch_id` is recorded and carried. The product patch-id streams `git diff` through a file into `git patch-id --verbatim` and no longer holds the diff in memory. The per-path walk compares only touched paths that changed at either head or either base, so a 6,000-path PR refuses or carries in seconds.
+- **`gstack-tree-receipt`** fails closed (`EVIDENCE_IDENTITY_UNKNOWN`) when git cannot produce the diff between the gated and current heads. Before, it read a failed diff as no changes.
 - **`gstack-review-log`** records a completed `review` or `review-fix-check` row claimed as `issues_found` as `clean` with zero counts when every finding's action is `fixed`, `auto-fixed`, `deferred` or `noted`. A skipped finding, a gate finding left open, a finding with no action, or an incomplete pass keeps the claimed status.
 - **Findings files** (`--findings`) count `auto-fixed`, `deferred` and `noted` actions as resolved, the same as `fixed`.
 - **`review_freshness`** for a pass whose tree moved now reads `content changed during review: the tree moved from <start> to <end> while <skill> ran …`, followed by the repair: keep the checkout frozen until every reviewer returns, then rerun on the frozen tree (docs/troubleshooting.md#review-content-changed).
 
 ### Tests
+- `test/ship-receipt-carry-large-diff.test.ts`: a PR that adds 6,000 receipts under a long directory name writes a receipt with its product patch-id and carries across a base merge. Negative control: one rescored receipt still refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`.
 - `test/review-notes-only-pass.test.ts`: a pass with three notes is recorded clean and reads CURRENT. Negative controls: a skipped finding, an open gate finding, a finding with no action, and an incomplete pass keep `issues_found`. Also checks the reason a pass gets when its tree moved.

lib/tree-receipt.ts

@@ -170,7 +170,8 @@ export function buildReceipt(o: ReceiptOptions): TreeReceipt | { error: string;
   const versionRel = o.versionRel ?? ('rel' in cv ? cv.rel : 'rel' in gv ? gv.rel : 'VERSION');
   const oldV = 'version' in gv ? gv.version : null;
   const newV = 'version' in cv ? cv.version : null;
-  const diffText = git(o.repoRoot, ['diff', '--unified=0', '--no-color', '--no-ext-diff', gatedHead, currentHead]) ?? '';
+  const diffText = git(o.repoRoot, ['diff', '--unified=0', '--no-color', '--no-ext-diff', gatedHead, currentHead]);
+  if (diffText === null) return { error: `git diff ${gatedHead.slice(0, 12)} ${currentHead.slice(0, 12)} failed, so the changed files cannot be classified (GST-448)`, code: 'EVIDENCE_IDENTITY_UNKNOWN' };
   const files = classifyFiles(o.repoRoot, parseUnifiedDiff(diffText), gatedHead, currentHead, o.policy, versionRel, oldV, newV);
   const residual = files.filter(f => f.verdict === 'changed').map(f => f.path);
   const stampFiles = files.filter(f => f.verdict === 'stamp').map(f => f.path);

test/ship-receipt-carry-large-diff.test.ts

@@ -0,0 +1,53 @@
+/**
+ * gstack-ship-receipt carry on a PR whose own diff is large (GST-448, harness-evals#147): a results PR that adds thousands
+ * of run-receipt files has a name-only listing and a product diff over the child-process output buffer, so carry refused
+ * with "git could not compute HEAD's product patch-id: git diff … failed" after every base move.
+ */
+import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import { EXIT } from '../lib/headless-artifacts';
+import { extractReceipt } from '../lib/ship-receipt';
+import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture';
+
+setDefaultTimeout(120_000);
+const repos: FixtureRepo[] = [];
+afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
+const DIR = `results/${'harness-eval-run-receipts-'.repeat(8)}`;
+/** `count` run receipts under a long directory name: about 1.4 MB of paths and 2 MB of diff, past the 1 MB default buffer. */
+const receipts = (count: number, tag: string) => Object.fromEntries(Array.from({ length: count }, (_, i) => [`${DIR}/${tag}-run-${String(i).padStart(5, '0')}.json`, `{"run":${i},"tag":"${tag}","score":0.${i % 10}}\n`]));
+
+describe('carry with a large product diff (GST-448)', () => {
+  // Value: protects=a results PR keeps its receipt across a base move when its own diff is megabytes of added files
+  //   (harness-evals#147 paid ~3h of /ship per base move); fails_when=carry buffers git's name-only listing or the diff in
+  //   a bounded child-process buffer and refuses; seam=none; no_claim=gh pr edit (--body stands in)
+  test('a base merge carries the receipt of a PR that adds 6000 files', () => {
+    const repo = makeFixtureRepo(); repos.push(repo);
+    commit(repo, receipts(6000, 'base'), 'results: 6000 run receipts');
+    const body = path.join(repo.root, 'body.md');
+    const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '9', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]);
+    expect(w.status).toBe(EXIT.ok);
+    const reviewed = extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.product_patch_id;
+    expect(reviewed).toMatch(/^[0-9a-f]{40}$/);
+    repo.git('checkout', '-q', 'main');
+    write(repo.work, 'docs/README.md', '# docs moved on main\n');
+    repo.git('add', '-A'); repo.git('commit', '-qm', 'main moves on'); repo.git('push', '-q', 'origin', 'main');
+    repo.git('checkout', '-q', 'feature');
+    repo.git('merge', '-q', '--no-edit', 'origin/main');
+    const out = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
+    expect([out.status, out.out.split('\n').find(l => l.startsWith('CARRY:')), out.err.split('\n').find(l => l.includes('RECEIPT_CARRY'))]).toEqual([EXIT.ok, expect.stringMatching(/^CARRY: carried/), undefined]);
+    expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.product_patch_id).toBe(reviewed);
+  });
+
+  // Value: protects=negative control: the large-diff path still refuses when the PR's own receipts change after review;
+  //   fails_when=the streamed patch-id reads as unchanged for a changed diff; seam=none; no_claim=none
+  test('one changed receipt in the large diff still refuses the carry', () => {
+    const repo = makeFixtureRepo(); repos.push(repo);
+    commit(repo, receipts(6000, 'base'), 'results: 6000 run receipts');
+    const body = path.join(repo.root, 'body.md');
+    expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '9', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok);
+    commit(repo, { [`${DIR}/base-run-00042.json`]: '{"run":42,"tag":"base","score":0.9}\n' }, 'results: rescore run 42');
+    const out = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
+    expect([out.status, out.out.split('\n').find(l => l.startsWith('CARRY:'))]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)']);
+  });
+});