fix(review): the remnants after decision 226 - out_of_scope hold tested through finalize, the high-confidence exception and the carry prose said as the code does

- tests/lib/review-rounds.test.mjs: round 2 whose Claude Security payload
  holds the round-1 BLOCKER only in out_of_scope keeps it counted through
  the real finalize and verdict gate (BLOCK, gate exit 1, not under
  Resolved since round 1); a clean run resolves it (F-V4; red under the
  mutation that drops out_of_scope from raisedOutside).
- README.md, SECURITY.md: "blocks only at confidence high" names the
  exception of decision 222 (counted at high in an earlier round, it stays
  counted when it comes back below high or outside the scope) (F-V5).
- commands/trekreview.md Phase 5: an engine finding is held by a matching
  finding in review.md (engine MAJOR/BLOCKER put back first) or a matching
  below_threshold / out_of_scope entry; an engine MINOR or SUGGESTION the
  coordinator left out holds nothing, a known issue (decision 226).
- tests/lib/review-prose-pins.test.mjs: both sentences pinned; red on the
  old prose.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-11-08 20:41:12 +01:00
commit b94b060035
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
5 changed files with 88 additions and 9 deletions

View file

@ -500,8 +500,11 @@ a dependency in Voyage's manifest. Without it, `/trekreview` uses Claude Code's
origin's default branch). With neither engine able to look (no Claude Security
and no `origin` remote, or a branch that is already on origin's default
branch), security is "not measured", never 0, and the verdict gate exits 2. A
Claude Security finding blocks only at confidence `high`; the rest are listed
under `## Declined to Judge` and keep the review from `ALLOW`.
Claude Security finding blocks only at confidence `high`, or when it was
counted at `high` in an earlier round of the same run: it then stays counted
when it comes back below `high` or outside the review's scope, until it is
fixed. The rest are listed under `## Declined to Judge` and keep the review
from `ALLOW`.
**One stance for every reviewer.** Both reviewers and the coordinator preload
the `voyage:reviewing-adversarially` skill, and `/trekreview` names it when it

View file

@ -90,9 +90,11 @@ base. That check is a trace, not a signature (a known issue): code the
reviewed run executed can write a report folder of its own, which `convert`
reads when its name sorts last among the matching ones, or delete the real
report before `convert` reads it, and Voyage does not detect either. A
Claude Security finding blocks only at confidence `high`; the rest are listed
under Declined to Judge and keep the review from `ALLOW`. Neither engine
edits code.
Claude Security finding blocks only at confidence `high`, or when it was
counted at `high` in an earlier round of the same run: it then stays counted
when it comes back below `high` or outside the review's scope, until it is
fixed. The rest are listed under Declined to Judge and keep the review from
`ALLOW`. Neither engine edits code.
## Out of scope

View file

@ -516,10 +516,14 @@ the round input:
Finalize (Phase 7) carries a previous finding until every reviewer that ran in
its round marks it resolved, and an engine finding until its engine ran this
round and did not raise it again — in its payload's `findings`, or outside
them in `below_threshold` or `out_of_scope` (decision 222: a Claude Security
finding counted at confidence `high` that comes back below it stays
counted). The engines (Phase 4.7) stay on the run range
round and nothing of this round holds it. Two things hold it: a matching
finding in `review.md` (an engine MAJOR or BLOCKER the coordinator left out
is put back into `review.md` first, at its own severity), or a matching entry
of that engine's `below_threshold` or `out_of_scope` (decision 222: a Claude
Security finding counted at confidence `high` that comes back below it stays
counted). An engine MINOR or SUGGESTION the coordinator left out of
`review.md` holds nothing, so the previous finding resolves (a known issue,
decision 226). The engines (Phase 4.7) stay on the run range
(`/security-review` takes no range; Claude Security scans from `{before_sha}`).
`brief-conformance-reviewer` additionally receives `sc_evidence_block` from

View file

