fix: pre-landing review fixes

Review pass 2 (below floor): import takes only autoplan-regate records with autoplan:<run>@gate_rev=N evidence; record diffs with --no-renames so a rename is a delete plus an add; a scoped-fix-check also checks the approval record for the branch's K1 grant, so a fresh checkout with an empty ledger still stops; a trailing }, N) counts as a timeout only where it closes a test callback; the re-gate words add paid, eval and model names; Step 19 sets NS on its own line so a path with spaces survives. Also launches gstack-review-read through runScriptSync (free-tests #3197 tripwire, run 41746166882).

Review-Finding: specialist-batched#1 reader=specialist-batched
Review-Finding: specialist-batched#3 reader=specialist-batched
Review-Finding: specialist-batched#4 reader=specialist-batched
Review-Finding: red-team#1 reader=red-team
Review-Finding: red-team#2 reader=red-team
Review-Finding: red-team#3 reader=red-team
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-no-stop

@@ -14,9 +14,10 @@
  */
 import { spawnSync } from 'node:child_process';
 import * as path from 'node:path';
-import { appendRows, buildRow, locate } from '../lib/approval-record';
-import { EXIT, NO_STOP_CLASSES, exitTableHelp } from '../lib/headless-artifacts';
+import { appendRows, buildRow, locate, readRecord } from '../lib/approval-record';
+import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
 import { appendLedger, checkNoStop, ledgerPath, readLedger } from '../lib/no-stop';
+import { runScriptSync } from '../lib/script-invocation';
 
 const HELP = `gstack-no-stop — record one of the five no-stop defaults (docs/review-policy.md#no-stop)
 
@@ -44,10 +45,12 @@ ${exitTableHelp()}`;
 const flag = (args: string[], name: string) => { const i = args.indexOf(name); return i >= 0 ? args[i + 1] : undefined; };
 const git = (args: string[]) => spawnSync('git', args, { cwd: process.cwd(), encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 });
 const NEEDS_RANGE = ['scoped-fix-check', 'test-timeout-fix', 'stamp-only-carry'];
