fix(ship-receipt): carry survives a base policy change that selects no PR file (GST-1415) (#4030)

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

bin/gstack-ship-receipt

@@ -51,7 +51,7 @@ import {
 import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs';
 import { comparePath, contextPacket, PACKET_SECTION_CAP, readScopedReview, touchedPaths } from '../lib/ship-patch';
 import { planRecheck, readRecheck, recheckPacket } from '../lib/ship-recheck';
-import { deeper, depthBelow, exceptionHead, exceptionLoopProblem, exceptionProblems, parseReviewLoopException, parseReviewTargeted, plainDocsSince, platformReceiptDepth, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince } from '../lib/ship-receipt-depth';
+import { carryExcludeBinds, deeper, depthBelow, exceptionHead, exceptionLoopProblem, exceptionProblems, parseReviewLoopException, parseReviewTargeted, plainDocsSince, platformReceiptDepth, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince } from '../lib/ship-receipt-depth';
 import { RISK_CLASSES, type RiskClass } from '../lib/review-risk';
 import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose';
 import { resolveStateRoot } from '../lib/state-root';
@@ -104,7 +104,8 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate <jso
   unreadable input records full).
   --review-exclude <digest|none> (the digest= on gstack-review-dispatch verify's REVIEW_EXCLUDE_RECORD: line, which the reviewers' briefs
   name; none without that line): must equal the digest HEAD has under the base policy's review_exclude_paths, else
-  RECEIPT_INVALID field=review_exclude and nothing is written; recorded as review_exclude. Carry and the queue compare that digest.
+  RECEIPT_INVALID field=review_exclude and nothing is written; recorded as review_exclude. Carry and the queue compare that digest; carry also
+  carries when neither the recorded selection nor HEAD's holds a file the PR changed.
   The check binds the digest to the base policy, not to a review: write does not read the review ledger, so the expected=
   digest a refusal prints passes too. After a refusal, rerun the review and pass the digest its verify prints.
   --no-stop <json|@file> (a JSON array of {class, reason, evidence}: each routine stop the run recorded and went on past, class one of
@@ -870,7 +871,7 @@ function cmdCarry(rest: string[], json: boolean): number {
     const now = review.depth;
     if (review.exclude.state === 'refused') return refused('RECEIPT_CARRY_REFUSED', `${review.exclude.lines.join('; ')}; run /ship on ${head.slice(0, 12)} once the base policy reads`);
     const digest = review.exclude.state === 'active' ? review.exclude.record.digest : 'none';
-    if (digest !== (receipt.review_exclude?.digest ?? 'none')) return refused('RECEIPT_CARRY_REFUSED', `review_exclude changed: recorded=${receipt.review_exclude?.digest ?? 'none'} now=${digest} source=${review.exclude.state === 'active' ? review.exclude.record.source : `${base} (no review_exclude_paths)`}; the receipt's digest is the --review-exclude argument write was given, and it names another selection than HEAD ${head.slice(0, 12)} has under the base policy (carry compares that digest, not the review ledger), so carry does not apply; run /ship on ${head.slice(0, 12)}`);
+    if (!carryExcludeBinds(review.exclude, receipt)) return refused('RECEIPT_CARRY_REFUSED', `review_exclude changed: recorded=${receipt.review_exclude?.digest ?? 'none'} now=${digest} source=${review.exclude.state === 'active' ? review.exclude.record.source : `${base} (no review_exclude_paths)`}; the receipt's digest is the --review-exclude argument write was given, and it names another selection than HEAD ${head.slice(0, 12)} has under the base policy and one of the two selections holds a file the PR changed (carry compares those, not the review ledger), so carry does not apply; run /ship on ${head.slice(0, 12)}`);
     const riskOf = (d: { class: string; risk?: readonly string[] }) => `${d.class}:${[...(d.risk ?? [])].sort().join(',')}`;
     if (recheck && riskOf(now) !== riskOf(receipt.review_depth)) return refused('RECEIPT_CARRY_REFUSED', `HEAD ${head.slice(0, 12)} is risk class ${riskOf(now)}, not the reviewed ${riskOf(receipt.review_depth)}, so a scoped re-check cannot stand in for review; run /ship on ${head.slice(0, 12)}`);
     if (depthBelow(receipt.review_depth.depth, now.depth)) return refused('RECEIPT_CARRY_REFUSED', `HEAD ${head.slice(0, 12)} needs ${now.depth} review depth (${now.reasons.join(', ')}), above the receipt's ${receipt.review_depth.depth}; run /ship on ${head.slice(0, 12)}`);

changelog.d/capy-gst-1415-carry-exclude-selection.md

@@ -0,0 +1 @@
+- **Receipt carry survives a base policy change that excludes none of the PR's files.** `gstack-ship-receipt carry` refused with `review_exclude changed` whenever main's `review_exclude_paths` digest moved, even when no file the PR changed fell under any pattern, so harness-evals #263 paid a full five-reader re-review of an unchanged product. Carry now also carries when neither the receipt's recorded selection nor HEAD's holds a file the PR changed (a pattern gained, added, removed or reverted), and the carried receipt records HEAD's digest so the queue binds it. A pattern change that selects a PR file, excluded or kept, and an unreadable or malformed base policy still refuse. (GST-1415)

docs/ship-policy.md

@@ -1474,7 +1474,8 @@ checks the digest against the base policy, so a stale selection cannot be
 recorded by accident, but it does not read the review ledger: a write given the
 `expected=` digest passes without a review, and only the review run shows that
 the reviewers read that selection.
-The queue makes one exception (GST-1408): a receipt with no `review_exclude`, written before the base gained the key, still binds when no file the head changes matches any current pattern, excluded or kept, because its reviewers read the same full diff; a recorded digest that differs, a matched file and an unreadable base policy still refuse, and carry still compares digests.
+The queue makes one exception (GST-1408): a receipt with no `review_exclude`, written before the base gained the key, still binds when no file the head changes matches any current pattern, excluded or kept, because its reviewers read the same full diff; a recorded digest that differs, a matched file and an unreadable base policy still refuse.
+Carry makes the same exception in both directions (GST-1415): when the digests differ but neither the receipt's recorded selection (no `review_exclude`, or one whose list hash is the empty selection's and whose digest is built from it) nor HEAD's holds a file the PR changed, the receipt carries and records HEAD's digest, so a pattern the base gains, adds, removes or reverts does not void it; a selection that holds a PR file, excluded or kept, and an unreadable base policy still refuse.
 
 **Adopting it.** (1) Open a PR on the base that adds the patterns and
 `min_gstack_version` at least `REVIEW_EXCLUDE_KEY_VERSION`, and ship it like any

lib/review-exclude.ts

@@ -72,6 +72,10 @@ export interface ExcludeInput {
 }
 
 const sha256 = (s: string) => createHash('sha256').update(s).digest('hex');
+/** The list_sha256 of a selection that holds no file: no pattern matched the range, so applied, excluded and kept are all empty. */
+const EMPTY_LIST_SHA256 = sha256(JSON.stringify({ applied: [], excluded: [], kept: [] }));
+/** A recorded selection holds no file only when its list hash is the empty list's and its digest is built from that hash. */
+export const recordSelectsNoFile = (r: Pick<ExcludeRecord, 'patterns_sha256' | 'list_sha256' | 'digest'>) => r.list_sha256 === EMPTY_LIST_SHA256 && r.digest === sha256(`${r.patterns_sha256}\n${EMPTY_LIST_SHA256}`);
 const shellQuote = (s: string) => `'${s.replace(/'/g, "'\\''")}'`;
 const literal = (p: string) => `:(top,literal)${p}`;
 const argBytes = (paths: Iterable<string>) => { let n = 0; for (const p of paths) n += Buffer.byteLength(shellQuote(literal(p))) + 1; return n; };

lib/ship-receipt-depth.ts

@@ -22,7 +22,7 @@ import * as path from 'node:path';
 import { baseContainsHead } from './default-base';
 import type { ShipReceipt } from './headless-artifacts';
 import { collectCommit } from './review-collect';
-import { reviewExclude, type Exclusion } from './review-exclude';
+import { recordSelectsNoFile, reviewExclude, type Exclusion } from './review-exclude';
 import { isUnavailable } from './ship-policy';
 import { loadPolicyAt } from './ship-policy-at';
 import { classifyPath, depthOfCommitInput, gateScanNeeded, platformFits, scanNumbers, type Depth } from './review-depth';
@@ -285,11 +285,23 @@ function headInputs(root: string, baseRef: string, head: string, r?: ShipReceipt
  * or kept (GST-1408): the selection then equals the full diff that receipt's review read, so a pattern the base gained later
  * changes nothing the reviewers saw.
  */
+/** No file the head changed matches any pattern the base policy declares, so each reviewer reads the full diff (GST-1408). */
+const selectsNoFile = (x: Exclusion) => x.state === 'none' || x.state === 'head-only' || (x.state === 'active' && x.record.unmatched.length === x.record.patterns.length);
+/**
+ * Carry's exclusion check: the recorded digest against the one HEAD has under the current base policy, except that a policy
+ * change alone does not void the receipt (GST-1415): when neither the receipt's selection nor HEAD's holds a file the PR
+ * changed, each reviewer read the full diff and still would, whatever the patterns. A policy that cannot be read never binds.
+ */
+export function carryExcludeBinds(x: Exclusion, r: ShipReceipt): boolean {
+  if (x.state === 'refused') return false;
+  const e = r.review_exclude;
+  return (x.state === 'active' ? x.record.digest : 'none') === (e?.digest ?? 'none') || (selectsNoFile(x) && (!e || recordSelectsNoFile(e)));
+}
 export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base: string): string | null {
   const expected = x.state === 'active' ? x.record.digest : 'none';
   const recorded = r.review_exclude?.digest ?? 'none';
   if (expected === recorded) return null;
-  if (recorded === 'none' && x.state === 'active' && x.record.unmatched.length === x.record.patterns.length) return null;
+  if (recorded === 'none' && selectsNoFile(x)) return null;
   const source = x.state === 'active' ? x.record.source : r.review_exclude?.source ?? base;
   const h = head.slice(0, 12);
   return `field=review_exclude expected=${expected} recorded=${recorded} source=${source}: the receipt records another selection than ${base}'s review_exclude_paths gives ${h}; the receipt is unchanged; fix: rerun /ship's review on ${h} (gstack-review-dispatch pass --base ${base.replace(/^(refs\/remotes\/)?origin\//, '')}), then gstack-ship-receipt write --pr <n> --review-exclude <d> --body <body> with the digest gstack-review-dispatch verify --ledger <ledger> prints on REVIEW_EXCLUDE_RECORD: (write checks the digest against the base policy and does not read the review ledger, so only the rerun shows the reviewers read this selection)`;

test/review-exclude.test.ts

@@ -420,6 +420,79 @@ describe('the receipt binds the selection the reviewers read', () => {
       expect(b.code).toBe('POLICY_INVALID');
     }, 240_000);
   });
+
+  // Value: protects=a base policy change that selects no file the PR changed (a pattern gained, added beside another, removed or
+  //   reverted) no longer voids the receipt at carry, and the queue binds the carried receipt, while a changed pattern that selects
+  //   a PR file (excluded or kept) and an unreadable or malformed base policy still refuse; fails_when=carry compares the policy
+  //   digest alone (GST-1415: harness-evals #263 re-ran a full five-reader review of an unchanged product), or the acceptance
+  //   spreads to a selection that holds a PR file; why_new=GST-1415; seam=gstack-ship-receipt carry after a base merge, then
+  //   bindShippedReceipt on the carried receipt; no_claim=the queue's own comparison of uncarried receipts (GST-1408 above)
+  describe('carry after a base policy change (GST-1415)', () => {
+    const CODE = { 'lib/reader.js': 'exports.p = 1;\n', 'docs/guide.md': '# guide\n' };
+    const NO_KEY = JSON.stringify({}) + '\n';
+    const shipped = (files: Record<string, string>, basePolicy: string[] | null) => {
+      const repo = makeFixtureRepo({ prefix: 'gstack-exclude-carry-', baseFiles: basePolicy ? { '.gstack/ship-policy.json': policyText(basePolicy) } : {} });
+      repos.push(repo);
+      commit(repo, files);
+      const body = path.join(repo.root, 'body.md');
+      const extra = [...targetedAtHead(repo)];
+      if (basePolicy) {
+        const expected = writeReceipt(repo, body, extra).err.match(/field=review_exclude expected=([0-9a-f]{64}) /)?.[1];
+        expect(expected).toBeDefined();
+        extra.push('--review-exclude', expected!);
+      }
+      const w = writeReceipt(repo, body, extra);
+      expect({ status: w.status, err: w.err }).toMatchObject({ status: EXIT.ok });
+      return { repo, body, recorded: receiptIn(body).review_exclude?.digest ?? 'none' };
+    };
+    const carry = (repo: FixtureRepo, body: string) => runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
+
+    test('a pattern change that selects no PR file carries, and the queue binds the carried receipt', () => {
+      const cases: Array<{ name: string; before: string[] | null; after: string }> = [
+        { name: 'gained', before: null, after: policyText(['receipts/**']) },
+        { name: 'added beside another', before: ['receipts/**'], after: policyText(['receipts/**', 'results/**']) },
+        { name: 'removed beside another', before: ['receipts/**', 'results/**'], after: policyText(['receipts/**']) },
+        { name: 'reverted', before: ['receipts/**'], after: NO_KEY },
+      ];
+      for (const c of cases) {
+        const { repo, body, recorded } = shipped(CODE, c.before);
+        onBase(repo, { '.gstack/ship-policy.json': c.after });
+        const carried = carry(repo, body);
+        expect({ name: c.name, status: carried.status, err: carried.status === EXIT.ok ? '' : carried.err }).toEqual({ name: c.name, status: EXIT.ok, err: '' });
+        const head = repo.git('rev-parse', 'HEAD');
+        const now = receiptIn(body);
+        expect({ name: c.name, head: now.head, moved: (now.review_exclude?.digest ?? 'none') !== recorded }).toEqual({ name: c.name, head, moved: true });
+        const b = bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8'), undefined, [head]);
+        expect({ name: c.name, ok: b.ok, message: b.ok ? '' : b.message }).toEqual({ name: c.name, ok: true, message: '' });
+      }
+    }, 240_000);
+
+    test('a pattern change that selects a PR file, excluded or kept, still refuses carry', () => {
+      const touches: Array<Record<string, string>> = [{ 'receipts/a/result.json': '{"ok": true}\n' }, { 'receipts/run.js': 'exports.run = 1;\n' }];
+      const changes: Array<[string[] | null, string]> = [[null, policyText(['receipts/**'])], [['receipts/**'], NO_KEY]];
+      for (const touched of touches) {
+        for (const [before, after] of changes) {
+          const { repo, body, recorded } = shipped({ ...CODE, ...touched }, before);
+          onBase(repo, { '.gstack/ship-policy.json': after });
+          const refused = carry(repo, body);
+          expect({ touched: Object.keys(touched)[0], before, status: refused.status }).toEqual({ touched: Object.keys(touched)[0], before, status: EXIT.refused });
+          expect(refused.err).toContain(`review_exclude changed: recorded=${recorded} `);
+          expect(refused.err).toContain('(RECEIPT_CARRY_REFUSED;');
+        }
+      }
+    }, 240_000);
+
+    test('an unreadable or malformed base policy still refuses carry', () => {
+      for (const text of [JSON.stringify({ review_exclude_paths: 'receipts/**', min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION }) + '\n', '{ not json\n']) {
+        const { repo, body } = shipped(CODE, null);
+        onBase(repo, { '.gstack/ship-policy.json': text });
+        const refused = carry(repo, body);
+        expect({ text, status: refused.status }).toEqual({ text, status: EXIT.refused });
+        expect(refused.err).toContain('RECEIPT_CARRY_REFUSED');
+        expect(refused.err).toContain('so its review_exclude_paths cannot be read');
+      }
+    }, 240_000);
+  });
 });
 
 describe('a branch cannot hide code under a base pattern', () => {