mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 10:40:32 +01:00
fix: finish counts a post-review fix-check chain's first link; review-plan plans the next chained fix-check (GST-1676) (#4093)
* finish counts a post-review fix-check chain's first link; review-plan plans the next chained fix-check (GST-1676) Replay: GST-1676 (repins lib/review-depth.ts) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
678276396f
commit
688e3ab9d1
11 files changed
+140
-19
No files matched your search
+13
-8
@@ -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 <fp> 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 <sha> the fix range's base (this pass's PASS_HEAD): after a fixed blocker on a diff below full depth, a
|
||||
--range-base <sha> 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 <approval id> 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 <n> 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 <n> the PR whose platform review counts under review.platform on (default: this branch's PR)
|
||||
--declare-risk <classes> 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 <the commit it read> --targeted-ran <the targeted_patch_id it read> (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 <stop head sha>)' 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 <stop head sha>)' 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');
|
||||
|
||||
@@ -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 <last fix-check's commit> --grant <new 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)
|
||||
@@ -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 <that commit>
|
||||
--grant <id>`, 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.
|
||||
|
||||
<a id="transitions"></a>
|
||||
### 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 <approval id>`) | 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
|
||||
|
||||
+12
-6
@@ -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.
|
||||
|
||||
<a id="fix-check-chain"></a>**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 <paths>
|
||||
(fix-check chain of <n> from <range_base12>: at <commit12> grant <id>; …)`. A
|
||||
|
||||
+5
-1
@@ -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;
|
||||
}
|
||||
|
||||
+4
-3
@@ -249,8 +249,9 @@ type FixLink = { row: Record<string, any>; 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;
|
||||
|
||||
@@ -1 +1 @@
|
||||
{"sha":"76da578bca44e918a22014b68deb9664a4bdc04c3f4ac6b2a1360cf811f8c820","replay":"GST-1598"}
|
||||
{"sha":"34685744162ba50d743abbf7c6e2e1ee57bf5ae0e6fd82347bca28f50a155b3f","replay":"GST-1676"}
|
||||
@@ -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)."}
|
||||
@@ -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('<a id="transitions"></a>'), doc.indexOf('A **fix-check** is one reviewer'));
|
||||
|
||||
@@ -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; };
|
||||
|
||||
@@ -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 }> = {}) => {
|
||||
|
||||
Reference in new issue
Block a user