Let a clean follow-up review count after an earlier one found a blocker (GST-1973) (#4188)

* Add failing tests for fix-check chains whose earlier links found issues (GST-1973)

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

* Count fix-check chains whose earlier links found issues a later clean link read (GST-1973)

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

* List the new follow-up review test with the other release-script tests in the test-only ratchet (GST-1973)

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

* Match finding paths written with a leading ./ and test the remaining refusal guards (GST-1973)

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

* Name the changelog entry after the branch and say it in plain words (GST-1973)

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

* Reword the changelog entry in plain words without tracker codes (GST-1973)

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

* Refuse a follow-up review chain when no later clean review read an earlier finding's file (GST-1973)

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

* Say in the changelog entry that a finding no later clean review read now refuses the chain (GST-1973)

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

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-09 06:37:08 +00:00
1 parent b5c690cea3
commit 88c76ebefa
6 files changed
+291 -17

No files matched your search

@@ -0,0 +1,4 @@
---
important: blocker
---
- **/ship's finish now counts a chain of follow-up reviews after a granted follow-up review found a blocker.** A granted follow-up review that finds issues has to be logged `converged:false`, and finish refused every chain holding such a link, so a later clean follow-up review that read the fix could not count and only a fresh /ship review recovered the receipt, past the two-round cap. An earlier link that found issues now links when it finished its read on its own commit, its findings account for its counts, and every open or blocking finding names a file it read (a leading `./` on a finding's path names the same file). It counts no path itself, a path its findings name counts only through a clean follow-up review after it, and the `FIX_CHECK:` line marks it `(found issues)`. The chain must still be contiguous, every link still needs its own grant, and the newest link must still be a clean read; a gap, a range that starts mid-link, a newest link with findings, an unfinished read, and findings with no file all still refuse. A finding an earlier link logged on a file that no later clean follow-up review's range touched, even a file the branch never changed, now refuses the chain too, where before it dropped out and the chain counted.
+12 -7
View File
@@ -1870,17 +1870,22 @@ fix-check unbound the earlier one and lanes logged a synthetic combined row
(#4063: fc3 at 265d868, then fc4 over the merge). finish now walks back from the
newest row: while a row's `range_base` doesn't hold the reviewed tree, the link
before it is the newest earlier row whose commit is that `range_base`. The chain
counts only when it reaches a row whose `range_base` holds the reviewed tree and
every link is a clean, committed read with its own approved
`review_loop_grant`. A link's grant may name any stop head inside its
`range_base..commit`, its own commit included: the post-review flow grants
counts only when it reaches a row whose `range_base` holds the reviewed tree,
every link is a committed read with its own approved `review_loop_grant`, and
the newest link is clean. An earlier link that found issues (logged
`converged:false`, as review-army.md requires) still links when it finished its
read bound to its own commit, its findings account for its counts, and every
open or blocking finding names a file it read (GBRA-386, GST-1973): it counts
no path itself, a path its findings name counts only through a clean link
after it, and the line marks it `(found issues)`. A link's grant may name any
stop head inside its `range_base..commit`, its own commit included: the post-review flow grants
each fix-check at the commit it reads, so link 1's stop head is link 2's
`range_base` (founderstar#634, GST-1676). Grants are assigned newest link
first and a grant counts for one link only, so one grant never covers two
fix-checks: a lone grant naming that boundary goes to the newer link and leaves
the older one ungranted. A path counts as `fix_checked` when some link's range
touched it and it is unchanged since that link (or its patch against the base is
still the one that link read). The line reads `FIX_CHECK: reviewed <paths>
the older one ungranted. A path counts as `fix_checked` when some clean link's
range touched it and it is unchanged since that link (or its patch against the
base is still the one that link read). The line reads `FIX_CHECK: reviewed <paths>
(fix-check chain of <n> from <range_base12>: at <commit12> grant <id>; …)`. A
`range_base` no earlier row ends at is a gap and refuses with the single-row
reason (`range_base <sha12> does not hold the reviewed tree`); a link that is
+2 -2
View File
@@ -97,7 +97,7 @@ export function parseHunks(diff: string, untracked: readonly string[] = []): Hun
return map;
}
function normalizePath(p: string): string {
export function normalizePath(p: string): string {
return p.replace(/\\/g, '/').replace(/^\.\//, '');
}
@@ -124,7 +124,7 @@ export function locusOf(finding: ScopeFinding, diffHunks: HunkMap, rangeHunks: H
return 'outside';
}
function fingerprintPath(fp: unknown): string | undefined {
export function fingerprintPath(fp: unknown): string | undefined {
if (typeof fp !== 'string' || fp.startsWith('shared-libs:')) return undefined;
const [path] = fp.split(':');
return path || undefined;
+30 -8
View File
@@ -22,6 +22,8 @@ import { mechanicalPaths, writeMechanicalRecord } from './ship-mechanical';
import { uidSidecars } from './ship-sidecar';
import type { MechanicalConstant } from './ship-policy-mechanical';
import { reviewFreshness } from './review-evidence';
import { isCriticalFinding, isOpenByAction } from './headless-artifacts';
import { fingerprintPath, normalizePath } from './review-scope';
import { forbidSpecs } from './review-fetch';
import { readRuledNotes, ruledNoteText } from './ruled-notes';
import { parseReviewLoopException, type ReviewLoopException } from './ship-receipt-depth';
@@ -255,8 +257,11 @@ type FixLink = { row: Record<string, any>; commit: string; from: string };
* head inside its range_base..commit, its own commit included (the post-review flow grants each fix-check at the commit it
* reads, GST-1676); a grant counts for one link only, assigned newest link first, so one grant naming the boundary or two
* links' commits covers one, and the older link needs a grant of its own.
* A range_base no earlier row ends at is a gap, and the chain does not count. A path counts as fix_checked when a link's
* range touched it and it is unchanged since that link, or its patch against the base is still the one the link read.
* A range_base no earlier row ends at is a gap, and the chain does not count. A link before the newest may, instead of a clean
* read, be a finished read bound to its own committed content that found issues (logged converged:false, GST-1973) when its
* findings account for its counts and each open or blocking one names a file it read: it counts no path itself, and a path its
* findings name counts only through a clean link after it, and a finding no later clean link's range touched refuses the chain. A path counts as fix_checked when a clean link's range touched it and it is unchanged since that link,
* or its patch against the base is still the one the link read.
*/
function fixChecked(cwd: string, o: { reviewed: string; now: string; baseRef: string; parent?: string; currentBase?: string; candidates: string[] }, r: Report): { counted: boolean; paths: string[] } | null {
const rows: Record<string, any>[] = bin(cwd, 'gstack-review-read', [], 60_000).stdout.split('\n---CONFIG---')[0].split('\n')
@@ -288,31 +293,48 @@ function fixChecked(cwd: string, o: { reviewed: string; now: string; baseRef: st
if (g) used.add(g.id);
granted[k] = g;
}
const ruledOn = (l: FixLink) => new Set(rulings.filter(n => n.links.includes(l)).map(n => n.finding));
const why = (l: FixLink) => {
const fresh = reviewFreshness(l.row, l.row.wtree, new Set(rulings.filter(n => n.links.includes(l)).map(n => n.finding)));
const fresh = reviewFreshness(l.row, l.row.wtree, ruledOn(l));
return fresh?.status !== 'CURRENT' ? fresh?.reason ?? 'not a review row'
: l.row.tree !== l.row.wtree || !onBranch(l.commit) ? 'the row was logged on content no branch commit holds'
: !onBranch(l.from) ? 'the row names no range_base commit on this branch' : '';
};
const at = (l: FixLink) => (chain.length > 1 ? `; fix-check at ${l.commit.slice(0, 12)}, link ${chain.indexOf(l) + 1} of ${chain.length}` : '');
const whys = new Map(chain.map(l => [l, why(l)]));
const clean = new Set(chain.filter(l => !whys.get(l)));
const flagged = (l: FixLink): string[] | null => {
const b = l.row.review_binding, findings: Record<string, any>[] = Array.isArray(l.row.findings) ? l.row.findings.filter((f: unknown) => !!f && typeof f === 'object') : [];
const paths = findings.filter(f => (f.blocking === true || isCriticalFinding(f) || isOpenByAction(f)) && !ruledOn(l).has(f.fingerprint))
.map(f => typeof f.path === 'string' ? f.path : fingerprintPath(f.fingerprint)).map(p => p && normalizePath(p));
const bound = l.row.completed === true && l.row.scope !== 'fix-range' && (b?.state === 'incomplete' || b?.state === 'verified') && !!l.row.tree && b.start_wtree === l.row.tree && b.end_wtree === l.row.tree;
const accounted = Array.isArray(l.row.findings) && findings.length >= Number(l.row.issues_found ?? 0) && findings.filter(isCriticalFinding).length >= Number(l.row.critical ?? 0);
return bound && accounted && onBranch(l.commit) && onBranch(l.from) && paths.every(p => !!p && git(cwd, ['cat-file', '-e', `${l.commit}:${p}`]).status === 0) ? paths as string[] : null;
};
const handedOn = new Map<FixLink, string[]>(chain.slice(0, -1).filter(l => !clean.has(l)).flatMap(l => (p => p ? [[l, p] as const] : [])(flagged(l))));
const newestFirst = [...chain].reverse();
const stale = newestFirst.map(l => ({ l, w: why(l) })).find(x => x.w);
const stale = newestFirst.map(l => ({ l, w: handedOn.has(l) ? '' : whys.get(l)! })).find(x => x.w);
const ungranted = newestFirst.find(l => !granted[chain.indexOf(l)]);
const ranges = new Map(chain.filter(l => clean.has(l)).map(l => [l, touchedPaths(cwd, l.from, l.commit)]));
const uncovered = [...handedOn].map(([d, ps]) => ({ d, p: ps.find(p => !chain.slice(chain.indexOf(d) + 1).some(j => ranges.get(j)?.has(p))) })).find(x => x.p);
const notCounted = !chain.length ? '' : stale ? `${stale.w}${at(stale.l)}` : gap ? `${gap}${at(chain[0]!)}`
: uncovered ? `a finding on ${uncovered.p} that no later clean fix-check's range touched${at(uncovered.d)}`
: ungranted ? `no approved review_loop_grant names a stop head inside its range_base..commit${at(ungranted)}` : '';
if (notCounted) r.add(`FIX_CHECK: not counted (${notCounted})`);
if (!chain.length || notCounted) return { counted: false, paths: [] };
const read: ((p: string) => boolean)[] = [];
for (const l of chain) {
const range = touchedPaths(cwd, l.from, l.commit), after = touchedPaths(cwd, l.commit, o.now);
for (const [k, l] of chain.entries()) {
if (!clean.has(l)) continue;
const later = new Set(chain.slice(k + 1).flatMap(d => handedOn.get(d) ?? []));
const range = ranges.get(l), after = touchedPaths(cwd, l.commit, o.now);
if (!range || !after) { r.stop(`git could not diff the fix-check range ${l.from.slice(0, 12)}..${l.commit.slice(0, 12)} against the reviewed and current trees: stage 2 is blocked`); return null; }
const sides = { reviewedBase: patchBase(cwd, o.baseRef, l.commit, o.parent), reviewed: l.commit, currentBase: o.currentBase ?? git(cwd, ['merge-base', o.baseRef, 'HEAD']).stdout.trim(), current: o.now };
read.push(p => range.has(p) && (!after.has(p) || (!!sides.reviewedBase && !!sides.currentBase && comparePath(cwd, sides, p) === 'same')));
read.push(p => !later.has(p) && range.has(p) && (!after.has(p) || (!!sides.reviewedBase && !!sides.currentBase && comparePath(cwd, sides, p) === 'same')));
}
const paths = o.candidates.filter(p => read.some(f => f(p))).sort();
const list = paths.length ? short(paths) : 'none';
r.add(chain.length === 1 ? `FIX_CHECK: reviewed ${list} (fix-check at ${chain[0]!.commit.slice(0, 12)}, range from ${chain[0]!.from.slice(0, 12)}, grant ${granted[0]!.id})`
: `FIX_CHECK: reviewed ${list} (fix-check chain of ${chain.length} from ${chain[0]!.from.slice(0, 12)}: ${chain.map((l, k) => `at ${l.commit.slice(0, 12)} grant ${granted[k]!.id}`).join('; ')})`);
: `FIX_CHECK: reviewed ${list} (fix-check chain of ${chain.length} from ${chain[0]!.from.slice(0, 12)}: ${chain.map((l, k) => `at ${l.commit.slice(0, 12)} grant ${granted[k]!.id}${clean.has(l) ? '' : ' (found issues)'}`).join('; ')})`);
return { counted: true, paths };
}
+242
View File
@@ -0,0 +1,242 @@
/**
* GST-1973 (GBRA-386, gbrain #8881): a granted fix-check that finds a blocker is logged converged:false
* (ship/sections/review-army.md), so its row never reads CURRENT. finish's chain walk refused every chain with such a
* link, and a later clean link that read the fix could not count: only a fresh /ship review recovered the receipt,
* past R51's two-round cap. An earlier link with findings now hands its paths on: the chain still has to be contiguous
* and granted link by link, its newest link must be a clean bound read, and only clean links count paths. A blocking
* finding on an earlier link closes only when a later clean link's range touched its path.
*/
import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test';
import * as path from 'node:path';
import { lineOf, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture';
setDefaultTimeout(90_000);
const BIN = path.dirname(RELEASE);
const LANE = 'tests=printf " 3 pass\\n 0 fail\\nRan 3 tests across 1 file. [5.00ms]\\n"';
const ships: ShipFx[] = [];
afterEach(() => { while (ships.length) ships.pop()!.cleanup(); });
const TEST_V1 = "import { expect, test } from 'bun:test';\ntest('a', () => { expect(2).toBe(2); });\n";
const TEST_V2 = "import { expect, test } from 'bun:test';\ntest('a', () => { expect(2).toBe(2); }, 1_000);\n";
const CLEAN = { completed: true, converged: true, status: 'clean', issues_found: 0, critical: 0, informational: 0, findings: [] };
/** The shape of GBRA-386's 990e7b6e row: a granted fix-check that found one CRITICAL and two filed notes, logged converged:false. */
const found = (p = 'src/x.ts', o: Record<string, unknown> = {}) => ({
completed: true, converged: false, status: 'issues_found', issues_found: 3, critical: 1, informational: 2,
findings: [
{ fingerprint: `${p}:93:red-team`, severity: 'CRITICAL', action: 'deferred', locus: 'fix-range', kind: 'defect', blocking: true, raised_by: 'red-team' },
{ fingerprint: 'src/x.ts:121:testing', severity: 'INFORMATIONAL', action: 'noted', locus: 'fix-range', kind: 'test-gap', blocking: false, raised_by: 'testing', issue: 8897 },
{ fingerprint: 'src/x.ts:121:red-team', severity: 'INFORMATIONAL', action: 'noted', locus: 'fix-range', kind: 'defect', blocking: false, raised_by: 'red-team', issue: 8897 },
],
...o,
});
const NONCONVERGED = 'missing start capture or incomplete/nonconverged pass';
const STOP = 'STOP: behavior, tests or build inputs changed since review (';
const run = (f: ShipFx, bin: string, args: string[]) => f.run(path.join(BIN, bin), args, { GSTACK_APPROVALS_MIRROR: '0' });
const headOf = (f: ShipFx) => f.git('rev-parse', 'HEAD').stdout.trim();
const at = (c: string) => c.slice(0, 12);
const stopLine = (out: string) => (l => l.slice(0, l.indexOf(')') + 1))(out.split('\n').find(l => l.startsWith(STOP)) ?? '');
const notCountedAt = (c: string) => new RegExp(`^FIX_CHECK: not counted \\(.*; fix-check at ${at(c)}, link 1 of 2\\)$`);
/** The reviewed tree is the feature head; src/y.ts is a second code path for the finding-path cases. */
function reviewedShape() {
const f = shipFixture({ 'src/x.ts': 'export const x = 1;\n', 'src/y.ts': 'export const y = 1;\n' }, { 'src/x.ts': 'export const x = 2;\n', 'test/x.test.ts': TEST_V1 });
ships.push(f);
return { f, t: headOf(f), reviewed: f.run(path.join(BIN, 'gstack-wtree'), []).stdout.trim(), reviewedBase: f.git('merge-base', 'origin/main', 'HEAD').stdout.trim() };
}
const edit = (f: ShipFx, files: Record<string, string>, msg: string) => { f.write(files); f.commit(msg); return headOf(f); };
/** One granted fix-check at HEAD reading range_base..HEAD; the owner's grant names the commit it reads (GST-1676). */
function link(f: ShipFx, from: string, row: Record<string, unknown>, o: { grant?: boolean } = {}) {
const commit = headOf(f);
let grant = '';
if (o.grant !== false) {
const g = run(f, 'gstack-approvals', ['add', '--ask', `Owner grant: one scoped fix-check (review_loop_grant ${at(commit)})`, '--words', 'one scoped fix-check', '--by', 'gstack minder (GSTA-36)', '--scope', 'this /ship invocation', '--json']);
expect(g.status, g.stderr).toBe(0);
grant = JSON.parse(g.stdout).added[0] as string;
}
const start = run(f, 'gstack-review-log', ['--start', 'review-fix-check']);
expect(start.status, start.stderr).toBe(0);
const logged = run(f, 'gstack-review-log', [JSON.stringify({ skill: 'review-fix-check', timestamp: '2026-11-09T00:00:00Z', via: 'ship', review_scope: 'frozen', range_base: from, ...row }), '--finish', start.stdout.trim()]);
expect(logged.status, logged.stderr).toBe(0);
return { commit, grant };
}
function finish(f: ShipFx, reviewed: string, reviewedBase: string) {
const post = path.join(f.root, 'post.json');
expect(f.run(path.join(BIN, 'gstack-docs-candidate'), ['snapshot', '--out', post, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', '--docs', 'README.md']).status).toBe(0);
return f.run(RELEASE, ['finish', '--base', 'main', '--reviewed-tree', reviewed, '--docs-post', post, '--reviewed-base', reviewedBase, '--lane', LANE]);
}
describe('finish counts a fix-check chain whose earlier links found issues a later clean link read (GST-1973)', () => {
// Value: protects=the GBRA-386 shape counts: T..A and A..B each found a CRITICAL (logged converged:false), B..C read the fixes
// clean, and every path C's range touched counts; fails_when=finish refuses link 2 as nonconverged (FIX_CHECK: not counted ...
// link 2 of 3) and src/x.ts routes as behavior, so only a fresh review recovers the receipt; seam=finish's chain walk
test('T to A (findings) to B (findings) to C (clean) counts at C', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
const a = link(f, t, found());
edit(f, { 'src/x.ts': 'export const x = 4;\n' }, 'fix fix-check 1 findings');
const b = link(f, a.commit, found());
edit(f, { 'src/x.ts': 'export const x = 5;\n', 'test/x.test.ts': TEST_V2 }, 'fix fix-check 2 CRITICAL');
const c = link(f, b.commit, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).toBe(`FIX_CHECK: reviewed src/x.ts,test/x.test.ts (fix-check chain of 3 from ${at(t)}: at ${at(a.commit)} grant ${a.grant} (found issues); at ${at(b.commit)} grant ${b.grant} (found issues); at ${at(c.commit)} grant ${c.grant})`);
expect([r.status, lineOf(r.stdout, 'ROUTE')], r.stdout).toEqual([0, expect.stringMatching(/ behavior=0 .* fix_checked=2$/)]);
});
// Value: protects=one fix round after a fix-check that found a blocker counts: T..A found a CRITICAL, A..C read the fix clean;
// fails_when=finish refuses link 1 as nonconverged; seam=finish's chain walk
test('T to A (findings) to C (clean) counts at C', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
const a = link(f, t, found());
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'fix fix-check 1 CRITICAL');
const c = link(f, a.commit, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).toBe(`FIX_CHECK: reviewed src/x.ts,test/x.test.ts (fix-check chain of 2 from ${at(t)}: at ${at(a.commit)} grant ${a.grant} (found issues); at ${at(c.commit)} grant ${c.grant})`);
expect([r.status, lineOf(r.stdout, 'ROUTE')], r.stdout).toEqual([0, expect.stringMatching(/ behavior=0 .* fix_checked=2$/)]);
});
});
describe('finish still refuses fix-check chains that do not read the fixes (GST-1973)', () => {
// Value: protects=contiguity: a commit after the link with findings that no fix-check read breaks the chain; fails_when=a clean
// link counts from any branch commit once an earlier link found issues; seam=finish's chain walk
test('a gap between the link with findings and the clean link refuses', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
link(f, t, found());
const gap = edit(f, { 'src/x.ts': 'export const x = 4;\n' }, 'an unread edit');
edit(f, { 'test/x.test.ts': TEST_V2 }, 'the clean link fix');
link(f, gap, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).toBe(`FIX_CHECK: not counted (range_base ${at(gap)} does not hold the reviewed tree)`);
expect([r.status, stopLine(r.stdout)], r.stdout).toEqual([3, `${STOP}src/x.ts,test/x.test.ts)`]);
});
// Value: protects=a clean link joins only at the previous link's end: a range_base inside the earlier link's range is no link's
// end commit; fails_when=finish accepts any range_base the earlier link's range holds; seam=finish's chain walk
test('a clean link whose range starts inside the earlier link\'s range, not at its end, refuses', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
const mid = edit(f, { 'src/y.ts': 'export const y = 2;\n' }, 'first half of the fix');
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'second half of the fix');
link(f, t, found());
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'fix the CRITICAL');
link(f, mid, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).toBe(`FIX_CHECK: not counted (range_base ${at(mid)} does not hold the reviewed tree)`);
expect([r.status, stopLine(r.stdout)], r.stdout).toEqual([3, `${STOP}src/x.ts,src/y.ts,test/x.test.ts)`]);
});
// Value: protects=the newest link must be at the current tree for what it read: an edit after the clean link routes as
// behavior; fails_when=a counted chain covers edits made after its last link; seam=finish's per-path read test
test('a clean last link that is not at the current tree leaves the later edit as behavior', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
const a = link(f, t, found());
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'fix fix-check 1 CRITICAL');
link(f, a.commit, CLEAN);
edit(f, { 'src/x.ts': 'export const x = 9;\n' }, 'an edit after the last fix-check');
const r = finish(f, reviewed, reviewedBase);
expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).not.toMatch(/reviewed [^ ]*src\/x\.ts/);
expect([r.status, stopLine(r.stdout)], r.stdout).toEqual([3, expect.stringMatching(/^STOP: behavior, tests or build inputs changed since review \([^)]*src\/x\.ts/)]);
});
// Value: protects=the newest link must itself be a clean, converged read: a chain ending in a link with findings never counts,
// alone or after a clean link; fails_when=finish accepts a link with findings anywhere in the chain; seam=finish's chain walk
test('a non-converged last link refuses, alone or after a clean link', () => {
const one = reviewedShape();
edit(one.f, { 'src/x.ts': 'export const x = 3;\n', 'test/x.test.ts': TEST_V2 }, 'fix the round-2 blocker');
link(one.f, one.t, found());
const r1 = finish(one.f, one.reviewed, one.reviewedBase);
expect([r1.status, lineOf(r1.stdout, 'FIX_CHECK')], r1.stdout).toEqual([3, `FIX_CHECK: not counted (${NONCONVERGED})`]);
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
const a = link(f, t, CLEAN);
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'a second fix');
const c = link(f, a.commit, found());
const r = finish(f, reviewed, reviewedBase);
expect([r.status, lineOf(r.stdout, 'FIX_CHECK')], r.stdout).toEqual([3, `FIX_CHECK: not counted (${NONCONVERGED}; fix-check at ${at(c.commit)}, link 2 of 2)`]);
expect(stopLine(r.stdout)).toBe(`${STOP}src/x.ts,test/x.test.ts)`);
});
// Value: protects=a blocking finding closes only when a later clean link read its path: an earlier clean link's read of
// src/y.ts does not survive a later link that found a CRITICAL there, when no clean link after it touched src/y.ts;
// fails_when=the chain counts src/y.ts from the first link's read; seam=finish's per-path read test
test('a clean link that did not read the path an earlier finding named leaves that path as behavior', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/y.ts': 'export const y = 2;\n' }, 'fix the round-2 blocker in y');
const a = link(f, t, CLEAN);
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'a fix in x');
const b = link(f, a.commit, found('src/y.ts'));
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'a second fix in x');
link(f, b.commit, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).toMatch(/^FIX_CHECK: not counted \(a finding on src\/y\.ts that no later clean fix-check's range touched; fix-check at [0-9a-f]{12}, link 2 of 3\)$/);
expect([r.status, stopLine(r.stdout)], r.stdout).toEqual([3, expect.stringMatching(/^STOP: behavior, tests or build inputs changed since review \([^)]*src\/y\.ts/)]);
});
// Value: protects=a finding path written ./src/y.ts names the same file as git's src/y.ts, so it still masks the earlier read;
// fails_when=finish compares the reviewer's raw path to git's root-relative paths and counts src/y.ts; seam=flagged()'s paths
test('a finding path written with a leading ./ still names the file a later clean link must read', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/y.ts': 'export const y = 2;\n' }, 'fix the round-2 blocker in y');
const a = link(f, t, CLEAN);
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'a fix in x');
const b = link(f, a.commit, found('./src/y.ts'));
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'a second fix in x');
link(f, b.commit, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).toMatch(/^FIX_CHECK: not counted \(a finding on src\/y\.ts that no later clean fix-check's range touched; fix-check at [0-9a-f]{12}, link 2 of 3\)$/);
expect([r.status, stopLine(r.stdout)], r.stdout).toEqual([3, expect.stringMatching(/^STOP: behavior, tests or build inputs changed since review \([^)]*src\/y\.ts/)]);
});
// Value: protects=an earlier link's finding counts as handled only when a later clean link's range touched its file: a CRITICAL
// on a file the branch never changed, a doubled ./ spelling and a directory all refuse the chain; fails_when=a finding on a path
// no routing candidate holds drops out and the chain counts; seam=finish's chain walk (cross-family trail read, GSTA-425)
test('a finding no later clean link read refuses the chain, even on a file the branch never changed', () => {
for (const p of ['src/y.ts', '././src/x.ts', 'src']) {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
const a = link(f, t, found(p));
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'fix fix-check 1 findings');
link(f, a.commit, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect([r.status, lineOf(r.stdout, 'FIX_CHECK')], r.stdout).toEqual([3, expect.stringMatching(notCountedAt(a.commit))]);
}
});
// Value: protects=every link needs its own approved review_loop_grant, the link that found issues included; fails_when=a link
// with findings rides on the clean link's grant; seam=finish's per-link grant assignment
test('a link with findings and no grant of its own refuses the chain', () => {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
const a = link(f, t, found(), { grant: false });
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'fix fix-check 1 CRITICAL');
link(f, a.commit, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect([r.status, lineOf(r.stdout, 'FIX_CHECK')], r.stdout).toEqual([3, expect.stringMatching(notCountedAt(a.commit))]);
expect(stopLine(r.stdout)).toBe(`${STOP}src/x.ts,test/x.test.ts)`);
});
// Value: protects=an earlier link hands its paths on only when its row says what it found: an unfinished read, a blocking
// finding with no path, and claimed counts the findings list does not hold all refuse; fails_when=finish trusts a link whose
// findings it cannot place; seam=finish's chain walk
test('an earlier link that did not finish its read, or whose findings cannot be placed, refuses', () => {
const rows = [
found('src/x.ts', { completed: false, status: 'unavailable' }),
found('src/x.ts', { findings: [{ fingerprint: 'p2/adversarial-native#1', severity: 'CRITICAL', action: 'deferred', blocking: true, raised_by: 'adversarial-native' }], issues_found: 1, informational: 0 }),
found('src/x.ts', { issues_found: 5 }),
found('src/x.ts', { critical: 2 }),
found('src/gone.ts'),
];
for (const row of rows) {
const { f, t, reviewed, reviewedBase } = reviewedShape();
edit(f, { 'src/x.ts': 'export const x = 3;\n' }, 'fix the round-2 blocker');
const a = link(f, t, row);
edit(f, { 'src/x.ts': 'export const x = 4;\n', 'test/x.test.ts': TEST_V2 }, 'fix fix-check 1 CRITICAL');
link(f, a.commit, CLEAN);
const r = finish(f, reviewed, reviewedBase);
expect([r.status, lineOf(r.stdout, 'FIX_CHECK')], r.stdout).toEqual([3, expect.stringMatching(notCountedAt(a.commit))]);
}
});
});
+1
View File
@@ -230,6 +230,7 @@ const BASELINE = [
'test/shared-libs-source-reads.test.ts', // slices test/skill-e2e-shared-libs-paths.test.ts source (recorded 2026-10 when the ratchet learned paid-source slicing)
'test/shared-libs-stage-actor.test.ts', // slices test/skill-e2e-shared-libs.test.ts source (recorded 2026-10 when the ratchet learned paid-source slicing)
'test/ship-coverage-audit-af.test.ts', // slices test/skill-e2e-workflow.test.ts source (recorded 2026-10 when the ratchet learned paid-source slicing)
'test/ship-finish-fixed-links.test.ts', // GST-1973: runs bin/gstack-ship-release finish through test/helpers/ship-bin-fixture.ts's RELEASE path, like test/ship-release.test.ts
'test/ship-release.test.ts', // GSTA-167 PR 4: runs bin/gstack-ship-release through test/helpers/ship-bin-fixture.ts's RELEASE path, so it names no bin/ literal itself
'test/ship-section-fixture.test.ts',
'test/skill-budget-regression.test.ts',