diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 87ecbfda6..29a6ebe02 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -41,6 +41,7 @@ import { effectiveReaders, READERS_POLICY_DEFAULT } from '../lib/ship-policy-rea import { classifyDiff, diffCommand, keepCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, ROLE_EFFORT, SCOPED_RANGE_LINES, type Depth, type History } from '../lib/review-depth'; import { classList, targetedBriefSha, targetedClasses, targetedPatchId } from '../lib/review-targeted'; import { targetedDelta } from '../lib/review-targeted-delta'; +import { docsChangeSince } from '../lib/ship-receipt-depth'; import { RISK_CLASSES, type RiskClass } from '../lib/review-risk'; import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check'; @@ -90,7 +91,11 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do behavior STOP): they plan as a fix to a gate-class path, never a note, so below the round cap it is a pass on the range (lean: a fix-check), and at the cap it stops for the owner, whose --grant adds one scoped fix-check; after a fix-check, each new grant adds one more (a chain link, - --range-base at the last fix-check's commit); pass --pass, --last and --fix-checks as the loop left them + --range-base at the last fix-check's commit); pass --pass, --last and --fix-checks as the loop left them. + After a fix-check, edits since --range-base that are only release stamps (the base policy's stamp rule, + line-exact) and docs (docsChangeSince, the receipt's classifier), with no executable, symlink or gitlink mode and no gate-scan + tag in the range, are no gate-class edit: done, or targeted + when a docs claim needs the targeted read on a targeted-class diff (GST-1702) --pr the PR whose platform review counts under review.platform on (default: this branch's PR) --declare-risk risk classes the lane declares for the diff (comma list of ${RISK_CLASSES.join(', ')}): they join the detected classes and only raise the depth; RISK_DECLARED: prints both (GST-1129) @@ -240,14 +245,21 @@ function main(): number { const brief = targetedBriefSha(); const ran = !['uncommitted', 'none'].includes(patch) && flag('--targeted-ran') === patch; const postReview = has('--post-review'); + const fixCheckAt = postReview && pass > 0 && last === 'fix-check' && fixRange?.runtime_lines != null ? git(['rev-parse', '--verify', '--quiet', `${rangeBase}^{commit}`])?.trim() : undefined; + const afterFixCheck = fixCheckAt ? docsChangeSince(root, fixCheckAt, head, ref) : null; + const rawSince = fixCheckAt ? git(['diff', '--raw', '--no-renames', fixCheckAt, 'HEAD']) : null; + const carried = afterFixCheck && afterFixCheck.verdict !== 'code' && rawSince !== null && !/^:\d+ (100755|120000|160000) /m.test(rawSince) && gateTagsOf([fixCheckAt!]) === 0 ? afterFixCheck : null; const plan = planReview({ ...collected, ...exclusionInput, ...riskPaths, declared: declared as RiskClass[], shape: { panel: review.panel, readerModel: review.reader_model, platform: review.platform, head, platformReview: platform?.ok ? platform.review : null }, scope, gateTagged, targeted: { classes: targeted.classes, ran }, numbers: scanNumbers(kept(added)), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host', criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'), - history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: (has('--fixed-below') || (postReview && pass > 0)) && last !== 'fix-check', blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || !!fixRange?.touched_gate || (postReview && pass > 0), pass1Depth, rangeRuntimeLines: fixRange?.runtime_lines, granted: grant, fixChecks: has('--fix-checks') ? fixChecks : undefined, postReview, chained: inLoopLink }, + history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: (has('--fixed-below') || (postReview && pass > 0)) && last !== 'fix-check', blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || (!carried && (!!fixRange?.touched_gate || (postReview && pass > 0))), pass1Depth, rangeRuntimeLines: fixRange?.runtime_lines, granted: grant, fixChecks: has('--fix-checks') ? fixChecks : undefined, postReview, chained: inLoopLink }, }); - if (postReview && pass > 0) plan.next.reason = `edits after the review bound (Step 16 stage 2.1) plan as a fix to a gate-class path, never a note: ${plan.next.reason}`; + if (carried) plan.next.reason = `the edits after the last fix-check ${fixCheckAt!.slice(0, 12)} are ${carried.paths.length ? `${carried.verdict === 'plain' ? 'plain docs' : `docs (${carried.paths.join(', ')})`}${carried.stamps.length ? ' and release stamps' : ''}` : 'release stamps'} (docsChangeSince), no gate-class edit (GST-1702): ${plan.next.reason}`; + else if (postReview && pass > 0) plan.next.reason = `edits after the review bound (Step 16 stage 2.1) plan as a fix to a gate-class path, never a note: ${plan.next.reason}`; const ranArg = flag('--targeted-ran'); - const delta = plan.next.step === 'done' && rangeBase && ranArg && targeted.classes.length && !ran ? targetedDelta(root, ref, rangeBase, ranArg, !!grant) : null; + const delta = plan.next.step !== 'done' || !targeted.classes.length || ran ? null + : carried?.verdict === 'claim' ? { state: 'dispatch' as const, range_base: fixCheckAt!, paths: [...carried.paths] } + : rangeBase && ranArg ? targetedDelta(root, ref, rangeBase, ranArg, !!grant) : null; if (delta?.state === 'dispatch') { const why = grant ? `the owner's grant ${grant} covers the fix round that changed ${delta.paths.join(', ')}` : `the only change since is docs whose claim the record cannot carry (${delta.paths.join(', ')})`; plan.next = { step: 'targeted', reason: `the loop is done and ${why} after the targeted read at ${delta.range_base.slice(0, 12)}: one targeted reader reads HEAD for ${classList(targeted.classes)}; it is not a review round` }; diff --git a/changelog.d/capy-gst-1702-post-review-stamps.md b/changelog.d/capy-gst-1702-post-review-stamps.md new file mode 100644 index 000000000..4226a45a2 --- /dev/null +++ b/changelog.d/capy-gst-1702-post-review-stamps.md @@ -0,0 +1 @@ +- **Release stamps and a docs-audit edit after the last fix-check no longer stop /ship for an owner grant.** `gstack-review-plan --post-review --last fix-check` now reads the edits since the fix-check's commit with the receipt's classifier (`docsChangeSince`): lines the base policy's `stamp_files` and `stamp_patterns` cover (VERSION, `package.json`, `.claude-plugin/plugin.json`, a `since: 'TBD'` row moved to the version) and docs edits are no gate-class edit, so the plan prints `NEXT: done`, or `NEXT: targeted` when a docs claim on a money, gate, auth or secret diff needs the targeted read. Before, any such edit planned `NEXT: stop` ("the fix-check edited a gate-class path"), and GBRA-341 (gbrain #8705) needed a second grant and a fix-check on a 39-line stamp delta. A stamp match is line-exact, so any other line in a stamp file still stops, as do any code edit, an executable or symlink file with a docs name, and a commit whose message the gate scan tags as a CI skip marker. (GST-1702) diff --git a/docs/review-policy.md b/docs/review-policy.md index 499b25e18..2950ab21f 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -121,7 +121,15 @@ incomplete coverage, which blocks until the reviewer is rerun. own grant is refused there, so each link still needs the owner's word. In the loop, a new grant naming the last fix-check's commit (`--range-base`) plans the next link the same way; a grant recorded before a later logged fix-check - already bought a link and is refused (GST-1681). + already bought a link and is refused (GST-1681). Edits after the last + fix-check that are only release stamps under the base policy's stamp rule + (line-exact: one other line in a stamp file makes the file an edit) and docs + are no gate-class edit: the plan reads them with the receipt's classifier + (`docsChangeSince`, the [targeted-only read](#targeted-only-read-after-the-loop)'s) + and prints `NEXT: done`, or `NEXT: targeted` when a docs claim on a + targeted-class diff needs the read, with no grant (GST-1702). An executable, + symlink or gitlink file in that range, or a commit the gate scan tags there + (a `[skip ci]` message), still stops. ### Transitions @@ -146,6 +154,7 @@ incomplete coverage, which blocks until the reviewer is rerun. | a stop (any row above) on gate class before Step 11's voices ran | Step 11's voices (adversarial, outside, targeted), then ask again with `--adversarial-ran` | | a round-2 full pass that stops for a fixed blocking finding or a gate-class edit, with the owner's recorded grant (`--grant `) | one scoped fix-check (round 3), after Step 11 on gate class | | a round-2 fix-check that stops the same way, with the owner's recorded grant and `--fix-checks 1` | one scoped fix-check (round 3), after Step 11 on gate class | +| `--post-review` after a fix-check whose later edits are only release stamps and docs (`docsChangeSince`) | done, or the one targeted reader when a docs claim on a targeted-class diff needs it | | `--post-review` after a fix-check (edits after the review bound), with a new recorded grant at a stop head after the last fix-check's commit | one scoped fix-check, the next link of the fix-check chain, after Step 11 on gate class | | a fix-check that stops in the loop, with a new recorded grant naming its own commit (`--range-base`, the stop it answers), after any number of fix-checks | one scoped fix-check, the next link of the fix-check chain, after Step 11 on gate class | diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 148d1a525..51bf5f362 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1819,7 +1819,10 @@ that doesn't carry each of those lines word for word. STOP tells the lane to plan the inserted Step 9 with `gstack-review-plan --post-review` (plus `--pass`, `--last` and `--fix-checks` as the loop left them). Edits after the review bound plan as a fix to a gate-class path, never -as a note. Below the round cap that's a pass on the range (a fix-check under +as a note. The exception is a range after the last fix-check whose edits are +only release stamps and docs (`docsChangeSince`): it plans `NEXT: done`, or +`NEXT: targeted` for a docs claim on a targeted-class diff, with no grant +([review-policy.md](review-policy.md#transitions), GST-1702). Below the round cap that's a pass on the range (a fix-check under the lean panel). At the cap it stops for the owner, whose recorded grant (`--grant`) adds one scoped fix-check, never `done`. When the last step was a fix-check, pass `--range-base` at that fix-check's commit: a new grant at a diff --git a/test/review-plan-post-review-stamps.test.ts b/test/review-plan-post-review-stamps.test.ts new file mode 100644 index 000000000..e518193fc --- /dev/null +++ b/test/review-plan-post-review-stamps.test.ts @@ -0,0 +1,183 @@ +/** + * GST-1702 (GBRA-341, gbrain #8705): after an owner-granted fix-check, only release stamps and a docs-audit edit followed (VERSION, + * package.json, .claude-plugin/plugin.json, a BOOTSTRAP stamp comment, the CHANGELOG re-heading, a behavior-change row's + * since: 'TBD' moved to the version under stamp_patterns and stamp_placeholders, and a docs line). gstack-ship-receipt write + * carries the targeted record across that delta (GST-1689's docsChangeSince), and gstack-review-plan --post-review --last + * fix-check classifies the same delta with the same classifier: it is no gate-class edit, so the plan is done, or targeted when + * the docs rule says a claim needs the read. Any non-stamp line in a stamp file still counts as an edit and stops. + */ +import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { checkoutTree } from '../lib/review-brief'; +import { BASE_POLICY, cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; + +setDefaultTimeout(180_000); +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); + +const POLICY = { + ...BASE_POLICY, + min_gstack_version: '1.94.16.0', + stamp_paths: [], + stamp_files: ['VERSION', 'package.json', 'CHANGELOG.md', '.claude-plugin/plugin.json', 'BOOTSTRAP_FOR_AGENTS.md', 'src/core/behavior-change-notice.ts'], + stamp_patterns: ['"version": "{version}"', "since: '{version}'", ''], + stamp_placeholders: ['TBD'], +}; +const plugin = (v: string, extra = '') => `{\n "name": "fixture",\n "version": "${v}"${extra}\n}\n`; +const bootstrap = (v: string, line = '') => `# Bootstrap\n\n\n\nRun setup first.\n${line}`; +const NOTICE_BASE = "export const NOTICES = [\n { id: 'old', since: '1.2.3.0' },\n];\n"; + +const commit = (repo: FixtureRepo, files: Record, msg: string) => { + for (const [f, t] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(repo.work, f)), { recursive: true }); fs.writeFileSync(path.join(repo.work, f), t); } + repo.git('add', '-A'); repo.git('commit', '-q', '-m', msg); + return repo.git('rev-parse', 'HEAD'); +}; +const read = (repo: FixtureRepo, f: string) => fs.readFileSync(path.join(repo.work, f), 'utf8'); + +/** + * A branch with the GBRA-341 shape up to the fix-check: a money-class change (or a plain code change with money false), a + * behavior-change row at since: 'TBD' and an [Unreleased] CHANGELOG entry, then the fix-check's commit, which the targeted + * reader read clean through the real dispatch steps. + */ +function fixChecked(money = true): { repo: FixtureRepo; fc: string; record: string } { + const repo = makeFixtureRepo({ prefix: 'gst1702-', policy: POLICY }); + repos.push(repo); + commit(repo, { '.claude-plugin/plugin.json': plugin('1.2.3.0'), 'BOOTSTRAP_FOR_AGENTS.md': bootstrap('1.2.3.0'), 'src/core/behavior-change-notice.ts': NOTICE_BASE }, 'manifests'); + repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', '-b', 'feat'); + const changelog = read(repo, 'CHANGELOG.md').replace('# Changelog\n', '# Changelog\n\n## [Unreleased]\n\n- Adds the new notice.\n'); + const [file, code] = money ? ['src/charge.ts', 'export const spend_cap_usd = 5;\n'] : ['src/greet.ts', 'export const greeting = "hi";\n']; + commit(repo, { [file]: code, 'src/core/behavior-change-notice.ts': NOTICE_BASE.replace('];', " { id: 'new', since: 'TBD' },\n];"), 'CHANGELOG.md': changelog }, 'feature'); + const fc = commit(repo, { [file]: code + 'export const retries = 2;\n' }, 'granted fix-check fix'); + const out = path.join(repo.root, 'read'); + const ledger = path.join(out, 'review-ledger.jsonl'); + const record = path.join(repo.root, 'targeted.json'); + if (!money) return { repo, fc, record }; + const render = runBin(repo, 'gstack-review-dispatch', ['render', '--role', 'targeted', '--classes', 'money', '--diff-base', repo.git('merge-base', 'origin/main', 'HEAD'), '--out', out]); + expect(render.status, render.err).toBe(0); + expect(runBin(repo, 'gstack-review-dispatch', ['sent', '--ledger', ledger, '--role', 'targeted', '--sent', /file=(\S+)/.exec(render.out)![1]!]).status).toBe(0); + fs.writeFileSync(path.join(out, 'targeted.out'), `REVIEWED_TREE: ${checkoutTree(repo.work)}\nNO FINDINGS\n`); + expect(runBin(repo, 'gstack-review-dispatch', ['collect', '--ledger', ledger, '--role', 'targeted', '--output', path.join(out, 'targeted.out'), '--merged', path.join(out, 'merged.jsonl')]).status).toBe(0); + fs.writeFileSync(path.join(out, 'judged.jsonl'), ''); + expect(runBin(repo, 'gstack-review-dispatch', ['judge', '--ledger', ledger, '--merged', path.join(out, 'merged.jsonl'), '--judged', path.join(out, 'judged.jsonl')]).status).toBe(0); + expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger, '--targeted-out', record]).out).toContain('TARGETED_RECORD: classes=money status=done findings=0 '); + return { repo, fc, record }; +} + +/** /ship's release stamp to 1.2.4.0, committed: VERSION, package.json, plugin.json, the BOOTSTRAP comment, the TBD row and the CHANGELOG heading. */ +function stamp(repo: FixtureRepo, edits: { plugin?: string; notice?: string; bootstrap?: string } = {}): void { + commit(repo, { + VERSION: '1.2.4.0\n', + 'package.json': read(repo, 'package.json').replace('"version": "1.2.3"', '"version": "1.2.4"'), + '.claude-plugin/plugin.json': plugin('1.2.4.0', edits.plugin), + 'BOOTSTRAP_FOR_AGENTS.md': bootstrap('1.2.4.0', edits.bootstrap), + 'src/core/behavior-change-notice.ts': read(repo, 'src/core/behavior-change-notice.ts').replace("since: 'TBD'", "since: '1.2.4.0'") + (edits.notice ?? ''), + 'CHANGELOG.md': read(repo, 'CHANGELOG.md').replace('## [Unreleased]', '## [1.2.4.0] - 2026-11-06'), + }, 'release stamp 1.2.4.0'); +} +const docsAudit = (repo: FixtureRepo, line = 'Read the overview first, then the walkthrough.') => commit(repo, { 'docs/run-order.md': `# Run order\n\n${line}\n` }, 'Step 14.5 docs audit'); + +const write = (repo: FixtureRepo, record: string) => runBin(repo, 'gstack-ship-receipt', ['write', '--base', 'main', '--session-kind', 'unattended', '--review-targeted', `@${record}`, '--json']); +/** GBRA-341's invocation: Step 16's Step 9 after the granted fix-check. */ +const postReview = (repo: FixtureRepo, fc: string) => runBin(repo, 'gstack-review-plan', ['--base', 'main', '--gate-tagged', '0', '--post-review', '--pass', '1', '--last', 'fix-check', '--fix-checks', '1', '--range-base', fc]); +const dispatches = (out: string) => out.split('\n').filter(l => l.startsWith('DISPATCH: ')); + +describe('GST-1702: release stamps and a docs-audit edit after the last fix-check are no gate-class edit', () => { + // Value: protects=GBRA-341's delta (manifest stamps, a BOOTSTRAP stamp comment, a since: 'TBD' row moved to the version in a + // code file, the CHANGELOG re-heading and a plain docs line) after the fix-check the targeted reader read: write binds the + // record and the post-review plan is done; fails_when=write refuses RECEIPT_INVALID or the plan stops on 'the fix-check edited + // a gate-class path' (GBRA-341 needed a second owner grant and a fix-check on a 39-line stamp delta); + // seam=gstack-ship-receipt write --review-targeted and gstack-review-plan --post-review --last fix-check + test('stamps plus a plain docs-audit edit: write binds the record and the post-review plan is done', () => { + const { repo, fc, record } = fixChecked(); + stamp(repo); + docsAudit(repo); + const w = write(repo, record); + expect(w.status, w.err).toBe(EXIT.ok); + expect(JSON.parse(w.out).review_targeted).toMatchObject({ classes: ['money'], status: 'done' }); + const p = postReview(repo, fc); + expect(p.status, p.err).toBe(0); + expect(p.out).toContain('NEXT: done'); + expect(p.out).not.toContain('edited a gate-class path'); + expect(p.out).toContain(`the edits after the last fix-check ${fc.slice(0, 12)} are plain docs and release stamps`); + expect(dispatches(p.out)).toEqual([]); + }); + + // Value: protects=stamps alone after the fix-check plan done too; fails_when=an empty docs remainder counts as an edit; + // seam=gstack-review-plan --post-review + test('stamps alone: the post-review plan is done', () => { + const { repo, fc } = fixChecked(); + stamp(repo); + const p = postReview(repo, fc); + expect(p.out).toContain('NEXT: done'); + expect(p.out).toContain(`the edits after the last fix-check ${fc.slice(0, 12)} are release stamps`); + expect(dispatches(p.out)).toEqual([]); + }); + + // Value: protects=a stamp match is line-exact: a real field added to plugin.json, or a code line added to the behavior-change + // file, beside the stamps still stops for the owner; fails_when=a stamp file's other lines ride the stamp; + // seam=gstack-review-plan --post-review + test('a non-stamp line in plugin.json or in the behavior-change file still stops', () => { + for (const edits of [{ plugin: ',\n "main": "dist/index.js"' }, { notice: 'export const NOTICE_LIMIT = 3;\n' }]) { + const { repo, fc } = fixChecked(); + stamp(repo, edits); + docsAudit(repo); + const p = postReview(repo, fc); + expect(p.status, p.err).toBe(0); + 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, never a note: the fix-check edited a gate-class path"); + } + }); + + // Value: protects=on a money-class PR a docs-audit line with a numeric claim after the fix-check still plans the one targeted + // reader on HEAD (and write still refuses the record); fails_when=the carry lets a claim through; seam=gstack-review-plan + // --post-review and gstack-ship-receipt write + test('a docs edit with a numeric claim on a money-class PR plans the targeted read', () => { + const { repo, fc, record } = fixChecked(); + stamp(repo); + docsAudit(repo, 'Each run now finishes 40% faster.'); + expect(write(repo, record).status).toBe(EXIT.fail); + const p = postReview(repo, fc); + expect(p.status, p.err).toBe(0); + expect(p.out).toContain('NEXT: targeted'); + expect(dispatches(p.out)).toEqual(['DISPATCH: targeted targeted effort=full classes=money']); + expect(p.out).toContain(`TARGETED_DELTA: range_base=${fc} paths=docs/run-order.md classes=money`); + }); + + // Value: protects=a numeric claim on a PR with no targeted class needs no read: the plan is done; fails_when=the claim stops or + // dispatches a reader the docs rule does not ask for; seam=gstack-review-plan --post-review + test('a docs edit with a numeric claim on a PR with no targeted class is done', () => { + const { repo, fc } = fixChecked(false); + stamp(repo); + docsAudit(repo, 'Each run now finishes 40% faster.'); + const p = postReview(repo, fc); + expect(p.status, p.err).toBe(0); + expect(p.out).toContain('NEXT: done'); + expect(dispatches(p.out)).toEqual([]); + }); + + // Value: protects=an executable file with a docs extension after the fix-check is code to the gate, not a docs-audit edit; + // fails_when=the carry reads the path name alone and plans done for an unreviewed script; seam=gstack-review-plan --post-review + test('an executable docs-extension file after the fix-check still stops', () => { + const { repo, fc } = fixChecked(false); + stamp(repo); + fs.mkdirSync(path.join(repo.work, 'docs'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'docs/task.md'), '#!/bin/sh\necho hi\n', { mode: 0o755 }); + repo.git('add', '-A'); repo.git('update-index', '--chmod=+x', 'docs/task.md'); repo.git('commit', '-q', '-m', 'Step 14.5 docs audit'); + const p = postReview(repo, fc); + expect(p.status, p.err).toBe(0); + 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'); + }); + + // Value: protects=a commit the gate scan tags after the fix-check (a [skip ci] message on a docs-audit commit) keeps the stop; + // fails_when=the carry drops the range's gate tag and plans done; seam=gstack-review-plan --post-review + test('a docs-audit commit with [skip ci] after the fix-check still stops', () => { + const { repo, fc } = fixChecked(false); + stamp(repo); + commit(repo, { 'docs/run-order.md': '# Run order\n\nRead the overview first.\n' }, 'Step 14.5 docs audit [skip ci]'); + const p = postReview(repo, fc); + expect(p.status, p.err).toBe(0); + 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'); + }); +});