diff --git a/bin/gstack-pregate b/bin/gstack-pregate index b16e6d4d2..cdf79cbd7 100755 --- a/bin/gstack-pregate +++ b/bin/gstack-pregate @@ -36,7 +36,7 @@ import { buildContext, defaultUserConfigPath, describeRefusal, loadedDotenvFiles import { CHECKS, CHECK_IDS, PREGATE_GUARDS_OUT_REL, PREGATE_OUT_REL, PREGATE_TIER2, PREPUSH_CHECK_IDS, clearDecision, clearObligation, keepSameTreeClears, readPregateFile, renderTable, runPregate, verifyPregate, writePregateFile } from '../lib/pregate/index'; import { describeLane, discoverLanes } from '../lib/pregate/workflows'; import { REMOTE_GATE_LANE } from '../lib/pregate/checks/lanes'; -import { describeSubstitution } from '../lib/pregate/remote-receipt'; +import { baseBranchOf, describeSubstitution } from '../lib/pregate/remote-receipt'; import { approvalGrants, describeExceptions, type FlakyInput } from '../lib/pregate/flaky-exception'; import { ensureTmpExcluded } from '../lib/restamp'; import { RESULT_CODES, resultCodeSuffix, resultLine } from '../lib/result-codes'; @@ -177,7 +177,12 @@ async function cmdRun(root: string, rest: string[], json: boolean): Promise (or set GSTACK_SPEND_CAP_USD)\n'); return EXIT.usage; } remote = { spendCapUsd: cap }; } else if (flag(rest, '--spend-cap') !== undefined) { process.stderr.write('gstack-pregate: --spend-cap applies only with --remote\n'); return EXIT.usage; } - const base = resolveBaseBranch(root, flag(rest, '--base')); + const out = stage === 'guards' && !flag(rest, '--out') ? path.join(root, PREGATE_GUARDS_OUT_REL) : outPath(root, rest); + const prev = fs.existsSync(out) ? readPregateFile(out).file : null; + const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8', timeout: 15_000 }).stdout?.trim(); + const recordedBase = flag(rest, '--base') === undefined && prev?.head && prev.head === head ? baseBranchOf(prev.base) : null; + const base = recordedBase ?? resolveBaseBranch(root, flag(rest, '--base')); + if (recordedBase) { const fallback = resolveBaseBranch(root); if (fallback !== recordedBase) process.stderr.write(`gstack-pregate: no --base; using ${recordedBase}, the base of the record at this head (${path.relative(root, out)}), not ${fallback}; pass --base to change it\n`); } const userConfigPath = defaultUserConfigPath(root); const loaded = buildContext({ repoRoot: root, base, policyFrom, allowRepoCommands: allow, timeoutMs, stallTimeoutMs, remote, knownChecks: [...CHECK_IDS, ...PREGATE_TIER2], userConfigPath, runId: flag(rest, '--run') }); if ('refused' in loaded) { @@ -199,7 +204,6 @@ async function cmdRun(root: string, rest: string[], json: boolean): Promise] [--fix-list --hunks --tests [--callers ]] [-- ] runs gstack-review-plan and writes /pass-manifest.json: stage first (audit readers marked first in --extras), specialists (with the other --extras readers), red-team, voices (Step 11). Renders first and specialists now; prints - the plan, MANIFEST: and one READER: line per reader. A plan whose NEXT is not full, scoped or fix-check prints PASS: none. + the plan, MANIFEST: and one READER: line per reader. A plan whose NEXT is not full, scoped, fix-check or targeted prints + PASS: none, except NEXT: voices under --voices. --voices (Step 11 on its own --out) keeps only the plan's voices, so it renders nothing its program never sends; it takes no --extras, and a reviewer pass refuses to resume its manifest. is letters, digits and dashes, read as lowercase (P1 is p1): the manifest, reader titles and a resume use that form. @@ -405,7 +406,7 @@ function pass(): number { const step = p.next.step; const fixChecks = step === 'fix-check' ? [] : p.dispatches.filter(d => d.role === 'fix-check'); const planLines = [...text.stdout.trimEnd().split('\n'), ...fixChecks.map(d => `FIX_CHECK: planned effort=${d.effort} (Step 3.5 runs it on its own; not in this manifest)`)]; - if (step !== 'full' && step !== 'scoped' && step !== 'fix-check' && step !== 'targeted') { process.stdout.write([...planLines, `PASS: none next=${step} (no reviewer pass; follow NEXT)`].join('\n') + '\n'); return EXIT.ok; } + if (step !== 'full' && step !== 'scoped' && step !== 'fix-check' && step !== 'targeted' && !(step === 'voices' && args.includes('--voices'))) { process.stdout.write([...planLines, `PASS: none next=${step} (no reviewer pass; follow NEXT)`].join('\n') + '\n'); return EXIT.ok; } const rangeBase = step === 'scoped' ? git(['rev-parse', '--verify', '--quiet', `${p.fix_range?.base ?? ''}^{commit}`])?.trim() : undefined; if (step === 'scoped' && !rangeBase) throw new BriefError('a scoped plan names no readable fix range base'); const mergeBase = mergeBaseWith(base); diff --git a/changelog.d/gst-1775-ship-tooling-fixes.md b/changelog.d/gst-1775-ship-tooling-fixes.md new file mode 100644 index 000000000..ce560de88 --- /dev/null +++ b/changelog.d/gst-1775-ship-tooling-fixes.md @@ -0,0 +1 @@ +- **Three /ship tooling bugs from gbrain#8730 are fixed.** When Step 16 stage 2.1's post-review Step 9 plans `NEXT: voices` on a gate-class diff, `gstack-review-dispatch pass --voices` now renders the voices that plan lists instead of printing `PASS: none`, so plan and dispatch agree. A same-tree pregate clear now survives a `--stage tests` run without `--base`, which keeps the base of the record at its head instead of taking `origin/HEAD` (when that names another branch, the run says so), and it survives a base branch that moved between runs, because the carry compares the policy's source and the gate command, not the base commit its label names (a receipt a stamp-only substitution accepted still needs the same base commit). `gstack-pregate clear --receipt` also accepts a stage run's own log, where the gate's `PREGATE_REMOTE:` line sits under `lanes:`, but only when that run's lanes check reads `requires-remote`, so the log of a run whose gate exited non-zero is refused. `gstack-review-scope summary` no longer reads a fix-check ledger in a `p` directory as an unbound pass N. It counts as the fix-check it is. (GST-1775) diff --git a/docs/pregate.md b/docs/pregate.md index 8947c7d19..d5f60d2cc 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -534,7 +534,9 @@ the supervision above. Close the obligation one of four ways: its preflight checks) and the VM only as the opt-in (under `--no-ci-carry` it names the VM route); or - after running it, `gstack-pregate clear --remote pregate.json/remote_gate - --receipt ` with the gate's output, validated as `--remote` validates it + --receipt ` with the gate's output (or a `gstack-pregate --remote` log, + where the line sits under `lanes:`, accepted only when that log's lanes check + reads `requires-remote`), validated as `--remote` validates it (a run URL is refused, `PREGATE_RECEIPT_INVALID` for a bad receipt; a receipt for a tree that differs from this one only by the repo's stamp rule is accepted and recorded, see [below](#remote-gate-stamp-only)), or @@ -1285,13 +1287,20 @@ pregate reran `gstack-pregate` on the tree the lane had already cleared `macos-named-regressions` and `windows-smoke` on from CI runs, and the fresh record dropped both receipts, so accept stopped on `requires-remote (2 uncleared)` and never exited 0 (#4063, GST-1595). Now, when `gstack-pregate` -writes over a record for the same head, tree, policy and base branch, each uncleared -obligation takes the earlier receipt for the same lane, platform, workflow and -job, with its `cleared_by`, stamp-only substitution and flaky exception, as long -as the earlier obligation listed every test the new one needs. Each kept +writes over a record for the same head, tree, policy source and base branch (the +base branch may have moved: the policy label's base commit does not count, +GST-1775), each uncleared +obligation takes the earlier receipt for the same lane, platform, workflow, job +and gate command, with its `cleared_by`, stamp-only substitution and flaky exception, as long +as the earlier obligation listed every test the new one needs. A receipt a +stamp-only substitution accepted carries only while the base commit is the same: +the base's stamp rules accepted it, and a moved base may have changed them. Each kept receipt prints `PREGATE_CLEAR_KEPT: receipt= (recorded at this tree ; …)` (on stderr under `--json`), and accept relays the lines -before its `PREGATE:` line. A record for another head or tree keeps nothing, +before its `PREGATE:` line. A run without `--base` at the record's head keeps +that record's base branch rather than `origin/HEAD`'s, and says so on stderr when +they differ, so a `--stage tests` rerun cannot drop the clears by reading +another branch (GST-1775). A record for another head or tree keeps nothing, so any commit after the clear, even an amend that keeps the tree, needs a new clear at the new head: a receipt and its flaky exception were vetted against the head they name. A rerun with a diff --git a/docs/review-policy.md b/docs/review-policy.md index 026a58dab..0c3f48420 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -683,7 +683,7 @@ puts in the receipt: | `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: diff-plus-blocking` or `all`) | | `review_loop.blocking_found` | every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row finding from the same reviewer at the same path:line accounts for (each row finding pairs with one per ledger), so an adversarial, outside-voice or not-yet-classified blocker counts and one whose newest classification is below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | | `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | -| `review_loop.unbound` | what could not bind (GST-1168), each printed as `REVIEW_SCOPE_UNBOUND: : `: a row with no start token and no readable time, a `pN` ledger with no review row of pass N, a row finding's `ledger_id` no ledger collected, a `Review-Finding` trailer naming nothing; absent when 0 | +| `review_loop.unbound` | what could not bind (GST-1168), each printed as `REVIEW_SCOPE_UNBOUND: : `: a row with no start token and no readable time, a `pN` ledger with no review row of pass N (a fix-check ledger in a `pN` directory is not a pass ledger, GST-1775), a row finding's `ledger_id` no ledger collected, a `Review-Finding` trailer naming nothing; absent when 0 | | `review_loop.ledgers_read` | dispatch ledgers the count read; absent under `--no-ledger` | | `REVIEW_SCOPE_VOIDED:` (stderr, not a receipt field) | an abandoned pass (GBEV-3): a `pN` ledger never judged that no row names, replaced by a later `pM` ledger (M > N) that was judged or is named. Its findings are left out of every count above instead of counting as unbound and unrecorded. A fix-check ledger and the last pass are never voided | diff --git a/lib/pregate/index.ts b/lib/pregate/index.ts index 49cadf924..8064e2875 100644 --- a/lib/pregate/index.ts +++ b/lib/pregate/index.ts @@ -319,7 +319,9 @@ export function clearDecision(file: PregateFile, lane: string, input: ClearInput let text: string; try { text = fs.readFileSync(input.receiptPath, 'utf8'); } catch (e: any) { return { refused: 'PREGATE_RECEIPT_INVALID', message: `cannot read ${input.receiptPath}: ${e.message}` }; } - const report = parseRemoteReport(text); + const lanesRows = [...text.matchAll(/^(?:tests|preflight) +lanes +(\S+)/gm)].map(m => m[1]); + if (/^ +lanes: +PREGATE_REMOTE: /m.test(text) && (!lanesRows.length || lanesRows.some(s => s !== 'requires-remote'))) return { refused: 'PREGATE_RECEIPT_INVALID', message: `${input.receiptPath} prints its PREGATE_REMOTE: line under a lanes check that is ${lanesRows.join(', ') || 'missing'}, not requires-remote, so the gate did not pass there; ${lane} stays uncleared` }; + const report = parseRemoteReport(text.replace(/^ +lanes: +(?=PREGATE_REMOTE: )/gm, '')); if (!report) return { refused: 'PREGATE_RECEIPT_INVALID', message: `${input.receiptPath} has no PREGATE_REMOTE: line; ${lane} stays uncleared` }; const own = remoteReportProblem(report, report.tree); if (own) return { refused: 'PREGATE_RECEIPT_INVALID', message: `${own}; ${lane} stays uncleared` }; @@ -490,19 +492,22 @@ export function clearObligation(file: PregateFile, lane: string, receipt: string * A rerun at the same tree keeps the clears its predecessor recorded (GST-1627): `gstack-ship-release accept` reruns the * pregate on the final tree after the lane cleared macos-named-regressions and windows-smoke from CI runs at that tree, and * a fresh record dropped them, so accept never passed. A receipt carries only into an uncleared obligation for the same - * lane, platform, workflow and job whose tests the earlier one covered, at the same head, under the same policy and base - * branch, with its flaky exception when it had one. A record for another head or tree (a same-tree amend gets new clears, + * lane, platform, workflow, job and gate command whose tests the earlier one covered, at the same head, under the same policy source and base + * branch (a base branch that moved between the runs keeps them: the policy label names the base commit, GST-1775), with its + * flaky exception when it had one. A receipt accepted through a stamp-only substitution carries only while the base commit is + * unchanged: the base's stamp rules decided it, and a moved base may have changed them. A record for another head or tree (a same-tree amend gets new clears, * since a receipt and its exception were vetted against the head they name), or a rerun with a failed or unfinished check (a remote gate * that timed out this time), keeps nothing. Returns one PREGATE_CLEAR_KEPT line per lane. */ export function keepSameTreeClears(prev: PregateFile | null, file: PregateFile): string[] { - if (!prev || prev.head !== file.head || prev.tree !== file.tree || prev.policy !== file.policy || prev.policy_digest !== file.policy_digest || baseBranchOf(prev.base) !== baseBranchOf(file.base)) return []; + const source = (label: string | undefined) => (label ?? '').replace(/^(\S+?)@[0-9a-f]{7,40}(?=:|\s|$)/, '$1'); + if (!prev || prev.head !== file.head || prev.tree !== file.tree || source(prev.policy) !== source(file.policy) || prev.policy_digest !== file.policy_digest || baseBranchOf(prev.base) !== baseBranchOf(file.base)) return []; if (file.checks.some(c => c.status === 'fail' || c.status === 'incomplete' || c.status === 'not_installed')) return []; const kept: string[] = []; for (const r of file.requires_remote ?? []) { if (r.receipt) continue; - const old = prev.requires_remote?.find(o => o.receipt && o.lane === r.lane && o.platform === r.platform && o.workflow === r.workflow && o.job === r.job && r.tests.every(t => o.tests.includes(t))); - if (!old?.receipt) continue; + const old = prev.requires_remote?.find(o => o.receipt && o.lane === r.lane && o.platform === r.platform && o.workflow === r.workflow && o.job === r.job && o.command === r.command && r.tests.every(t => o.tests.includes(t))); + if (!old?.receipt || (old.substitution && (prev.base !== file.base || prev.policy !== file.policy))) continue; clearObligation(file, r.lane, old.receipt, old.cleared_by ?? 'an earlier run at this tree', old.substitution, prev.exceptions?.find(e => e.lane === r.lane)); kept.push(`PREGATE_CLEAR_KEPT: ${r.lane} receipt=${old.receipt} (recorded at this tree ${file.tree.slice(0, 12)}; docs/pregate.md#same-tree-clears)`); } diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 35cbe206d..2f4da5004 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -468,8 +468,8 @@ export function summarizeReviewLoop(rows: readonly Record[], since: } } for (const p of ledgerPasses) { - const n = /^p(\d+)$/.exec(p.name ?? '')?.[1]; - if (n && !passes.some(r => String(r.pass) === n)) unbound.push(`pass ${n} (ledger ${p.name}): no review-log row, so its fixes and dispositions cannot be counted`); + const n = passNo(p); + if (n !== undefined && Number.isFinite(n) && !passes.some(r => String(r.pass) === String(n))) unbound.push(`pass ${n} (ledger ${p.name}): no review-log row, so its fixes and dispositions cannot be counted`); } const known = ledgerPasses.some(p => p.ids) ? new Set(ledgerPasses.flatMap(p => p.ids ?? [])) : null; if (known) for (const [, s] of seen) if (s.ledgerId && !known.has(s.ledgerId)) unbound.push(`ledger_id ${s.ledgerId}: no dispatch ledger of this run collected it`); diff --git a/test/pregate-remote.test.ts b/test/pregate-remote.test.ts index ca781b02e..fc7a2cf00 100644 --- a/test/pregate-remote.test.ts +++ b/test/pregate-remote.test.ts @@ -26,6 +26,7 @@ const GATE_SCRIPT = [ "if (process.env.GATE_SILENT) process.exit(0);", "const spent = process.env.GATE_SPENT ? ` spent=${process.env.GATE_SPENT}` : '';", "console.log(`PREGATE_REMOTE: tree=${process.env.GATE_TREE || process.env.PREGATE_TREE} tests=${process.env.GATE_TESTS ?? 3} failed=${process.env.GATE_FAILED ?? 0}${spent}`);", + "if (process.env.GATE_EXIT) process.exitCode = Number(process.env.GATE_EXIT);", ].join('\n') + '\n'; function gated(pregate: Record = { ...BASE_PREGATE, remote_gate: GATE }): FixtureRepo { @@ -101,6 +102,38 @@ describe('small machines record the test tier for the remote gate (C4)', () => { expect(ok.out).not.toContain('attested'); }); + // GBRA-347 (gbrain#8730): a stage run's log prints the gate's line under the lanes check (` lanes: PREGATE_REMOTE: …`), + // and `clear --receipt` on that log refused "has no PREGATE_REMOTE: line". + // Value: protects=a gstack-pregate --remote log is a receipt clear accepts, validated like the bare line; fails_when=clear + // refuses the indented form, takes a line indented under another check, or takes the log of a run whose gate exited 1; + // seam=gstack-pregate clear --receipt + test('`clear --receipt` takes the stage run\'s own log, where the gate\'s line sits under lanes:', async () => { + const repo = gated(); + const run = runBin(repo, 'gstack-pregate', ['--base', 'main', '--stage', 'tests', '--remote', '--spend-cap', '5', '--allow-repo-commands']); + expect(run.status, run.err).toBe(EXIT.ok); + const log = path.join(repo.root, 'stage-run.log'); + fs.writeFileSync(log, run.out); + expect(run.out).toMatch(/^ +lanes: +PREGATE_REMOTE: tree=[0-9a-f]{40} tests=3 failed=0$/m); + expect(run.out).not.toMatch(/^PREGATE_REMOTE: /m); + const failed = runBin(repo, 'gstack-pregate', ['--base', 'main', '--stage', 'tests', '--remote', '--spend-cap', '5', '--allow-repo-commands'], { GATE_EXIT: '1' }); + expect(failed.out).toMatch(/^ +lanes: +PREGATE_REMOTE: tree=[0-9a-f]{40} tests=3 failed=0$/m); + const failedLog = path.join(repo.root, 'failed-run.log'); + fs.writeFileSync(failedLog, failed.out); + const fileAt = path.join(repo.work, '.gstack/tmp/pregate.json'); + const { file } = await lanesOn(repo, { cores: 4, testBackend: 'local' }); + fs.writeFileSync(fileAt, JSON.stringify(file)); + const clear = (receipt: string) => runBin(repo, 'gstack-pregate', ['clear', '--remote', 'pregate.json/remote_gate', '--receipt', receipt]); + const other = path.join(repo.root, 'other-check.log'); + fs.writeFileSync(other, ` guards: PREGATE_REMOTE: tree=${file.tree} tests=3 failed=0\n`); + expect(clear(other).err).toContain('has no PREGATE_REMOTE: line'); + const refused = clear(failedLog); + expect(refused.status).not.toBe(EXIT.ok); + expect(refused.err).toContain('under a lanes check that is incomplete, not requires-remote'); + const ok = clear(log); + expect(ok.status, ok.err).toBe(EXIT.ok); + expect(ok.out).toContain(`PREGATE_CLEARED: pregate.json/remote_gate receipt=remote-gate tree=${file.tree.slice(0, 12)} tests=3 `); + }); + test('`clear` on a test_backend obligation records the run URL as attested, not validated, and says so', async () => { const repo = gated(BASE_PREGATE); const fileAt = path.join(repo.work, '.gstack/tmp/pregate.json'); diff --git a/test/review-plan-post-review-stamps.test.ts b/test/review-plan-post-review-stamps.test.ts index e518193fc..5968e8ffe 100644 --- a/test/review-plan-post-review-stamps.test.ts +++ b/test/review-plan-post-review-stamps.test.ts @@ -181,3 +181,52 @@ describe('GST-1702: release stamps and a docs-audit edit after the last fix-chec expect(p.out).toContain('NEXT: stop (edits after the review bound (Step 16 stage 2.1) plan as a fix to a gate-class path'); }); }); + +/** + * GST-1775 (GBRA-347, gbrain#8730): /ship Step 16 stage 2.1 inserted a Step 9 for edits after the review bound on a gate-class + * diff. `gstack-review-plan --post-review --pass 2 --last full` printed NEXT: voices with the voices' DISPATCH lines, and Step 11's + * `gstack-review-dispatch pass --voices` with the same plan flags printed PASS: none, so the voices the plan asked for could not run. + */ +function postReviewBranch(): { repo: FixtureRepo; flags: string[] } { + const repo = makeFixtureRepo({ prefix: 'gst1775-voices-' }); + repos.push(repo); + repo.git('checkout', '-q', '-b', 'feat'); + const edit = (code: string, msg: string) => { + fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'src/charge.ts'), code); + repo.git('add', '-A'); repo.git('commit', '-q', '-m', msg); + return repo.git('rev-parse', 'HEAD'); + }; + const rangeBase = edit('export const spend_cap_usd = 5;\n', 'feature'); + edit('export const spend_cap_usd = 5;\nexport const retries = 2;\n', 'edit after the review bound'); + return { repo, flags: ['--gate-tagged', '1', '--post-review', '--pass', '2', '--last', 'full', '--range-base', rangeBase, '--pass1-depth', 'full'] }; +} +const dispatchLines = (out: string) => out.split('\n').filter(l => l.startsWith('DISPATCH: ')).map(l => l.split(' ')[1]); + +describe('GST-1775: a post-review NEXT: voices plan dispatches through pass --voices', () => { + // Value: protects=Step 11 renders exactly the voices a NEXT: voices plan lists, so plan and dispatch agree; + // fails_when=pass --voices prints PASS: none for NEXT: voices, or renders a reader the plan does not list; + // seam=gstack-review-plan --post-review and gstack-review-dispatch pass --voices; no_claim=the readers' replies + test('pass --voices builds the voices manifest the NEXT: voices plan names; without --voices it is still no reviewer pass', () => { + const { repo, flags } = postReviewBranch(); + const plan = runBin(repo, 'gstack-review-plan', ['--base', 'main', ...flags]); + expect(plan.status, plan.err).toBe(0); + expect(plan.out).toContain('NEXT: voices'); + const planned = dispatchLines(plan.out); + expect(planned).toEqual(['adversarial-native', 'outside-voice', 'targeted']); + + const out = path.join(repo.root, 'p2-voices'); + const pass = runBin(repo, 'gstack-review-dispatch', ['pass', '--base', 'main', '--out', out, '--pass', 'p2-voices', '--voices', '--', ...flags]); + expect(pass.status, pass.err).toBe(0); + expect(pass.out).not.toContain('PASS: none'); + expect(pass.out).toMatch(/MANIFEST: file=\S+ pass=p2-voices readers=3 first=0 specialists=0 red-team=0 voices=3/); + const m = JSON.parse(fs.readFileSync(pass.out.match(/MANIFEST: file=(\S+)/)![1]!, 'utf8')); + expect(m.next).toBe('voices'); + expect(m.voices_only).toBe(true); + expect(m.readers.map((r: any) => [r.role, r.stage]).sort()).toEqual(planned.map(r => [r, 'voices']).sort()); + + const noVoices = runBin(repo, 'gstack-review-dispatch', ['pass', '--base', 'main', '--out', path.join(repo.root, 'p3'), '--pass', 'p3', '--', ...flags]); + expect(noVoices.status, noVoices.err).toBe(0); + expect(noVoices.out).toContain('PASS: none next=voices'); + }); +}); diff --git a/test/review-scope-ledger-count.test.ts b/test/review-scope-ledger-count.test.ts index b18d962f6..de9a27c48 100644 --- a/test/review-scope-ledger-count.test.ts +++ b/test/review-scope-ledger-count.test.ts @@ -298,4 +298,26 @@ describe('GST-986 item 5: fix-check passes and their findings count from the led const s = loop(summary(repo, '--run', run)); expect(s).toMatchObject({ passes: 1, blocking_found: 1, fixed: 0, fix_checks: 1, unrecorded: 1 }); }); + + // GST-1775 (gbrain#8730): a granted fix-check run with out /p4 left its ledger in p4, and its row was logged + // pass:2 skill:review-fix-check, so summary named p4 an unbound pass 4 and the receipt carried unbound=1. + // Value: protects=a fix-check ledger binds to its review-fix-check row whatever its directory is called; + // fails_when=a fix-check ledger named p is read as pass N; seam=gstack-review-scope summary --run + test('a fix-check ledger in a p directory is a fix-check, never an unbound pass N', () => { + const repo = branchWith([ + review(0, 1, [{ fingerprint: 'lib/a.ts:10:logic', severity: 'CRITICAL', blocking: true, action: 'fixed', raised_by: 'red-team' }]), + review(20, 2, []), + { ...fixCheck(40, []), pass: 2 }, + ]); + const run = path.join(repo.root, 'run'); + const step = (dir: string, readers: Array<[string, string]>) => { pass(path.join(run, dir), readers); Bun.sleepSync(5); }; + step('p1', [['red-team', '[CRITICAL] lib/a.ts:10 the cap is never enforced']]); + step('p2', [['red-team', CLEAN]]); + step('p4', [['fix-check', CLEAN]]); + const r = summary(repo, '--run', run); + expect(r.err).not.toContain('REVIEW_SCOPE_UNBOUND'); + const s = loop(r); + expect(s).toMatchObject({ passes: 2, blocking_found: 1, fixed: 1, fix_checks: 1, rounds: 3, ledgers_read: 3 }); + expect(s.unbound).toBeUndefined(); + }); }); diff --git a/test/ship-accept-same-tree-clears.test.ts b/test/ship-accept-same-tree-clears.test.ts index a4ce4d87d..d86859148 100644 --- a/test/ship-accept-same-tree-clears.test.ts +++ b/test/ship-accept-same-tree-clears.test.ts @@ -85,6 +85,40 @@ describe('accept keeps the remote-lane clears made at its tree (GST-1627)', () = }); }); +describe('same-tree clears survive a stage run and a moved base (GST-1775)', () => { + // GBRA-347 (gbrain#8730): after `gstack-pregate clear --run-url`, `gstack-pregate --stage tests` with no --base wrote a record + // against origin/HEAD, which named another PR's branch, so the clear was dropped; accept's `--base master` run then + // dropped every clear with no PREGATE_CLEAR_KEPT line. The policy label also names the base commit, so a base that + // moved between the runs dropped them too. + // Value: protects=a clear at this head and tree survives a no --base stage run and accept's full run, and a base advance; + // fails_when=a stage run takes origin/HEAD over the record's base, or the policy label's base commit decides the carry; + // seam=bin/gstack-pregate's run writer under gstack-ship-release accept + test('a stage run with no --base keeps the record\'s base and the clear, and accept still exits 0', () => { + const f = cleared(); + f.git('push', '-q', 'origin', 'main:other-pr'); + f.git('fetch', '-q', 'origin'); + expect(f.git('remote', 'set-head', 'origin', 'other-pr').status).toBe(0); + const stage = f.run(path.join(BIN, 'gstack-pregate'), ['--stage', 'tests', '--allow-repo-commands']); + expect([stage.status, lineOf(stage.stdout, 'PREGATE_CLEAR_KEPT')], stage.stdout + stage.stderr).toEqual([0, expect.stringMatching(new RegExp(`^PREGATE_CLEAR_KEPT: ${LANE} `))]); + expect(stage.stderr).toContain('gstack-pregate: no --base; using main, the base of the record at this head (.gstack/tmp/pregate.json), not other-pr'); + expect(record(f).base).toStartWith('origin/main@'); + const out = accept(f); + expect([out.status, lineOf(out.stdout, 'PREGATE_CLEAR_KEPT'), lineOf(out.stdout, 'PREGATE')], out.stdout).toEqual([0, + expect.stringMatching(new RegExp(`^PREGATE_CLEAR_KEPT: ${LANE} receipt=${RUN_URL} `)), 'PREGATE: requires-remote cleared (final tree)']); + }); + + test('a base that moved between the runs keeps the clear at the same head and tree', () => { + const f = cleared(); + const before = record(f).policy; + f.advanceBase({ 'README.md': '# Fx moved\n' }); + f.git('fetch', '-q', 'origin'); + const out = pregate(f); + expect([lineOf(out.stdout, 'PREGATE_CLEAR_KEPT'), lineOf(out.stdout, 'PREGATE')], out.stdout + out.stderr).toEqual([ + expect.stringMatching(new RegExp(`^PREGATE_CLEAR_KEPT: ${LANE} receipt=${RUN_URL} `)), expect.stringMatching(/^PREGATE: requires-remote cleared /)]); + expect(record(f).policy).not.toBe(before); + }); +}); + describe('keepSameTreeClears (GST-1627)', () => { const obligation = (o: Partial = {}): PregateRemote => ({ lane: LANE, platform: 'macos', workflow: 'other.yml', job: 'named-regressions', tests: ['test/a.test.ts'], receipt: null, ...o }); const file = (remote: PregateRemote[], o: Partial = {}): PregateFile => ({ schema_version: 1, tree: 'a'.repeat(40), head: 'b'.repeat(40), base: 'origin/main@cccccccccccc', policy: 'base origin/main', checks: [{ id: 'lanes', status: 'requires-remote' }], requires_remote: remote, ...o }); @@ -100,10 +134,41 @@ describe('keepSameTreeClears (GST-1627)', () => { const kept = (next: PregateFile) => { keepSameTreeClears(prev(), next); return next.requires_remote![0]!.receipt; }; expect(kept(file([obligation()], { base: 'origin/main@dddddddddddd' }))).toBe(RUN_URL); for (const next of [file([obligation()], { head: 'f'.repeat(40) }), file([obligation()], { tree: 'e'.repeat(40) }), file([obligation()], { policy: 'head' }), file([obligation()], { base: 'origin/release@cccccccccccc' }), - file([obligation({ tests: ['test/a.test.ts', 'test/b.test.ts'] })]), file([obligation({ job: 'smoke', lane: LANE })]), file([obligation({ platform: 'windows' })])]) { + file([obligation({ tests: ['test/a.test.ts', 'test/b.test.ts'] })]), file([obligation({ job: 'smoke', lane: LANE })]), file([obligation({ platform: 'windows' })]), file([obligation({ command: 'node scripts/other-gate.js' })])]) { expect(kept(next)).toBeNull(); } expect(kept(file([obligation()], { checks: [{ id: 'lanes', status: 'incomplete' }] }))).toBeNull(); expect(keepSameTreeClears(null, file([obligation()]))).toEqual([]); }); + + // Value: protects=the policy label's base commit never decides the carry, its source does (GST-1775); fails_when=a moved base + // drops the clear, or a policy from another branch, the head or a user file carries it; seam=keepSameTreeClears + test('compares the policy source, not the base commit its label names', () => { + const at = (policy: string, base = 'origin/main@cccccccccccc') => ({ policy, base }); + const carried = (from: { policy: string; base: string }, to: { policy: string; base: string }) => { + const next = file([obligation()], to); + keepSameTreeClears(file([obligation({ receipt: RUN_URL })], from), next); + return next.requires_remote![0]!.receipt; + }; + const main = (sha: string) => at(`origin/main@${sha}:.gstack/pregate.json`, `origin/main@${sha}`); + expect(carried(main('cccccccccccc'), main('dddddddddddd'))).toBe(RUN_URL); + for (const other of [at('origin/release@dddddddddddd:.gstack/pregate.json', 'origin/release@dddddddddddd'), at('head (unreviewed) .gstack/pregate.json'), at('user /home/u/.gstack/pregate.json'), at('origin/main@dddddddddddd (no .gstack/pregate.json)')]) { + expect(carried(main('cccccccccccc'), other)).toBeNull(); + } + }); + + // Value: protects=a receipt a stamp-only substitution accepted carries only at the base commit whose stamp rules accepted it + // (GST-1775 cross-family read: a moved base may tighten them, and base policies carry no content digest); fails_when=the + // substitution carries past a moved base, or a same-base rerun drops it; seam=keepSameTreeClears + test('a stamp-only substitution carries at the same base commit and fails closed when the base moved', () => { + const substitution = { gated_tree: 'd'.repeat(40), accepted_for: 'a'.repeat(40), policy: 'origin/main@cccccccccccc:.gstack/ship-policy.json', files: [{ path: 'VERSION', rule: 'stamp_files' }] }; + const main = (sha: string) => ({ policy: `origin/main@${sha}:.gstack/pregate.json`, base: `origin/main@${sha}` }); + const carried = (to: { policy: string; base: string }) => { + const next = file([obligation()], to); + const kept = keepSameTreeClears(file([obligation({ receipt: RUN_URL, substitution })], main('cccccccccccc')), next); + return [kept.length, next.requires_remote![0]!.receipt, next.requires_remote![0]!.substitution ?? null]; + }; + expect(carried(main('cccccccccccc'))).toEqual([1, RUN_URL, substitution]); + expect(carried(main('dddddddddddd'))).toEqual([0, null, null]); + }); });