ship-receipt write: the targeted record survives a main merge; classes since the read cover the PR's own change (GST-1593)

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

bin/gstack-ship-receipt

@@ -290,7 +290,7 @@ function cmdWrite(rest: string[], json: boolean): number {
   const product = reviewedProduct(repoRoot, base, head, pregateFile);
   let targeted: ReturnType<typeof parseReviewTargeted> | undefined;
   if (flag(rest, '--review-targeted') !== undefined) {
-    try { targeted = parseReviewTargeted(jsonArg(flag(rest, '--review-targeted')), product.product_patch_id ?? 'none', read => reviewedProduct(repoRoot, base, read, pregateFile).product_patch_id, read => targetedClassesSince(repoRoot, read, headSha ?? head), read => plainDocsSince(repoRoot, read, headSha ?? head)); } catch (e: any) { targeted = { ok: false, message: `--review-targeted: ${e.message}` }; }
+    try { targeted = parseReviewTargeted(jsonArg(flag(rest, '--review-targeted')), product.product_patch_id ?? 'none', read => reviewedProduct(repoRoot, base, read, pregateFile).product_patch_id, read => targetedClassesSince(repoRoot, read, headSha ?? head, base), read => plainDocsSince(repoRoot, read, headSha ?? head, base)); } catch (e: any) { targeted = { ok: false, message: `--review-targeted: ${e.message}` }; }
   }
   if (targeted && !targeted.ok) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `${targeted.message}; nothing written`) + '\n'); return EXIT.fail; }
   const approval = approvalForReceipt({ repoRoot, base, head, citationText: [bodyText, ...flags(rest, '--approval')].join('\n'), runDirs: flags(rest, '--approval-run') });

lib/ship-receipt-depth.ts

@@ -479,16 +479,72 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string,
 }
 
 /**
- * The targeted classes of what changed from `from` to the checked-out `head`, with gstack-gate-diff run over that range whenever
- * a non-docs path changed (a test edit that skips a case is gate class); null when `from` is not an ancestor of `head`, the gate
- * scan did not run, or a class input is unreadable.
+ * A commit on `from` that holds what the PR's own change did after `from`: every path whose part of the PR's diff against its
+ * merge base with origin/<base> (product_patch_id's basis, with tests and metadata kept) differs between `from` and `head`, at
+ * `head`'s content. A main merge moves the merge base with the files it brings, so a file only main changed keeps `from`'s
+ * content (GST-1593); a branch edit, or a merge resolution that changes the PR's own hunks, moves the path. With no resolvable
+ * merge base every path that changed from `from` to `head` moves. Null when git cannot build it.
  */
