fix(gate): the threat model applies to new findings only (decision 221)

Undone: item 2 of the Part 2 fix round (1be961a). recordVerdict no longer
drops a copy rule A carried as counted when a finding with the same id is
backed this round, and no longer recomputes the counts, `lowered` and the
stop-rule summary for it; the isLowered helper and the SEVERITIES import it
needed are gone. Why: the id is (file, line, rule_key), so a DIFFERENT
backed security finding at the same line and key knocked a counted security
finding inside the threat model out of the count (round 2 PASS, its text
gone), against decisions 218 and 220.

The rule now, as decided: the threat model applies only to this round's
new findings (the deduped findings, before countRound); a finding counted
in an earlier round stays counted, through rule A's carry, until a reviewer
marks it resolved. The gate never counts less. Items 1 and 3-8 of the fix
round are untouched.

Tests: the fix-round case for item 2 now asserts the decided rule (the
carried copy stays counted, BLOCK, the stop rule fires not-fewer, the gate
stops); a new case pins that a different backed finding at the same line
and key leaves the counted one, with its own text, in the count.

The plan-gate header and the known_outside comment, and Handover 4's
known_outside sentence, state the rule. CHANGELOG Unreleased, Known issues:
a finding counted earlier stays counted when an Outside: line approved
later puts it outside, or when it moves to another plan line; the way on is
the operator's choice at the stop rule.

Part 2 of the E plan, decision 221.

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

CHANGELOG.md

@@ -236,6 +236,7 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
 
 ### Known issues
 
+- **A finding counted in an earlier critic round stays counted when the threat model later puts it outside** (E, decision 221). The brief's threat model applies to a round's new findings only: a finding already counted stays counted, through rule A's carry, when an `Outside:` line approved after it was counted puts it outside, or when it moves to another plan line; the plan loop then stops, and the way on is the operator's choice at the stop rule. The gate never counts less.
 - **`tests/fixtures/bakeoff-rich/` has no reader** (cleanup R-b). It stays for V3 (decision 202); with `gold-eval` gone nothing reads `gold.json`, and its README still describes the removed bakeoff harness as current.
 - **The query guard has no caller** (cleanup R-b). `lib/validators/query-privacy-gate.mjs` and its tests stay, but its callers were the removed STORM steps, so no research query passes through it; its header and `skills/researching-sources/` say so. Left for the design (187).
 - **Eleven agents take the session's effort** (cleanup R-b). Nine agents pin `effort: high`; the other eleven inherit the session level, so a session started below high runs them below high. Left for the design (187).

docs/HANDOVER-CONTRACTS.md

