diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index fde4e7432..1d42836fc 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -64,7 +64,8 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do refuses (exit 2): the plan dispatches it itself --reviewed-numbers the numeric-claim fingerprint the outside voice already reviewed --depth-floor standard|full raise the depth to at least this (never lowers it); /ship passes its prompt flag on every run - --range-base the fix range's base (this pass's PASS_HEAD): after a fixed blocker on a diff below full depth, a + --range-base the fix range's base (this pass's PASS_HEAD; with --post-review --last fix-check, the last fix-check's + commit): after a fixed blocker on a diff below full depth, a committed range of at most ${SCOPED_RANGE_LINES} runtime lines plans a scoped pass; anything unread plans the full pass --no-tiers specialists, audits and watchers at the host's default effort too (as gstack-config review_effort_tiers off does) --json the plan as JSON (depth, runtime_lines, rules, errors with input and repair, gate_scan, per-dispatch exclude) @@ -78,13 +79,16 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do --grant the owner's grant of one scoped fix-check after round 2, recorded with gstack-approvals add (approved, its ask or scope naming review_loop_grant and the stop head's sha, at or after --range-base and at or before HEAD; needs --range-base): the stop after a round-2 full pass with a fixed blocking finding or a - gate-class edit plans that fix-check instead; a fix-check never earns another; any other row is refused (exit 3) + gate-class edit plans that fix-check instead; a fix-check never earns another, except that with + --post-review --last fix-check a new grant at a stop head after --range-base (the last fix-check's + commit) plans the next link of the fix-check chain; any other row is refused (exit 3) --fix-checks fix-checks already run in this invocation: the owner's grant adds one only after the first, in the lean loop and after a round-2 fix-check in the full loop (GEVA-223) --post-review plan the Step 9 that /ship Step 16 stage 2.1 inserted for edits after the review bound (finish's 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; pass --pass, --last and --fix-checks as the loop left them + --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 --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) @@ -138,8 +142,8 @@ function main(): number { const grant = flag('--grant'); if (has('--grant') && (!grant || grant.startsWith('-'))) { process.stderr.write('gstack-review-plan: --grant needs the approval id gstack-approvals add printed\n'); return EXIT.usage; } const rangeBase = flag('--range-base'); - if (has('--range-base') && (!rangeBase || rangeBase.startsWith('-'))) { process.stderr.write('gstack-review-plan: --range-base needs the fix range\'s base commit (this pass\'s PASS_HEAD)\n'); return EXIT.usage; } - if (grant && !rangeBase) { process.stderr.write('gstack-review-plan: --grant needs --range-base (round 2\'s PASS_HEAD), which binds the grant to the stop it answers\n'); return EXIT.usage; } + if (has('--range-base') && (!rangeBase || rangeBase.startsWith('-'))) { process.stderr.write('gstack-review-plan: --range-base needs the fix range\'s base commit (this pass\'s PASS_HEAD; with --post-review --last fix-check, the last fix-check\'s commit)\n'); return EXIT.usage; } + if (grant && !rangeBase) { process.stderr.write('gstack-review-plan: --grant needs --range-base (round 2\'s PASS_HEAD; with --post-review --last fix-check, the last fix-check\'s commit), which binds the grant to the stop it answers\n'); return EXIT.usage; } if (list('--force').includes('targeted')) { process.stderr.write('gstack-review-plan: --force targeted: the targeted reader is no specialist; the plan dispatches it whenever the diff has a targeted class, and after a finished loop for a docs claim added since its read: pass --range-base --targeted-ran (docs/review-policy.md#targeted-only-read-after-the-loop)\n'); return EXIT.usage; } const declared = list('--declare-risk'); if (has('--declare-risk') && (!declared.length || declared.some(c => !(RISK_CLASSES as readonly string[]).includes(c)))) { process.stderr.write(`gstack-review-plan: --declare-risk ${flag('--declare-risk') ?? ''}: each class is one of ${RISK_CLASSES.join(', ')} (a declaration only raises the depth)\n`); return EXIT.usage; } @@ -151,8 +155,9 @@ function main(): number { const row = ('file' in loc ? readRecord(loc).rows : []).find(r => r.id === grant && r.outcome === 'approved' && `${r.ask} ${r.scope}`.includes('review_loop_grant')); const passHead = git(['rev-parse', '--verify', '--quiet', `${rangeBase}^{commit}`])?.trim(); const ancestor = (a: string, d: string) => git(['merge-base', '--is-ancestor', a, d]) !== null; - const stopHead = row && passHead ? `${row.ask} ${row.scope}`.match(/\b[0-9a-f]{7,40}\b/g)?.map(s => git(['rev-parse', '--verify', '--quiet', `${s}^{commit}`])?.trim()).find(s => s && ancestor(passHead, s) && ancestor(s, 'HEAD')) : undefined; - if (!stopHead) { process.stderr.write(`gstack-review-plan: --grant ${grant}: no approved row ${grant} naming review_loop_grant and this invocation's stop head (a commit at or after --range-base ${rangeBase}, at or before HEAD) in this branch's approval record (${'file' in loc ? 'gstack-approvals list' : loc.message}); record the owner's grant with gstack-approvals add --ask '... (review_loop_grant )' first\n`); return EXIT.refused; } + const chained = has('--post-review') && last === 'fix-check'; + const stopHead = row && passHead ? `${row.ask} ${row.scope}`.match(/\b[0-9a-f]{7,40}\b/g)?.map(s => git(['rev-parse', '--verify', '--quiet', `${s}^{commit}`])?.trim()).find(s => s && ancestor(passHead, s) && ancestor(s, 'HEAD') && !(chained && s === passHead)) : undefined; + if (!stopHead) { process.stderr.write(`gstack-review-plan: --grant ${grant}: no approved row ${grant} naming review_loop_grant and this invocation's stop head (a commit ${chained ? `after --range-base ${rangeBase}, the last fix-check's commit, whose own grant does not carry over` : `at or after --range-base ${rangeBase}`}, at or before HEAD) in this branch's approval record (${'file' in loc ? 'gstack-approvals list' : loc.message}); record the owner's grant with gstack-approvals add --ask '... (review_loop_grant )' first\n`); return EXIT.refused; } } const collected = collectLive(root, base); if (collected.errors.length) { @@ -231,7 +236,7 @@ function main(): number { 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 }, + 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 }, }); 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'); diff --git a/changelog.d/capy-gst-1676-fix-check-chain-link-1.md b/changelog.d/capy-gst-1676-fix-check-chain-link-1.md new file mode 100644 index 000000000..6165b09e2 --- /dev/null +++ b/changelog.d/capy-gst-1676-fix-check-chain-link-1.md @@ -0,0 +1 @@ +- **A post-review fix-check chain counts its first link, and the planner plans the next link.** In the post-review flow each granted fix-check's stop head is the commit it reads, so link 1's grant names link 1's own commit, which is also link 2's range_base. finish only accepted an earlier link's grant at a stop head before its commit, so link 1 never counted once link 2 existed and the whole chain routed as behavior (founderstar#634: `FIX_CHECK: not counted (… link 1 of 2)`). A link's grant may now name its own commit; grants are still assigned newest link first, one link per grant, so a lone grant at the boundary covers the newer link only and the older link stays ungranted. `gstack-review-plan --post-review --last fix-check --range-base --grant ` now plans the next chained fix-check instead of a stop that asked for the grant it then ignored, and refuses the last link's own grant, so lanes no longer hand-render the dispatch. (GST-1676) diff --git a/docs/review-policy.md b/docs/review-policy.md index e3601fcc8..79f9cb415 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -113,6 +113,12 @@ incomplete coverage, which blocks until the reviewer is rerun. including a grant for a stop head before `--range-base` (an earlier /ship). A fix-check never earns another, so the grant cannot repeat, and a stop after the granted fix-check still goes to the owner. The cap approves nothing. + Edits that /ship Step 16 later finds after the review bound are a new stop: a + new grant at a stop head after the last fix-check's commit, passed as + `gstack-review-plan --post-review --last fix-check --range-base + --grant `, plans one more fix-check, the next link of the + [fix-check chain](ship-policy.md#fix-check-chain) (GST-1676). The last link's + own grant is refused there, so each link still needs the owner's word. ### Transitions @@ -137,6 +143,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 (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** is one reviewer reading the touched hunks, the fix list, the test results and the callers of every changed symbol, at full effort. It is diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 3bb845fe7..c3cd6581d 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1821,8 +1821,11 @@ STOP tells the lane to plan the inserted Step 9 with `gstack-review-plan 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 the lean panel). At the cap it stops for the owner, whose recorded grant -(`--grant`) adds one scoped fix-check, never `done`. So `finish` never asks for -a step the planner refuses. +(`--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 +stop head after it plans the next link of the chain below (GST-1676), and the +last link's own grant is refused. So `finish` never asks for a step the +planner refuses. **Chained fix-checks (GST-1627).** A second granted fix-check after the first one (a merge resolution, say) logs its row with @@ -1834,10 +1837,13 @@ newest row: while a row's `range_base` doesn't hold the reviewed tree, the link before it is the newest earlier row whose commit is that `range_base`. The chain counts only when it reaches a row whose `range_base` holds the reviewed tree and every link is a clean, committed read with its own approved -`review_loop_grant`. An earlier link's grant must name a stop head before its -commit, since that commit is where the next link starts, and a grant counts for -one link only (the newest link that names it), so one grant never covers two -fix-checks. A path counts as `fix_checked` when some link's range +`review_loop_grant`. A link's grant may name any stop head inside its +`range_base..commit`, its own commit included: the post-review flow grants +each fix-check at the commit it reads, so link 1's stop head is link 2's +`range_base` (founderstar#634, GST-1676). Grants are assigned newest link +first and a grant counts for one link only, so one grant never covers two +fix-checks: a lone grant naming that boundary goes to the newer link and leaves +the older one ungranted. A path counts as `fix_checked` when some link's range touched it and it is unchanged since that link (or its patch against the base is still the one that link read). The line reads `FIX_CHECK: reviewed (fix-check chain of from : at grant ; …)`. A diff --git a/lib/review-depth.ts b/lib/review-depth.ts index ce966e96c..3f3188573 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -384,6 +384,8 @@ export interface History { lean?: boolean; /** Fix-checks already run in this invocation (lean: the grant adds one only after the first). */ fixChecks?: number; + /** The Step 9 /ship Step 16 stage 2.1 inserted for edits after the review bound (gstack-review-plan --post-review). */ + postReview?: boolean; } export interface NextStep { step: Step; reason: string } @@ -398,13 +400,15 @@ export interface NextStep { step: Step; reason: string } * The cap limits fix rounds, not Step 11's voices: on gate class any stop waits for them (GST-967), * and the owner's recorded grant (`granted`) turns the stop after a round-2 full pass with a fixed finding, or after a round-2 * fix-check when `fixChecks` says it was the only one (GEVA-223), into its one fix-check; a fix-check never earns another, so - * the grant cannot repeat. + * the grant cannot repeat. Edits after the review bound (`postReview`) that follow a fix-check are a new stop, and a new grant + * there adds one fix-check, the next link of the chain finish counts (GST-1676, docs/ship-policy.md#fix-check-chain). */ export function nextStep(h: History, depth?: Depth): NextStep { const n = h.lean ? leanStep(h, depth) : loopStep(h, depth); if (n.step !== 'stop') return n; if (h.gate && !h.adversarialRan) return { step: 'voices', reason: `gate class: Step 11's ${h.lean ? 'one reader' : 'voices (adversarial, outside, targeted)'} run${h.lean ? 's' : ''} before the owner's exit, then ask again with --adversarial-ran (${n.reason})` }; const afterFixCheck = h.last === 'fix-check' && h.fullPasses === FULL_PASS_CAP - 1 && h.fixChecks === 1; + if (h.postReview && h.last === 'fix-check' && h.granted && !h.blockingOpen) return { step: 'fix-check', reason: `the owner's grant ${h.granted} adds one scoped fix-check for the edits after the last fix-check, a new link of the fix-check chain (docs/ship-policy.md#fix-check-chain); a later post-review edit needs a grant of its own` }; if (!h.lean && h.granted && !h.blockingOpen && (afterFixCheck || (h.last === 'full' && h.fullPasses === FULL_PASS_CAP))) return { step: 'fix-check', reason: `the owner's grant ${h.granted} adds one scoped fix-check after round ${FULL_PASS_CAP} (review_loop_exception), ${afterFixCheck ? 'never a second' : 'never a fourth round'}` }; return n; } diff --git a/lib/ship-release.ts b/lib/ship-release.ts index 1008081d7..a2041e377 100644 --- a/lib/ship-release.ts +++ b/lib/ship-release.ts @@ -249,8 +249,9 @@ type FixLink = { row: Record; commit: string; from: string }; * The granted fix-checks finish counts (GST-1130, GST-1627): the newest review-fix-check row and, while its range_base does * not hold the reviewed tree, the newest earlier row whose commit is that range_base, back to a row whose range_base holds * it. Every link must be a completed clean read on committed branch content with its own approved review_loop_grant, a stop - * head inside its range_base..commit (before the commit for an earlier link, since that commit is the next link's start); - * a grant counts for one link only, the newest link that names it, so one grant naming two links' commits covers one. + * head inside its range_base..commit, its own commit included (the post-review flow grants each fix-check at the commit it + * reads, GST-1676); a grant counts for one link only, assigned newest link first, so one grant naming the boundary or two + * links' commits covers one, and the older link needs a grant of its own. * A range_base no earlier row ends at is a gap, and the chain does not count. A path counts as fix_checked when a link's * range touched it and it is unchanged since that link, or its patch against the base is still the one the link read. */ @@ -279,7 +280,7 @@ function fixChecked(cwd: string, o: { reviewed: string; now: string; baseRef: st for (let k = chain.length - 1; k >= 0; k--) { const l = chain[k]!; const g = grants.filter(g => !used.has(g.id)) - .flatMap(g => (`${g.ask} ${g.scope}`.match(/\b[0-9a-f]{7,40}\b/g) ?? []).map(commitOf).filter(c => !!c && inRange(l, c) && (k === chain.length - 1 || c !== l.commit)).map(stop => ({ g, stop }))) + .flatMap(g => (`${g.ask} ${g.scope}`.match(/\b[0-9a-f]{7,40}\b/g) ?? []).map(commitOf).filter(c => !!c && inRange(l, c)).map(stop => ({ g, stop }))) .reduce<{ g: typeof grants[number]; stop: string } | undefined>((best, c) => (!best || ancestor(best.stop, c.stop) ? c : best), undefined)?.g; if (g) used.add(g.id); granted[k] = g; diff --git a/test/fixtures/review-pins/files/lib/review-depth.ts.json b/test/fixtures/review-pins/files/lib/review-depth.ts.json index 704b9003f..9388d608f 100644 --- a/test/fixtures/review-pins/files/lib/review-depth.ts.json +++ b/test/fixtures/review-pins/files/lib/review-depth.ts.json @@ -1 +1 @@ -{"sha":"76da578bca44e918a22014b68deb9664a4bdc04c3f4ac6b2a1360cf811f8c820","replay":"GST-1598"} +{"sha":"34685744162ba50d743abbf7c6e2e1ee57bf5ae0e6fd82347bca28f50a155b3f","replay":"GST-1676"} diff --git a/test/fixtures/review-pins/replays/GST-1676.json b/test/fixtures/review-pins/replays/GST-1676.json new file mode 100644 index 000000000..05d1559a0 --- /dev/null +++ b/test/fixtures/review-pins/replays/GST-1676.json @@ -0,0 +1 @@ +{"replay":"GST-1676","justification":"GST-1676 (chained post-review fix-check): lib/review-depth.ts is main's GST-1598 bytes plus one History field, postReview, and one transition in nextStep: with postReview set, last fix-check, a recorded grant and no open blocker, the stop becomes one fix-check (the next link of finish's fix-check chain, docs/ship-policy.md#fix-check-chain). Only bin/gstack-review-plan --post-review sets postReview, and it binds that grant to a stop head after --range-base (the last fix-check's commit). Deterministic no-loss replay, no paid run: the GST-967 grid (MAIN_PLANS) and the GST-972 grid (MAIN_972_PLANS) in test/review-depth.test.ts hash unchanged and the GST-967 transition-table invariant passes, since none of them sets postReview; so no plan main builds drops, renames or re-efforts a reader, no stop main prints without a grant changes, and no brief body moves (MAIN_SECTIONS unchanged)."} diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts index 8331f3de5..e616d0aea 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -111,6 +111,20 @@ describe('GST-967: the round cap limits fix rounds, not the gate-class voices; a expect(nextStep({ ...h, gate: true, adversarialRan: true }).step).toBe('fix-check'); for (const o of [{ fixChecks: 2 }, { fixChecks: undefined }, { blockingOpen: true }, { granted: undefined }, { fullPasses: FULL_PASS_CAP }]) expect([o, nextStep({ ...h, ...o }).step]).toEqual([o, 'stop']); }); + // Value: protects=after a fix-check, edits after the review bound (--post-review) with a new recorded grant plan one more fix-check, + // the next link of finish's chain, in either panel and after Step 11 on gate class; without a grant, with a blocker open, or + // outside --post-review it stays a stop; fails_when=the planner tells the lane to get a grant and then ignores it (GST-1676); + // seam=nextStep; no_claim=the CLI's binding of the grant to a stop head after the last fix-check's commit + test('a new grant after a fix-check plans the next chained fix-check for edits after the review bound (GST-1676)', () => { + const h = { fullPasses: FULL_PASS_CAP, last: 'fix-check', touchedGate: true, fixedBelow: false, granted: 'a-060fed2b73ee', fixChecks: 2, postReview: true } as const; + expect(nextStep(h)).toMatchObject({ step: 'fix-check', reason: expect.stringContaining('a-060fed2b73ee') }); + expect(nextStep({ ...h, lean: true }).step).toBe('fix-check'); + expect(nextStep({ ...h, fullPasses: 1, fixChecks: 1 }).step).toBe('fix-check'); + expect(nextStep({ ...h, gate: true }).step).toBe('voices'); + expect(nextStep({ ...h, gate: true, adversarialRan: true }).step).toBe('fix-check'); + for (const o of [{ granted: undefined }, { blockingOpen: true }, { postReview: false }, { postReview: undefined }]) expect([o, nextStep({ ...h, ...o }).step]).toEqual([o, 'stop']); + expect(nextStep({ ...h, last: 'full', fullPasses: 1, fixedBelow: true }).step).toBe('full'); + }); test('docs/review-policy.md\'s transitions table and rule 3 say both', () => { const doc = fs.readFileSync(path.join(ROOT, 'docs', 'review-policy.md'), 'utf8'); const table = doc.slice(doc.indexOf(''), doc.indexOf('A **fix-check** is one reviewer')); diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts index 3b2f9c47b..99e554c6b 100644 --- a/test/review-plan-cli.test.ts +++ b/test/review-plan-cli.test.ts @@ -122,6 +122,40 @@ describe('gstack-review-plan', () => { expect(plan([...capStop.slice(0, -2), '--grant', id]).status).toBe(2); expect(plan([...capStop, '--grant', '--json']).status).toBe(2); }, 30_000); + // Value: protects=a granted fix-check chain after the review bound is planned, not hand-rendered: once a fix-check ran and /ship + // Step 16 finds later edits, --post-review --last fix-check with a new grant at a stop head after --range-base (the last + // fix-check's commit) plans one more fix-check; the last link's grant (naming its own commit, the new range_base) is refused, + // no grant or an open blocker stops, and outside --post-review a fix-check still never earns another; + // fails_when=the planner prints stop despite the new grant (GST-1676, founderstar#634) or lets the previous link's grant buy + // the next link; seam=bin/gstack-review-plan, nextStep; no_claim=finish's chain count (test/ship-finish-ruled-notes.test.ts) + test('GST-1676: --post-review after a fix-check plans the next chained fix-check for a new grant, never for the last link\'s', () => { + const r = repo({ 'src/a.ts': 'export const a = 1;\n' }); + const env: NodeJS.ProcessEnv = { ...process.env, GSTACK_STATE_ROOT: r.state, GSTACK_APPROVALS_MIRROR: '0' }; + delete env.GSTACK_APPROVALS_DIR; + const git = (...a: string[]) => spawnSync('git', a, { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout.trim(); + const commit = (n: number) => { fs.appendFileSync(path.join(r.dir, 'src/a.ts'), `export const v${n} = ${n};\n`); git('commit', '-qam', `c${n}`); return git('rev-parse', 'HEAD'); }; + const add = (stop: string, n: string) => { + const a = spawnSync(path.join(path.dirname(BIN), 'gstack-approvals'), ['add', '--ask', `Owner grant: scoped fix-check ${n} (review_loop_grant ${stop.slice(0, 12)})`, '--words', 'do it', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env }); + expect(a.status, a.stderr).toBe(0); + return JSON.parse(a.stdout).added[0] as string; + }; + commit(1); + const link1 = commit(2); + const g1 = add(link1, 'fc1'); + const link2 = commit(3); + const g2 = add(link2, 'fc2'); + const plan = (a: string[]) => spawnSync(BIN, ['--base', 'main', '--gate-tagged', '0', '--pass', '2', '--last', 'fix-check', '--fix-checks', '2', '--range-base', link1, ...a], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env }); + const chained = plan(['--post-review', '--grant', g2]); + expect(chained.status, chained.stderr).toBe(0); + expect(chained.stdout).toContain(`NEXT: fix-check (edits after the review bound (Step 16 stage 2.1) plan as a fix to a gate-class path, never a note: the owner's grant ${g2} adds one scoped fix-check for the edits after the last fix-check, a new link of the fix-check chain`); + expect(chained.stdout).toContain('DISPATCH: fix-check fix-check effort=full'); + expect(plan(['--post-review']).stdout).toContain('NEXT: stop'); + expect(plan(['--post-review', '--grant', g2, '--blocking-open']).stdout).toContain('NEXT: stop'); + expect(plan(['--fixed-blocking', '--grant', g2]).stdout).toContain('NEXT: stop'); + const reused = plan(['--post-review', '--grant', g1]); + expect([reused.status, reused.stdout]).toEqual([3, '']); + expect(reused.stderr).toContain(`--grant ${g1}: no approved row ${g1} naming review_loop_grant and this invocation's stop head (a commit after --range-base ${link1}, the last fix-check's commit`); + }, 30_000); test('--fix-check-prompt renders the fix-check reviewer prompt from the parent\'s files and refuses a missing one', () => { const r = repo({ 'docs/howto.md': '# x\n' }); const w = (n: string, t: string) => { const f = path.join(r.state, n); fs.writeFileSync(f, t); return f; }; diff --git a/test/ship-finish-ruled-notes.test.ts b/test/ship-finish-ruled-notes.test.ts index 9d1875c86..6f04ea3a5 100644 --- a/test/ship-finish-ruled-notes.test.ts +++ b/test/ship-finish-ruled-notes.test.ts @@ -302,6 +302,54 @@ describe('finish chains contiguous fix-check rows (GST-1627)', () => { }); }); +/** + * GST-1676 (founderstar#634, FOUN-117): the post-review flow grants each fix-check at its own stop head, which is the + * commit that fix-check reads. Link 1's grant names link 1's commit (05a0db1c) and link 2's names link 2's (9ab970db), + * so the boundary is both link 1's stop head and link 2's range_base. + */ +function postReviewShape() { + const f = shipFixture({ 'src/x.ts': 'export const x = 1;\n' }, { 'src/x.ts': 'export const x = 2;\n', 'test/x.test.ts': TEST_V1 }); + ships.push(f); + const passHead = headOf(f); + const reviewed = f.run(path.join(BIN, 'gstack-wtree'), []).stdout.trim(); + const reviewedBase = f.git('merge-base', 'origin/main', 'HEAD').stdout.trim(); + f.write({ 'src/x.ts': 'export const x = 3;\n' }); f.commit('edit after the review bound'); + const link1 = headOf(f); + return { f, passHead, reviewed, reviewedBase, link1 }; +} + +describe('finish counts a post-review fix-check chain whose grants name each link\'s own commit (GST-1676)', () => { + // Value: protects=the #634 shape binds: link 1's grant names its own commit, which is also link 2's range_base, and link 2's + // grant names link 2's commit, so each link has its own grant and the chain counts; fails_when=finish refuses an earlier + // link's grant at its own commit (FIX_CHECK: not counted ... link 1 of 2) and src/x.ts routes as behavior; + // seam=finish's per-link grant assignment + test('#634: two post-review links, each granted at its own commit, and finish counts the chain', () => { + const { f, passHead, reviewed, reviewedBase, link1 } = postReviewShape(); + const g1 = grantLink(f, link1, 'fc1'); + logLink(f, passHead); + f.write({ 'test/x.test.ts': TEST_V2 }); f.commit('a second edit after the review bound'); + const link2 = headOf(f); + const g2 = grantLink(f, link2, 'fc2'); + logLink(f, link1); + const r = finishAfterAudit(f, reviewed, reviewedBase); + expect(lineOf(r.stdout, 'FIX_CHECK'), r.stdout).toBe(`FIX_CHECK: reviewed src/x.ts,test/x.test.ts (fix-check chain of 2 from ${passHead.slice(0, 12)}: at ${link1.slice(0, 12)} grant ${g1}; at ${link2.slice(0, 12)} grant ${g2})`); + expect([r.status, lineOf(r.stdout, 'ROUTE')], r.stdout).toEqual([0, expect.stringMatching(/ behavior=0 .* fix_checked=2$/)]); + }); + + // Value: protects=a grant at the boundary still counts once: with only link 1's grant (at link 1's commit) recorded, the newer + // link takes it and link 1 stays ungranted; fails_when=one grant row counts for both links once an earlier link may use its + // own commit; seam=finish's newest-first, one-grant-per-link assignment + test('only link 1\'s grant: it goes to the newer link, and link 1 is ungranted', () => { + const { f, passHead, reviewed, reviewedBase, link1 } = postReviewShape(); + grantLink(f, link1, 'fc1'); + logLink(f, passHead); + f.write({ 'test/x.test.ts': TEST_V2 }); f.commit('a second edit after the review bound'); + logLink(f, link1); + const r = finishAfterAudit(f, reviewed, reviewedBase); + expect([r.status, lineOf(r.stdout, 'FIX_CHECK')], r.stdout).toEqual([3, `FIX_CHECK: not counted (no approved review_loop_grant names a stop head inside its range_base..commit; fix-check at ${link1.slice(0, 12)}, link 1 of 2)`]); + }); +}); + describe('ruledNotes reads only approved owner rulings (GST-1537)', () => { const HEAD = '0123456789abcdef0123456789abcdef01234567'; const row = (finding: string, o: Partial<{ ask: string; scope: string; by: string; outcome: string }> = {}) => {