Files
gstack/test/review-scope.test.ts
Garry Tanandcapy-ai[bot] 576839cb8c v1.91.256.0 feat(ship): review loop converges (frozen scope, 3-pass cap, NO FINDINGS) and receipts carry across conflict-only merges (#3481)
* feat(ship): review loop converges (frozen finding scope, 3-pass cap, NO FINDINGS first-class) and receipts carry across conflict-only merges

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

* test(ship): keep the Step 9 first-entry and Fix-First phrasing the skip/requeue contract asserts; fill the review record's pass, review_scope and range_base placeholders in the caller template test; narrow a test type

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

* fix: pre-landing review fixes (pass 1) and the review-loop budget wording

Two blocking findings from /ship's own pass 1, both reproduced in the QA smoke before the fix:
gstack-ship-receipt carry hashed the product diff with git patch-id --stable, which strips
whitespace, so a space moved in a reviewed shell line carried as "no product change"; and it
classed package.json and every lockfile as release metadata, so a dependency swap carried too.
The patch-id is now --verbatim, package.json and VERSION are metadata only while the change is a
version stamp (lib/pregate/stamp-only.ts), lockfiles are product unless the policy names them,
the test globs name code extensions, a dirty working tree refuses before anything is compared,
and a receipt carried twice keeps the reviewed head as carry.from.

gstack-review-scope: an unresolvable base is POLICY_SOURCE_UNAVAILABLE instead of a silent
working-tree read; --range-base is validated; git runs with core.quotePath=false; range_base is
null on pass 1; a blocking finding with no path is fix; category "testing" is a test finding
below the floor; defer searches once, files nothing on a failed search and exits 0 unfiled;
summary refuses zero matching passes and the receipt write is chained to it with &&; a finding
the diff causes elsewhere names the changed line in `cause` and classify follows it.

Review-loop budget wording (ideas from tstack's sol-loop and pstack's interrogate, rewritten;
NOTICE.md): the blocker / should / nit glossary and two stop rules in Step 9.4, the checklist
and docs/review-policy.md; a lead-judgment stage in the finding merge both /review and /ship
render. The core read diffs $RANGE_BASE from pass 2 on. Caps and pins re-measured with reasons.

Step 7 coverage audit: four tests added in existing files, the --review-loop case moved to its
own test (the shared test tripped bun's 5 s default).

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

* fix: pin the renumbered review-army stages and point the wording test at product code

CI at c3e27fc failed three free-tier tests, all this branch's own: the
workflow-required-steps fixture and shared-libs-rendering still pinned the
pre-Lead-judgment stage numbers, and test-of-test-ratchet counted
review-loop-budget-wording.test.ts as test-only code because it read only
generated markdown. The fixture and rendering test now pin stages 1-8, and the
wording test renders both armies through generateReviewArmy and reads the
authored ship template as well as the generated sections.

Pass-2 maintainability notes folded in: docs/troubleshooting.md names the two
new RECEIPT_CARRY_REFUSED causes (dirty tree, unreadable status) and the two new
REVIEW_SCOPE_INPUT causes (non-hex --range-base, summary with no matching
pass); gstack-review-scope's help states that classify reads the base branch's
policy and refuses an unresolvable base unless --policy is passed.

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

* fix: date a review-log row from its binding start when it has no timestamp

The Step 9.4 template asks the writer to fill `timestamp`; the logger stamps
`review_binding.started_at` but never `timestamp`. `gstack-review-scope
summary` dropped such a row silently, so this PR's own pass 2 was missing from
REVIEW_LOOP (passes=1 for a two-pass loop). A row now dates from `timestamp`,
else from its binding start; a row with neither is still not counted.

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

* fix: pass-4 review fixes (carry stamp rule, deleted-file locus, scanned defer, diff prefixes, row time)

Five findings at the blocking floor from the Step 11 outside voice (gpt-6.1-sol)
and the native adversarial pass, fixed as a fourth pass approved by GStack
Minder under the three-pass policy:

- carry: a path the policy names under stamp_files, stamp_paths or
  release_outputs is metadata only while its change is a stamp (versionStampOnly
  for a manifest kind, stampOnlyText otherwise), the rule the tree receipt
  applies; a logic change in a stamped source file is product. Every
  metadata-listed path is read at both heads and a non-stamp change drops its
  globs from the product exclusion.
- review-scope: parseHunks keeps a deleted file as a whole changed location, so
  a cause anchored in a removed export is the diff's to fix, not deferred.
- defer: issues are filed through lib/gstack-post's publication scan; a HIGH
  finding in the title or body refuses and a MEDIUM one needs a human, both
  recorded as unfiled with the reason; --repo names the destination.
- classify: both diffs pin --src-prefix=a/ --dst-prefix=b/ --no-textconv, so
  diff.mnemonicPrefix cannot send every finding outside.
- summary: a row dates from review_binding.started_at before the writer-typed
  timestamp, so a date-only value cannot drop a real pass.

Docs: ship-policy.md#receipt-carry, review-policy.md, the Step 9.4 template
and the CHANGELOG gate-integrity note record the rule and the pass.

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

* test(review-army): assert the Red Team branch the run took in the N+1 case

Under the blocking floor the Performance specialist rates a 12-line N+1
INFORMATIONAL, so the conditional Red Team does not activate; the case
encoded the old severity (N+1 is CRITICAL, a Red Team always follows). It
now asserts the branch the run took: a Red Team that ran must follow the
Performance specialist; no Red Team is correct only when the report's own
count says 0 critical and its Red Team line names that reason. The N+1
detection assertion is unchanged. The free contract test gains the
informational-not-activated and informational-claimed controls and the
budget test's synthetic report carries the count line.

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

* v1.91.256.0 stamp

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>
2026-10-26 20:48:01 +00:00

424 lines
33 KiB
TypeScript

/**
* Harness lane E: the /ship review loop's finding scope (lib/review-scope.ts,
* bin/gstack-review-scope). Five Captains hit a loop that re-read the whole
* diff each pass and fixed neighbours in pre-existing code until the cycle
* cap. Under `review_scope: frozen` pass 1 fixes the diff and defers blocking
* findings in unchanged code to issues; pass 2+ fixes only blocking findings
* in the range since the previous pass (or the diff) and notes the rest.
*/
import { afterEach, describe, expect, test } from 'bun:test';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { EXIT } from '../lib/headless-artifacts';
import {
isBlocking, kindOf, locusOf, parseHunks, renderDeferredIssue, renderDeferredSection, reviewLoopLine, scopeFindings, scopeLine, summarizeReviewLoop,
type ScopedFinding,
} from '../lib/review-scope';
import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture';
const ROOT = path.resolve(import.meta.dir, '..');
const repos: FixtureRepo[] = [];
const fixture = () => { const r = makeFixtureRepo({ prefix: 'review-scope-' }); repos.push(r); return r; };
afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
const DIFF = [
'diff --git a/src/auth.ts b/src/auth.ts', 'index 1..2 100644', '--- a/src/auth.ts', '+++ b/src/auth.ts',
'@@ -10,2 +10,4 @@ export function check() {', '+ const a = 1;', '+ const b = 2;', '@@ -40 +42,0 @@', '- old();',
'diff --git a/src/new.ts b/src/new.ts', 'new file mode 100644', '--- /dev/null', '+++ b/src/new.ts', '@@ -0,0 +1,3 @@', '+a', '+b', '+c',
'diff --git a/src/gone.ts b/src/gone.ts', 'deleted file mode 100644', '--- a/src/gone.ts', '+++ /dev/null', '@@ -1,3 +0,0 @@', '-a', '-b', '-c',
].join('\n');
describe('hunk parsing and locus', () => {
test('parseHunks reads new-side ranges, whole added files, untracked files, and keeps a deleted file as a whole changed location', () => {
const hunks = parseHunks(DIFF, ['scratch/untracked.ts']);
expect(hunks['src/auth.ts']).toEqual([[10, 13], [42, 42]]);
expect(hunks['src/new.ts']).toBe('whole');
expect(hunks['scratch/untracked.ts']).toBe('whole');
expect(hunks['src/gone.ts']).toBe('whole');
});
// A removed export is a changed location: a caller it breaks reports the caller's path with a cause
// in the deleted file, and the pass-4 outside review showed that anchor landed outside and deferred.
test('a cause anchored in a deleted file is the diff, so the broken caller is fixed here, not deferred', () => {
const diff = parseHunks(DIFF);
expect(locusOf({ path: 'src/consumer.ts', line: 80, cause: { path: 'src/gone.ts', line: 2 } }, diff, null)).toBe('diff');
const [scoped] = scopeFindings([{ fingerprint: 'src/consumer.ts:80:api', path: 'src/consumer.ts', line: 80, severity: 'CRITICAL', cause: { path: 'src/gone.ts' } }], { policy: 'frozen', pass: 1, diffHunks: diff, rangeHunks: null });
expect(scoped!.locus).toBe('diff');
expect(scoped!.scope).toBe('fix');
});
test('locusOf: inside a hunk or within 3 lines is the diff, farther is outside, an added file is whole, the range wins over the diff', () => {
const diff = parseHunks(DIFF);
expect(locusOf({ path: 'src/auth.ts', line: 11 }, diff, null)).toBe('diff');
expect(locusOf({ path: 'src/auth.ts', line: 16 }, diff, null)).toBe('diff');
expect(locusOf({ path: 'src/auth.ts', line: 17 }, diff, null)).toBe('outside');
expect(locusOf({ path: 'src/auth.ts', line: 200 }, diff, null)).toBe('outside');
expect(locusOf({ path: 'src/auth.ts' }, diff, null)).toBe('diff');
expect(locusOf({ path: 'src/new.ts', line: 999 }, diff, null)).toBe('diff');
expect(locusOf({ path: 'lib/other.ts', line: 1 }, diff, null)).toBe('outside');
expect(locusOf({ fingerprint: 'src/auth.ts:11:security' }, diff, null)).toBe('diff');
expect(locusOf({ fingerprint: 'shared-libs:abc' }, diff, null)).toBe('outside');
expect(locusOf({ severity: 'CRITICAL', summary: 'somewhere' }, diff, null)).toBe('outside');
// a defect the diff causes in an unchanged caller names the changed line in `cause`; the locus follows it
expect(locusOf({ path: 'lib/caller.ts', line: 80, cause: { path: 'src/auth.ts', line: 11 } }, diff, null)).toBe('diff');
expect(locusOf({ path: 'lib/caller.ts', line: 80, cause: { path: 'src/auth.ts', line: 300 } }, diff, null)).toBe('outside');
expect(locusOf({ path: 'lib/caller.ts', line: 80, cause: { path: 'src/new.ts' } }, diff, null)).toBe('diff');
const range = parseHunks('diff --git a/src/auth.ts b/src/auth.ts\n--- a/src/auth.ts\n+++ b/src/auth.ts\n@@ -11 +11 @@\n-x\n+y\n');
expect(locusOf({ path: 'src/auth.ts', line: 11 }, diff, range)).toBe('fix-range');
expect(locusOf({ path: 'src/auth.ts', line: 42 }, diff, range)).toBe('diff');
});
test('kind and the blocking floor: CRITICAL defects block; advisories, test gaps and INFORMATIONAL items never do', () => {
expect(kindOf({ advisory: true, severity: 'INFORMATIONAL' })).toBe('advisory');
expect(kindOf({ category: 'test-gap', severity: 'CRITICAL' })).toBe('test-gap');
// the Testing specialist's schema (review/specialists/testing.md) emits category "testing" for every finding it returns
const real = { severity: 'CRITICAL', confidence: 7, path: 'src/auth.ts', line: 42, category: 'testing', summary: 'denied path has no test', fingerprint: 'src/auth.ts:42:testing', specialist: 'testing' };
expect(kindOf(real)).toBe('test-gap');
expect(isBlocking(real)).toBe(false);
expect(fs.readFileSync(path.join(ROOT, 'review/specialists/testing.md'), 'utf8')).toContain('\"category\":\"testing\"');
expect(kindOf({ category: 'security', severity: 'CRITICAL' })).toBe('defect');
expect(isBlocking({ category: 'security', severity: 'CRITICAL' })).toBe(true);
expect(isBlocking({ category: 'security', severity: 'critical' })).toBe(true);
expect(isBlocking({ category: 'security', severity: 'INFORMATIONAL' })).toBe(false);
expect(isBlocking({ category: 'test-coverage', severity: 'CRITICAL' })).toBe(false);
expect(isBlocking({ advisory: true, severity: 'CRITICAL' })).toBe(false);
});
});
describe('scope decisions', () => {
const diff = parseHunks(DIFF);
const range = parseHunks('diff --git a/src/auth.ts b/src/auth.ts\n--- a/src/auth.ts\n+++ b/src/auth.ts\n@@ -11 +11 @@\n-x\n+y\n');
const findings = [
{ fingerprint: 'src/auth.ts:11:security', path: 'src/auth.ts', line: 11, severity: 'CRITICAL', category: 'security', summary: 'in range' },
{ fingerprint: 'src/auth.ts:42:logic', path: 'src/auth.ts', line: 42, severity: 'CRITICAL', category: 'logic', summary: 'in diff' },
{ fingerprint: 'src/auth.ts:42:naming', path: 'src/auth.ts', line: 42, severity: 'INFORMATIONAL', category: 'naming', summary: 'in diff, informational' },
{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts', line: 7, severity: 'CRITICAL', category: 'data-loss', summary: 'pre-existing' },
{ fingerprint: 'lib/legacy.ts:9:style', path: 'lib/legacy.ts', line: 9, severity: 'INFORMATIONAL', category: 'style', summary: 'pre-existing wording' },
{ fingerprint: 'shared-libs:deadbeef', severity: 'INFORMATIONAL', advisory: true, category: 'shared-libs', summary: 'advice' },
];
const by = (scoped: ScopedFinding[]) => Object.fromEntries(scoped.map(f => [f.fingerprint, `${f.locus}/${f.kind}/${f.blocking ? 'blocking' : 'below'}/${f.scope}`]));
test('all: every finding is fixed, whatever its locus (the pre-frozen loop)', () => {
const scoped = scopeFindings(findings, { policy: 'all', pass: 2, diffHunks: diff, rangeHunks: range });
expect(scoped.every(f => f.scope === 'fix')).toBe(true);
expect(by(scoped)['lib/legacy.ts:7:data-loss']).toBe('outside/defect/blocking/fix');
});
test('frozen pass 1: the diff is fixed, blocking findings outside are deferred, the rest are notes', () => {
const scoped = scopeFindings(findings, { policy: 'frozen', pass: 1, diffHunks: diff, rangeHunks: range });
expect(by(scoped)).toEqual({
'src/auth.ts:11:security': 'diff/defect/blocking/fix',
'src/auth.ts:42:logic': 'diff/defect/blocking/fix',
'src/auth.ts:42:naming': 'diff/defect/below/fix',
'lib/legacy.ts:7:data-loss': 'outside/defect/blocking/defer',
'lib/legacy.ts:9:style': 'outside/defect/below/note',
'shared-libs:deadbeef': 'outside/advisory/below/note',
});
expect(scopeLine('frozen', 1, scoped)).toBe('REVIEW_SCOPE: policy=frozen pass=1 findings=6 fix=3 defer=1 note=2 blocking=3');
// a blocking finding nobody could place is fixed now, never filed against '(no path)'
const unplaced = scopeFindings([{ severity: 'CRITICAL', category: 'logic', summary: 'somewhere' }], { policy: 'frozen', pass: 2, diffHunks: diff, rangeHunks: range });
expect(by(unplaced)['undefined']).toBe('outside/defect/blocking/fix');
});
test('diff-plus-blocking (the documented fallback, not the default): a blocking finding outside the diff is fixed here instead of filed; everything else is as frozen', () => {
const p1 = by(scopeFindings(findings, { policy: 'diff-plus-blocking', pass: 1, diffHunks: diff, rangeHunks: range }));
expect(p1['lib/legacy.ts:7:data-loss']).toBe('outside/defect/blocking/fix');
expect(p1['lib/legacy.ts:9:style']).toBe('outside/defect/below/note');
expect(p1['src/auth.ts:42:naming']).toBe('diff/defect/below/fix');
const p2 = by(scopeFindings(findings, { policy: 'diff-plus-blocking', pass: 2, diffHunks: diff, rangeHunks: range }));
expect(p2['lib/legacy.ts:7:data-loss']).toBe('outside/defect/blocking/fix');
expect(p2['src/auth.ts:42:naming']).toBe('diff/defect/below/note');
expect(Object.values(p2).some(v => String(v).endsWith('/defer'))).toBe(false);
});
test('frozen pass 2+: only blocking findings in the range or the diff are fixed; everything below the floor is a note', () => {
const scoped = scopeFindings(findings, { policy: 'frozen', pass: 2, diffHunks: diff, rangeHunks: range });
expect(by(scoped)).toEqual({
'src/auth.ts:11:security': 'fix-range/defect/blocking/fix',
'src/auth.ts:42:logic': 'diff/defect/blocking/fix',
'src/auth.ts:42:naming': 'diff/defect/below/note',
'lib/legacy.ts:7:data-loss': 'outside/defect/blocking/defer',
'lib/legacy.ts:9:style': 'outside/defect/below/note',
'shared-libs:deadbeef': 'outside/advisory/below/note',
});
});
});
describe('deferral rendering', () => {
const finding: ScopedFinding = { fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts', line: 7, severity: 'CRITICAL', category: 'data-loss', summary: 'Unbounded delete\nwithout a where clause', fix: 'add the tenant filter', specialist: 'security', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' };
test('the issue names the origin PR, the finding and a marker that keeps a rerun from filing twice; the body section references and never closes', () => {
const issue = renderDeferredIssue(finding, { repo: 'acme/app', pr: 42, branch: 'feat/x', head: 'a'.repeat(40) });
expect(issue.title).toBe('Deferred review finding: lib/legacy.ts:7 — Unbounded delete without a where clause');
expect(issue.body).toContain('acme/app#42 (head aaaaaaaaaaaa)');
expect(issue.body).toContain('- **Severity:** CRITICAL');
expect(issue.body).toContain('- **Suggested fix:** add the tenant filter');
expect(issue.body).toContain('- **Found by:** security');
expect(issue.marker).toBe('<!-- gstack-deferred-finding: lib/legacy.ts:7:data-loss -->');
expect(issue.body.trimEnd().endsWith(issue.marker)).toBe(true);
const section = renderDeferredSection(
[{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts:7', severity: 'CRITICAL', summary: 'Unbounded delete', issue: 77 },
{ fingerprint: 'lib/old.ts:1:auth', path: 'lib/old.ts:1', severity: 'CRITICAL', summary: 'Missing owner check', issue: null, reason: 'gh is not installed' }],
[{ path: 'src/auth.ts', line: 42, severity: 'INFORMATIONAL', summary: 'rename b', advisory: true }]);
expect(section).toContain('## Deferred findings');
expect(section).toContain('- Deferred: #77 — CRITICAL `lib/legacy.ts:7` Unbounded delete');
expect(section).toContain('- Deferred: unfiled (gh is not installed) — CRITICAL `lib/old.ts:1` Missing owner check');
expect(section).toContain('## Review notes');
expect(section).toContain('- INFORMATIONAL [ADVISORY] `src/auth.ts:42` rename b');
expect(section).not.toMatch(/\b(fixes|closes|resolves)\b/i);
expect(renderDeferredSection([], [])).toBe('');
});
});
describe('loop summary', () => {
test('counts this invocation\'s passes and unique blocking, deferred, noted and fixed findings', () => {
const rows = [
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T10:00:00Z', review_scope: 'frozen', findings: [
{ fingerprint: 'a:1:x', blocking: true, action: 'auto-fixed' }, { fingerprint: 'b:1:y', blocking: true, action: 'deferred', issue: 5 }, { fingerprint: 'c:1:z', action: 'noted' }] },
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T10:30:00Z', review_scope: 'frozen', findings: [
{ fingerprint: 'b:1:y', blocking: true, action: 'deferred', issue: 5 }, { fingerprint: 'c:1:z', action: 'noted' }] },
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T09:00:00Z', findings: [{ fingerprint: 'old:1:q', blocking: true, action: 'fixed' }] },
{ skill: 'adversarial-review', timestamp: '2026-10-26T10:15:00Z', findings: [{ fingerprint: 'd:1:w', blocking: true }] },
{ skill: 'review', timestamp: '2026-10-26T10:40:00Z', findings: [{ fingerprint: 'e:1:v', blocking: true }] },
];
const s = summarizeReviewLoop(rows, '2026-10-26T09:30:00Z');
expect(s).toEqual({ passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen' });
expect(reviewLoopLine(s)).toBe('REVIEW_LOOP: passes=2 blocking_found=2 deferred=1 noted=1 fixed=1 scope=frozen');
});
// A pass logged without the template's timestamp (the logger stamps review_binding.started_at, never
// timestamp) is still this invocation's pass; dropping it silently under-reported passes on #3481.
test('a row dates from the logger\'s binding start before the writer\'s timestamp; a row with neither is not counted', () => {
const rows = [
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T10:00:00Z', review_scope: 'frozen', findings: [{ fingerprint: 'a:1:x', blocking: true, action: 'auto-fixed' }] },
{ skill: 'review', via: 'ship', review_scope: 'frozen', review_binding: { state: 'changed', started_at: '2026-10-26T10:30:00.000Z' }, findings: [{ fingerprint: 'c:1:z', action: 'noted' }] },
// A writer-typed date-only timestamp parses as midnight, before --since; the logger's start keeps the pass.
{ skill: 'review', via: 'ship', timestamp: '2026-10-26', review_binding: { state: 'verified', started_at: '2026-10-26T10:45:00.000Z' }, findings: [{ fingerprint: 'd:1:w', action: 'noted' }] },
{ skill: 'review', via: 'ship', review_binding: { state: 'changed', started_at: '2026-10-26T09:00:00.000Z' }, findings: [{ fingerprint: 'old:1:q', action: 'noted' }] },
{ skill: 'review', via: 'ship', findings: [{ fingerprint: 'undated:1:q', action: 'noted' }] },
];
expect(summarizeReviewLoop(rows, '2026-10-26T09:30:00Z')).toEqual({ passes: 3, blocking_found: 1, deferred: 0, noted: 2, fixed: 1, scope: 'frozen' });
});
});
describe('gstack-review-scope (bin)', () => {
test('classify reads the branch diff and the pass range from git, honours the policy default, and refuses bad input with REVIEW_SCOPE_INPUT / REVIEW_SCOPE_RANGE', () => {
const repo = fixture();
const src = path.join(repo.work, 'src');
fs.mkdirSync(src, { recursive: true });
fs.writeFileSync(path.join(src, 'legacy.ts'), Array.from({ length: 30 }, (_, i) => `legacy ${i + 1}`).join('\n') + '\n');
fs.writeFileSync(path.join(src, 'auth.ts'), Array.from({ length: 30 }, (_, i) => `line ${i + 1}`).join('\n') + '\n');
repo.git('add', '.'); repo.git('commit', '-qm', 'seed'); repo.git('push', '-q', 'origin', 'main');
repo.git('checkout', '-qb', 'feat');
const lines = fs.readFileSync(path.join(src, 'auth.ts'), 'utf8').split('\n');
lines[9] = 'line 10 changed';
fs.writeFileSync(path.join(src, 'auth.ts'), lines.join('\n'));
repo.git('commit', '-qam', 'change line 10');
const pass1Head = repo.git('rev-parse', 'HEAD');
const findings = JSON.stringify([
{ fingerprint: 'src/auth.ts:10:logic', path: 'src/auth.ts', line: 10, severity: 'CRITICAL', category: 'logic', summary: 'wrong' },
{ fingerprint: 'src/auth.ts:25:logic', path: 'src/auth.ts', line: 25, severity: 'CRITICAL', category: 'logic', summary: 'neighbour' },
{ fingerprint: 'src/legacy.ts:3:security', path: 'src/legacy.ts', line: 3, severity: 'CRITICAL', category: 'security', summary: 'old' },
{ fingerprint: 'src/legacy.ts:4:style', path: 'src/legacy.ts', line: 4, severity: 'INFORMATIONAL', category: 'style', summary: 'wording' },
]);
const f = path.join(repo.root, 'findings.json');
fs.writeFileSync(f, findings);
// diff.mnemonicPrefix prints c/ and w/ instead of a/ and b/; classify pins the prefixes, so the user's git config cannot send every finding outside.
repo.git('config', 'diff.mnemonicPrefix', 'true');
const p1 = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '1', '--findings', f, '--json']);
expect(p1.status).toBe(EXIT.ok);
expect(p1.err.trim()).toBe('REVIEW_SCOPE: policy=frozen pass=1 findings=4 fix=1 defer=2 note=1 blocking=3');
const j1 = JSON.parse(p1.out);
expect(j1.policy).toBe('frozen');
expect(j1.findings.map((x: ScopedFinding) => `${x.fingerprint}=${x.locus}/${x.scope}`)).toEqual([
'src/auth.ts:10:logic=diff/fix', 'src/auth.ts:25:logic=outside/defer', 'src/legacy.ts:3:security=outside/defer', 'src/legacy.ts:4:style=outside/note']);
// pass 1 "fixed" line 10 and touched line 25 (a regression in the fix range)
lines[9] = 'line 10 fixed'; lines[24] = 'line 25 touched by the fix';
fs.writeFileSync(path.join(src, 'auth.ts'), lines.join('\n'));
repo.git('commit', '-qam', 'fix: pre-landing review fixes');
const p2 = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '2', '--range-base', pass1Head, '--findings', f, '--json']);
expect(p2.status).toBe(EXIT.ok);
expect(JSON.parse(p2.out).findings.map((x: ScopedFinding) => `${x.fingerprint}=${x.locus}/${x.scope}`)).toEqual([
'src/auth.ts:10:logic=fix-range/fix', 'src/auth.ts:25:logic=fix-range/fix', 'src/legacy.ts:3:security=outside/defer', 'src/legacy.ts:4:style=outside/note']);
const all = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '2', '--range-base', pass1Head, '--policy', 'all', '--findings', f, '--json']);
expect(JSON.parse(all.out).findings.every((x: ScopedFinding) => x.scope === 'fix')).toBe(true);
const noRange = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '2', '--findings', f]);
expect(noRange.status).toBe(EXIT.fail);
expect(noRange.err).toContain('(REVIEW_SCOPE_INPUT;');
const badRange = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '2', '--range-base', 'f'.repeat(40), '--findings', f]);
expect(badRange.status).toBe(EXIT.fail);
expect(badRange.err).toContain('(REVIEW_SCOPE_RANGE;');
// a --range-base on pass 1 is not applied, so the envelope does not claim it
const p1Range = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '1', '--range-base', pass1Head, '--findings', f, '--json']);
expect(JSON.parse(p1Range.out).range_base).toBeNull();
for (const args of [['--pass', '0'], ['--pass', '1', '--policy', 'loose'], ['--pass', '2', '--range-base', '--output=/tmp/x']]) {
const r = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', ...args, '--findings', f]);
expect(r.status).toBe(EXIT.fail);
expect(r.err).toContain('(REVIEW_SCOPE_INPUT;');
}
// the reviewed policy is the base's; an unresolvable base is refused by name, never read from the working tree
const noBase = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'nonesuch', '--pass', '1', '--findings', f]);
expect(noBase.status).toBe(EXIT.refused);
expect(noBase.err).toContain('(POLICY_SOURCE_UNAVAILABLE)');
expect(noBase.err).toContain('pass --policy to classify without the base');
fs.writeFileSync(f, '{"not":"an array"}');
const bad = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '1', '--findings', f]);
expect(bad.status).toBe(EXIT.fail);
expect(bad.err).toContain('(REVIEW_SCOPE_INPUT;');
const d = path.join(repo.root, 'deferred.json'); fs.writeFileSync(d, '{}');
const badRender = runBin(repo, 'gstack-review-scope', ['render', '--deferred', d]);
expect(badRender.status).toBe(EXIT.fail);
expect(badRender.err).toContain('expected the JSON array defer --out writes');
});
test('classify sees a non-ASCII changed path under its own name, so a CRITICAL finding there is fixed, not deferred to an issue', () => {
const repo = fixture();
fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true });
fs.writeFileSync(path.join(repo.work, 'src', 'café.ts'), 'export const a = 1;\n');
repo.git('add', '.'); repo.git('commit', '-qm', 'seed'); repo.git('push', '-q', 'origin', 'main');
repo.git('checkout', '-qb', 'feat');
fs.writeFileSync(path.join(repo.work, 'src', 'café.ts'), 'export const a = 2;\n');
repo.git('commit', '-qam', 'change');
fs.writeFileSync(path.join(repo.work, 'src', 'naïve.ts'), 'export const n = 1;\n');
const f = path.join(repo.root, 'findings.json');
fs.writeFileSync(f, JSON.stringify([
{ path: 'src/café.ts', line: 1, severity: 'CRITICAL', category: 'logic', summary: 'wrong' },
{ path: 'src/naïve.ts', line: 1, severity: 'CRITICAL', category: 'logic', summary: 'untracked' },
]));
const r = runBin(repo, 'gstack-review-scope', ['classify', '--base', 'main', '--pass', '1', '--findings', f, '--json']);
expect(r.status).toBe(EXIT.ok);
expect(JSON.parse(r.out).findings.map((x: ScopedFinding) => `${x.path}=${x.locus}/${x.scope}`)).toEqual(['src/café.ts=diff/fix', 'src/naïve.ts=diff/fix']);
});
test('defer without gh keeps the finding deferred as unfiled and still exits 0 (the PR body carries the reason), searches once before creating, and files nothing when the search fails; --dry-run files nothing', () => {
const repo = fixture();
const classified = path.join(repo.root, 'classified.json');
fs.writeFileSync(classified, JSON.stringify([
{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts', line: 7, severity: 'CRITICAL', category: 'data-loss', summary: 'Unbounded delete', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' },
{ fingerprint: 'src/auth.ts:42:naming', path: 'src/auth.ts', line: 42, severity: 'INFORMATIONAL', category: 'naming', summary: 'rename b', locus: 'diff', kind: 'defect', blocking: false, scope: 'note' },
{ fingerprint: 'src/auth.ts:11:security', path: 'src/auth.ts', line: 11, severity: 'CRITICAL', category: 'security', summary: 'fixed', locus: 'diff', kind: 'defect', blocking: true, scope: 'fix' },
]));
const dry = runBin(repo, 'gstack-review-scope', ['defer', '--findings', classified, '--pr', '42', '--dry-run', '--json']);
expect(dry.status).toBe(EXIT.ok);
expect(JSON.parse(dry.out)).toEqual([{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts:7', severity: 'CRITICAL', summary: 'Unbounded delete', issue: null, reason: 'dry run' }]);
const noGh = path.join(repo.root, 'nogh');
fs.mkdirSync(noGh);
fs.writeFileSync(path.join(noGh, 'gh'), '#!/bin/sh\necho "gh: not logged in" >&2; exit 4\n', { mode: 0o755 });
const out = path.join(repo.root, 'deferred.json');
const unfiled = runBin(repo, 'gstack-review-scope', ['defer', '--findings', classified, '--pr', '42', '--out', out], { PATH: `${noGh}:${process.env.PATH}` });
expect(unfiled.status).toBe(EXIT.ok);
expect(unfiled.err).toMatch(/^DEFERRED: unfiled \(gh issue list failed: gh: not logged in\) CRITICAL lib\/legacy\.ts:7$/m);
expect(unfiled.out).toContain('- Deferred: unfiled (gh issue list failed: gh: not logged in) — CRITICAL `lib/legacy.ts:7` Unbounded delete');
// the search failed, so nothing was created: a rate-limited rerun cannot duplicate the issue the marker exists to find
const listFails = path.join(repo.root, 'listfails');
fs.mkdirSync(listFails);
const created = path.join(listFails, 'created');
fs.writeFileSync(path.join(listFails, 'gh'), `#!/bin/sh\nif [ "$1" = issue ] && [ "$2" = list ]; then echo "gh: HTTP 403 rate limit" >&2; exit 1; fi\nif [ "$1" = issue ] && [ "$2" = create ]; then touch "${created}"; echo https://example.test/o/r/issues/1; exit 0; fi\nexit 1\n`, { mode: 0o755 });
const rateLimited = runBin(repo, 'gstack-review-scope', ['defer', '--findings', classified, '--pr', '42', '--json'], { PATH: `${listFails}:${process.env.PATH}` });
expect(rateLimited.status).toBe(EXIT.ok);
expect(JSON.parse(rateLimited.out)[0]).toMatchObject({ issue: null, reason: 'gh issue list failed: gh: HTTP 403 rate limit' });
expect(fs.existsSync(created)).toBe(false);
const rendered = runBin(repo, 'gstack-review-scope', ['render', '--deferred', out, '--findings', classified]);
expect(rendered.status).toBe(EXIT.ok);
expect(rendered.out).toContain('## Deferred findings');
expect(rendered.out).toContain('## Review notes');
expect(rendered.out).toContain('- INFORMATIONAL `src/auth.ts:42` rename b');
expect(rendered.out).not.toContain('src/auth.ts:11');
});
test('defer through gh creates one issue per deferred finding and reuses an open issue that already carries the marker', () => {
const repo = fixture();
const shim = path.join(repo.root, 'gh-shim');
fs.mkdirSync(shim);
const log = path.join(shim, 'calls.log');
fs.writeFileSync(path.join(shim, 'gh'), [
'#!/bin/sh', `echo "$*" >> "${log}"`,
'if [ "$1" = issue ] && [ "$2" = list ]; then',
` echo '[{"number":9,"url":"https://example.test/acme/app/issues/9","body":"x <!-- gstack-deferred-finding: lib/legacy.ts:7:data-loss --> y"},{"number":3,"url":"https://example.test/acme/app/issues/3","body":"<!-- gstack-deferred-finding: other:1:x -->"}]'; exit 0; fi`,
'if [ "$1" = issue ] && [ "$2" = create ]; then echo "https://example.test/acme/app/issues/10"; exit 0; fi',
'echo "unsupported $*" >&2; exit 1', '',
].join('\n'), { mode: 0o755 });
const classified = path.join(repo.root, 'classified.json');
fs.writeFileSync(classified, JSON.stringify([
{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts', line: 7, severity: 'CRITICAL', category: 'data-loss', summary: 'Unbounded delete', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' },
{ fingerprint: 'lib/old.ts:1:auth', path: 'lib/old.ts', line: 1, severity: 'CRITICAL', category: 'auth', summary: 'Missing owner check', fix: 'check owner', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' },
// Reviewer text is untrusted: a security finding that quotes the credential it found must not reach a public issue.
{ fingerprint: 'lib/keys.ts:3:security', path: 'lib/keys.ts', line: 3, severity: 'CRITICAL', category: 'security', summary: 'Hardcoded token ghp_' + 'Qm4zR7tK2wX9pL5vN8sD1fG6hJ3yB0cA7eU2', locus: 'outside', kind: 'defect', blocking: true, scope: 'defer' },
]));
const r = runBin(repo, 'gstack-review-scope', ['defer', '--findings', classified, '--pr', '42', '--repo', 'acme/app', '--json'], { PATH: `${shim}:${process.env.PATH}` });
expect(r.status).toBe(EXIT.ok);
const records = JSON.parse(r.out);
expect(records.slice(0, 2)).toEqual([
{ fingerprint: 'lib/legacy.ts:7:data-loss', path: 'lib/legacy.ts:7', severity: 'CRITICAL', summary: 'Unbounded delete', issue: 9, url: 'https://example.test/acme/app/issues/9', reason: 'already filed' },
{ fingerprint: 'lib/old.ts:1:auth', path: 'lib/old.ts:1', severity: 'CRITICAL', summary: 'Missing owner check', issue: 10, url: 'https://example.test/acme/app/issues/10' },
]);
expect(records[2].issue).toBeNull();
expect(records[2].reason).toMatch(/^publication scan refused: HIGH /);
expect(r.err).toContain('DEFERRED: #9 CRITICAL lib/legacy.ts:7');
expect(r.err).toContain('DEFERRED: #10 CRITICAL lib/old.ts:1');
expect(r.err).toContain('DEFERRED: unfiled (publication scan refused: HIGH ');
const calls = fs.readFileSync(log, 'utf8');
expect(calls.match(/^issue create /gm)?.length).toBe(1);
expect(calls.match(/^issue list /gm)?.length).toBe(1);
expect(calls).toContain('--search "gstack-deferred-finding" in:body');
expect(calls).toContain('issue create --title=Deferred review finding: lib/old.ts:1 — Missing owner check --body-file=- --repo acme/app');
expect(calls).not.toContain('Qm4zR7tK2wX9pL5vN8sD1fG6hJ3yB0cA7eU2');
});
// Generated by /ship Step 7 (coverage audit of the harness lane E diff).
// Value: protects=summary reads this branch's review log through gstack-review-read and prints the REVIEW_LOOP counters that
// `gstack-ship-receipt write --review-loop` consumes, counting only this invocation's ship-via review passes and tolerating a
// malformed row and a branch with no log; fails_when=the row filter, the ---CONFIG--- split, the --since guard or the --out
// write breaks, or a missing log is treated as an error instead of zero passes; why_new=only the pure summarizeReviewLoop is
// tested, never the bin against a review log on disk; seam=none; no_claim=review-log writing (rows are written directly, not
// through gstack-review-log)
test('summary reads the branch review log through gstack-review-read, counts this invocation\'s passes, writes --out, and refuses a missing --since', () => {
const repo = fixture();
repo.git('checkout', '-qb', 'feat');
const slug = runBin(repo, 'gstack-slug', ['--get', 'SLUG']).out.trim();
const branch = runBin(repo, 'gstack-slug', ['--get', 'BRANCH']).out.trim();
expect(slug).not.toBe('');
expect(branch).toBe('feat');
const logDir = path.join(repo.home, 'projects', slug);
fs.mkdirSync(logDir, { recursive: true });
const rows = [
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T09:00:00Z', status: 'clean', review_scope: 'frozen', findings: [{ fingerprint: 'old:1:q', blocking: true, action: 'fixed' }] },
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T10:00:00Z', status: 'issues_found', review_scope: 'frozen', findings: [
{ fingerprint: 'a:1:x', blocking: true, action: 'auto-fixed' }, { fingerprint: 'b:1:y', blocking: true, action: 'deferred', issue: 5 }, { fingerprint: 'c:1:z', action: 'noted' }] },
{ skill: 'adversarial-review', via: 'ship', timestamp: '2026-10-26T10:15:00Z', findings: [{ fingerprint: 'd:1:w', blocking: true }] },
{ skill: 'review', via: 'ship', timestamp: '2026-10-26T10:30:00Z', status: 'clean', review_scope: 'frozen', findings: [
{ fingerprint: 'b:1:y', blocking: true, action: 'deferred', issue: 5 }, { fingerprint: 'c:1:z', action: 'noted' }] },
{ skill: 'review', timestamp: '2026-10-26T10:40:00Z', findings: [{ fingerprint: 'e:1:v', blocking: true, action: 'fixed' }] },
];
fs.writeFileSync(path.join(logDir, `${branch}-reviews.jsonl`), rows.map(r => JSON.stringify(r)).join('\n') + '\n{not json\n');
const expected = { passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen' };
const out = path.join(repo.root, 'review-loop.json');
const r = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-26T09:30:00Z', '--out', out, '--json']);
expect(r.status).toBe(EXIT.ok);
expect(r.err).toContain('REVIEW_LOOP: passes=2 blocking_found=2 deferred=1 noted=1 fixed=1 scope=frozen');
expect(JSON.parse(r.out)).toEqual(expected);
expect(JSON.parse(fs.readFileSync(out, 'utf8'))).toEqual(expected);
const plain = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-26T08:00:00Z']);
expect(plain.status).toBe(EXIT.ok);
expect(plain.out.trim()).toBe('REVIEW_LOOP: passes=3 blocking_found=3 deferred=1 noted=1 fixed=2 scope=frozen');
const noSince = runBin(repo, 'gstack-review-scope', ['summary', '--json']);
expect(noSince.status).toBe(EXIT.fail);
expect(noSince.err).toContain('(REVIEW_SCOPE_INPUT;');
expect(noSince.out).toBe('');
const badSince = runBin(repo, 'gstack-review-scope', ['summary', '--since', 'yesterday']);
expect(badSince.status).toBe(EXIT.fail);
expect(badSince.err).toContain('(REVIEW_SCOPE_INPUT;');
// zero matching passes is a wrong --since or a log written elsewhere, never a loop to record: refused, nothing written
const bare = fixture();
const noneOut = path.join(bare.root, 'review-loop.json');
const none = runBin(bare, 'gstack-review-scope', ['summary', '--since', '2026-10-26T09:30:00Z', '--out', noneOut, '--json']);
expect(none.status).toBe(EXIT.fail);
expect(none.err).toContain('(REVIEW_SCOPE_INPUT;');
expect(none.err).toContain('REVIEW_LOOP: passes=0');
expect(fs.existsSync(noneOut)).toBe(false);
const late = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-27T00:00:00Z']);
expect(late.status).toBe(EXIT.fail);
});
});