@@ -253,7 +253,7 @@ prose form (relaxed in S26 so the template's prose emission parses).
    - at least one step has `End-to-end:` with a backticked command — the "run the thing" step;
    - the optional step fields `Interface in:`, `Interface out:`, `Review focus:` are never empty when present (consumers tolerate their absence);
    - `ceremony` is one of the three classes; an `architectural` plan has `## Design` with the subsections Architecture, Data Flow, Interfaces, Data Models, Error Handling, Module Map, Architecture Rules, Requirements Check, plus `## Test Strategy`; the module map (mermaid) has no cycle; every architecture rule says "checked by `<command>`" (with a Proof column naming an existing step) or "advisory — `.claude/rules/<file>.md`".
-6. **`## Critic Verdict`** (written by `lib/validators/plan-gate.mjs --record`, never by hand): one fenced `json` block, schema `voyage.plan-verdict/1` — `round`, `max_rounds` (3), `extra_rounds`, `plan_sha256`, `verdict` (`BLOCK` | `REVISE` | `PASS`), `counts`, `findings` (deduplicated, each with `severity`). `plan_sha256` hashes the plan minus its review log (the `> **Plan quality:` line and the sections Critic Verdict, Revisions, Plan Quality Score; an old Adversarial Pass 2 section is plan text and hashed), with CRLF and line-end whitespace normalized. The verdict sits inside the plan because Claude Code's plan approval hands a hook the plan file's text and nothing beside it. **Optional rule-A fields** (decision 43; `DIFF_FIELDS` in `lib/review/critic-diff.mjs`): each one is optional, and absent means round-1 behaviour, so an older block gives the same exit codes. Top level: `diff_base` (the sha256 the round was counted against), `diff_note` (one line: why a round from round 2 on was counted fully), `resolved` (the ids of previous findings that left the count). Per finding: `op_line` and `op_line_end` (the cited lines in the operative text), `line_end`, `rule_keys` (the union of the keys dedup merged), `original_severity` (what a reviewer rated, set whenever the counted `severity` differs), `lowered_by` (`rule A: outside the diff since round <P>` or `rule B: <caught_by>`), `caught_by` (the test, `Regression:` command or gate a rule-B lowering names), `lowering_refused` (why a rule-B lowering was refused) and `carried_from` (the round a carried finding first appeared). `severity` is always the counted severity. `recordVerdict` called without `base`, `baseNote` and `planPaths` records as before, plus the rule-B record, and a per-finding `rule_keys` from dedup is always passed through (an optional field). **Optional stop-rule fields** (decisions 217, 220; `lib/review/stop-rule.mjs`): `rounds` (one `{round, blocking, newByClass}` per recorded round, carried from the previous verdict; `newByClass` is `null` for a round counted in full) and `stop_rule` (`{fired, reason: same-class | not-fewer | null, classes, perRound}`); a verdict without them starts its history at the next round. `accepted_open.under` is `"stop-rule"` for an acceptance made after the stop rule fired. **Optional threat-model fields** (E 2.4, decisions 164, 218; `lib/review/threat-model.mjs`): `known_outside` (one `{id, rule_key, rule_keys, text, line}` per security finding of plan-critic or scope-guardian whose `threat_model_ref` equals the text of an `Outside:` line of the brief's `## Threat Model`, `line` being that text; it left the counted findings and the stop rule, and stays listed in later rounds until a reviewer marks it resolved or it is reported again under its id) and `brief_path` (the brief `--record --brief` read, stored relative to the repo's top level and read from there whatever the working directory, carried when a later round has none); a verdict without them lists no known issue.
+6. **`## Critic Verdict`** (written by `lib/validators/plan-gate.mjs --record`, never by hand): one fenced `json` block, schema `voyage.plan-verdict/1` — `round`, `max_rounds` (3), `extra_rounds`, `plan_sha256`, `verdict` (`BLOCK` | `REVISE` | `PASS`), `counts`, `findings` (deduplicated, each with `severity`). `plan_sha256` hashes the plan minus its review log (the `> **Plan quality:` line and the sections Critic Verdict, Revisions, Plan Quality Score; an old Adversarial Pass 2 section is plan text and hashed), with CRLF and line-end whitespace normalized. The verdict sits inside the plan because Claude Code's plan approval hands a hook the plan file's text and nothing beside it. **Optional rule-A fields** (decision 43; `DIFF_FIELDS` in `lib/review/critic-diff.mjs`): each one is optional, and absent means round-1 behaviour, so an older block gives the same exit codes. Top level: `diff_base` (the sha256 the round was counted against), `diff_note` (one line: why a round from round 2 on was counted fully), `resolved` (the ids of previous findings that left the count). Per finding: `op_line` and `op_line_end` (the cited lines in the operative text), `line_end`, `rule_keys` (the union of the keys dedup merged), `original_severity` (what a reviewer rated, set whenever the counted `severity` differs), `lowered_by` (`rule A: outside the diff since round <P>` or `rule B: <caught_by>`), `caught_by` (the test, `Regression:` command or gate a rule-B lowering names), `lowering_refused` (why a rule-B lowering was refused) and `carried_from` (the round a carried finding first appeared). `severity` is always the counted severity. `recordVerdict` called without `base`, `baseNote` and `planPaths` records as before, plus the rule-B record, and a per-finding `rule_keys` from dedup is always passed through (an optional field). **Optional stop-rule fields** (decisions 217, 220; `lib/review/stop-rule.mjs`): `rounds` (one `{round, blocking, newByClass}` per recorded round, carried from the previous verdict; `newByClass` is `null` for a round counted in full) and `stop_rule` (`{fired, reason: same-class | not-fewer | null, classes, perRound}`); a verdict without them starts its history at the next round. `accepted_open.under` is `"stop-rule"` for an acceptance made after the stop rule fired. **Optional threat-model fields** (E 2.4, decisions 164, 218; `lib/review/threat-model.mjs`): `known_outside` (one `{id, rule_key, rule_keys, text, line}` per security finding of plan-critic or scope-guardian whose `threat_model_ref` equals the text of an `Outside:` line of the brief's `## Threat Model`, `line` being that text; it left the counted findings and the stop rule, and stays listed in later rounds until a reviewer marks it resolved or it is reported again under its id; the threat model applies to the round's new findings only, so a finding counted in an earlier round stays counted until a reviewer marks it resolved, decision 221) and `brief_path` (the brief `--record --brief` read, stored relative to the repo's top level and read from there whatever the working directory, carried when a later round has none); a verdict without them lists no known issue.
 7. **Every plan_version, no version bump** (a plan without the field validates unchanged; voyage-lekerepo trial 2, 2026-10-08):
    - **K10 — `Regression:`** (optional step field): ONE backticked command the criteria runner's allowlist runs, with no pipe and no `&&` (first backticked span, as for `Verify:`). The executor runs it green before the step's checkpoint; a step without it runs nothing new. `Verify:` stays the step's own red-first test. One that can never run is `PLAN_STEP_REGRESSION_NOT_RUNNABLE` (an error; a warning under `--soft`).
    - **K11 — `On failure:`** opens with one of `revert`, `retry`, `skip`, `escalate` (case-insensitive; a leading `**`, `_`, backtick or `-` is ignored, other leading characters are not; `onFailureAction` in `lib/parsers/plan-schema.mjs`), the words `/trekexecute` reads; any other opening is the warning `PLAN_STEP_ON_FAILURE_UNKNOWN`. A missing `On failure:` is unchanged (the executor defaults to `escalate`).

lib/validators/plan-gate.mjs

@@ -123,9 +123,11 @@
 // `threat_model_ref` equals an `Outside:` line's text exactly — leaves the
 // counted set and the stop rule and goes to the optional verdict field
 // `known_outside` ({id, rule_key, rule_keys, text, line}), carried to later
-// rounds until a reviewer marks it resolved (or it counts again under its id);
-// a copy of it that rule A carried from an earlier round as counted leaves the
-// count too. The verdict stores `brief_path`, relative to the repo's top level
+// rounds until a reviewer marks it resolved (or it counts again under its id).
+// The threat model applies to this round's new findings only (decision 221): a
+// finding counted in an earlier round stays counted, through rule A's carry,
+// until a reviewer marks it resolved; the gate never counts less. The verdict
+// stores `brief_path`, relative to the repo's top level
 // (carried when a later round has no --brief), and the gate reads it from
 // there whatever its working directory. A threat
 // model that is not usable backs nothing: every security finding counts, and
@@ -143,7 +145,7 @@ import { createHash } from 'node:crypto';
 import { dirname, join, resolve } from 'node:path';
 import { validatePlanContent } from './plan-validator.mjs';
 import { splitSections, fenceTracker, extractPlanVersion, versionAtLeast } from '../parsers/plan-schema.mjs';
-import { normalizeSeverity, isSecurity, SEVERITIES } from '../review/plan-review-dedup.mjs';
+import { normalizeSeverity, isSecurity } from '../review/plan-review-dedup.mjs';
 import { countRound, lineDiff, oneLine } from '../review/critic-diff.mjs';
 import { roundSummary, decideStop, describeStop, STOP_REASONS } from '../review/stop-rule.mjs';
 import { usableInRepo, repoRelativeBrief, backedLine } from '../review/threat-model.mjs';
@@ -292,13 +294,6 @@ function okKnownOutside(k) {
     typeof e.text === 'string' && typeof e.line === 'string' && e.line !== '');
 }
 
-/** A counted finding a reviewer rated higher than it counts (rule A or B). */
-const isLowered = x => {
-  const o = normalizeSeverity(x.original_severity);
-  const s = normalizeSeverity(x.severity);
-  return o !== null && s !== null && SEVERITIES.indexOf(o) < SEVERITIES.indexOf(s);
-};
-
 /** Optional `brief_path`: a non-empty string. */
 const okBriefPath = p => p === undefined || (typeof p === 'string' && p !== '');
 
@@ -437,12 +432,7 @@ export function recordVerdict(text, dedup, opts = {}) {
     findings: counted,
     marks: Array.isArray(dedup.marks) ? dedup.marks : [],
   });
