wave review-53cfb5bfe011: /ship pass 1 fixes

- lib/ship-receipt-depth.ts: a merged resume (land after a true merge) no longer recomputes the review_exclude digest over the emptied range, so it lands instead of RECEIPT_INVALID (failing-first land test with review_exclude_paths active).
- bin/gstack-review-dispatch render: a standalone targeted render under an active review_exclude_paths refuses and points at the pass manifest voices stage, instead of writing a brief with no selection that leaves verify dirty (failing-first test).
- test/default-base.test.ts: #3795 expectations follow main R51 depth (light for an 80-line code diff; runtime_lines=80 still proves the real base).
- test/review-dispatch.test.ts: the GST-897 fingerprint test renders from the clean checkout, like its neighbor, because CI rebuilds the tracked lib/diagram-render/dist/diagram-render.html and the reviewer diff ceiling refused the ROOT render.
This commit is contained in:
garrytan committed 2026-11-02 09:40:11 +00:00
1 parent 0ebc9bfb44
commit c423539133
6 files changed
+53 -15

No files matched your search

+1
View File
@@ -114,6 +114,7 @@ function render(): number {
const mb = mergeBaseWith(base);
const full = diffBase ? spawnSync('git', ['rev-parse', '--verify', '--quiet', `${diffBase}^{commit}`], { encoding: 'utf8', timeout: 30_000 }).stdout?.trim() : undefined;
if (!full || full !== mb) throw new BriefError(`the targeted reader reads the whole diff on every pass: --diff-base must be the merge base with ${base} (${mb ?? 'unresolved'}), got ${diffBase ?? 'none'}`);
if (!flag('--disclosure') && ceilingContext(process.cwd(), base).state === 'active') throw new BriefError(`${base}'s review_exclude_paths is active, so the targeted reader renders in the pass manifest's voices stage (gstack-review-dispatch stage --manifest <f> --stage voices), which carries the exclusion and its disclosure line`);
}
const checklists = flag('--checklists')?.split(',').map(s => s.trim()).filter(Boolean).map(c => (c.includes('/') ? c : path.join(ROOT, 'review/specialists', `${c}.md`)));
for (const c of checklists ?? []) if (!fs.existsSync(c)) throw new BriefError(`checklist not found: ${c}`);
+2 -2
View File
@@ -243,7 +243,7 @@ export function receiptDepthVerdict(root: string, baseRef: string, r: ShipReceip
return { ok: false, line, message: `the receipt ${why}, but head ${head.slice(0, 12)} needs ${need.depth} (${need.reasons.join(', ')}${errs ? `; ${errs}` : ''}); the receipt is unchanged; run /ship on ${head.slice(0, 12)}` };
}
/** bindReceipt plus the owner exceptions and the review depth: what READY and train build accept; the targeted line is reported, never refused on. A base that already contains the head refuses (BASE_CONTAINS_HEAD, GST-774) before any depth is computed, except for a PR GitHub already merged (`merged`: land resuming after a merge-only true merge), whose depth then reads the range with diff-scope's `--merged`. */
/** bindReceipt plus the owner exceptions and the review depth: what READY and train build accept; the targeted line is reported, never refused on. A base that already contains the head refuses (BASE_CONTAINS_HEAD, GST-774) before any depth is computed, except for a PR GitHub already merged (`merged`: land resuming after a merge-only true merge), whose depth then reads the range with diff-scope's `--merged` and whose exclusion digest is not recomputed over that emptied range. */
export function bindShippedReceipt(root: string, baseRef: string, body: string, pr: string | undefined, accepted: readonly string[], merged = false):
| { ok: true; receipt: ShipReceipt; head: string; line: string; depthLine: string; targetedLine: string }
| { ok: false; code: ReceiptBindingCode | 'BASE_CONTAINS_HEAD' | 'QUEUE_RECEIPT_DEPTH_RAISED' | 'QUEUE_RECEIPT_EXCEPTION_INVALID' | 'RECEIPT_INVALID' | Extract<Exclusion, { state: 'refused' }>['code']; message: string; line: string; depthLine?: string; targetedLine?: string } {
@@ -256,7 +256,7 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string,
if (problems.length) return { ok: false, code: 'QUEUE_RECEIPT_EXCEPTION_INVALID', message: `the receipt's owner exception does not stand: ${problems.join('; ')}`, line: b.line };
const at = headInputs(root, baseRef, b.head, b.receipt, merged);
if (at.x.state === 'refused') return { ok: false, code: at.x.code, message: at.x.lines.join('; '), line: b.line };
const mismatch = excludeMismatch(at.x, b.receipt, b.head, baseRef);
const mismatch = merged ? null : excludeMismatch(at.x, b.receipt, b.head, baseRef);
if (mismatch) return { ok: false, code: 'RECEIPT_INVALID', message: mismatch, line: b.line };
const d = receiptDepthVerdict(root, baseRef, b.receipt, b.head, at);
const gateTagged = boundScan(b.receipt, b.head) ?? (gateScanNeeded(at.c.paths, at.c.root) ? null : 0);
+7 -7
View File
@@ -128,14 +128,14 @@ describe('resolveBaseBranch skips an origin/HEAD that names the checked-out bran
});
describe('the review depth ship-queue ready binds', () => {
// Value: protects=a READY on the bug checkout never binds a light depth for an 80-line diff; fails_when=the queue compares the
// PR head with itself and reports required=light runtime_lines=0; seam=bindShippedReceipt
test('origin/<PR branch> refuses BASE_CONTAINS_HEAD; the resolved default base binds standard; the control binds the same', () => {
// Value: protects=a READY on the bug checkout binds the 80-line diff's real depth inputs (light under R51, runtime_lines=80);
// fails_when=the queue compares the PR head with itself and reports runtime_lines=0; seam=bindShippedReceipt
test('origin/<PR branch> refuses BASE_CONTAINS_HEAD; the resolved default base binds runtime_lines=80; the control binds the same', () => {
const body = path.join(fx.root, 'body.md');
const w = bin('gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body], fx.clone);
expect(w.status, w.err).toBe(0);
const text = fs.readFileSync(body, 'utf8');
expect(extractReceipt(text).receipt!.review_depth).toMatchObject({ depth: 'standard', runtime_lines: 80 });
expect(extractReceipt(text).receipt!.review_depth).toMatchObject({ depth: 'light', runtime_lines: 80 });
const head = git(fx.clone, 'rev-parse', 'HEAD');
withEnv(fx.env, () => {
const self = bindShippedReceipt(fx.clone, 'origin/pr-branch', text, '7', [head]);
@@ -144,9 +144,9 @@ describe('the review depth ship-queue ready binds', () => {
expect(self.code as string).toBe('BASE_CONTAINS_HEAD');
expect(self.message).toContain(head.slice(0, 12));
const real = bindShippedReceipt(fx.clone, `origin/${resolveBaseBranch(fx.clone)}`, text, '7', [head]);
expect(real.ok && real.depthLine).toContain('receipt=standard required=standard runtime_lines=80 class=code');
expect(real.ok && real.depthLine).toContain('receipt=light required=light runtime_lines=80 class=code');
const control = bindShippedReceipt(fx.control, `origin/${resolveBaseBranch(fx.control)}`, text, '7', [head]);
expect(control.ok && control.depthLine).toContain('receipt=standard required=standard runtime_lines=80 class=code');
expect(control.ok && control.depthLine).toContain('receipt=light required=light runtime_lines=80 class=code');
});
}, 120_000);
});
@@ -161,7 +161,7 @@ describe('gstack-review-plan', () => {
const depth = (out: string) => /^REVIEW_DEPTH: .*$/m.exec(out)?.[0];
const real = bin('gstack-review-plan', ['--base', 'main'], fx.clone);
expect(real.status, real.err).toBe(0);
expect(depth(real.out)).toContain('REVIEW_DEPTH: standard runtime_lines=80');
expect(depth(real.out)).toContain('REVIEW_DEPTH: light runtime_lines=80');
const control = bin('gstack-review-plan', ['--base', 'main'], fx.control);
expect(depth(control.out)).toBe(depth(real.out));
git(fx.control, 'checkout', '-q', 'main');
+6 -4
View File
@@ -782,16 +782,18 @@ describe('reading the REVIEWED_TREE line (GST-482 review fixes)', () => {
test('the fingerprint step opens the brief and its command prints a line collect accepts; GST-850 gpt-6.1-sol replies stay refused (GST-897)', () => {
const out = tmp();
const ledger = path.join(out, 'review-ledger.jsonl');
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
const file = field(run(['render', '--role', 'outside-voice', '--out', out, '--diff-base', head]).stdout, 'file');
const wt = cleanCheckout();
const wtTop = fs.realpathSync(wt);
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: wt, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
const file = field(run(['render', '--role', 'outside-voice', '--out', out, '--diff-base', head], wt).stdout, 'file');
const brief = fs.readFileSync(file, 'utf8');
const first = brief.split('\n')[0]!;
expect(first).toStartWith(`First step, before anything else: run \`cd '${top}' && echo "REVIEWED_TREE: `);
expect(first).toStartWith(`First step, before anything else: run \`cd '${wtTop}' && echo "REVIEWED_TREE: `);
expect(first).toContain('Copy that output line, not the command, as the first line of your final reply');
expect(brief.slice(brief.lastIndexOf('\n\n') + 2)).toContain('(the output, not the command)');
const command = /run `([^`]+)`/.exec(first)![1]!;
const printed = spawnSync('bash', ['-c', command], { cwd: os.tmpdir(), encoding: 'utf8', timeout: 30_000 }).stdout.trim();
expect(checkReviewedTree(printed + '\nNO FINDINGS', expected).ok).toBe(true);
expect(checkReviewedTree(printed + '\nNO FINDINGS', `${tree} ${wtTop}`).ok).toBe(true);
run(['sent', '--ledger', ledger, '--role', 'outside-voice', '--sent', file]);
for (const p of ['p1', 'p2']) {
const reply = path.join(ROOT, `test/fixtures/review-wrong-tree/gpt-6.1-sol-gst850-${p}.out`);
+15
View File
@@ -123,6 +123,21 @@ describe('what review_exclude_paths excludes and keeps', () => {
for (const b of briefs) expect(b).toContain(`(selection ${m.review_exclude.digest})`);
}, 180_000);
// Value: protects=Step 11's standalone targeted render cannot record a brief with no selection beside the pass's briefs;
// fails_when=render --role targeted without --disclosure writes a rendered event with no exclude digest, so verify sees
// mixed digests and never prints VERIFY: clean; why_new=/ship review of the GST-508 wave (Red Team); seam=gstack-review-dispatch render
test('a standalone targeted render under an active exclusion refuses and leaves the ledger as the pass wrote it', () => {
const repo = census();
const out = path.join(repo.root, 'pass');
expect(runBin(repo, 'gstack-review-dispatch', ['pass', '--base', 'main', '--out', out, '--pass', 'p1', '--', '--gate-tagged', '0']).status).toBe(EXIT.ok);
const ledger = path.join(out, 'review-ledger.jsonl');
const before = fs.readFileSync(ledger, 'utf8');
const r = runBin(repo, 'gstack-review-dispatch', ['render', '--role', 'targeted', '--classes', 'money', '--base', 'main', '--diff-base', mergeBase(repo), '--out', out]);
expect(r.status).not.toBe(EXIT.ok);
expect(r.err).toContain("review_exclude_paths is active, so the targeted reader renders in the pass manifest's voices stage");
expect(fs.readFileSync(ledger, 'utf8')).toBe(before);
}, 120_000);
// Value: protects=excluded receipts cannot dispatch the outside voice or the targeted money reader on lines no reader sees,
// and a credential line inside a matched file keeps it; fails_when=scanNumbers or targetedClasses read excluded lines, or
// a secret-bearing data file is excluded; why_new=task 9; seam=bin/gstack-review-plan; no_claim=pregate's credential scan
+22 -2
View File
@@ -14,8 +14,8 @@ import { EXIT } from '../lib/headless-artifacts';
import { BASE_POLICY, cleanup, makeFixtureRepo, readFile, runBin, type FixtureRepo } from './helpers/restamp-fixture';
import { shippedBody } from './helpers/ship-queue-train-fixture';
import { resolveReleaseConflicts } from '../lib/ship-queue-land';
import { receiptDepthAtHead } from '../lib/ship-receipt-depth';
import { mergePolicy } from '../lib/ship-policy';
import { receiptDepthAtHead, receiptExclude, receiptReviewAtHead } from '../lib/ship-receipt-depth';
import { mergePolicy, REVIEW_EXCLUDE_KEY_VERSION } from '../lib/ship-policy';
import { persist, repoIdentity } from '../lib/ship-queue-train-record';
const ROOT = path.resolve(import.meta.dir, '..');
@@ -583,6 +583,26 @@ describe('land: one pass, base, lock and history', () => {
expect(again.out).toMatch(/^REVIEW_DEPTH: receipt=light required=light /m);
expect(again.out).toContain('LANDED: ');
}, 90_000);
// Value: protects=the same merged resume when review_exclude_paths excluded files from the PR's range; fails_when=the queue
// recomputes the exclusion over the merged (empty) range and refuses the receipt's digest as RECEIPT_INVALID; why_new=the
// case above has no review_exclude_paths; seam=none; no_claim=GitHub's merge-commit shape beyond two parents
test('merge-only: a merged resume lands when review_exclude_paths excluded files from the range', () => {
const fx = setup({ ...POLICY, history: 'merge-only', review_exclude_paths: ['data/**'], min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION });
const head = openPr(fx, '15', { edit: work => { fs.mkdirSync(path.join(work, 'data', 'a'), { recursive: true }); fs.writeFileSync(path.join(work, 'data', 'a', 'result.json'), '{"ok": true}\n'); } });
fx.repo.git('checkout', '-q', '--detach', head);
const { depth, exclude } = receiptReviewAtHead(fx.repo.work, 'main', path.join(ROOT, 'bin'), head);
fx.repo.git('checkout', '-q', '--detach', 'origin/main');
const recorded = receiptExclude(exclude);
expect(recorded?.excluded).toBe(1);
fx.write(d => { d.prs['15'].body = shippedBody('15', head, { gstack_version: REVIEW_EXCLUDE_KEY_VERSION, review_depth: depth, review_exclude: recorded }); });
land(fx, ['--pr', '15']);
fx.write(d => { d.trueMerge = true; d.mergeErrorAfter = true; });
expect(land(fx, ['--pr', '15']).err).toContain('HTTP 502');
const again = land(fx, ['--pr', '15']);
expect(again.out + again.err).not.toMatch(/RECEIPT_INVALID|BASE_CONTAINS_HEAD|QUEUE_RECEIPT_DEPTH_RAISED/);
expect(again.out).toContain('LANDED: ');
}, 90_000);
});
// /ship adversarial (Claude) and outside-voice (Codex) findings (GSTA-154 PR A): each case failed or was unreachable before its fix.