fix(ship-release): a gate-edited test is never owner-accepted; an empty fix_check.files stops (GST-965)

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

changelog.d/gst-965-finish-review-loop-exception.md

@@ -1,7 +1,7 @@
 **`gstack-ship-release finish` now accepts the owner's edit after a fix-check instead of asking for another review round (GST-965).** On gbrain #8425, the fix-check found a blocker, the owner's pre-agreed fallback was applied and "then ship" was granted. Finish still classed the edit as behavior and stopped with `insert 5–11.5 → 12–14 → 16`, a third round the R51 cap forbids, so /ship could not get through Step 16 without running its stages by hand.
 
 ### Added
-- **`finish --review-loop-exception <json|@file>`** (`lib/ship-release.ts`) takes the same record `gstack-ship-receipt write --review-loop-exception` takes. Paths changed since `fix_check.head` that the branch already touched at that commit (narrowed to the record's `fix_check.files` when it lists them) route as `accepted=` on the `ROUTE:` line, a `REVIEW_LOOP_EXCEPTION:` line names them with the owner, date and channel, and the docs stage carries them (`accepted=` in the DOCS line). A path outside that scope still routes as behavior, and with `--reviewed-base` (as /ship passes it) so do a gate-edited test and a file too large for the scoped review. A malformed record, or a `fix_check.head` that is not HEAD or an ancestor of it, stops before the release commit. Without the flag nothing changes.
+- **`finish --review-loop-exception <json|@file>`** (`lib/ship-release.ts`) takes the same record `gstack-ship-receipt write --review-loop-exception` takes. Paths changed since `fix_check.head` that the branch already touched at that commit (narrowed to the record's `fix_check.files` when it lists them) route as `accepted=` on the `ROUTE:` line, a `REVIEW_LOOP_EXCEPTION:` line names them with the owner, date and channel, and the docs stage carries them (`accepted=` in the DOCS line). A path outside that scope, a gate-edited test (with or without `--reviewed-base`) and a file too large for the scoped review still route as behavior. A malformed record, or a `fix_check.head` that is not HEAD or an ancestor of it, stops before the release commit. Without the flag nothing changes.
 
 ### Tests
-- `test/ship-rereview-triggers.test.ts`: an owner fallback edit to a source file and a doc after the fix-check stops without the record and passes Step 16 with it; a new file outside the scope (even when `fix_check.files` lists it), a fix-check head off the branch, a malformed record and an unreadable record file stop; a `fix_check.files` list narrows what is accepted.
+- `test/ship-rereview-triggers.test.ts`: an owner fallback edit to a source file and a doc after the fix-check stops without the record and passes Step 16 with it; a new file outside the scope (even when `fix_check.files` lists it), a fix-check head off the branch, a malformed record, an unreadable record file and an empty `fix_check.files` stop; a `fix_check.files` list narrows what is accepted; a test weakened after the fix-check stops with and without `--reviewed-base`.

docs/review-policy.md

@@ -422,9 +422,9 @@ under `REVIEW_LOOP:`. Step 16 takes the same record:
 paths the branch touched at `fix_check.head` (narrowed to the record's
 `fix_check.files` when it lists them) and changed since as `accepted=` in
 `ROUTE:`, prints them on a `REVIEW_LOOP_EXCEPTION:` line, and carries them
-through the docs stage. A path outside that scope still routes as behavior,
-and with `--reviewed-base` (as /ship passes it) so do a gate-edited test and a
-file too large for the scoped review. Without the record nothing changes.
+through the docs stage. A path outside that scope, a gate-edited test and a
+file too large for the scoped review still route as behavior, and without the
+record nothing changes.
 
 The receipt's summary line prints `passes=<n> blocking_found=<k> deferred=<m>`,
 which is what a scorecard tracks (target: a median of 1 pass per PR). A

lib/ship-release.ts

@@ -204,7 +204,8 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string
     const atHead = fork ? touchedPaths(cwd, fork, head) : null;
     if (!since || !atHead) { r.stop(`git could not list the paths changed since fix-check ${head.slice(0, 12)} or the paths the branch touched at it against ${baseRef}: stage 2 is blocked`); return null; }
     const listed = loop.fix_check.files;
-    const forced = new Set([...gated, ...(packetBody?.over ?? [])]);
+    const unscanned = [...by.behavior, ...by.docs].filter(isTestCode);
+    const forced = new Set([...gated, ...(packetBody?.over ?? []), ...(gateEditedPaths(cwd, reviewed, now, unscanned) ?? unscanned)]);
     by.accepted = [...by.behavior, ...by.docs].filter(p => since.has(p) && atHead.has(p) && (!listed || listed.includes(p)) && !forced.has(p)).sort();
     by.behavior = by.behavior.filter(p => !by.accepted.includes(p));
     by.docs = by.docs.filter(p => !by.accepted.includes(p));
@@ -332,6 +333,7 @@ export async function finish(argv: readonly string[], cwd: string): Promise<Repo
     try { raw = JSON.parse(loopArg.startsWith('@') ? fs.readFileSync(path.resolve(cwd, loopArg.slice(1)), 'utf8') : loopArg); } catch (e) { return r.stop(`--review-loop-exception: ${firstLine((e as Error).message)}`); }
     const parsed = parseReviewLoopException(raw);
     if (!parsed.ok) return r.stop(`--review-loop-exception: ${parsed.message}`);
+    if (Array.isArray((raw as { fix_check?: { files?: unknown } }).fix_check?.files) && !parsed.exception.fix_check.files) return r.stop('--review-loop-exception: fix_check.files lists no paths; omit it to accept the paths the branch touched at fix_check.head');
     if (git(cwd, ['merge-base', '--is-ancestor', parsed.exception.fix_check.head, 'HEAD']).status !== 0) return r.stop(`--review-loop-exception: fix_check.head ${parsed.exception.fix_check.head.slice(0, 12)} is not HEAD or an ancestor of it`);
     loop = parsed.exception;
   }

test/ship-rereview-triggers.test.ts

@@ -718,6 +718,23 @@ describe('finish with an owner-accepted edit after a fix-check (GST-965, GBRA-27
     expect(lineOf(r.stdout, 'ROUTE')).toMatch(/ behavior=1 docs=1 metadata=0 tests=0 base=unknown context=0 accepted=1$/);
     expect(lineOf(r.stdout, 'REVIEW_LOOP_EXCEPTION')).toStartWith('REVIEW_LOOP_EXCEPTION: accepted src/x.ts (');
     expect([r.status, r.stdout.includes('STOP: behavior, tests or build inputs changed since review (src/new.ts)')]).toEqual([3, true]);
+    const none = finish(f, reviewed, post, ['--review-loop-exception', `@${exceptionFile(f, fixHead, { fix_check: { head: fixHead, findings: 1, files: [] } })}`]);
+    expect([none.status, none.stdout]).toEqual([3, expect.stringContaining('STOP: --review-loop-exception: fix_check.files lists no paths')]);
+  });
+
+  // Value: protects=a test weakened after the fix-check is never owner-accepted, with or without --reviewed-base;
+  //   fails_when=finish accepts a gate-edited test (a .skip, a removed assertion) because no base sides were given; seam=finish's route
+  test('a gate-edited test after the fix-check still routes as behavior under the exception', () => {
+    const f = fx({ 'src/x.ts': 'export const x = 1;\n' }, { 'src/x.ts': 'export const x = 2;\n', 'test/x.test.ts': "import { expect, test } from 'bun:test';\ntest('a', () => { expect(2).toBe(2); });\n" });
+    const fixHead = f.git('rev-parse', 'HEAD').stdout.trim();
+    const reviewed = wtree(f);
+    const reviewedBase = mergeBase(f);
+    const post = snapshot(f);
+    f.write({ 'test/x.test.ts': "import { test } from 'bun:test';\ntest.skip('a', () => {});\n" }); f.commit('weaken the test after the fix-check');
+    for (const extra of [[], ['--reviewed-base', reviewedBase]]) {
+      const r = finish(f, reviewed, post, ['--review-loop-exception', `@${exceptionFile(f, fixHead)}`, ...extra]);
+      expect([r.status, lineOf(r.stdout, 'ROUTE'), r.stdout.includes('STOP: behavior, tests or build inputs changed since review (test/x.test.ts)')]).toEqual([3, expect.stringMatching(/ behavior=1 .* accepted=0$/), true]);
+    }
   });
 });