-  // A previous finding countRound carried (rule A) whose id is backed this
-  // round leaves the counted set too: listed once, in known_outside, and out
-  // of the counts, the verdict and the stop rule's summary.
-  const leaving = c.findings.filter(x => x.carried_from !== undefined && x.carried_from !== null && backedIds.has(x.id));
-  const findings = c.findings.filter(x => !leaving.includes(x));
-  const lowered = c.lowered - leaving.filter(isLowered).length;
+  const findings = c.findings;
   const counts = { blocker: 0, major: 0, minor: 0 };
   for (const f of findings) counts[f.severity] += 1;
   const verdict = {
@@ -461,7 +451,9 @@ export function recordVerdict(text, dedup, opts = {}) {
   if (c.resolved !== null) verdict.resolved = c.resolved;
   // known_outside: this round's backed findings, after the previous round's
   // entries that no reviewer marked resolved and that are not reported again
-  // under their id (backed again: listed once; counted: in full, not listed).
+  // under their id. The threat model applies to this round's new findings only
+  // (decision 221): a finding counted in an earlier round stays counted, through
+  // rule A's carry, until a reviewer marks it resolved.
   const marks = Array.isArray(dedup.marks) ? dedup.marks : [];
   const countedIds = new Set(findings.map(x => x.id).filter(id => typeof id === 'string'));
   const kept = (Array.isArray(prev.verdict?.known_outside) ? prev.verdict.known_outside : [])
@@ -480,7 +472,7 @@ export function recordVerdict(text, dedup, opts = {}) {
     ? { applied: false, why: 'no --brief given' }
     : { applied: use.usable === true, why: use.usable === true ? null : (use.why ?? 'the threat model is not usable') };
   return {
-    code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered, threat_model: threat,
+    code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered: c.lowered, threat_model: threat,
     message: `round ${round} recorded: ${verdict.verdict}` +
       (knownOutside.length > 0 ? `; ${knownOutside.length} finding(s) outside the threat model listed as known issues` : '') +
       (use !== undefined && !threat.applied ? `; threat model not applied, every security finding counts: ${threat.why}` : '') +

tests/validators/plan-gate-threat-model.test.mjs

@@ -381,23 +381,46 @@ test('fix 1: a string entry in a reviewer\'s findings still counts as a blocker
   assert.equal(r.verdict.verdict, 'BLOCK');
 });
 
-test('fix 2: a finding counted in round 1 and backed in round 2 (same id, marked resolved: false) is listed once and counts nowhere', () => {
+// Decision 221: the threat model applies to this round's new findings only. A
+// finding counted in an earlier round stays counted (rule A's carry) until a
+// reviewer marks it resolved; the gate never counts less.
+function countedThenBacked(text2) {
   const t1 = buildPlan();
   const r1 = rec(t1, dd({ pc: [f('blocker', at(t1, R1_CHANGES), S1, 'SECURITY_RISK')] }));
   const id = r1.verdict.findings[0].id;
   const t2 = editA(r1.text);
   const r2 = rec(t2, dd({
-    pc: [f('blocker', at(t2, R1_CHANGES), S1, 'SECURITY_RISK', { threat_model_ref: OUTSIDE })],
+    pc: [f('blocker', at(t2, R1_CHANGES), text2, 'SECURITY_RISK', { threat_model_ref: OUTSIDE })],
     pcPrev: [{ id, resolved: false }], sgPrev: [{ id, resolved: false }],
   }), { base: baseOf(r1.text), threatModel: TM, briefPath: 'brief.md' });
+  return { id, r2 };
+}
+
+test('fix 2 (decision 221): a finding counted in round 1 and backed in round 2 stays counted until marked resolved', () => {
+  const { id, r2 } = countedThenBacked(S1);
   const v = r2.verdict;
-  assert.deepEqual(v.findings.map(x => x.id), [], 'the carried copy left the counted set');
-  assert.deepEqual(v.counts, { blocker: 0, major: 0, minor: 0 });
-  assert.equal(v.verdict, 'PASS');
-  assert.deepEqual((v.known_outside ?? []).map(e => e.id), [id]);
-  assert.equal(v.rounds.at(-1).blocking, 0, 'it left the stop-rule summary');
-  assert.equal(v.stop_rule.fired, false, JSON.stringify(v.stop_rule));
-  assert.equal(gatePlan(r2.text, { threatModel: TM }).code, 0);
+  assert.deepEqual(v.findings.map(x => [x.id, x.carried_from]), [[id, 1]], 'the carried copy stays counted');
+  assert.deepEqual(v.counts, { blocker: 1, major: 0, minor: 0 });
+  assert.equal(v.verdict, 'BLOCK');
+  assert.deepEqual((v.known_outside ?? []).map(e => e.id), [id], 'the backed re-report may stay listed');
+  assert.equal(v.rounds.at(-1).blocking, 1);
+  assert.equal(v.stop_rule.fired, true, 'the stop rule fires: the way on is the operator\'s choice');
+  assert.equal(v.stop_rule.reason, 'not-fewer');
+  assert.equal(gatePlan(r2.text, { threatModel: TM }).code, 1);
+});
+
+test('decision 221: a DIFFERENT backed finding at the same line and key never knocks out a counted one', () => {
+  const B = 'Step 1 writes the downloaded token into a world-readable cache file';
+  const { id, r2 } = countedThenBacked(B);
+  const v = r2.verdict;
+  assert.equal(v.verdict, 'BLOCK');
+  assert.deepEqual(v.counts, { blocker: 1, major: 0, minor: 0 });
+  const a = v.findings.find(x => x.id === id);
+  assert.ok(a, `A is still counted: ${JSON.stringify(v.findings)}`);
+  assert.equal(a.carried_from, 1);
+  assert.equal(a.text, S1, 'A keeps its own text');
+  const g = gatePlan(r2.text, { threatModel: TM });
+  assert.equal(g.code, 1, JSON.stringify(g.reasons));
 });
 
 test('fix 3: a merged finding whose security key is only in rule_keys is backed at record and in the bare gate alike', () => {