fix(plan): plan-critic and scope-guardian state rule B, mark previous findings and tag security findings

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

agents/plan-critic.md

@@ -275,12 +275,25 @@ result), or a behaviour the brief names with no test, is **major** — the tier
 of `MISSING_TEST` — never minor. Whenever a finding is the same class as a
 catalogue rule, use that rule's tier.
 
+Rule B: a major that a named test, `Regression:` command or gate in the plan
+will fail on during execution is minor — never a blocker, never a security finding.
+The 2026-10-08 rule still holds for rule B: a test that cannot fail on a wrong
+implementation catches nothing, so the finding stays major. Give
+`lowered_from` and `caught_by` when you lower a finding this way.
+
+A security finding uses the catalogue's security `rule_key`
+(`SECURITY_INJECTION`, or `SECURITY_REVIEW_VULNERABILITY` when no narrower key
+fits).
+
 Your rating decides what happens next: `/trekplan` records your findings with
 `plan-gate.mjs --record`, and the plan gate stops the plan while any BLOCKER
 or MAJOR is open — a MAJOR left after the last round goes to the operator
 (fix now, or build as is). A finding with no valid severity counts as a blocker. You may be run
-up to 3 times on successive revisions of the same plan; judge the plan in
-front of you, and do not repeat a finding the revision has fixed.
+up to 3 times on successive revisions of the same plan. From round 2 the
+prompt gives you the diff since the last reviewed text, and the plan's
+`## Critic Verdict` lists the previous findings. List every previous finding
+by id in `previous`, with `resolved: true` only when the plan now fixes it.
+Review the changed text. Cite the exact line, with `line_end` for a range.
 
 Dimensions 11 and 12 are scored inside the existing weights: 11 under Step
 quality, 12 under Coverage completeness and Specification quality.
@@ -359,8 +372,11 @@ this block inline *is* the hand-off; do not attempt to write a file.
 ```json
 {
   "agent": "plan-critic",
+  "previous": [
+    { "id": "<40-hex id from the current ## Critic Verdict>", "resolved": false }
+  ],
   "findings": [
-    { "file": "<plan path or source file>", "line": 0, "rule_key": "<dimension/short-id>", "severity": "blocker|major|minor", "text": "<one-line finding>" }
+    { "file": "<plan path or source file>", "line": 0, "line_end": 0, "rule_key": "<dimension/short-id>", "severity": "blocker|major|minor", "text": "<one-line finding>", "lowered_from": "major", "caught_by": "<the named test, Regression: command or gate>" }
   ]
 }
 ```
@@ -370,6 +386,12 @@ path + the step's line) or the source the finding concerns; `rule_key` is a
 short, stable id for the finding class (used for exact-match dedup).
 `severity` is exactly one of `blocker`, `major`, `minor`. Emit
 `"findings": []` when the plan is clean.
+`previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per
+finding the current `## Critic Verdict` lists; `resolved` is `true` or
+`false`. `line_end` (optional) is the last line of a cited range, an integer
+≥ `line`. `lowered_from` and `caught_by` go only on a finding lowered by rule
+B: `"lowered_from": "major"`, and `caught_by` names the test, `Regression:`
+command or gate that will fail on it; omit both otherwise.
 
 Be specific. Reference exact plan sections, step numbers, and file paths.
 Never use "generally" or "usually" — cite the specific problem in this specific plan.

agents/scope-guardian.md

@@ -94,10 +94,27 @@ result), or a behaviour the brief names with no test, is **major** — the tier
 of `MISSING_TEST` — never minor. Whenever a finding is the same class as a
 catalogue rule, use that rule's tier.
 