+const K1_BY = 'gstack no-stop default (K1)';
+const AUTOPLAN_EVIDENCE = /^autoplan:\S+@gate_rev=\d+$/;
 
 /** The commit the newest full or scoped pass reviewed (its review-log row's commit_full): round 2's PASS_HEAD. */
 function lastPassHead(): string | null {
-  const r = spawnSync(path.join(import.meta.dir, 'gstack-review-read'), [], { cwd: process.cwd(), encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 });
+  const r = runScriptSync(path.join(import.meta.dir, 'gstack-review-read'), [], { cwd: process.cwd(), encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 });
   const rows = (r.stdout ?? '').split('---CONFIG---')[0]!.split('\n').flatMap(l => { try { return [JSON.parse(l)]; } catch { return []; } });
   const pass = rows.filter(x => x.skill === 'review' && typeof x.commit_full === 'string').at(-1);
   return pass ? pass.commit_full : null;
@@ -60,7 +63,7 @@ function grant(head: string, detail: string, ledger: string): string | { error:
   const row = buildRow({
     ask: `No-stop default: one scoped fix-check past R51's cap (review_loop_grant ${head})`,
     words: `K1 no-stop default, docs/review-policy.md#no-stop: ${detail}`,
-    by: 'gstack no-stop default (K1)', scope: `review_loop_grant ${head}`, outcome: 'approved', source: `no-stop:${ledger.split('/').pop()}`,
+    by: K1_BY, scope: `review_loop_grant ${head}`, outcome: 'approved', source: `no-stop:${ledger.split('/').pop()}`,
   }, branch);
   if ('code' in row) return { error: row.message };
   const added = appendRows(loc, [row]);
@@ -81,6 +84,11 @@ function record(args: string[], json: boolean): number {
       process.stderr.write(`NO_STOP: refused ${cls} (--range-base ${rangeBase} is not the commit the last review pass read (${pass ? pass.slice(0, 12) : 'no review-log row'}), so the checked range would not be the range the fix-check covers); stop for the owner\n`);
       return EXIT.refused;
     }
+    const loc = locate({ cwd: process.cwd(), env: process.env, branch: git(['rev-parse', '--abbrev-ref', 'HEAD']).stdout.trim() });
+    if (!readLedger(file).some(r => r.class === cls) && !('code' in loc) && readRecord(loc).rows.some(r => r.by === K1_BY)) {
+      process.stderr.write(`NO_STOP: refused ${cls} (the approval record already holds this branch's K1 grant, from this or another checkout; a second goes to the owner); stop for the owner\n`);
+      return EXIT.refused;
+    }
   }
   let diff: string | undefined;
   if (rangeBase) {
@@ -91,7 +99,7 @@ function record(args: string[], json: boolean): number {
       if (merged.status !== 0) { process.stderr.write(`NO_STOP: refused ${cls} (git merge-tree ${base} ${rangeBase} failed: ${merged.stderr.trim() || 'conflict'}); stop for the owner\n`); return EXIT.refused; }
       from = merged.stdout.split('\n')[0]!.trim();
     }
-    const d = git(['diff', '--no-color', '--no-ext-diff', from, 'HEAD']);
+    const d = git(['diff', '--no-color', '--no-ext-diff', '--no-renames', from, 'HEAD']);
     if (d.status !== 0) { process.stderr.write(`gstack-no-stop record: git diff ${rangeBase} HEAD failed: ${d.stderr.trim()}\n`); return EXIT.fail; }
     diff = d.stdout;
   }
@@ -126,9 +134,12 @@ function main(argv: string[]): number {
     const file = ledgerPath(process.cwd());
     if (!src || !file) { process.stderr.write(`gstack-no-stop import: ${src ? 'not inside a git checkout' : '--from <file> is required'}\n`); return EXIT.usage; }
     const have = readLedger(file);
-    const fresh = readLedger(src).filter(r => (NO_STOP_CLASSES as readonly string[]).includes(r.class) && r.reason && r.evidence && !have.some(h => JSON.stringify(h) === JSON.stringify(r)));
+    const rows = readLedger(src);
+    const eligible = rows.filter(r => r.class === 'autoplan-regate' && r.reason && AUTOPLAN_EVIDENCE.test(r.evidence ?? ''));
+    const fresh = eligible.filter(r => !have.some(h => JSON.stringify(h) === JSON.stringify(r)));
     for (const r of fresh) appendLedger(file, r);
-    process.stdout.write(json ? JSON.stringify({ file, imported: fresh.length }) + '\n' : `NO_STOP: imported ${fresh.length} record(s) from ${src}\nNO_STOP_FILE: ${file}\n`);
+    const skipped = rows.length - eligible.length;
+    process.stdout.write(json ? JSON.stringify({ file, imported: fresh.length, skipped }) + '\n' : [`NO_STOP: imported ${fresh.length} record(s) from ${src}`, ...(skipped ? [`NO_STOP: skipped ${skipped} record(s): import takes only autoplan-regate records with autoplan:<run>@gate_rev=N evidence; record the other classes with gstack-no-stop record`] : []), `NO_STOP_FILE: ${file}`].join('\n') + '\n');
     return EXIT.ok;
   }
   if (cmd === 'list') {

lib/no-stop.ts

@@ -34,7 +34,7 @@ const VERSION = /\d+\.\d+\.\d+(?:\.\d+)?/g;
 const TEST_PATH = /(?:^|\/)(?:test|tests|__tests__|spec)\/|\.(?:test|spec)\.[cm]?[jt]sx?$/;
 const STAMP_PATHS = ['VERSION', 'package.json', 'CHANGELOG.md', 'agents-digest/gstack-AGENTS.md'];
 /** Plan-delta words that mean spend, a cap, an eval arm or a verdict changed: those re-gates go to the owner. */
-export const REGATE_WORDS = /\$|\b(?:spend|spending|spent|costs?|budgets?|prices?|caps?|capped|credits?|limits?|max|maximum|arms?|models?|judges?|verdicts?|thresholds?|bars?|pass(?:es)? at|trials?|samples?|stop rules?|kill rules?)\b|\bn\s*=/i;
+export const REGATE_WORDS = /\$|\b(?:spend|spending|spent|costs?|budgets?|prices?|caps?|capped|credits?|limits?|max|maximum|arms?|models?|judges?|verdicts?|thresholds?|bars?|pass(?:es)? at|trials?|samples?|stop rules?|kill rules?|paid|evals?|opus|sonnet|haiku|gpt[\w.-]*|gemini|claude)\b|\bn\s*=/i;
 
 /** Unified diff (git diff) to files, each with its one-for-one changed line pairs; a run of unequal - and + counts is unpaired. */
 export function parseDiff(diff: string): FileDiff[] {
@@ -65,8 +65,11 @@ export function parseDiff(diff: string): FileDiff[] {
   return files;
 }
 
-const timeoutValues = (s: string) => [...s.matchAll(TIMEOUT_VALUE)].filter(m => !m[1]!.startsWith('}') || /\)\s*;?\s*$/.test(s.slice(m.index! + m[0].length))).map(m => Number(m[2]!.replace(/_/g, '')));
-const maskTimeouts = (s: string) => s.replace(TIMEOUT_VALUE, (_m, key: string, _n: string, at: number) => (!key.startsWith('}') || /\)\s*;?\s*$/.test(s.slice(at + _m.length)) ? `${key}#` : _m));
+const TEST_CALL = /^\s*(?:test|it|describe|bench|beforeAll|beforeEach|afterAll|afterEach)(?:\.\w+)*\(/;
+/** A trailing `}, N)` is a timeout only where it closes a test callback: the line opens a test call, or the brace starts the line. */
+const isTimeout = (s: string, key: string, at: number, len: number) => !key.startsWith('}') || ((TEST_CALL.test(s) || !s.slice(0, at).trim()) && /\)\s*;?\s*$/.test(s.slice(at + len)));
+const timeoutValues = (s: string) => [...s.matchAll(TIMEOUT_VALUE)].filter(m => isTimeout(s, m[1]!, m.index!, m[0].length)).map(m => Number(m[2]!.replace(/_/g, '')));
+const maskTimeouts = (s: string) => s.replace(TIMEOUT_VALUE, (_m, key: string, _n: string, at: number) => (isTimeout(s, key, at, _m.length) ? `${key}#` : _m));
 
 /** Why the diff is not timeout-only, or null: every changed line pairs with one that differs only in larger timeout values (never an assertion). */
 export function notTimeoutOnly(files: FileDiff[]): string | null {

ship/sections/pr-body.md

@@ -220,8 +220,9 @@ per-lane counts (`{"<lane>":{"pass":N,"fail":N,"skip":N}}`) to
 
 ```bash
 rm -f .gstack/tmp/review-loop.json
+NS=$(~/.claude/skills/gstack/bin/gstack-no-stop list | sed -n 's/^NO_STOP_FILE: //p'); [ -s "$NS" ] || NS=
 ~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --no-ledger --out .gstack/tmp/review-loop.json &&
-~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr <pr-number> --base <base> --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) $(NS=$(~/.claude/skills/gstack/bin/gstack-no-stop list | sed -n 's/^NO_STOP_FILE: //p'); [ -s "$NS" ] && echo --no-stop @"$NS") --body "$PR_BODY_FILE"
+~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr <pr-number> --base <base> --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) ${NS:+--no-stop "@$NS"} --body "$PR_BODY_FILE"
 ```
 
 A Step 5 red-base pause adds `--base-red <pause file>` (a conditional receipt the queue

ship/sections/pr-body.md.tmpl

@@ -203,8 +203,9 @@ per-lane counts (`{"<lane>":{"pass":N,"fail":N,"skip":N}}`) to
 
 ```bash
 rm -f .gstack/tmp/review-loop.json
+NS=$(~/.claude/skills/gstack/bin/gstack-no-stop list | sed -n 's/^NO_STOP_FILE: //p'); [ -s "$NS" ] || NS=
 ~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --no-ledger --out .gstack/tmp/review-loop.json &&
-~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr <pr-number> --base <base> --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) $(NS=$(~/.claude/skills/gstack/bin/gstack-no-stop list | sed -n 's/^NO_STOP_FILE: //p'); [ -s "$NS" ] && echo --no-stop @"$NS") --body "$PR_BODY_FILE"
+~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr <pr-number> --base <base> --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) ${NS:+--no-stop "@$NS"} --body "$PR_BODY_FILE"
 ```
 
 A Step 5 red-base pause adds `--base-red <pause file>` (a conditional receipt the queue

test/no-stop.test.ts

@@ -79,6 +79,13 @@ describe('lib/no-stop: what qualifies', () => {
     expect(regateTouches('- Run 40 jobs per sweep\n+ Run 400 jobs per sweep').length).toBeGreaterThan(0);
     expect(regateTouches('- Keep the budgets\n+ Double the budgets').length).toBeGreaterThan(0);
   });
+
+  test('review pass 2 holes: a retry count is not a timeout, and dropping a paid eval or swapping a model is a re-gate', () => {
+    expect(check('test-timeout-fix', diff('test/a.test.ts', ['  retry({ fn }, 3);'], ['  retry({ fn }, 10);']))).toMatchObject({ ok: false });
+    expect(check('test-timeout-fix', diff('test/a.test.ts', ['  }, 5_000);'], ['  }, 30_000);']))).toMatchObject({ ok: true });
+    expect(regateTouches('- Run the paid eval\n+ Skip the paid eval').length).toBeGreaterThan(0);
+    expect(regateTouches('- Score it on Opus\n+ Score it on Haiku').length).toBeGreaterThan(0);
+  });
 });
 
 describe('gstack-no-stop record: the ledger, the grant and the receipt', () => {
@@ -136,6 +143,16 @@ describe('gstack-no-stop record: the ledger, the grant and the receipt', () => {
     expect(run('list', '--json').stdout).toBe(before.stdout);
   });
 
+  test('a checkout with an empty ledger still sees the branch\'s K1 grant in the approval record and stops', () => {
+    const file = JSON.parse(run('list', '--json').stdout).file as string;
+    const saved = fs.readFileSync(file, 'utf8');
+    fs.rmSync(file);
+    const r = run('record', '--class', 'scoped-fix-check', '--range-base', base, '--reason', 'again', '--evidence', 'commit x');
+    fs.writeFileSync(file, saved);
+    expect(r.status).toBe(3);
+    expect(r.stderr).toContain('the approval record already holds this branch\'s K1 grant');
+  });
+
   test('a class outside the five stops, and the range-based classes need --range-base', () => {
     expect(run('record', '--class', 'merge-anyway', '--reason', 'r', '--evidence', 'e').status).toBe(3);
     expect(run('record', '--class', 'stamp-only-carry', '--reason', 'r', '--evidence', 'e').status).toBe(2);
@@ -181,10 +198,22 @@ describe('gstack-no-stop record --class stamp-only-carry --base, and import', ()
     expect(r.stderr).toContain('app.ts is not a stamp file');
   });
 
-  test('import adds an autoplan run\'s records once and skips anything outside the five', () => {
+  test('a rename into changelog.d with an edit is a deletion plus an add, so it still stops', () => {
+    write('notes.md', 'one\ntwo\nthree\nfour\nfive\nsix\n'); git('add', '-A'); git('commit', '-qm', 'notes');
+    const from = git('rev-parse', 'HEAD');
+    fs.mkdirSync(path.join(work, 'changelog.d'), { recursive: true });
+    git('mv', 'notes.md', 'changelog.d/notes.md'); write('changelog.d/notes.md', 'one\ntwo\nthree\nfour\nfive\nseven\n'); git('add', '-A'); git('commit', '-qm', 'move');
+    const r = run('record', '--class', 'stamp-only-carry', '--range-base', from, '--reason', 'r', '--evidence', 'e');
+    expect(r.status).toBe(3);
+    expect(r.stderr).toContain('notes.md is not a stamp file');
+  });
+
+  test('import adds an autoplan run\'s re-gate records once and skips every other class', () => {
     const src = path.join(tmp, 'run-no-stop.json');
-    fs.writeFileSync(src, JSON.stringify([{ class: 'autoplan-regate', reason: 'plan edit, no spend', evidence: 'autoplan:run-1@gate_rev=2' }, { class: 'merge-anyway', reason: 'x', evidence: 'y' }]));
-    expect(run('import', '--from', src).stdout).toMatch(/^NO_STOP: imported 1 record\(s\)/m);
+    fs.writeFileSync(src, JSON.stringify([{ class: 'autoplan-regate', reason: 'plan edit, no spend', evidence: 'autoplan:run-1@gate_rev=2' }, { class: 'merge-anyway', reason: 'x', evidence: 'y' }, { class: 'stamp-only-carry', reason: 'x', evidence: 'y' }, { class: 'autoplan-regate', reason: 'x', evidence: 'hand-written' }]));
+    const first = run('import', '--from', src).stdout;
+    expect(first).toMatch(/^NO_STOP: imported 1 record\(s\)/m);
+    expect(first).toMatch(/^NO_STOP: skipped 3 record\(s\): import takes only autoplan-regate records/m);
     expect(run('import', '--from', src).stdout).toMatch(/^NO_STOP: imported 0 record\(s\)/m);
     expect(JSON.parse(run('list', '--json').stdout).records.map((r: NoStopRecord) => r.class)).toEqual(['stamp-only-carry', 'autoplan-regate']);
   });
@@ -201,7 +230,7 @@ describe('the no-stop defaults in the /ship and /autoplan sources', () => {
   test('a refused case and any other stop still stop; the receipt write carries the ledger', () => {
     expect(ship).toContain('three other routine stops go through `gstack-no-stop record` (`--help`); `recorded` goes on,\n`refused` stops.');
     expect(help).toContain('NO_STOP: refused (exit 3): the stop stands. Anything else still stops.');
-    expect(read('ship/sections/pr-body.md.tmpl')).toContain('echo --no-stop @"$NS"');
+    expect(read('ship/sections/pr-body.md.tmpl')).toContain('${NS:+--no-stop "@$NS"}');
     expect(read('ship/DRIVER.md.tmpl')).toContain('end the run, except `gstack-no-stop`');
     expect(read('autoplan/DRIVER.md.tmpl')).toContain("K1's `autoplan-regate` no-stop");
   });

test/size-ledger/ship/gsta258-l5-no-stop.json

@@ -1,4 +1,4 @@
 {
-  "bytes": 346,
-  "why": "GSTA-258 L5 (K1): the no-stop pointer in Step 5, the --no-stop receipt flag in Step 19's write, and the driver's gstack-no-stop exception"
+  "bytes": 349,
+  "why": "GSTA-258 L5 (K1): the no-stop pointer in Step 5, the --no-stop receipt flag in Step 19's write (NS set on its own line so a path with spaces survives), and the driver's gstack-no-stop exception"
 }