-export function targetedClassesSince(root: string, from: string, head: string): readonly TargetedClass[] | null {
+function ownChangeSince(root: string, base: string, from: string, head: string): string | null {
+  const run = (args: string[], env?: NodeJS.ProcessEnv, input?: string) => {
+    const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', env, input, timeout: 60_000, maxBuffer: 256 * 1024 * 1024 });
+    return r.status === 0 ? r.stdout : null;
+  };
+  const raw = (a: string, b: string) => {
+    const out = run(['diff', '--raw', '-z', '--no-renames', '--full-index', a, b]);
+    if (out === null) return null;
+    const t = out.split('\0');
+    const entries = new Map<string, string>();
+    for (let i = 0; i + 1 < t.length; i += 2) entries.set(t[i + 1]!, t[i]!);
+    return entries;
+  };
+  const hunks = (a: string, b: string, p: string) => run(['--literal-pathspecs', 'diff', '--no-color', '--no-ext-diff', '--no-renames', '--binary', a, b, '--', p])
+    ?.split('\n').filter(l => !l.startsWith('index ')).map(l => l.replace(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@/, '@@')).join('\n') ?? null;
+  const baseSha = baseCommit(root, `origin/${base.replace(/^origin\//, '')}`);
+  const mbFrom = baseSha.ok ? git(root, ['merge-base', baseSha.out, from]) : baseSha;
+  const mbHead = baseSha.ok ? git(root, ['merge-base', baseSha.out, head]) : baseSha;
+  let moved: string[];
+  if (mbFrom.ok && mbHead.ok) {
+    const was = raw(mbFrom.out, from), now = raw(mbHead.out, head);
+    if (!was || !now) return null;
+    moved = [...new Set([...was.keys(), ...now.keys()])].filter(p => {
+      if (was.get(p) === now.get(p)) return false;
+      const before = hunks(mbFrom.out, from, p);
+      return before === null || before !== hunks(mbHead.out, head, p);
+    });
+  } else {
+    const all = raw(from, head);
+    if (!all) return null;
+    moved = [...all.keys()];
+  }
+  const gitDir = git(root, ['rev-parse', '--absolute-git-dir']);
+  if (!gitDir.ok) return null;
+  const index = path.join(gitDir.out, `gstack-since-${process.pid}-${randomBytes(4).toString('hex')}.index`);
+  const env = { ...process.env, GIT_INDEX_FILE: index, GIT_AUTHOR_NAME: 'gstack-ship-receipt', GIT_AUTHOR_EMAIL: 'ship-receipt@gstack.local', GIT_COMMITTER_NAME: 'gstack-ship-receipt', GIT_COMMITTER_EMAIL: 'ship-receipt@gstack.local', GIT_AUTHOR_DATE: '2000-01-01T00:00:00Z', GIT_COMMITTER_DATE: '2000-01-01T00:00:00Z' };
+  try {
+    const listed = moved.length ? run(['--literal-pathspecs', 'ls-tree', '-z', '--full-tree', head, '--', ...moved]) : '';
+    if (listed === null) return null;
+    const atHead = new Map(listed.split('\0').filter(Boolean).map(l => [l.slice(l.indexOf('\t') + 1), l] as const));
+    const entries = moved.map(p => `${atHead.get(p) ?? `0 ${'0'.repeat(from.length)}\t${p}`}\0`).join('');
+    if (run(['read-tree', from], env) === null || run(['update-index', '-z', '--index-info'], env, entries) === null) return null;
+    const tree = run(['write-tree'], env)?.trim();
+    return (tree && run(['commit-tree', '--no-gpg-sign', tree, '-p', from, '-m', 'gstack-ship-receipt: the PR\'s own change since the targeted read'], env)?.trim()) || null;
+  } finally { fs.rmSync(index, { force: true }); }
+}
+
+/**
+ * The targeted classes of what the PR's own change did from `from` to `head` against origin/<base> (ownChangeSince), with
+ * gstack-gate-diff run over that change whenever a non-docs path moved (a test edit that skips a case is gate class); null when
+ * `from` is not an ancestor of `head`, the change cannot be built, the gate scan did not run, or a class input is unreadable.
+ */
+export function targetedClassesSince(root: string, from: string, head: string, base: string): readonly TargetedClass[] | null {
   if (!git(root, ['merge-base', '--is-ancestor', from, head]).ok) return null;
-  const at = headInputs(root, from, head, undefined, true);
+  const since = ownChangeSince(root, base, from, head);
+  if (!since) return null;
+  const at = headInputs(root, from, since, undefined, true);
   let gateTagged: number | null = 0;
   if (gateScanNeeded(at.c.paths, at.c.root)) {
-    const g = runScriptSync(path.join(BIN_DIR, 'gstack-gate-diff'), [from], { cwd: root, encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 });
+    const g = runScriptSync(path.join(BIN_DIR, 'gstack-gate-diff'), ['--commit', since], { cwd: root, encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 });
     const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? '');
     if (g.status !== 0 || !m) return null;
     gateTagged = m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0);
@@ -498,14 +554,16 @@ export function targetedClassesSince(root: string, from: string, head: string):
 }
 
 /**
- * Whether what changed from `from` to the checked-out `head` is plain docs, which a targeted record carries across (GST-1195):
- * every path docs class, R65's claim rule (platformFits: no numeric claim, no results, prereg or eval path), and no money,
- * security, secret or auth wording in the changed lines (the risk matchers plus DOCS_CLAIM_WORDS, since the matchers spare prose).
- * False when `from` is not an ancestor or the diff is unreadable.
+ * Whether what the PR's own change did from `from` to `head` against origin/<base> (ownChangeSince) is plain docs, which a
+ * targeted record carries across (GST-1195): every path docs class, R65's claim rule (platformFits: no numeric claim, no results,
+ * prereg or eval path), and no money, security, secret or auth wording in the changed lines (the risk matchers plus
+ * DOCS_CLAIM_WORDS, since the matchers spare prose). False when `from` is not an ancestor or the change is unreadable.
  */
