fix(gate): part 1 verifier findings — stop-rule wording, guard tests, unreadable round
- plan-gate: the bare gate's stale-verdict reason no longer says "run the next round" once the stop rule fired; it says there is no next round, as acceptOpen already does. - trekplan Phase 9: "REVISE after the last round" (Fix now with --extra-round) is qualified "and the stop rule did not fire", pinned in review-loop-docs.test.mjs next to the [STP] pins. - plan-gate-stop-rule tests: a planted stop-rule acceptance claiming an open security blocker stays STOP; a planted acceptance without under: "stop-rule" stays STOP; the "a blocker -> refused" test now asserts the blocker refusal; --accept-open under the stop rule covers an open MAJOR and the gate passes with a WARN. - review-rounds prepare: when the stop rule is judged (P >= 2) and either round's JSON block cannot be read, exit 2 (not measured) with "stop rule: a round has no readable JSON block" instead of skipping the rule. The [RND] differing-archive and [CAP] fixtures now use readable archives; one new [STP] test covers an unreadable archive and an unreadable review.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
08e8aa1c66
commit
07a3c06c57
6 changed files with 153 additions and 13 deletions
|
|
@ -1062,8 +1062,9 @@ duplicates. Source: research/05 R1 + R2.
|
|||
Phase 10, which lists the minors for the operator to pick from (operator
|
||||
decision 2026-10-08: a real weakness among the minors after the last round
|
||||
used to be carried with no way to a fix).
|
||||
- **REVISE after the last round**: the gate reports
|
||||
`PLAN_GATE_OPEN_MAJOR_FINAL`. Stop and ask the operator with
|
||||
- **REVISE after the last round**, and the stop rule did not fire: the gate reports
|
||||
`PLAN_GATE_OPEN_MAJOR_FINAL` (after the stop rule it reports
|
||||
`PLAN_GATE_STOP_RULE`, and only the stop rule below applies). Stop and ask the operator with
|
||||
AskUserQuestion — each open major in plain words, recommendation first
|
||||
(operator decision 2026-10-08):
|
||||
- **Fix now** — revise the plan for the listed majors, then record one more
|
||||
|
|
|
|||
|
|
@ -30,8 +30,8 @@
|
|||
// `carried_from`; class = `classOf`. Fired -> exit 1 with
|
||||
// `review-stop-rule: <describeStop line>` and `stop_rule`; nothing is
|
||||
// archived. A round whose trailing JSON block cannot be read gives no
|
||||
// summary, and the rule is then not judged (as a plan verdict without
|
||||
// history). The cap stays.
|
||||
// summary: exit 2 (not measured), `stop rule: a round has no readable
|
||||
// JSON block`, nothing archived. The cap stays.
|
||||
// - The round base is the previous `scope_sha_end`: a full sha that is an
|
||||
// ancestor of HEAD, else exit 2 (not measured).
|
||||
// - review.md is copied to `review-round-<run>-<P>.md`; an existing file
|
||||
|
|
@ -245,10 +245,9 @@ export function prepare({ projectDir, since, repo, dryRun = false } = {}) {
|
|||
const at = repo ?? projectDir;
|
||||
const a = prevBytes === null ? null : reviewRoundSummary(prevBytes.toString('utf8'), at);
|
||||
const b = reviewRoundSummary(text, at);
|
||||
if (a && b) {
|
||||
const d = decideStop([{ round: P - 1, ...a }, { round: P, ...b }]);
|
||||
if (d.fired) return { exit: 1, reason: `review-stop-rule: ${describeStop(d)}`, stop_rule: d };
|
||||
}
|
||||
if (!a || !b) return notMeasured('stop rule: a round has no readable JSON block');
|
||||
const d = decideStop([{ round: P - 1, ...a }, { round: P, ...b }]);
|
||||
if (d.fired) return { exit: 1, reason: `review-stop-rule: ${describeStop(d)}`, stop_rule: d };
|
||||
}
|
||||
const base = ownEnd;
|
||||
if (!base || !isAncestorOfHead(repo ?? projectDir, base)) {
|
||||
|
|
|
|||
|
|
@ -489,7 +489,12 @@ export function gatePlan(rawText) {
|
|||
reasons.push({ code: 'PLAN_GATE_VERDICT_MALFORMED', message: ex.error });
|
||||
} else {
|
||||
if (v.plan_sha256 !== planHash(text)) {
|
||||
reasons.push({ code: 'PLAN_GATE_VERDICT_STALE', message: `The plan changed after critic round ${v.round}; run the next round.` });
|
||||
reasons.push({
|
||||
code: 'PLAN_GATE_VERDICT_STALE',
|
||||
message: v.stop_rule?.fired === true
|
||||
? `The plan changed after critic round ${v.round}, and the stop rule fired, so there is no next round; restore the reviewed text, or stop and take the direction to the operator.`
|
||||
: `The plan changed after critic round ${v.round}; run the next round.`,
|
||||
});
|
||||
}
|
||||
const sev = f => normalizeSeverity(f.severity);
|
||||
const blockers = v.findings.filter(f => sev(f) !== 'major' && sev(f) !== 'minor');
|
||||
|
|
|
|||
|
|
@ -163,6 +163,15 @@ test('[STP] Phase 9: the stop rule gives no new round and asks Known issue or Ne
|
|||
'the round-3 blocker path says it never offers --extra-round after the stop rule');
|
||||
});
|
||||
|
||||
test('[STP] Phase 9: "REVISE after the last round" (Fix now with --extra-round) is only for a round where the stop rule did not fire', () => {
|
||||
const at = P9.indexOf('**REVISE after the last round**');
|
||||
assert.ok(at >= 0, 'Phase 9 has the "REVISE after the last round" bullet');
|
||||
const bullet = P9.slice(at, P9.indexOf('**The stop rule**', at));
|
||||
assert.ok(bullet.includes('`--extra-round`'), 'the bullet offers --extra-round');
|
||||
assert.ok(P9.includes('**REVISE after the last round**, and the stop rule did not fire:'),
|
||||
'the bullet is qualified as the BLOCK/REVISE bullet is');
|
||||
});
|
||||
|
||||
test('[STP] Phase 10 prints the round of max_rounds + extra_rounds', () => {
|
||||
assert.ok(P10.includes('**Critic rounds:** {round} of {max_rounds + extra_rounds}'), 'Phase 10 prints round of max_rounds + extra_rounds');
|
||||
assert.ok(!P10.includes('**Critic rounds:** {N} of {max}'), 'the old "{N} of {max}" is gone');
|
||||
|
|
|
|||
|
|
@ -235,6 +235,8 @@ function roundTwo(fx, findings, { outs = {}, engines = ['code-review', 'security
|
|||
|
||||
const MAJ_A3 = { severity: 'MAJOR', file: 'a.mjs', line: 3, title: 'Parser drops the last token', detail: 'parse() stops one token early' };
|
||||
const BLK_B3 = { severity: 'BLOCKER', file: 'b.mjs', line: 3, rule_key: 'BROKEN_SUCCESS_CRITERION', title: 'Build output is never written', detail: 'write() returns before the flush' };
|
||||
/** A second BLOCKER of BLK_B3's class, for archives the stop rule must read but not fire on. */
|
||||
const BLK_A5_RND = { severity: 'BLOCKER', file: 'a.mjs', line: 5, rule_key: 'BROKEN_SUCCESS_CRITERION', title: 'Second build path never flushes either', detail: 'emit() skips the final flush call' };
|
||||
|
||||
// ---- [RND] range and the fail-closed base ----------------------------------------
|
||||
|
||||
|
|
@ -454,11 +456,14 @@ test('[RND] an archive with other bytes is exit 2; a round field that disagrees
|
|||
assert.ok(existsSync(ROUNDS), 'lib/review/review-rounds.mjs must exist');
|
||||
const fx = rndRepo();
|
||||
fx.write(rndReview({ start: fx.start, end: fx.c1, findings: [MAJ_A3] }).text);
|
||||
fx.write('another review\n', fx.archive(2));
|
||||
// Another review with a readable JSON block (the stop rule reads it): two
|
||||
// BLOCKERs of another class, so the rule does not fire on the pair.
|
||||
const another = rndReview({ start: fx.start, end: fx.c1, findings: [BLK_B3, BLK_A5_RND] }).text;
|
||||
fx.write(another, fx.archive(2));
|
||||
const r = fx.prepare();
|
||||
assert.equal(r.status, 2, r.stdout + r.stderr);
|
||||
assert.match(String(parse(r).reason), new RegExp(`archive review-round-${fx.run}-2\\.md differs — not measured`), r.stdout);
|
||||
assert.equal(fx.read(fx.archive(2)), 'another review\n', 'the differing archive is left alone');
|
||||
assert.equal(fx.read(fx.archive(2)), another, 'the differing archive is left alone');
|
||||
rmSync(fx.archive(2));
|
||||
// round: 2 with no archive of round 1.
|
||||
fx.write(rndReview({ start: fx.start, end: fx.c1, findings: [MAJ_A3] }).text.replace('\nfindings:', '\nround: 2\nfindings:'));
|
||||
|
|
@ -706,8 +711,10 @@ test('[CAP] prepare refuses round 4 with exit 1', () => {
|
|||
const fx = rndRepo();
|
||||
const r3 = rndReview({ start: fx.start, end: fx.c2, findings: [MAJ_A3] }).text.replace('\nfindings:', '\nround: 3\nfindings:');
|
||||
fx.write(r3);
|
||||
fx.write('round 1\n', fx.archive(1));
|
||||
fx.write('round 2\n', fx.archive(2));
|
||||
// Earlier rounds with readable JSON blocks (the stop rule reads them): two
|
||||
// BLOCKERs of another class, so the rule does not fire before round 3.
|
||||
fx.write(rndReview({ start: fx.start, end: fx.c1, findings: [BLK_B3, BLK_A5_RND] }).text, fx.archive(1));
|
||||
fx.write(rndReview({ start: fx.start, end: fx.c2, findings: [BLK_B3, BLK_A5_RND] }).text, fx.archive(2));
|
||||
const before = fx.snapshot();
|
||||
const r = fx.prepare();
|
||||
assert.equal(r.status, 1, r.stdout + r.stderr);
|
||||
|
|
@ -1050,3 +1057,24 @@ test('[STP] the gate\'s pre-existing reading: a pre-existing MAJOR does not coun
|
|||
assert.equal(r.status, 1, r.stdout + r.stderr);
|
||||
assert.match(String(parse(r).reason), /^review-stop-rule: stop rule \(not-fewer\).*round 2: 2 blocking/, r.stdout);
|
||||
});
|
||||
|
||||
test('[STP] a round with no readable JSON block at P = 2: prepare is not measured (exit 2), --dry-run too, nothing written', () => {
|
||||
const fx = rndRepo();
|
||||
const r2 = rndReview({ start: fx.start, end: fx.c2, findings: [MAJ_A3] }).text.replace('\nfindings:', '\nround: 2\nfindings:');
|
||||
const r1 = rndReview({ start: fx.start, end: fx.c1, findings: [BLK_B3, BLK_A5_RND] }).text;
|
||||
const noBlock = (t) => t.slice(0, t.lastIndexOf('```json\n'));
|
||||
for (const [label, archived, current] of [
|
||||
['the previous archive', 'round 1\n', r2],
|
||||
['review.md', r1, noBlock(r2)],
|
||||
]) {
|
||||
fx.write(current);
|
||||
fx.write(archived, fx.archive(1));
|
||||
const before = fx.snapshot();
|
||||
for (const extra of [['--dry-run'], []]) {
|
||||
const r = fx.prepare(extra);
|
||||
assert.equal(r.status, 2, `${label} ${extra.join(' ')}: ${r.stdout}${r.stderr}`);
|
||||
assert.match(String(parse(r).reason), /^stop rule: a round has no readable JSON block — not measured/, r.stdout);
|
||||
assert.deepEqual(fx.snapshot(), before, `${label}: nothing written`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -177,6 +177,70 @@ test('gate: a fired stop rule with an open blocker stops with PLAN_GATE_STOP_RUL
|
|||
assert.equal(g.stop_rule.fired, true);
|
||||
});
|
||||
|
||||
const T3_FROM = 'Why: the brief asks for a parser CLI.';
|
||||
const T3_TO = 'Why: the brief asks for a parser CLI with clear errors.';
|
||||
|
||||
test('gate: a stale verdict after a fired stop rule never says "run the next round"; without the stop rule it does', () => {
|
||||
const { r1, r2 } = sameClassTwice();
|
||||
assert.ok(r2.text.includes(T3_FROM));
|
||||
const g = gatePlan(r2.text.replace(T3_FROM, T3_TO));
|
||||
assert.equal(g.code, 1);
|
||||
const stale = g.reasons.find(x => x.code === 'PLAN_GATE_VERDICT_STALE');
|
||||
assert.ok(stale, JSON.stringify(g.reasons));
|
||||
assert.doesNotMatch(stale.message, /run the next round/);
|
||||
assert.match(stale.message, /stop rule fired/);
|
||||
assert.match(stale.message, /no next round/);
|
||||
// Control: the stop rule did not fire after round 1.
|
||||
assert.equal(r1.verdict.stop_rule.fired, false);
|
||||
const c = gatePlan(r1.text.replace(T3_FROM, T3_TO));
|
||||
const cs = c.reasons.find(x => x.code === 'PLAN_GATE_VERDICT_STALE');
|
||||
assert.ok(cs, JSON.stringify(c.reasons));
|
||||
assert.match(cs.message, /run the next round/);
|
||||
});
|
||||
|
||||
/** The verdict block of `text` replaced by `fn(verdict)`. */
|
||||
const putVerdict = (text, fn) => text.replace(/```json\n[\s\S]*?\n```/, () => '```json\n' + JSON.stringify(fn(extractVerdict(text).verdict), null, 2) + '\n```');
|
||||
|
||||
test('gate guard: a planted stop-rule acceptance that claims an open SECURITY blocker leaves the gate at STOP', () => {
|
||||
const t1 = buildPlan();
|
||||
const r1 = rec(t1, dd({ pc: [f('blocker', at(t1, R1_CHANGES), B1, 'SECURITY_INJECTION')] }));
|
||||
const id = r1.verdict.findings[0].id;
|
||||
const t2 = editA(r1.text);
|
||||
const r2 = rec(t2, dd({
|
||||
pc: [f('blocker', at(t2, 'its error path'), B2, 'SECURITY_INJECTION')],
|
||||
pcPrev: [{ id, resolved: true }],
|
||||
}), { base: baseOf(r1.text) });
|
||||
assert.equal(r2.verdict.stop_rule?.fired, true);
|
||||
const planted = putVerdict(r2.text, v => ({
|
||||
...v,
|
||||
accepted_open: { round: v.round, plan_sha256: v.plan_sha256, accepted_at: NOW, under: 'stop-rule', findings: v.findings.map(x => ({ id: x.id ?? null, text: x.text ?? '' })) },
|
||||
}));
|
||||
const ex = extractVerdict(planted);
|
||||
assert.equal(ex.error, null);
|
||||
assert.equal(ex.verdict.accepted_open.under, 'stop-rule');
|
||||
assert.equal(planHash(planted), planHash(r2.text), 'the planted acceptance is review log');
|
||||
const g = gatePlan(planted);
|
||||
assert.equal(g.code, 1, JSON.stringify(g));
|
||||
assert.equal(g.status, 'stop');
|
||||
assert.ok(g.reasons.some(x => x.code === 'PLAN_GATE_STOP_RULE'), JSON.stringify(g.reasons));
|
||||
});
|
||||
|
||||
test('gate guard: a planted acceptance covering a blocker without `under: "stop-rule"` leaves the gate at STOP', () => {
|
||||
const { r2 } = sameClassTwice();
|
||||
const planted = putVerdict(r2.text, v => ({
|
||||
...v,
|
||||
accepted_open: { round: v.round, plan_sha256: v.plan_sha256, accepted_at: NOW, findings: v.findings.map(x => ({ id: x.id ?? null, text: x.text ?? '' })) },
|
||||
}));
|
||||
const ex = extractVerdict(planted);
|
||||
assert.equal(ex.error, null);
|
||||
assert.equal(ex.verdict.accepted_open.under, undefined);
|
||||
assert.equal(planHash(planted), planHash(r2.text), 'the planted acceptance is review log');
|
||||
const g = gatePlan(planted);
|
||||
assert.equal(g.code, 1, JSON.stringify(g));
|
||||
assert.equal(g.status, 'stop');
|
||||
assert.ok(g.reasons.some(x => x.code === 'PLAN_GATE_STOP_RULE'), JSON.stringify(g.reasons));
|
||||
});
|
||||
|
||||
// ---- --accept-open under the stop rule (220) ----------------------------------------
|
||||
|
||||
test('accept-open: after a fired stop rule it is allowed with rounds left, covers the blocker, and the gate warns', () => {
|
||||
|
|
@ -216,6 +280,40 @@ test('accept-open: without the stop rule it behaves as before (rounds left -> re
|
|||
const a = acceptOpen(r1.text, { now: NOW });
|
||||
assert.equal(a.code, 1);
|
||||
assert.match(a.message, /round\(s\) left/);
|
||||
// A blocker open, the stop rule not fired: refused as a blocker.
|
||||
const b1 = rec(t1, dd({ pc: [f('blocker', at(t1, R1_CHANGES), B1, 'dep/manifest')] }));
|
||||
assert.equal(b1.verdict.stop_rule?.fired, false);
|
||||
const b = acceptOpen(b1.text, { now: NOW });
|
||||
assert.equal(b.code, 1);
|
||||
assert.match(b.message, /a blocker is never accepted/);
|
||||
assert.equal(b.text, undefined, 'nothing is written');
|
||||
});
|
||||
|
||||
const M3 = 'Rollback wording in the second step names a script that the repository lacks';
|
||||
|
||||
test('accept-open: after a fired stop rule it covers an open MAJOR (no blocker) and the gate passes with a WARN', () => {
|
||||
const t1 = buildPlan();
|
||||
const r1 = rec(t1, dd({ pc: [f('major', at(t1, R1_CHANGES), M2, 'rollback/wording')] }));
|
||||
const id = r1.verdict.findings[0].id;
|
||||
const t2 = editA(r1.text);
|
||||
const r2 = rec(t2, dd({
|
||||
pc: [f('major', at(t2, 'its error path'), M3, 'rollback/missing-script')],
|
||||
pcPrev: [{ id, resolved: true }],
|
||||
}), { base: baseOf(r1.text) });
|
||||
assert.equal(r2.verdict.stop_rule?.fired, true, JSON.stringify(r2.verdict.stop_rule));
|
||||
assert.deepEqual(r2.verdict.findings.map(x => x.severity.toLowerCase()), ['major']);
|
||||
// Before the acceptance: STOP on the stop rule.
|
||||
assert.deepEqual(gatePlan(r2.text).reasons.map(x => x.code), ['PLAN_GATE_STOP_RULE']);
|
||||
const a = acceptOpen(r2.text, { now: NOW });
|
||||
assert.equal(a.code, 0, a.message);
|
||||
const v = extractVerdict(a.text).verdict;
|
||||
assert.equal(v.accepted_open.under, 'stop-rule');
|
||||
assert.deepEqual(v.accepted_open.findings.map(x => x.text), [M3]);
|
||||
const g = gatePlan(a.text);
|
||||
assert.equal(g.code, 0, JSON.stringify(g.reasons));
|
||||
assert.equal(g.status, 'pass');
|
||||
assert.ok(g.warnings.some(w => w.code === 'PLAN_GATE_OPEN_MAJOR' && w.message.includes(M3)), JSON.stringify(g.warnings));
|
||||
assert.ok(!g.warnings.some(w => w.code === 'PLAN_GATE_OPEN_BLOCKER'), JSON.stringify(g.warnings));
|
||||
});
|
||||
|
||||
// ---- known issue: the Pass 2 heading is plan text now ---------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue