docs(changelog): E in Unreleased - added, fixed, known issues and its door

CHANGELOG Unreleased gains E: the stop rule, "Known issue" after it, the
optional threat model, the critic's own security key SECURITY_RISK and the
security engine (Added); the Pass 2 heading, "6 of 3" and the dedup null
crash (Fixed); the known issues of decisions 220-226 and of E's own review
(the between-rounds holes, a fixed security MAJOR after the review stop
rule, the planted report folder, F5, the below-threshold check, the
unattended cost question, scan_prefix, no real /trekreview with Claude
Security, the plan side's rests, --brief next to --hash or --diff, the fix
round's test gaps, no Claude Security scan of E's own diff, copies and
repeated work); and one two-way door.

verdict-gate.mjs: the comment on the threat model's repo no longer says
"the answers' repo" (with a subdirectory as --repo the answers are read
from the top level and the threat model from the subdirectory; V2 of the
fix round's check). No code change.

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

View file

@ -45,6 +45,16 @@ Part B makes a WARN open the merge lock and the release check only when the oper
- **The start of the review range is bound to where the branch left main** (PXB). `lib/review/pre-existing.mjs` finds the fork point on the main branch's first-parent line (`main`, else `master`); a `scope_sha_start` other than that fork point or a later commit reached only through the approval log and `.claude/projects/`, a range that ends on main itself, or a repo with neither branch marks nothing pre-existing.
- **An engine finding missing from the review keeps the gate not measured** (FN1). The verdict gate runs the standard-path put-back over the engine payloads beside `review.md` and exits 2 for any MAJOR or BLOCKER whose row is missing from the review, also when `finalized: true` was written by hand; a finalize failure stops `/trekreview` in round 1 as in later rounds.
**E — stop rule, threat model, the critic's own security key, the security engine (decisions 164, 167, 175, 196, 217-226)**
E is the first build delivery of the approved design. The critic loop and the review rounds converged badly (four critic rounds without convergence in S202; `/security-review` gave 0 findings in 13 of 13 reviews), so a round now needs a reason to run, Voyage's own security findings can be placed by the brief, and security is judged by an engine that can be seen to look.
- **One stop rule for the plan loop and the review rounds** (decision 217). `lib/review/stop-rule.mjs` judges each round from round 2: two rounds in a row with new blocking findings of the same class (the catalogue rule key, else the critic's dimension), or blocking findings that do not get fewer, stop the loop. The operator gets the class and the counts per round with two choices, "Known issue" or "New mechanism", never "one more round". The plan gate stores each round in the verdict (`rounds`, `stop_rule`); after a fired stop rule `--record` refuses (`PLAN_GATE_STOP_RULE`, `--extra-round` included) and the bare gate stops with the operator line. `review-rounds.mjs prepare` (and `--dry-run`) refuses round 3 the same way (`review-stop-rule:`, exit 1, nothing archived); the cap of three stays.
- **"Known issue" after the stop rule** (decision 220). In the plan loop `--accept-open` is allowed with rounds left once the stop rule fired and covers open blockers and majors (`under: "stop-rule"`); the gate then warns. A counting security finding is never accepted (`PLAN_GATE_SECURITY_OPEN`). In the review a BLOCKER keeps the merge locked; "Known issue" answers the MAJORs through `/trekreview --answers`.
- **An optional threat model in the brief** (decisions 164, 218, 221). `## Threat Model` holds `- Inside: <text>` and `- Outside: <text>` bullets; present and malformed is `BRIEF_THREAT_MODEL_MALFORMED`, absent is the old rule, no `brief_version` bump. The intent hash covers it, so an edited threat model makes the approval stale. `/trekbrief` offers it when the task touches a guard, a gate, untrusted input, secrets or publishing. One reader, `lib/review/threat-model.mjs`, serves both gates: a Voyage security finding whose `threat_model_ref` equals an approved `Outside:` line exactly is a known issue (`known_outside`), not counted; an engine finding never is (nor one carrying the engines' key in `rule_key` or `rule_keys`). The plan gate takes the brief as `--record --brief <brief.md>` (`--brief` without `--record` is a usage error); the verdict gate reads it from `--repo`, else the review file's own work tree, never the working directory's. It applies to a round's new findings only: a finding counted earlier stays counted until it is marked resolved. Both gates re-read the brief every run: an entry the brief no longer backs counts in full (`PLAN_GATE_OUTSIDE_NOT_BACKED`; in the review, finalize's `outside` pass puts it back, and the verdict gate and the release check exit 2).
- **The critic's own security key** (decision 167). The rule catalogue gains `SECURITY_RISK` (BLOCKER, 18 keys); `SECURITY_REVIEW_VULNERABILITY` is the engines' key only. `plan-critic`, `scope-guardian` and the code reviewer's command-text part name `SECURITY_INJECTION` or `SECURITY_RISK`, need a concrete attack path and high confidence, and quote the `Outside:` line in `threat_model_ref` when one applies.
- **The security engine** (decisions 175, 196, 219, 222-224). `lib/review/security-engine.mjs choose` picks Claude Security when it is installed and the range has committed changes, else `/security-review` when `origin/HEAD...HEAD` is non-empty, else no engine (`no-engine`, `no-origin`, `empty-diff`): security "not measured", `missing-reviewer:security-engine`, verdict gate exit 2, never 0. `/trekreview` Phase 4.7 runs Claude Security's `scan changes` through its own skill over the review range; its cost question goes to the operator, and "No", an unanswerable question or a missing Workflow tool is recorded not run, never a silent fallback. `convert` reads Claude Security's report as data: only a stamp of a verified changes scan of exactly the reviewed head and base whose record count equals its `findings.total` and whose `scope` is empty counts; confidence `high` becomes a BLOCKER, the rest stand under Declined to Judge (`engine:below-threshold`, not counted, never `ALLOW`). From round 2 a counted engine finding that its engine raises again below `high` or outside the scope stays counted (decision 222). The probe (decision 175): Claude Security 0.12.0 found a planted JS command injection at confidence high and a planted shell injection in a command text at confidence medium.
### Changed
- **The "run the thing" step names Claude Code's built-in `run` skill.** The plan's run step (`/trekplan`), the executor's end-to-end part (`/trekexecute`) and the plan template each say in one sentence that the app is seen and driven with `run`, and that the verdict is still the `End-to-end:` command through the criteria runner, because `run` gives no exit code (decisions 54 and 64).
@ -217,6 +227,12 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
- **The texts next to the fixes say the same** (siblings found during the round): `/trekreview`, the review reference and the Arm B coordinator prompt no longer drop a finding whose rule key is unknown; `/trekbrief` writes its stats record on every exit of its manual and auto paths, says that `--gates` adds one pause before its auto chain, and that the chain ends at the plan; `/trekexecute` says `--gates` changes nothing there, checks the wave marker before a low-effort single session, and names no validator check that does not exist; plan mode goes through the plan approval before it prints the `/trekexecute` line; the commands read only the two-token `--gates <value>` form, and `--gates=true` is the gates shim's own form (`lib/util/autonomy-gate.mjs`), which it reads as `--gates true`.
**E**
- **The plan gate no longer leaves out the Pass 2 heading.** `REVIEW_LOG_SECTIONS` drops "Adversarial Pass 2 (v5.1.1 high-effort)", which no command writes, so a plan holding it hashes as any other plan; Handover 4's heading list follows.
- **`/trekplan`'s summary counts extra rounds.** Phase 10 prints the round as `round of max_rounds + extra_rounds` (it said "6 of 3").
- **A `null` in a reviewer's findings list no longer crashes the dedup** (S160). `plan-review-dedup.mjs` skips a `null` or `undefined` finding; any other entry that is not an object still counts as a blocker.
### Removed
**Cleanup R-a (decisions 188, 200-208).** What existed only for history or had no user, removed before the new design (187); git history keeps all of it.
@ -237,6 +253,20 @@ 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 (its id changes). Such a finding can also be listed in `known_outside` under its id, and two different findings at one line and key share that id, so one `previous` mark resolves both. It keeps counting in later rounds: the stop rule fires only when the blocking findings do not get fewer, and a counting security finding is never accepted as a known issue (`PLAN_GATE_SECURITY_OPEN`), so the way on is a reviewer marking it resolved or the operator's "New mechanism". The gate never counts less.
- **Review rounds still follow each finding's identity** (E, decision 226). Carry (`lib/review/review-rounds.mjs`) holds a counted engine finding only through a matching finding in `review.md` (engine MAJOR and BLOCKER findings are put back first) or a matching entry of that engine's `below_threshold` or `out_of_scope`. Two holes stay open, both of the same class as decisions 221 and 222: a counted `/code-review` finding that the engine raises again in its own findings below MAJOR (for example as a simplification) and that the coordinator leaves out of `review.md` is marked resolved, so a MAJOR can turn into ALLOW and a BLOCKER under a narrower key too (F-V1); and a counted engine finding whose file is renamed between rounds and that comes back below `high` or out of scope on the new path is marked resolved (F-V2). The other way, any below-threshold Claude Security entry on the same file holds a previous Claude Security finding on that file, so a fixed BLOCKER can stay counted (F-V3, fail-safe). Whether to keep this tracking at all goes to the review delivery (V), with "replace" as the starting point: each round sees the whole change again and the stop rule counts only blocking findings.
- **The plan loop has holes of the same class** (E, decisions 221, 226; found by E's own review). When a round is counted in full (no diff base, for example a missing snapshot), the threat model applies to every finding of the round, so a finding counted in round 1 and raised again with a `threat_model_ref` leaves the count. And `--record` carries the earlier `known_outside` entries without checking them against the brief again: an entry whose `Outside:` line was removed stays out of the counts and the stop rule; the bare gate stops on it (`PLAN_GATE_OUTSIDE_NOT_BACKED`), so the plan cannot pass, but the recorded verdict does not count it.
- **A fixed security MAJOR cannot be confirmed after the review stop rule** (E, decision 220). When the stop rule refuses round 3, a MAJOR answered `fixed` waits for a round that does not come, and a security finding is never accepted, so the merge stays locked until the operator takes the direction.
- **A planted Claude Security report folder is read** (E, decision 223). `convert` reads the newest matching `CLAUDE-SECURITY-<ts>/` folder by name; code the reviewed run executed can write one of its own that sorts last, or delete the real one. The stamp check is a trace, not a signature, as `SECURITY.md` says.
- **A below-threshold Claude Security finding misfiled as `known_outside` comes back as a BLOCKER** (E, F5). Finalize's `outside` pass puts an engine finding listed in `known_outside` back at its catalogue tier; stricter than decision 219 and only after a coordinator error.
- **The verdict gate does not itself check that a below-threshold finding kept the verdict from ALLOW** (E). Claude Security's `below_threshold` list is never compared with `review.md`, so when the coordinator leaves an entry out of `## Declined to Judge`, nothing deterministic keeps the verdict from `ALLOW`.
- **An unattended `/trekexecute` stops at Claude Security's cost question** (E, by design). Nobody can answer it, so the engine is recorded not run, security is not measured, the verdict gate exits 2 and the merge stays closed.
- **A non-empty `scan_prefix` may mean a partial scan** (E). `convert` accepts a stamp whose `scan_prefix` is a relative folder; whether Claude Security then scanned only part of the range is not measured.
- **No real `/trekreview` with Claude Security has run** (E). The engine is tested with fixture reports and one real 0.12.0 report from the probe; the model-run parts of Phase 4.7 (the skill, its question, the Workflow run) and the coordinator's handling are not measured. No real `/trekbrief` or `/trekplan` session has used a threat model either.
- **Smaller rests of the threat model's plan side** (E, part 2). A `known_outside` engine finding whose title the coordinator rewrote, or cut at `JUDGE_TITLE_MAX`, can be put back twice (the row matches on title); a brief outside the repo or behind a symlink is not tested; the plan-gate hook is not run end to end with `known_outside`; a covered finding that shares its id with a counted one drops out of the list in the next round (the count never gets smaller).
- **`--brief` next to `--hash` or `--diff` is still ignored** (E, the check of its review's fix round). The plan gate refuses `--brief` without `--record`, but `--hash`, `--diff` and `--accept-open` are handled before `--record` is read, so `--hash <plan> --record --brief <brief>` exits 0 without reading the brief. No shipped command passes such a line; whether the gate should refuse two mode flags together is left open.
- **Test gaps of the review's fix round** (E). No test covers the verdict gate's fallback to the review file's own work tree (when it is lost, nothing is backed: the safe side), the engines' key matched whatever its case and spaces, or `--hash`/`--diff` with `--brief`; one test's reason pattern matches every not-backed reason, so its exit code is what catches a revert.
- **E's own review has no Claude Security scan** (E). Claude Security's cost question for the scan of the branch went unanswered, so no scan was started and the security of E's own diff is not measured by an engine; `/code-review high` and the check of its fix round read it.
- **Copies and repeated work in E's code** (E, its own review). `review-rounds.mjs` and `review-answers.mjs` each keep a `payloadOf` (they differ in one check); `plan-gate.mjs` imports two functions named `resolveRepo` (one aliased) and the `--record --brief` path still resolves the top level more than once; a verdict-gate run on a review with `known_outside` verifies the approval log twice and parses `review.md` more than once.
- **`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).
@ -309,6 +339,7 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
- two-way: fix round 4.5b2 part C (decisions 135, 137) — a revert restores part B's behaviour.
- two-way: cleanup R-a (decisions 188, 200-208) — revert its commits; the removed files come back from git, and no format, flag value or record shape changed except the `/trekreview --workflow` flag and the autonomy gate's `--state`/`--event` arguments, which no shipped command used.
- two-way, one exception: cleanup R-b (decisions 175, 189-191, 209) — revert its commits; the removed files, hooks, phases and flags come back from git. One-way for briefs written after R-b: they carry no `phase_signals`, so after a revert the old validator refuses each one that declares `brief_version` 2.1 or later (`BRIEF_V51_MISSING_SIGNALS`) until the signals are added by hand.
- two-way: E — the stop rule, the threat model, `SECURITY_RISK` and the security engine (decisions 164, 167, 175, 196, 217-226) — revert its commits. A brief with `## Threat Model` then validates with the section ignored and its intent hash changes back, so its approval reads stale once and is given again; a plan verdict's `rounds`, `stop_rule`, `known_outside` and `brief_path` and a review's `known_outside` are fields the old readers skip, so an entry under `known_outside` is no longer re-checked against the brief; `.review-engine-payload-claude-security.json` is no longer read.
## v6.5.0 — 2026-10-18 — Three skills for brief, criteria and review, English labels, and three trial-5 fixes

View file

@ -373,8 +373,8 @@ export function decideOpenFindings(text, { repo, reviewPath } = {}) {
fixed_pending: [],
};
}
// The threat model is read from the answers' repo: --repo, else the review
// file's own work tree, never the working directory's (none: nothing is backed).
// The threat model is read from --repo, else the review file's own work
// tree, never the working directory's (none: nothing is backed).
const outside = notBackedOutside(text, repo ?? (reviewPath ? answerLogRepo(dirname(reviewPath)) : null));
if (outside.entries.length > 0) {
const what = (e) => (e !== null && typeof e === 'object'