+Rule B: a major that a named test, `Regression:` command or gate in the plan
+will fail on during execution is minor — never a blocker, never a security finding.
+The 2026-10-08 rule still holds for rule B: a test that cannot fail on a wrong
+implementation catches nothing, so the finding stays major. Give
+`lowered_from` and `caught_by` when you lower a finding this way.
+
+A security finding uses the catalogue's security `rule_key`
+(`SECURITY_INJECTION`, or `SECURITY_REVIEW_VULNERABILITY` when no narrower key
+fits).
+
 Your findings go through `plan-gate.mjs --record` with plan-critic's; an open
 blocker or major stops the plan. `severity` is exactly `blocker`, `major` or `minor`;
 an unrated finding counts as a blocker.
 
+You may be run up to 3 times on successive revisions of the same plan. From
+round 2 the prompt gives you the diff since the last reviewed text, and the
+plan's `## Critic Verdict` lists the previous findings. List every previous
+finding by id in `previous`, with `resolved: true` only when the plan now
+fixes it. Review the changed text. Cite the exact line, with `line_end` for a
+range.
+
 ## Output format
 
 ```
@@ -142,8 +159,11 @@ this block inline *is* the hand-off; do not attempt to write a file.
 ```json
 {
   "agent": "scope-guardian",
+  "previous": [
+    { "id": "<40-hex id from the current ## Critic Verdict>", "resolved": false }
+  ],
   "findings": [
-    { "file": "<plan path or source file>", "line": 0, "rule_key": "<creep|gap|dependency|...>", "severity": "blocker|major|minor", "text": "<one-line finding>" }
+    { "file": "<plan path or source file>", "line": 0, "line_end": 0, "rule_key": "<creep|gap|dependency|...>", "severity": "blocker|major|minor", "text": "<one-line finding>", "lowered_from": "major", "caught_by": "<the named test, Regression: command or gate>" }
   ]
 }
 ```
@@ -152,6 +172,12 @@ One object per scope-creep / gap / dependency finding. `file`/`line` point at th
 plan step (or the brief requirement) the finding concerns; `rule_key` is a short,
 stable id for the finding class (used for exact-match dedup against plan-critic).
 Emit `"findings": []` when the plan is fully aligned.
+`previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per
+finding the current `## Critic Verdict` lists; `resolved` is `true` or
+`false`. `line_end` (optional) is the last line of a cited range, an integer
+≥ `line`. `lowered_from` and `caught_by` go only on a finding lowered by rule
+B: `"lowered_from": "major"`, and `caught_by` names the test, `Regression:`
+command or gate that will fail on it; omit both otherwise.
 
 ## When to use — examples
 

tests/lib/critic-severity-rules.test.mjs