@ -482,6 +482,30 @@ test('[E32] README and SECURITY.md — Claude Security recommended, proprietary
assert.ok(!/claude-security/.test(manifest), 'the manifest names no Claude Security dependency');
});
test('[E3] README and SECURITY.md — "blocks only at confidence high" names the exception of decision 222: counted at high in an earlier round, it stays counted when it comes back below', () => {
for (const rel of ['README.md', 'SECURITY.md']) {
const t = flat(read(rel));
const at = t.indexOf('A Claude Security finding blocks only at confidence `high`');
assert.ok(at >= 0, `${rel}: the confidence sentence is there`);
const passage = t.slice(at, t.indexOf('keep the review from `ALLOW`', at));
assert.match(passage, /counted at (confidence )?`high` in an earlier round/, `${rel}: the earlier-round exception is named`);
assert.match(passage, /stays counted/, `${rel}: it stays counted`);
}
});
test('[E3] trekreview — Phase 5 says what holds an engine finding between rounds, as the code does (decision 226)', () => {
const p = flat(section(CMD, '## Phase 5'));
const at = p.indexOf('Finalize (Phase 7) carries a previous finding');
assert.ok(at >= 0, 'the carry sentence is there');
const s = p.slice(at, p.indexOf('The engines (Phase 4.7) stay on the run range', at));
assert.ok(!s.includes("in its payload's `findings`"), 'an entry in the payload\'s findings alone never holds it');
assert.match(s, /a matching finding in `review\.md`/, 'a matching finding in review.md holds it');
assert.match(s, /`below_threshold` or `out_of_scope`/, 'an entry in below_threshold or out_of_scope holds it');
assert.match(s, /put back/, 'the put-back of an engine MAJOR or BLOCKER is named');
assert.match(s, /engine MINOR or SUGGESTION the coordinator left out of `review\.md` holds nothing/, 'what holds nothing is said');
assert.match(s, /decision 226/, 'the known issue is named');
});
test('[E32] Handover 6 — names the engine claude-security and its payload file with below_threshold', () => {
const h6 = section(read('docs/HANDOVER-CONTRACTS.md'), '## Handover 6');
assert.ok(h6.includes('.review-engine-payload-claude-security.json'));

View file

@ -1199,3 +1199,49 @@ test('[E3] decision 222: round 2 whose Claude Security payload holds the round-1
assert.equal(o.status, 0, o.stdout + o.stderr);
assert.deepEqual(jsonOf(fx.read()).resolved, [id], 'Control: the engine is the payload\'s, never the entry\'s');
});
test('[E3] decision 222: round 2 whose Claude Security payload holds the round-1 BLOCKER only in out_of_scope keeps it counted through finalize: BLOCK, never under Resolved since round 1', () => {
const fx = rndRepo();
const { prev } = roundOne(fx, [CS_PREV], { engines: ['code-review', 'claude-security'] });
const [id] = prev.findings.map((x) => x.id);
/** Round 2's Claude Security report of start..c2 at confidence high, converted with `scope` as the review's scope. */
const claudeSecurity = (records, scope) => {
const dir = mkdtempSync(join(tmpdir(), 'review-rounds-cs-oos-'));
made.push(dir);
writeReport(dir, { head: fx.c2, base: fx.start, records });
const scopeFile = join(dir, 'scope.txt');
writeFileSync(scopeFile, scope);
const r = run([SECURITY, 'convert', '--report-dir', dir, '--base', fx.start, '--head', fx.c2, '--scope', scopeFile]);
assert.equal(r.status, 0, `security-engine convert: ${r.stdout}${r.stderr}`);
writeFileSync(join(fx.project, '.review-engine-payload-claude-security.json'), r.stdout);
return JSON.parse(r.stdout);
};
const resolvedPart = (text) => text.slice(text.indexOf('## Resolved since round 1'), text.indexOf('## Declined to Judge'));
const raise = [record({ file: 'b.mjs', line: 3, title: CS_PREV.title })];
// b.mjs left the review's scope: the coordinator declined the raise as engine:out-of-scope, no finding, WARN.
roundTwo(fx, [], { outs: { [CCR]: [], [BCR]: [] }, engines: ['code-review'], verdict: 'WARN' });
const pl = claudeSecurity(raise, 'a.mjs\n');
assert.deepEqual([pl.findings.length, pl.below_threshold.length, pl.out_of_scope.length], [0, 0, 1],
'Control: Claude Security raised it again, at high, outside the review scope only');
const f = fx.finalize2();
assert.equal(f.status, 0, f.stdout + f.stderr);
const text = fx.read();
const payload = jsonOf(text);
assert.deepEqual(payload.ran.engines, ['claude-security', 'code-review'], 'Claude Security ran this round');
assert.deepEqual(payload.resolved, [], 'the round-1 BLOCKER is not resolved');
assert.deepEqual(payload.findings.map((x) => [x.severity, x.carried_from]), [['BLOCKER', id]], 'it stays counted, carried');
assert.equal(payload.verdict, 'BLOCK');
assert.match(fmOf(text), /^verdict: BLOCK$/m);
assert.match(text, /^## Resolved since round 1$/m, text);
assert.ok(!resolvedPart(text).includes(CS_PREV.title) && !resolvedPart(text).includes(id), `not under Resolved since round 1:\n${resolvedPart(text)}`);
const gate = fx.gate();
assert.equal(gate.status, 1, gate.stdout + gate.stderr);
assert.equal(parse(gate).reason, 'verdict BLOCK', gate.stdout);
// Control: Claude Security ran and did not raise it at all: resolved.
roundTwo(fx, [], { outs: { [CCR]: [], [BCR]: [] }, engines: ['code-review'], verdict: 'WARN' });
claudeSecurity([], 'a.mjs\n');
const c = fx.finalize2();
assert.equal(c.status, 0, c.stdout + c.stderr);
assert.deepEqual(jsonOf(fx.read()).resolved, [id], 'Control: a clean Claude Security run resolves it');
assert.ok(resolvedPart(fx.read()).includes(CS_PREV.title), 'Control: listed under Resolved since round 1');
});