-export function plainDocsSince(root: string, from: string, head: string): boolean {
+export function plainDocsSince(root: string, from: string, head: string, base: string): boolean {
   if (!git(root, ['merge-base', '--is-ancestor', from, head]).ok) return false;
-  const c = collectCommit(root, from, head);
+  const since = ownChangeSince(root, base, from, head);
+  if (!since) return false;
+  const c = collectCommit(root, from, since);
   if (c.errors.length || !c.paths.length || !c.paths.every(p => classifyPath(p, c.root) === 'docs') || c.perPath.some(p => p.binary)) return false;
   const lines = c.paths.flatMap(p => [...(c.added?.[p] ?? []), ...(c.removed?.[p] ?? [])]);
   if (lines.some(l => l.toLowerCase().split(/[^a-z0-9]+/).some(t => DOCS_CLAIM_WORDS.has(t) || AUTH_PATH_PREFIXES.some(a => t.startsWith(a))))) return false;

test/review-targeted-record.test.ts

@@ -12,7 +12,7 @@ import { EXIT } from '../lib/headless-artifacts';
 import { checkoutTree } from '../lib/review-brief';
 import { TARGETED_RULES, targetedBriefSha } from '../lib/review-targeted';
 import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt';
-import { receiptDepthAtHead } from '../lib/ship-receipt-depth';
+import { receiptDepthAtHead, targetedClassesSince } from '../lib/ship-receipt-depth';
 import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture';
 
 setDefaultTimeout(180_000);
@@ -293,3 +293,77 @@ describe('/ship\'s receipt writes pass the targeted record (item 3, #3853)', ()
     expect(expand()).toEqual(['--review-targeted', `@${recordPath}`]);
   });
 });