@@ -0,0 +1,90 @@
+// tests/lib/critic-severity-rules.test.mjs
+// The two plan reviewers (plan-critic, scope-guardian) must state rule B word for
+// word as lib/review/critic-diff.mjs exports it, keep the 2026-10-08 rule that a
+// test which cannot fail on a wrong implementation is major, tag security
+// findings with the catalogue's security keys, and carry the round-2 fields
+// (`previous`, `line_end`, `lowered_from`, `caught_by`) in their json template.
+// The name is the brief's; lib/util/test-census.mjs counts this file as a
+// behaviour test (it does not match PIN_FILE_RE), not as a doc pin.
+
+import { test } from 'node:test';
+import { strict as assert } from 'node:assert';
+import { readFileSync } from 'node:fs';
+import { join, dirname } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { RULE_B } from '../../lib/review/critic-diff.mjs';
+import { RULE_CATALOGUE } from '../../lib/review/rule-catalogue.mjs';
+
+const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
+const read = rel => readFileSync(join(ROOT, rel), 'utf-8');
+const norm = s => s.replace(/\s+/g, ' ');
+
+const AGENTS = ['agents/plan-critic.md', 'agents/scope-guardian.md'];
+
+// The one json template an agent file carries: the fenced block after
+// "Machine-readable findings block".
+function jsonTemplate(text, rel) {
+  const head = text.indexOf('Machine-readable findings block');
+  assert.ok(head >= 0, `${rel}: machine-readable findings block heading missing`);
+  const m = /```json\n([\s\S]*?)\n```/.exec(text.slice(head));
+  assert.ok(m, `${rel}: no fenced json template after the heading`);
+  return JSON.parse(m[1]);
+}
+
+test('the catalogue has exactly two security keys: SECURITY_INJECTION and SECURITY_REVIEW_VULNERABILITY', () => {
+  const keys = RULE_CATALOGUE.filter(r => r.category === 'security').map(r => r.rule_key).sort();
+  assert.deepEqual(keys, ['SECURITY_INJECTION', 'SECURITY_REVIEW_VULNERABILITY']);
+});
+
+for (const rel of AGENTS) {
+  test(`${rel}: states rule B verbatim (never a blocker, never a security finding)`, () => {
+    const t = norm(read(rel));
+    assert.ok(t.includes(norm(RULE_B)), `${rel} must contain the RULE_B sentence from critic-diff.mjs`);
+    assert.ok(t.includes('never a blocker, never a security finding'));
+    assert.ok(
+      t.includes('The 2026-10-08 rule still holds for rule B: a test that cannot fail on a wrong implementation catches nothing, so the finding stays major.'),
+      `${rel} must say the 2026-10-08 rule still holds for rule B`,
+    );
+    assert.ok(t.includes('Give `lowered_from` and `caught_by` when you lower a finding this way.'));
+    assert.ok(!/\brule A\b/i.test(t), `${rel} must not state rule A (plan Design § Gotchas)`);
+  });
+
+  test(`${rel}: keeps the 2026-10-08 rule (cannot fail on a wrong implementation is major; catalogue tier)`, () => {
+    const raw = read(rel);
+    assert.match(raw, /cannot fail on a wrong\s+implementation[\s\S]{0,200}\*\*major\*\*/);
+    assert.ok(norm(raw).includes("Whenever a finding is the same class as a catalogue rule, use that rule's tier"));
+  });
+
+  test(`${rel}: a security finding uses the catalogue's security rule_key`, () => {
+    const t = norm(read(rel));
+    const keys = RULE_CATALOGUE.filter(r => r.category === 'security').map(r => r.rule_key);
+    for (const k of keys) assert.ok(t.includes(`\`${k}\``), `${rel} must name \`${k}\``);
+    assert.ok(t.includes("A security finding uses the catalogue's security `rule_key`"));
+  });
+
+  test(`${rel}: round 2+ lists every previous finding by id and cites the exact line`, () => {
+    const t = norm(read(rel));
+    assert.ok(t.includes('From round 2 the prompt gives you the diff since the last reviewed text'), `${rel}: round-2 diff sentence missing`);
+    assert.ok(t.includes('List every previous finding by id in `previous`, with `resolved: true` only when the plan now fixes it.'));
+    assert.ok(t.includes('Cite the exact line, with `line_end` for a range.'));
+  });
+
+  test(`${rel}: the json template carries previous, line_end, lowered_from and caught_by beside the old fields`, () => {
+    const tpl = jsonTemplate(read(rel), rel);
+    assert.equal(typeof tpl.agent, 'string');
+    assert.ok(Array.isArray(tpl.findings) && tpl.findings.length >= 1, 'findings array with one example');
+    assert.ok(Array.isArray(tpl.previous) && tpl.previous.length >= 1, 'previous array with one example');
+    assert.ok('id' in tpl.previous[0] && 'resolved' in tpl.previous[0], 'previous entries carry id and resolved');
+    const f = tpl.findings[0];
+    for (const k of ['file', 'line', 'rule_key', 'severity', 'text', 'line_end', 'lowered_from', 'caught_by']) {
+      assert.ok(k in f, `finding template lacks "${k}"`);
+    }
+  });
+}
+
+test('plan-critic no longer tells the critic to judge only the plan in front of it', () => {
+  assert.ok(
+    !norm(read('agents/plan-critic.md')).includes('judge the plan in front of you'),
+    'the round-2 paragraph replaces "judge the plan in front of you"',
+  );
+});