+
+describe('a main merge after the read keeps the record when the PR\'s own change is unchanged (GST-1593, harness-evals #293)', () => {
+  const mainCommit = (repo: FixtureRepo, files: Record<string, string>, msg: string) => {
+    repo.git('checkout', '-q', 'main');
+    for (const [file, text] of Object.entries(files)) commit(repo, file, text, msg);
+    repo.git('push', '-q', 'origin', 'main');
+    repo.git('checkout', '-q', 'feat');
+    repo.git('fetch', '-q', 'origin');
+  };
+  const mainBrings = { 'lib/budget-ledger.ts': 'export const spend_cap_usd = 40;\n', 'lib/secrets.ts': 'export const key = process.env.OPENAI_API_KEY;\n' };
+  const readAndMerge = () => {
+    const { repo, base, head } = moneyBranch();
+    const record = path.join(repo.root, 'targeted.json');
+    expect(verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS'), record).status).toBe(0);
+    mainCommit(repo, mainBrings, 'main: budget ledger and secrets');
+    return { repo, head, record };
+  };
+
+  // Value: protects=a main merge that brings money- and secret-tagged files leaves the PR's product patch unchanged, so the
+  //   record binds (HARN-1 on harness-evals #293, #292, #225: targetedClassesSince diffed across the merge commit and voided it);
+  //   fails_when=the classes since the read count files only main brought in; seam=gstack-ship-receipt write --review-targeted
+  test('read, then a main merge that brings money and secret files: write binds', () => {
+    const { repo, head, record } = readAndMerge();
+    repo.git('merge', '-q', '--no-edit', 'origin/main');
+    const bound = write(repo, ['--review-targeted', `@${record}`]);
+    expect(bound.status, bound.err).toBe(EXIT.ok);
+    expect(JSON.parse(bound.out).review_targeted).toMatchObject({ classes: ['money'], status: 'done' });
+    expect(targetedClassesSince(repo.work, head, repo.git('rev-parse', 'HEAD'), 'main')).toEqual([]);
+  });
+
+  // Value: protects=a branch-side money edit or skipped test after the read still voids the record across a main merge;
+  //   fails_when=the PR-own basis drops the branch's own change; seam=targetedClassesSince and write --review-targeted
+  test('after a main merge, a branch-side money edit and a branch-side skipped test are refused', () => {
+    const { repo, head, record } = readAndMerge();
+    repo.git('merge', '-q', '--no-edit', 'origin/main');
+    const merged = repo.git('rev-parse', 'HEAD');
+    commit(repo, 'src/charge.ts', 'export const spend_cap_usd = 50;\n', 'money edit after the merge');
+    expect(targetedClassesSince(repo.work, head, repo.git('rev-parse', 'HEAD'), 'main')).toContain('money');
+    const money = write(repo, ['--review-targeted', `@${record}`]);
+    expect(money.status).toBe(EXIT.fail);
+    expect(money.err).toContain(`the targeted reader read ${head.slice(0, 12)}`);
+
+    repo.git('reset', '-q', '--hard', merged);
+    commit(repo, 'test/charge.test.ts', 'import { test, expect } from "bun:test";\ntest.skip("cap", () => { expect(5).toBe(5); });\n', 'skip a case after the merge');
+    expect(targetedClassesSince(repo.work, head, repo.git('rev-parse', 'HEAD'), 'main')).toContain('gate');
+    const skipped = write(repo, ['--review-targeted', `@${record}`]);
+    expect(skipped.status).toBe(EXIT.fail);
+    expect(skipped.err).toContain(`the targeted reader read ${head.slice(0, 12)}`);
+  });
+
+  // Value: protects=a plain docs commit after the read still carries the record when a main merge also brought money and secret
+  //   files (GST-1195's docs carry over the same PR-own basis); fails_when=plainDocsSince reads main's files as the branch's;
+  //   seam=gstack-ship-receipt write --review-targeted
+  test('after a main merge, a plain docs commit binds', () => {
+    const { repo, record } = readAndMerge();
+    repo.git('merge', '-q', '--no-edit', 'origin/main');
+    commit(repo, 'docs/run-order.md', '# Run order\n\nRead the overview first, then the walkthrough.\n', 'Step 14.5 docs');
+    const docs = write(repo, ['--review-targeted', `@${record}`]);
+    expect(docs.status, docs.err).toBe(EXIT.ok);
+  });
+
+  // Value: protects=a merge whose resolution changes the PR's own hunks is a change the reader never read; fails_when=the merge
+  //   commit's own edit to the PR's file is credited to main; seam=targetedClassesSince and write --review-targeted
+  test('a main merge that also changes the PR\'s own hunks is refused', () => {
+    const { repo, head, record } = readAndMerge();
+    repo.git('merge', '-q', '--no-commit', '--no-ff', 'origin/main');
+    fs.writeFileSync(path.join(repo.work, 'src', 'charge.ts'), 'export const spend_cap_usd = 9;\n');
+    repo.git('add', '-A'); repo.git('commit', '-q', '--no-edit');
+    expect(targetedClassesSince(repo.work, head, repo.git('rev-parse', 'HEAD'), 'main')).toEqual(['money']);
+    const evil = write(repo, ['--review-targeted', `@${record}`]);
+    expect(evil.status).toBe(EXIT.fail);
+    expect(evil.err).toContain(`the targeted reader read ${head.slice(0, 12)}`);
+  });
+});