fix(gate): part 2 verifier findings — non-object findings, carried backed copy, rule_keys, marks, approval prose, brief_path, engine put-back once

1. plan-review-dedup skips only a null or undefined finding; any other
   entry that is not an object (a string, a number, an array) goes on as
   before E, a finding with no valid severity that the plan gate counts as
   a blocker (it was dropped, so a string blocker recorded PASS).
2. plan-gate --record: a previous finding that rule A carried as counted and
   whose id is backed this round leaves the counted set, the counts, the
   verdict and the stop rule's summary (it was counted and listed in
   known_outside at once: BLOCK, not-fewer, and --accept-open refused).
3. A known_outside entry keeps rule_keys; extractVerdict accepts it (a list
   of strings), and the bare gate re-checks with the same keys the recorder
   used, so a merged finding whose security key is only in rule_keys is a
   WARN, not a STOP.
4. plan-critic, scope-guardian and /trekplan Phase 9's round-2 sentences say
   the Critic Verdict also lists known_outside and each is marked by id in
   previous. Code already accepts such a mark (dedup passes it; countRound
   ignores ids it does not carry; --record removes the entry on true);
   pinned in diff mode and full count.
5. The approval prose names ## Threat Model where it named only ## Intent +
   ## Goal: skills/annotating/SKILL.md (description, step 4, guard list),
   docs/command-modes.md, Handover 1's intent_approved_hash row, README
   (approval, Approve row, intent approval gate) and CLAUDE.md's /trekbrief
   row.
6. threat-model.mjs gains usableInRepo (a brief path relative to the repo
   top level) and repoRelativeBrief; --record stores brief_path relative to
   the top level, never absolute, and both gates read it from there, so the
   gate run from a subdirectory reads the same brief. reviewThreatModel
   uses usableInRepo. Handover 4 and the gate header say so.
7. review-finalize's outside pass puts an entry back only when no finding
   carries its row yet (engine-compare rowOf): an engine finding listed in
   known_outside while its payload is saved went back twice.
8. A test runs scripts/release-check.mjs on a review whose known_outside
   entry the brief does not back: exit 2 (control: backed passes the review
   check).

Existing tests changed: plan-gate-threat-model's entry-keys assertion now
expects rule_keys (item 3), and its planted non-security entry drops the
security key from rule_keys too, since the gate reads rule_keys (item 3).

Part 2 fix round of the E plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-11-08 10:50:20 +01:00
commit 1be961aab1
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
17 changed files with 298 additions and 52 deletions

View file

@ -30,7 +30,7 @@ Voyage — a contract-driven Claude Code pipeline: brief, research, plan, execut
| Command | Description | Orchestrator model | | Command | Description | Orchestrator model |
|---------|-------------|--------------------| |---------|-------------|--------------------|
| `/trekbrief` | Brief — one question round sized by class (`ceremony: small\|bounded\|architectural`, heaviest in doubt, said aloud, only moved up), run in the form of `voyage:interviewing` (asked for by name; criteria by `voyage:writing-criteria`, which `brief-reviewer` is also told to judge by, with `voyage:reviewing-adversarially`), produces a task brief with explicit research plan; ends with the operator explicitly approving `## Intent` + `## Goal` ("sounds good" is not approval), logged in the target repo's hash-chained approval log and committed (the brief too when `artifact_store: git`); the operator's notes from the annotation page go through `voyage:annotating` (`--approve <dir>` does only the approval); optionally orchestrates the pipeline | session | | `/trekbrief` | Brief — one question round sized by class (`ceremony: small\|bounded\|architectural`, heaviest in doubt, said aloud, only moved up), run in the form of `voyage:interviewing` (asked for by name; criteria by `voyage:writing-criteria`, which `brief-reviewer` is also told to judge by, with `voyage:reviewing-adversarially`), produces a task brief with explicit research plan; ends with the operator explicitly approving `## Intent` + `## Goal` (+ `## Threat Model` when present; "sounds good" is not approval), logged in the target repo's hash-chained approval log and committed (the brief too when `artifact_store: git`); the operator's notes from the annotation page go through `voyage:annotating` (`--approve <dir>` does only the approval); optionally orchestrates the pipeline | session |
| `/trekresearch` | Research — deep local + external research, produces structured research brief. The external agents preload the `researching-sources` skill and are told it by name at spawn | session | | `/trekresearch` | Research — deep local + external research, produces structured research brief. The external agents preload the `researching-sources` skill and are told it by name at spawn | session |
| `/trekplan` | Plan — brief-reviewer, explore, plan, review. Requires `--brief` or `--project`; halts unless the brief's intent is approved (hash-bound, stale on edit). Auto-discovers `architecture/overview.md` if present. Plan 1.9: the file first on every line — every `Files:` entry and `Test first:` `File:` line opens with its backticked path(s). From 1.8: `Test first:` per step + one `End-to-end:` step; every check written by `voyage:writing-criteria` (named where the plan is written and where its three reviewers start); a design part for `ceremony: architectural` (absent = architectural); critic loop of max 3 rounds, recorded in the plan, rounds 2+ in diff mode (rule A, counted by the gate; rule B in the reviewers), and `plan-gate.mjs` (0 go / 1 stop / 2 could not run) decides; open minors after it are listed for the operator to pick | session | | `/trekplan` | Plan — brief-reviewer, explore, plan, review. Requires `--brief` or `--project`; halts unless the brief's intent is approved (hash-bound, stale on edit). Auto-discovers `architecture/overview.md` if present. Plan 1.9: the file first on every line — every `Files:` entry and `Test first:` `File:` line opens with its backticked path(s). From 1.8: `Test first:` per step + one `End-to-end:` step; every check written by `voyage:writing-criteria` (named where the plan is written and where its three reviewers start); a design part for `ceremony: architectural` (absent = architectural); critic loop of max 3 rounds, recorded in the plan, rounds 2+ in diff mode (rule A, counted by the gate; rule B in the reviewers), and `plan-gate.mjs` (0 go / 1 stop / 2 could not run) decides; open minors after it are listed for the operator to pick | session |
| `/trekexecute` | Execute — disciplined plan/session-spec executor: refuses a plan whose logged approval does not cover its current sha256 (K2); one fresh `step-implementer` helper per plan step, one controller that reads the diff (spec + quality verdicts), proof of red / floor guard / step check (K9) / `Regression:` (K10) / done line as commands, fix rounds that go up (max 5: helper → plan → operator), architecture rules (K5) + plan-file sweep (K6) after the last step, the app seen with Claude Code's built-in `run` while the `End-to-end:` command stays the verdict (`run` gives no exit code), then its own review (`/simplify` → `/trekreview` → verdict gate, K7; BLOCK keeps the merge into main closed), closed stop list + decision log in the final report | session | | `/trekexecute` | Execute — disciplined plan/session-spec executor: refuses a plan whose logged approval does not cover its current sha256 (K2); one fresh `step-implementer` helper per plan step, one controller that reads the diff (spec + quality verdicts), proof of red / floor guard / step check (K9) / `Regression:` (K10) / done line as commands, fix rounds that go up (max 5: helper → plan → operator), architecture rules (K5) + plan-file sweep (K6) after the last step, the app seen with Claude Code's built-in `run` while the `End-to-end:` command stays the verdict (`run` gives no exit code), then its own review (`/simplify` → `/trekreview` → verdict gate, K7; BLOCK keeps the merge into main closed), closed stop list + decision log in the final report | session |

View file

@ -159,7 +159,7 @@ Success criteria are proposed in the form of `voyage:writing-criteria`, so the r
Then `brief-reviewer` scores the draft on six dimensions (completeness, consistency, testability, scope clarity, research-plan validity, memory alignment) and gates publication, up to 1 / 2 / 3 review iterations by class; force-stop yields a `brief_quality: partial` brief with the failing dimensions documented. Then `brief-reviewer` scores the draft on six dimensions (completeness, consistency, testability, scope clarity, research-plan validity, memory alignment) and gates publication, up to 1 / 2 / 3 review iterations by class; force-stop yields a `brief_quality: partial` brief with the failing dimensions documented.
**Approval, logged and committed.** Every run ends with you approving `## Intent` + `## Goal` (only an explicit "Approve" counts). The approval is appended to the target repo's hash-chained approval log, `.claude/voyage/approvals.jsonl`, bound to the intent's sha256, and committed. The log is always committed; the brief only when you chose "in git" for this task (`artifact_store: git`). "Only on this machine" keeps the project folder out of git via `.git/info/exclude`. The log is a trace, not a signature: any session on your machine could write it, and the commit is what makes it visible. **Approval, logged and committed.** Every run ends with you approving `## Intent` + `## Goal` (+ `## Threat Model` when the brief has one; only an explicit "Approve" counts). The approval is appended to the target repo's hash-chained approval log, `.claude/voyage/approvals.jsonl`, bound to the intent's sha256, and committed. The log is always committed; the brief only when you chose "in git" for this task (`artifact_store: git`). "Only on this machine" keeps the project folder out of git via `.git/info/exclude`. The log is a trace, not a signature: any session on your machine could write it, and the commit is what makes it visible.
Output: `.claude/projects/{YYYY-MM-DD}-{slug}/brief.md` Output: `.claude/projects/{YYYY-MM-DD}-{slug}/brief.md`
@ -171,7 +171,7 @@ Borrowed, with credit: size classes and the ratchet from obra/superpowers (MIT),
|------|-------|----------| |------|-------|----------|
| **Default** | `/trekbrief <task>` | Question round sized by class, then review and approval. | | **Default** | `/trekbrief <task>` | Question round sized by class, then review and approval. |
| **Quick** | `/trekbrief --quick <task>` | Proposes `small` as the class; still said aloud, overridable, and raised if the task proves bigger. | | **Quick** | `/trekbrief --quick <task>` | Proposes `small` as the class; still said aloud, overridable, and raised if the task proves bigger. |
| **Approve** | `/trekbrief --approve <project-dir \| brief-file>` | No interview: show an existing brief's `## Intent` + `## Goal` verbatim and ask the operator to approve them. The same step (Phase 4h) ends every normal `/trekbrief` run. | | **Approve** | `/trekbrief --approve <project-dir \| brief-file>` | No interview: show an existing brief's `## Intent` + `## Goal` (+ `## Threat Model` when present) verbatim and ask the operator to approve them. The same step (Phase 4h) ends every normal `/trekbrief` run. |
`/trekbrief` is **always interactive**. There is no foreground/background mode — the interview requires user input. `/trekbrief` is **always interactive**. There is no foreground/background mode — the interview requires user input.
@ -269,7 +269,7 @@ Output:
`--brief` or `--project` is **required**. `/trekplan` with no brief exits with an error and a pointer to `/trekbrief`. `--brief` or `--project` is **required**. `/trekplan` with no brief exits with an error and a pointer to `/trekbrief`.
**Intent approval gate.** `/trekplan` halts before planning anything unless the brief's `## Intent` + `## Goal` carry the operator's approval. `/trekbrief` asks for it at the end of every interview (Phase 4h, also in `--quick`), or run `/trekbrief --approve <project-dir | brief-file>` later. The approval is a hash over those two sections: edit either one afterwards and `/trekplan` halts again until the new text is approved. Review briefs (`type: trekreview`) are exempt. The marker is a trace that the approval step ran, **not a signature**. Any session on the same machine could write it. **Intent approval gate.** `/trekplan` halts before planning anything unless the brief's `## Intent` + `## Goal` (+ `## Threat Model` when present) carry the operator's approval. `/trekbrief` asks for it at the end of every interview (Phase 4h, also in `--quick`), or run `/trekbrief --approve <project-dir | brief-file>` later. The approval is a hash over those sections: edit any of them afterwards and `/trekplan` halts again until the new text is approved. Review briefs (`type: trekreview`) are exempt. The marker is a trace that the approval step ran, **not a signature**. Any session on the same machine could write it.
### What the plan contains ### What the plan contains

View file

@ -309,7 +309,9 @@ 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 prompt gives you the diff since the last reviewed text, and the plan's
`## Critic Verdict` lists the previous findings. List every previous finding `## Critic Verdict` lists the previous findings. List every previous finding
by id in `previous`, with `resolved: true` only when the plan now fixes it. 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. The `## Critic Verdict` also lists `known_outside`, the security findings the
brief's threat model puts outside. Mark each of them by id in `previous` the
same way. 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 Dimensions 11 and 12 are scored inside the existing weights: 11 under Step
quality, 12 under Coverage completeness and Specification quality. quality, 12 under Coverage completeness and Specification quality.
@ -408,7 +410,8 @@ instance hides a class that keeps coming back.
`severity` is exactly one of `blocker`, `major`, `minor`. Emit `severity` is exactly one of `blocker`, `major`, `minor`. Emit
`"findings": []` when the plan is clean. `"findings": []` when the plan is clean.
`previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per `previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per
finding the current `## Critic Verdict` lists; `resolved` is `true` or finding the current `## Critic Verdict` lists, its `known_outside` entries
included; `resolved` is `true` or
`false`. `line_end` (optional) is the last line of a cited range, an integer `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 ≥ `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:` B: `"lowered_from": "major"`, and `caught_by` names the test, `Regression:`

View file

@ -127,7 +127,9 @@ 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 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 plan's `## Critic Verdict` lists the previous findings. List every previous
finding by id in `previous`, with `resolved: true` only when the plan now 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 fixes it. The `## Critic Verdict` also lists `known_outside`, the security
findings the brief's threat model puts outside. Mark each of them by id in
`previous` the same way. Review the changed text. Cite the exact line, with `line_end` for a
range. range.
## Output format ## Output format
@ -197,7 +199,8 @@ by class (the catalogue key, else the part before the first `/`), so a key
made for one instance hides a class that keeps coming back. made for one instance hides a class that keeps coming back.
Emit `"findings": []` when the plan is fully aligned. Emit `"findings": []` when the plan is fully aligned.
`previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per `previous` (from round 2; omit it in round 1) holds one `{id, resolved}` per
finding the current `## Critic Verdict` lists; `resolved` is `true` or finding the current `## Critic Verdict` lists, its `known_outside` entries
included; `resolved` is `true` or
`false`. `line_end` (optional) is the last line of a cited range, an integer `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 ≥ `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:` B: `"lowered_from": "major"`, and `caught_by` names the test, `Regression:`

View file

@ -990,7 +990,7 @@ End your response with the REQUIRED machine-readable `json` findings block
dedup helper. You have no Write tool — return the block inline, do not write a file. dedup helper. You have no Write tool — return the block inline, do not write a file.
Read local files only inside the repo under exploration, plus paths this prompt gives you (the brief, the plan, and templates it names)." Read local files only inside the repo under exploration, plus paths this prompt gives you (the brief, the plan, and templates it names)."
From round 2 the prompt adds one sentence, followed by the `--diff` output: From round 2 the prompt adds one sentence, followed by the `--diff` output:
"Previous findings are in the plan's `## Critic Verdict`; the diff since the last reviewed text is below. Mark each previous finding by id in `previous`." "Previous findings are in the plan's `## Critic Verdict`, its `known_outside` entries included; the diff since the last reviewed text is below. Mark each previous finding by id in `previous`."
**scope-guardian** — scope alignment check. **scope-guardian** — scope alignment check.
Prompt: "Check this implementation plan against the brief. Prompt: "Check this implementation plan against the brief.
@ -1006,7 +1006,7 @@ End your response with the REQUIRED machine-readable
tool — return the block inline, do not write a file. tool — return the block inline, do not write a file.
Read local files only inside the repo under exploration, plus paths this prompt gives you (the brief, the plan, and templates it names)." Read local files only inside the repo under exploration, plus paths this prompt gives you (the brief, the plan, and templates it names)."
From round 2 the prompt adds one sentence, followed by the `--diff` output: From round 2 the prompt adds one sentence, followed by the `--diff` output:
"Previous findings are in the plan's `## Critic Verdict`; the diff since the last reviewed text is below. Mark each previous finding by id in `previous`." "Previous findings are in the plan's `## Critic Verdict`, its `known_outside` entries included; the diff since the last reviewed text is below. Mark each previous finding by id in `previous`."
After both complete, **extract each reviewer's trailing `json` findings block** After both complete, **extract each reviewer's trailing `json` findings block**
and pipe both into the dedup helper via **stdin** — the reviewers are read-only and pipe both into the dedup helper via **stdin** — the reviewers are read-only

View file

@ -72,7 +72,7 @@ Every validator exposes a CLI: `node lib/validators/<name>.mjs --json <path>` re
| `source` | string | optional | `interview \| manual` | | | `source` | string | optional | `interview \| manual` | |
| `brief_quality` | string | optional | `complete \| partial` | Set when iteration cap is hit | | `brief_quality` | string | optional | `complete \| partial` | Set when iteration cap is hit |
| `framing` | string | **required at ≥ 2.2** (v5.5) | `preserve \| refine \| replace \| new-direction` | How this brief relates to prior operator intent. Enum-checked on any version when present; missing → `BRIEF_MISSING_FRAMING` at `brief_version ≥ 2.2`. Layer 1 of the framing-alignment defense. | | `framing` | string | **required at ≥ 2.2** (v5.5) | `preserve \| refine \| replace \| new-direction` | How this brief relates to prior operator intent. Enum-checked on any version when present; missing → `BRIEF_MISSING_FRAMING` at `brief_version ≥ 2.2`. Layer 1 of the framing-alignment defense. |
| `intent_approved_hash` | string | optional in the schema; **required by `/trekplan`** (veikart steg 1) | `"sha256:<64 hex>"` | Hash over the normalized `## Intent` + `## Goal`. Written only by `lib/validators/intent-approval.mjs --stamp`, which `/trekbrief` runs on the operator's explicit "Approve" (Phase 4h, or `--approve <project-dir \| brief-file>`). | | `intent_approved_hash` | string | optional in the schema; **required by `/trekplan`** (veikart steg 1) | `"sha256:<64 hex>"` | Hash over the normalized `## Intent` + `## Goal` (+ `## Threat Model` when present). Written only by `lib/validators/intent-approval.mjs --stamp`, which `/trekbrief` runs on the operator's explicit "Approve" (Phase 4h, or `--approve <project-dir \| brief-file>`). |
| `intent_approved_at` | string | optional | ISO-8601 | When the stamp was written. Informational; the gate reads the hash. | | `intent_approved_at` | string | optional | ISO-8601 | When the stamp was written. Informational; the gate reads the hash. |
| `brief_reviewed_hash` / `brief_reviewed_at` | string | optional, additive (no `brief_version` bump) | `"sha256:<64 hex>"` / ISO-8601 | Hash over the brief's body and the frontmatter the reviewer judges (everything but this stamp and the intent marker), written by `lib/validators/brief-review-stamp.mjs --stamp` when `/trekbrief`'s brief-reviewer gate passes. `/trekplan` Phase 4b skips its own brief review only while the hash is current; absent (any other producer) or stale (body or reviewed frontmatter such as `framing` changed) means it reviews. A trace, not a signature. | | `brief_reviewed_hash` / `brief_reviewed_at` | string | optional, additive (no `brief_version` bump) | `"sha256:<64 hex>"` / ISO-8601 | Hash over the brief's body and the frontmatter the reviewer judges (everything but this stamp and the intent marker), written by `lib/validators/brief-review-stamp.mjs --stamp` when `/trekbrief`'s brief-reviewer gate passes. `/trekplan` Phase 4b skips its own brief review only while the hash is current; absent (any other producer) or stale (body or reviewed frontmatter such as `framing` changed) means it reviews. A trace, not a signature. |
| `ceremony` | string | optional (K1) | `small \| bounded \| architectural` | The brief's size class: sets `/trekbrief`'s question budget and review cap, and what downstream phases scale to. **Absent (or unreadable) = `architectural`**, the heaviest, so another producer's brief is never treated lighter by accident. Enum-checked when present (`BRIEF_INVALID_CEREMONY`). Read it through `briefCeremony(fm)` in `brief-validator.mjs`. | | `ceremony` | string | optional (K1) | `small \| bounded \| architectural` | The brief's size class: sets `/trekbrief`'s question budget and review cap, and what downstream phases scale to. **Absent (or unreadable) = `architectural`**, the heaviest, so another producer's brief is never treated lighter by accident. Enum-checked when present (`BRIEF_INVALID_CEREMONY`). Read it through `briefCeremony(fm)` in `brief-validator.mjs`. |
@ -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; - 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); - 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`". - `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, 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, 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) 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): 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`). - **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`). - **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`).

View file

@ -9,7 +9,7 @@ Per-command flag tables, imported from `CLAUDE.md` via pointer.
| _(default)_ | Question round sized by class (`small` / `bounded` / `architectural`), brief-review gate, approval → brief.md with research plan | | _(default)_ | Question round sized by class (`small` / `bounded` / `architectural`), brief-review gate, approval → brief.md with research plan |
| `--quick` | Proposes `small` as the class (still said aloud, overridable, raised when the task proves bigger) → brief.md with research plan | | `--quick` | Proposes `small` as the class (still said aloud, overridable, raised when the task proves bigger) → brief.md with research plan |
| `--gates [true\|false\|closed\|open]` | (v3.4.0) Autonomy flag: bare, `true` or `closed` = on; `false` or `open` = off; any other next token is not its value. See `## Autonomy mode` in `docs/operations.md`. | | `--gates [true\|false\|closed\|open]` | (v3.4.0) Autonomy flag: bare, `true` or `closed` = on; `false` or `open` = off; any other next token is not its value. See `## Autonomy mode` in `docs/operations.md`. |
| `--approve <project-dir \| brief-file>` | (veikart steg 1) No interview: show the existing brief's `## Intent` + `## Goal` verbatim, ask the operator to approve, and on "Approve" stamp `intent_approved_hash` + `intent_approved_at`. The remedy `/trekplan` names when it halts on `BRIEF_INTENT_NOT_APPROVED` / `BRIEF_INTENT_APPROVAL_STALE`. | | `--approve <project-dir \| brief-file>` | (veikart steg 1) No interview: show the existing brief's `## Intent` + `## Goal` (+ `## Threat Model` when present) verbatim, ask the operator to approve, and on "Approve" stamp `intent_approved_hash` + `intent_approved_at`. The remedy `/trekplan` names when it halts on `BRIEF_INTENT_NOT_APPROVED` / `BRIEF_INTENT_APPROVAL_STALE`. |
Always interactive. Phase 3 puts the task in a size class (`ceremony:`, K1; heaviest in doubt, said aloud, overridable, only moved up) and asks one round of questions within the class budget (`small` 4, `bounded` 8, `architectural` 12 per round), recommendation first, facts looked up instead of asked, a visible Deferred list; it also asks where the brief is stored (`artifact_store: git | local`). Phase 4 runs a `brief-reviewer` stop-gate with 1 / 2 / 3 review iterations by class. Phase 4h takes only an explicit "Approve" («høres bra ut» and an empty answer are refused by `lib/util/approval-log.mjs answer`), stamps the brief, appends the approval to the target repo's log (`approval-log.mjs append`, K2) and commits it (`lib/util/artifact-commit.mjs`: the log always, the brief when `git`). After that, asks the user to choose manual (print commands) or auto (Claude runs research + plan in foreground). Always interactive. Phase 3 puts the task in a size class (`ceremony:`, K1; heaviest in doubt, said aloud, overridable, only moved up) and asks one round of questions within the class budget (`small` 4, `bounded` 8, `architectural` 12 per round), recommendation first, facts looked up instead of asked, a visible Deferred list; it also asks where the brief is stored (`artifact_store: git | local`). Phase 4 runs a `brief-reviewer` stop-gate with 1 / 2 / 3 review iterations by class. Phase 4h takes only an explicit "Approve" («høres bra ut» and an empty answer are refused by `lib/util/approval-log.mjs answer`), stamps the brief, appends the approval to the target repo's log (`approval-log.mjs append`, K2) and commits it (`lib/util/artifact-commit.mjs`: the log always, the brief when `git`). After that, asks the user to choose manual (print commands) or auto (Claude runs research + plan in foreground).

View file

@ -36,8 +36,10 @@
// `threat_model_ref` (the text of the brief's `Outside:` line it quotes), and // `threat_model_ref` (the text of the brief's `Outside:` line it quotes), and
// a merge keeps it only when every merged copy carries the same one; one copy // a merge keeps it only when every merged copy carries the same one; one copy
// without it, or two different ones, drop it, so the plan gate counts the // without it, or two different ones, drop it, so the plan gate counts the
// merged finding in full. A non-object in a reviewer's findings list (a // merged finding in full. A `null` or `undefined` in a reviewer's findings
// `null`) is skipped instead of taking the CLI down (the S160 known issue). // list is skipped instead of taking the CLI down (the S160 known issue); any
// other entry that is not an object (a string, a number, an array) stays a
// finding with no valid severity, which the plan gate counts as a blocker.
// //
// Provenance is preserved on the surviving finding's `raised_by` array. // Provenance is preserved on the surviving finding's `raised_by` array.
// Severity is kept (normalized to blocker | major | minor, else null). A merge // Severity is kept (normalized to blocker | major | minor, else null). A merge
@ -218,8 +220,9 @@ function normalizeAgentPayload(payload, fallbackAgent) {
const agent = (typeof payload.agent === 'string' && payload.agent.length > 0) const agent = (typeof payload.agent === 'string' && payload.agent.length > 0)
? payload.agent ? payload.agent
: fallbackAgent; : fallbackAgent;
// A non-object entry (a `null`) is no finding: skipped, never thrown on (S160). // A `null` or `undefined` entry holds nothing: skipped, never thrown on (S160).
const findings = Array.isArray(payload.findings) ? payload.findings.filter(f => f !== null && typeof f === 'object') : []; // Any other entry, an object or not, goes on as before (no valid severity = a blocker).
const findings = Array.isArray(payload.findings) ? payload.findings.filter(f => f !== null && f !== undefined) : [];
const previous = Array.isArray(payload.previous) ? payload.previous : []; const previous = Array.isArray(payload.previous) ? payload.previous : [];
return { agent, pairs: findings.map(f => ({ agent, finding: f })), previous }; return { agent, pairs: findings.map(f => ({ agent, finding: f })), previous };
} }

View file

@ -27,8 +27,11 @@
// goes back into the findings at its catalogue tier with no id, a `### ` // goes back into the findings at its catalogue tier with no id, a `### `
// entry with a `**Source:**` line saying why, the frontmatter list, counts, // entry with a `**Source:**` line saying why, the frontmatter list, counts,
// a verdict that is only raised and, as a BLOCKER, its `### Step R<n>`. // a verdict that is only raised and, as a BLOCKER, its `### Step R<n>`.
// The entries that stay are listed under `## Outside the Threat Model`; // An entry whose row a finding already carries (the engine pass put the
// a malformed entry stays too (the verdict gate exits 2 on it). // same engine finding back, engine-compare.mjs `rowOf`) leaves the list
// without a second copy. The entries that stay are listed under
// `## Outside the Threat Model`; a malformed entry stays too (the verdict
// gate exits 2 on it).
// - instruction, right after engine: a finding under `CLAUDE_MD_DEVIATION` // - instruction, right after engine: a finding under `CLAUDE_MD_DEVIATION`
// or `CLAUDE_MD_STALE` whose `instruction_ref` is missing or names a file // or `CLAUDE_MD_STALE` whose `instruction_ref` is missing or names a file
// outside the repo's tracked instruction set (lib/review/instruction-scope.mjs // outside the repo's tracked instruction set (lib/review/instruction-scope.mjs
@ -122,7 +125,7 @@ import { isSecurityFinding } from './review-answers.mjs';
import { carry, frontmatterField, MAX_REVIEW_ROUNDS, runId } from './review-rounds.mjs'; import { carry, frontmatterField, MAX_REVIEW_ROUNDS, runId } from './review-rounds.mjs';
import { applyPolicy, readPolicy } from './review-policy.mjs'; import { applyPolicy, readPolicy } from './review-policy.mjs';
import { ENGINES } from './engine-findings.mjs'; import { ENGINES } from './engine-findings.mjs';
import { ENGINE_OF_REVIEWER, putBack } from './engine-compare.mjs'; import { ENGINE_OF_REVIEWER, putBack, rowOf, trimmedTitle } from './engine-compare.mjs';
import { backedLine, isKnownEntry, reviewThreatModel } from './threat-model.mjs'; import { backedLine, isKnownEntry, reviewThreatModel } from './threat-model.mjs';
import { getRule } from './rule-catalogue.mjs'; import { getRule } from './rule-catalogue.mjs';
import { isMain } from '../util/is-main.mjs'; import { isMain } from '../util/is-main.mjs';
@ -288,9 +291,15 @@ function passOutside(payload, ctx) {
else kept.push(e); else kept.push(e);
} }
const why = tm ? 'no `Outside:` line of the brief equals its threat_model_ref, or it is not a Voyage security finding' : use.why; const why = tm ? 'no `Outside:` line of the brief equals its threat_model_ref, or it is not a Voyage security finding' : use.why;
// A row a finding already carries (the engine pass put the same engine
// finding back) is counted once: the entry leaves the list, no second copy.
const rows = new Set(payload.findings.filter(isPlainObject).map(rowOf));
for (const e of back) { for (const e of back) {
const rule = getRule(e.rule_key.trim().toUpperCase()); const rule = getRule(e.rule_key.trim().toUpperCase());
const f = { ...e, id: null, rule_key: rule ? rule.rule_key : e.rule_key, severity: rule ? rule.severity : 'BLOCKER' }; const f = { ...e, id: null, rule_key: rule ? rule.rule_key : e.rule_key, severity: rule ? rule.severity : 'BLOCKER' };
const held = rows.has(rowOf(f)) || (typeof f.title === 'string' && rows.has(rowOf({ ...f, title: trimmedTitle(f.title) })));
if (held) continue;
rows.add(rowOf(f));
payload.findings.push(f); payload.findings.push(f);
ctx.added.add(f); ctx.added.add(f);
ctx.outsideBack.set(f, why); ctx.outsideBack.set(f, why);

View file

@ -21,9 +21,16 @@
// Otherwise every security finding counts, and `why` says why. // Otherwise every security finding counts, and `why` says why.
// backedLine(finding, tm) -> the `Outside:` text the finding's // backedLine(finding, tm) -> the `Outside:` text the finding's
// `threat_model_ref` equals exactly, else null. // `threat_model_ref` equals exactly, else null.
// reviewThreatModel(reviewText, repo) -> usable() for the brief a review // usableInRepo(briefPath, repo) -> usable() for a brief path relative to
// names in its `brief_path` (a relative path is read from the repo's top // the repo's top level (an absolute one as it is); both gates read a
// level); the review side's `known_outside` (E 2.5) is checked with it. // stored `brief_path` this way, whatever directory they run in.
// repoRelativeBrief(briefPath, repo) -> a brief path given from the
// working directory, made relative to the repo's top level (forward
// slashes; `..` when outside it), never absolute: what the plan gate
// stores as `brief_path`.
// reviewThreatModel(reviewText, repo) -> usableInRepo() for the brief a
// review names in its `brief_path`; the review side's `known_outside`
// (E 2.5) is checked with it.
// isKnownEntry(e) -> a review `known_outside` entry's shape: a // isKnownEntry(e) -> a review `known_outside` entry's shape: a
// finding with a string `rule_key` and `file`, an integer `line` >= 0 // finding with a string `rule_key` and `file`, an integer `line` >= 0
// and a non-empty `threat_model_ref`. // and a non-empty `threat_model_ref`.
@ -33,7 +40,7 @@
// every run, so a finding whose line is gone counts in full. // every run, so a finding whose line is gone counts in full.
import { readFileSync } from 'node:fs'; import { readFileSync } from 'node:fs';
import { isAbsolute, join } from 'node:path'; import { isAbsolute, join, relative, sep } from 'node:path';
import { parseDocument } from '../util/frontmatter.mjs'; import { parseDocument } from '../util/frontmatter.mjs';
import { verifyLog, normalizeHex, normalizeSubject, resolveRepo } from '../util/approval-log.mjs'; import { verifyLog, normalizeHex, normalizeSubject, resolveRepo } from '../util/approval-log.mjs';
import { checkIntentApprovalContent } from '../validators/intent-approval.mjs'; import { checkIntentApprovalContent } from '../validators/intent-approval.mjs';
@ -93,8 +100,38 @@ export function usable(briefPath, repo) {
} }
/** /**
* The threat model a review applies: usable() for the brief at the review's * usable() for a brief path relative to the repo's top level (an absolute
* frontmatter `brief_path`, a relative path read from the repo's top level. * path as it is), so a stored `brief_path` reads the same brief from any
* working directory.
* @param {string} briefPath
* @param {string} [repo] - the repo (default: the git top level of the working directory)
* @returns {{usable: boolean, why: string|null, threatModel: object}}
*/
export function usableInRepo(briefPath, repo) {
if (typeof briefPath !== 'string' || briefPath.trim() === '') return { usable: false, why: 'no brief path was given', threatModel: EMPTY };
let root;
try { root = resolveRepo(repo); } catch (e) {
return { usable: false, why: `the repo could not be read: ${e.message}`, threatModel: EMPTY };
}
return usable(isAbsolute(briefPath) ? briefPath : join(root, briefPath), root);
}
/**
* A brief path given from the working directory, relative to the repo's top
* level: forward slashes, `..` when the brief is outside the repo, never absolute.
* @param {string} briefPath
* @param {string} [repo] - the repo (default: the git top level of the working directory)
* @returns {string}
*/
export function repoRelativeBrief(briefPath, repo) {
const root = resolveRepo(repo);
const s = normalizeSubject(root, briefPath);
return isAbsolute(s) ? relative(root, s).split(sep).join('/') : s;
}
/**
* The threat model a review applies: usableInRepo() for the brief at the
* review's frontmatter `brief_path`, a relative path read from the repo's top level.
* @param {string} reviewText - review.md * @param {string} reviewText - review.md
* @param {string} [repo] - the repo (default: the git top level of the working directory) * @param {string} [repo] - the repo (default: the git top level of the working directory)
* @returns {{usable: boolean, why: string|null, threatModel: object}} * @returns {{usable: boolean, why: string|null, threatModel: object}}
@ -103,11 +140,7 @@ export function reviewThreatModel(reviewText, repo) {
const doc = parseDocument(typeof reviewText === 'string' ? reviewText : ''); const doc = parseDocument(typeof reviewText === 'string' ? reviewText : '');
const bp = doc.parsed?.frontmatter?.brief_path; const bp = doc.parsed?.frontmatter?.brief_path;
if (typeof bp !== 'string' || bp.trim() === '') return { usable: false, why: 'the review names no brief_path', threatModel: EMPTY }; if (typeof bp !== 'string' || bp.trim() === '') return { usable: false, why: 'the review names no brief_path', threatModel: EMPTY };
let root; return usableInRepo(bp, repo);
try { root = resolveRepo(repo); } catch (e) {
return { usable: false, why: `the repo could not be read: ${e.message}`, threatModel: EMPTY };
}
return usable(isAbsolute(bp) ? bp : join(root, bp), root);
} }
/** A review `known_outside` entry's shape (E 2.5): string rule_key and file, integer line >= 0, non-empty threat_model_ref. */ /** A review `known_outside` entry's shape (E 2.5): string rule_key and file, integer line >= 0, non-empty threat_model_ref. */

View file

@ -122,9 +122,12 @@
// log). A backed finding — Voyage's own security finding (no engine) whose // log). A backed finding — Voyage's own security finding (no engine) whose
// `threat_model_ref` equals an `Outside:` line's text exactly — leaves the // `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 // counted set and the stop rule and goes to the optional verdict field
// `known_outside` ({id, rule_key, text, line}), carried to later rounds until // `known_outside` ({id, rule_key, rule_keys, text, line}), carried to later
// a reviewer marks it resolved (or it counts again under its id); the verdict // rounds until a reviewer marks it resolved (or it counts again under its id);
// stores `brief_path` (carried when a later round has no --brief). A threat // 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
// (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 // model that is not usable backs nothing: every security finding counts, and
// `threat_model.why` says why. The bare gate re-reads the brief at // `threat_model.why` says why. The bare gate re-reads the brief at
// `brief_path` on every run: an entry still backed is a warning // `brief_path` on every run: an entry still backed is a warning
@ -140,10 +143,10 @@ import { createHash } from 'node:crypto';
import { dirname, join, resolve } from 'node:path'; import { dirname, join, resolve } from 'node:path';
import { validatePlanContent } from './plan-validator.mjs'; import { validatePlanContent } from './plan-validator.mjs';
import { splitSections, fenceTracker, extractPlanVersion, versionAtLeast } from '../parsers/plan-schema.mjs'; import { splitSections, fenceTracker, extractPlanVersion, versionAtLeast } from '../parsers/plan-schema.mjs';
import { normalizeSeverity, isSecurity } from '../review/plan-review-dedup.mjs'; import { normalizeSeverity, isSecurity, SEVERITIES } from '../review/plan-review-dedup.mjs';
import { countRound, lineDiff, oneLine } from '../review/critic-diff.mjs'; import { countRound, lineDiff, oneLine } from '../review/critic-diff.mjs';
import { roundSummary, decideStop, describeStop, STOP_REASONS } from '../review/stop-rule.mjs'; import { roundSummary, decideStop, describeStop, STOP_REASONS } from '../review/stop-rule.mjs';
import { usable as threatModelUsable, backedLine } from '../review/threat-model.mjs'; import { usableInRepo, repoRelativeBrief, backedLine } from '../review/threat-model.mjs';
import { resolveRepo } from '../verification/repo-tools.mjs'; import { resolveRepo } from '../verification/repo-tools.mjs';
import { isMain } from '../util/is-main.mjs'; import { isMain } from '../util/is-main.mjs';
@ -281,13 +284,21 @@ function okStopRule(s) {
const isStrOrNull = x => x === null || typeof x === 'string'; const isStrOrNull = x => x === null || typeof x === 'string';
/** Optional `known_outside`: a list of {id: string|null, rule_key: string|null, text: string, line: non-empty string}. */ /** Optional `known_outside`: a list of {id: string|null, rule_key: string|null, rule_keys?: string[], text: string, line: non-empty string}. */
function okKnownOutside(k) { function okKnownOutside(k) {
if (k === undefined) return true; if (k === undefined) return true;
return Array.isArray(k) && k.every(e => isPlainObject(e) && isStrOrNull(e.id) && isStrOrNull(e.rule_key) && return Array.isArray(k) && k.every(e => isPlainObject(e) && isStrOrNull(e.id) && isStrOrNull(e.rule_key) &&
(e.rule_keys === undefined || (Array.isArray(e.rule_keys) && e.rule_keys.every(x => typeof x === 'string'))) &&
typeof e.text === 'string' && typeof e.line === 'string' && e.line !== ''); 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. */ /** Optional `brief_path`: a non-empty string. */
const okBriefPath = p => p === undefined || (typeof p === 'string' && p !== ''); const okBriefPath = p => p === undefined || (typeof p === 'string' && p !== '');
@ -407,9 +418,15 @@ export function recordVerdict(text, dedup, opts = {}) {
const counted = []; const counted = [];
for (const f of dedup.findings) { for (const f of dedup.findings) {
const line = tm ? backedLine(f, tm) : null; const line = tm ? backedLine(f, tm) : null;
if (line !== null) backed.push({ id: f.id ?? null, rule_key: f.rule_key ?? null, text: typeof f.text === 'string' ? f.text : '', line }); if (line === null) { counted.push(f); continue; }
else counted.push(f); // rule_keys kept, so the bare gate re-checks the entry with the same keys (keysOf).
backed.push({
id: f.id ?? null, rule_key: f.rule_key ?? null,
...(Array.isArray(f.rule_keys) ? { rule_keys: f.rule_keys.filter(k => typeof k === 'string') } : {}),
text: typeof f.text === 'string' ? f.text : '', line,
});
} }
const backedIds = new Set(backed.map(e => e.id).filter(id => id !== null));
const c = countRound({ const c = countRound({
round, round,
previous: prev.verdict ?? null, previous: prev.verdict ?? null,
@ -420,7 +437,12 @@ export function recordVerdict(text, dedup, opts = {}) {
findings: counted, findings: counted,
marks: Array.isArray(dedup.marks) ? dedup.marks : [], marks: Array.isArray(dedup.marks) ? dedup.marks : [],
}); });
const findings = c.findings; // 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 counts = { blocker: 0, major: 0, minor: 0 }; const counts = { blocker: 0, major: 0, minor: 0 };
for (const f of findings) counts[f.severity] += 1; for (const f of findings) counts[f.severity] += 1;
const verdict = { const verdict = {
@ -441,7 +463,6 @@ export function recordVerdict(text, dedup, opts = {}) {
// entries that no reviewer marked resolved and that are not reported again // 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 (backed again: listed once; counted: in full, not listed).
const marks = Array.isArray(dedup.marks) ? dedup.marks : []; const marks = Array.isArray(dedup.marks) ? dedup.marks : [];
const backedIds = new Set(backed.map(e => e.id).filter(id => id !== null));
const countedIds = new Set(findings.map(x => x.id).filter(id => typeof id === 'string')); 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 : []) const kept = (Array.isArray(prev.verdict?.known_outside) ? prev.verdict.known_outside : [])
.filter(e => !knownResolved(e, marks) && !backedIds.has(e.id) && !countedIds.has(e.id)); .filter(e => !knownResolved(e, marks) && !backedIds.has(e.id) && !countedIds.has(e.id));
@ -459,7 +480,7 @@ export function recordVerdict(text, dedup, opts = {}) {
? { applied: false, why: 'no --brief given' } ? { applied: false, why: 'no --brief given' }
: { applied: use.usable === true, why: use.usable === true ? null : (use.why ?? 'the threat model is not usable') }; : { applied: use.usable === true, why: use.usable === true ? null : (use.why ?? 'the threat model is not usable') };
return { return {
code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered: c.lowered, threat_model: threat, code: 0, text: placeSection(text, renderSection(verdict)), verdict, lowered, threat_model: threat,
message: `round ${round} recorded: ${verdict.verdict}` + message: `round ${round} recorded: ${verdict.verdict}` +
(knownOutside.length > 0 ? `; ${knownOutside.length} finding(s) outside the threat model listed as known issues` : '') + (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}` : '') + (use !== undefined && !threat.applied ? `; threat model not applied, every security finding counts: ${threat.why}` : '') +
@ -548,7 +569,8 @@ function checkKnownOutside(entries, use, reasons, warnings) {
const backed = []; const backed = [];
const unbacked = []; const unbacked = [];
for (const e of entries) { for (const e of entries) {
(tm && backedLine({ rule_key: e.rule_key, threat_model_ref: e.line }, tm) !== null ? backed : unbacked).push(e); const asFinding = { rule_key: e.rule_key, ...(Array.isArray(e.rule_keys) ? { rule_keys: e.rule_keys } : {}), threat_model_ref: e.line };
(tm && backedLine(asFinding, tm) !== null ? backed : unbacked).push(e);
} }
if (backed.length > 0) { if (backed.length > 0) {
warnings.push({ warnings.push({
@ -631,8 +653,10 @@ export function gatePlan(rawText, opts = {}) {
} }
const known = Array.isArray(v.known_outside) ? v.known_outside : []; const known = Array.isArray(v.known_outside) ? v.known_outside : [];
if (known.length > 0) { if (known.length > 0) {
// brief_path is relative to the repo's top level, so the gate reads the
// same brief from any working directory inside the repo.
const use = opts.threatModel ?? (typeof v.brief_path === 'string' const use = opts.threatModel ?? (typeof v.brief_path === 'string'
? threatModelUsable(v.brief_path) ? usableInRepo(v.brief_path)
: { usable: false, why: 'the verdict names no brief_path', threatModel: null }); : { usable: false, why: 'the verdict names no brief_path', threatModel: null });
checkKnownOutside(known, use, reasons, warnings); checkKnownOutside(known, use, reasons, warnings);
} }
@ -826,8 +850,9 @@ if (isMain(import.meta.url)) {
const dir = snapshotDir(planPath); const dir = snapshotDir(planPath);
const opts = { extraRound: args.includes('--extra-round'), reviewed, planPaths: planPathsOf(planPath) }; const opts = { extraRound: args.includes('--extra-round'), reviewed, planPaths: planPathsOf(planPath) };
if (briefArg !== undefined) { if (briefArg !== undefined) {
opts.briefPath = briefArg; // Stored relative to the repo's top level, never absolute; read from there.
opts.threatModel = threatModelUsable(briefArg); opts.briefPath = repoRelativeBrief(briefArg);
opts.threatModel = usableInRepo(opts.briefPath);
} }
const lb = loadBase(text, dir); const lb = loadBase(text, dir);
if (lb.verdict) { if (lb.verdict) {

View file

@ -1,6 +1,6 @@
--- ---
name: annotating name: annotating
description: "The operator's own annotation loop for a Voyage brief, plan or review (a research document too, when asked): build the page with scripts/annotate.mjs so the person writes every note, take the pasted Copy Prompt back, and revise note by note. Fix and Change edit only the text the note points at; a Question is answered and changes nothing unless the person says so; nothing the notes did not ask for is edited. A note on ## Intent or ## Goal of an approved brief makes the approval stale, and a changed plan needs a new approval, said before the edit. The page is built again on the revised file. Use when someone wants to comment on, annotate or mark up a brief, plan, review or research document in the browser, or pastes back the notes from that page (Fix, Change, Question)." description: "The operator's own annotation loop for a Voyage brief, plan or review (a research document too, when asked): build the page with scripts/annotate.mjs so the person writes every note, take the pasted Copy Prompt back, and revise note by note. Fix and Change edit only the text the note points at; a Question is answered and changes nothing unless the person says so; nothing the notes did not ask for is edited. A note on ## Intent or ## Goal (or ## Threat Model) of an approved brief makes the approval stale, and a changed plan needs a new approval, said before the edit. The page is built again on the revised file. Use when someone wants to comment on, annotate or mark up a brief, plan, review or research document in the browser, or pastes back the notes from that page (Fix, Change, Question)."
--- ---
# Annotating # Annotating
@ -46,7 +46,7 @@ approval.
anchored to, and the comment. Work from the quote and the comment, one note at a time. anchored to, and the comment. Work from the quote and the comment, one note at a time.
4. **Before any edit, check what the edit costs.** 4. **Before any edit, check what the edit costs.**
- A brief with `intent_approved_hash` in its frontmatter, and a Fix or Change in `## Intent` or - A brief with `intent_approved_hash` in its frontmatter, and a Fix or Change in `## Intent` or
`## Goal`: say in one line, before the edit, that the edit makes the approval stale, that `## Goal` (or `## Threat Model`, when the brief has one): say in one line, before the edit, that the edit makes the approval stale, that
`/trekplan` halts until the new text is approved, and how to approve it `/trekplan` halts until the new text is approved, and how to approve it
(`/trekbrief --approve <project dir>`). Then edit. (`/trekbrief --approve <project dir>`). Then edit.
- A plan the person approved: any edit gives it a new sha256, the approval log no longer covers it, - A plan the person approved: any edit gives it a new sha256, the approval log no longer covers it,
@ -84,7 +84,7 @@ approval.
- `tests/lib/doc-consistency.test.mjs` and `tests/lib/skills-annotating-prose-pins.test.mjs` (in - `tests/lib/doc-consistency.test.mjs` and `tests/lib/skills-annotating-prose-pins.test.mjs` (in
`npm test`): the script `scripts/annotate.mjs` exists, the three producing commands call it, print `npm test`): the script `scripts/annotate.mjs` exists, the three producing commands call it, print
the `file://` link and the `open` line, and name this skill. Deterministic, but only at test time. the `file://` link and the `open` line, and name this skill. Deterministic, but only at test time.
- `lib/validators/intent-approval.mjs` (inside `/trekplan`): an edit to `## Intent` or `## Goal` - `lib/validators/intent-approval.mjs` (inside `/trekplan`): an edit to `## Intent` or `## Goal` (or `## Threat Model`)
after approval makes the approval stale, and planning halts until the new text is approved. after approval makes the approval stale, and planning halts until the new text is approved.
- `lib/util/approval-log.mjs` (inside `/trekexecute`, K2): a plan whose current sha256 the approval - `lib/util/approval-log.mjs` (inside `/trekexecute`, K2): a plan whose current sha256 the approval
log does not cover is refused. Both checks hold whether or not the edit's cost was said. log does not cover is refused. Both checks hold whether or not the edit's cost was said.

View file

@ -84,6 +84,12 @@ for (const rel of AGENTS) {
assert.ok(t.includes('`rule_key` names the class, never the instance'), `${rel} must say rule_key names the class, never the instance`); assert.ok(t.includes('`rule_key` names the class, never the instance'), `${rel} must say rule_key names the class, never the instance`);
}); });
test(`${rel}: round 2+ marks each known_outside entry by id in previous too (E fix round)`, () => {
const t = norm(read(rel));
assert.match(t, /`## Critic Verdict` also lists `known_outside`[^.]*\. Mark each of them by id in `previous` the same way/);
assert.match(t, /`previous` \(from round 2; omit it in round 1\) holds one `\{id, resolved\}` per finding the current `## Critic Verdict` lists, its `known_outside` entries included/);
});
test(`${rel}: round 2+ lists every previous finding by id and cites the exact line`, () => { test(`${rel}: round 2+ lists every previous finding by id and cites the exact line`, () => {
const t = norm(read(rel)); 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('From round 2 the prompt gives you the diff since the last reviewed text'), `${rel}: round-2 diff sentence missing`);

View file

@ -49,6 +49,10 @@ test('Phase 9 hands the brief to plan-critic and --brief to --record; the threat
assert.match(p9, /`threat_model`/); assert.match(p9, /`threat_model`/);
assert.match(p9, /PLAN_GATE_KNOWN_OUTSIDE/); assert.match(p9, /PLAN_GATE_KNOWN_OUTSIDE/);
assert.match(p9, /PLAN_GATE_OUTSIDE_NOT_BACKED/); assert.match(p9, /PLAN_GATE_OUTSIDE_NOT_BACKED/);
// Fix round: both round-2 sentences name the known_outside entries to mark.
const round2 = p9.split('\n').filter(l => l.startsWith('"Previous findings are in the plan\'s `## Critic Verdict`'));
assert.equal(round2.length, 2, 'one round-2 sentence per reviewer');
for (const l of round2) assert.match(l, /its `known_outside` entries included/);
}); });
test('Phase 10 runs the plan gate before offering "execute", and covers plan mode', () => { test('Phase 10 runs the plan gate before offering "execute", and covers plan mode', () => {

View file

@ -520,6 +520,16 @@ test('a null in a reviewer\'s findings list no longer crashes dedup (the S160 kn
assert.equal(r.findings[0].rule_key, 'PC1'); assert.equal(r.findings[0].rule_key, 'PC1');
}); });
test('only null and undefined are skipped: a string, a number or an array entry stays a finding with no valid severity, as before E', () => {
const one = dedupFindings([{ agent: 'plan-critic', payload: { findings: [null, undefined, 'BLOCKER: rm -rf on an unquoted path'] } }]);
assert.equal(one.findings.length, 1, JSON.stringify(one.findings));
assert.equal(one.findings[0].severity, null, 'no valid severity: the plan gate counts it as a blocker');
// Several such entries carry no text, so the Jaccard pass merges them, as it did before E.
const many = dedupFindings([{ agent: 'plan-critic', payload: { findings: [7, ['x'], 'str'] } }]);
assert.ok(many.findings.length >= 1, JSON.stringify(many.findings));
for (const f of many.findings) assert.equal(f.severity, null);
});
test('CLI --stdin: a null finding no longer crashes it (exit 0)', () => { test('CLI --stdin: a null finding no longer crashes it (exit 0)', () => {
const r = spawnSync(process.execPath, [SHIM, '--stdin'], { const r = spawnSync(process.execPath, [SHIM, '--stdin'], {
input: JSON.stringify({ plan_critic: { findings: [null] }, scope_guardian: { findings: [] } }), input: JSON.stringify({ plan_critic: { findings: [null] }, scope_guardian: { findings: [] } }),

View file

@ -28,6 +28,7 @@ import { validateReviewContent } from '../../lib/validators/review-validator.mjs
import { stampIntentApproval } from '../../lib/validators/intent-approval.mjs'; import { stampIntentApproval } from '../../lib/validators/intent-approval.mjs';
import { appendApproval, normalizeHex, normalizeSubject, resolveRepo } from '../../lib/util/approval-log.mjs'; import { appendApproval, normalizeHex, normalizeSubject, resolveRepo } from '../../lib/util/approval-log.mjs';
import { childEnv } from '../helpers/child-env.mjs'; import { childEnv } from '../helpers/child-env.mjs';
import { releaseCheck } from '../../scripts/release-check.mjs';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const FINALIZE = join(ROOT, 'lib', 'review', 'review-finalize.mjs'); const FINALIZE = join(ROOT, 'lib', 'review', 'review-finalize.mjs');
@ -287,6 +288,58 @@ test('gate: a backed entry turns not backed once the brief\'s approval is stale
assert.match(JSON.parse(g2.stdout).reason, /BRIEF_INTENT_APPROVAL_STALE/); assert.match(JSON.parse(g2.stdout).reason, /BRIEF_INTENT_APPROVAL_STALE/);
}); });
// ---- Part 2 fix round ---------------------------------------------------------------
test('fix 7: an engine finding the coordinator listed in known_outside while its payload is saved goes back once', () => {
const fx = tmRepo();
const engineFinding = {
id: 'c'.repeat(40), severity: 'BLOCKER', rule_key: 'SECURITY_INJECTION', file: 'commands/up.md', line: 5,
brief_ref: 'NFR — security (Claude Code /security-review)', title: 'Unquoted name reaches rm',
detail: 'rm -rf "$NAME" with a name from the request.', recommended_action: 'Quote and contain the name.', engine: 'security-review',
};
writeFileSync(join(fx.project, '.review-engine-payload-security-review.json'),
JSON.stringify({ reviewer: 'claude-security-review', findings: [engineFinding], out_of_scope: [] }));
const { id: _id, ...asListed } = engineFinding;
fx.write(tmReview({ start: fx.start, end: fx.end, known: [{ ...asListed, threat_model_ref: OUTSIDE }] }));
const r = fx.finalize();
assert.equal(r.status, 0, r.stdout + r.stderr);
const json = jsonOf(fx.read());
const at5 = json.findings.filter((f) => f.file === 'commands/up.md' && f.line === 5);
assert.equal(at5.length, 1, `put back once: ${JSON.stringify(json.findings.map((f) => [f.id, f.rule_key, f.engine]))}`);
assert.equal(at5[0].severity, 'BLOCKER');
assert.deepEqual(json.known_outside ?? [], []);
assert.equal(json.verdict, 'BLOCK');
const v = fx.validate();
assert.equal(v.status, 0, v.stdout + v.stderr);
});
test('fix 8: scripts/release-check.mjs exits 2 on a review whose known_outside entry the brief does not back', () => {
const fx = tmRepo();
fx.write(tmReview({ start: fx.start, end: fx.end, known: [entry({ threat_model_ref: INSIDE })] }));
const r = releaseCheck({ repo: fx.repo, review: fx.reviewPath, version: '1.0.0', suiteTimeoutMs: 30000 });
assert.equal(r.exit, 2, JSON.stringify(r.checks));
const rev = r.checks.find((c) => c.id === 'review');
assert.equal(rev.status, 'error');
assert.match(rev.detail, /threat model/i);
// Control: the same review with a backed entry passes the review check.
fx.write(tmReview({ start: fx.start, end: fx.end, known: [entry()] }));
const c = releaseCheck({ repo: fx.repo, review: fx.reviewPath, version: '1.0.0', suiteTimeoutMs: 30000 });
assert.equal(c.checks.find((x) => x.id === 'review').status, 'pass', JSON.stringify(c.checks));
});
test('fix 5: the approval prose says a ## Threat Model is covered too, when the brief has one', () => {
const line = (rel, needle) => read(rel).split('\n').find((l) => l.includes(needle)) ?? '';
assert.match(line('docs/command-modes.md', '| `--approve <project-dir'), /`## Threat Model`/);
assert.match(line('docs/HANDOVER-CONTRACTS.md', '| `intent_approved_hash` |'), /`## Threat Model`/);
assert.match(line('CLAUDE.md', '| `/trekbrief` |'), /`## Threat Model`/);
assert.match(line('README.md', '**Approval, logged and committed.**'), /`## Threat Model`/);
assert.match(line('README.md', '| **Approve** |'), /`## Threat Model`/);
assert.match(line('README.md', '**Intent approval gate.**'), /`## Threat Model`/);
const skill = read('skills/annotating/SKILL.md');
assert.match(skill.split('\n').find((l) => l.startsWith('description:')), /## Threat Model/);
assert.equal((flat(skill).match(/`## Intent` or `## Goal` \(or `## Threat Model`/g) ?? []).length, 2, 'the Process step and the guard list');
});
// ---- prose -------------------------------------------------------------------------- // ---- prose --------------------------------------------------------------------------
function subsection(text, heading) { function subsection(text, heading) {

View file

@ -23,7 +23,7 @@ import { test } from 'node:test';
import { strict as assert } from 'node:assert'; import { strict as assert } from 'node:assert';
import { resolve, join } from 'node:path'; import { resolve, join } from 'node:path';
import { spawnSync } from 'node:child_process'; import { spawnSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, readFileSync, rmSync, realpathSync } from 'node:fs'; import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, realpathSync } from 'node:fs';
import { tmpdir } from 'node:os'; import { tmpdir } from 'node:os';
import { fileURLToPath } from 'node:url'; import { fileURLToPath } from 'node:url';
import { gatePlan, recordVerdict, planHash, operativeText, extractVerdict } from '../../lib/validators/plan-gate.mjs'; import { gatePlan, recordVerdict, planHash, operativeText, extractVerdict } from '../../lib/validators/plan-gate.mjs';
@ -122,9 +122,10 @@ test('record: a backed security finding leaves the counted set and goes to known
assert.deepEqual(v.findings, []); assert.deepEqual(v.findings, []);
assert.equal(v.known_outside?.length, 1, JSON.stringify(v)); assert.equal(v.known_outside?.length, 1, JSON.stringify(v));
const e = v.known_outside[0]; const e = v.known_outside[0];
assert.deepEqual(Object.keys(e).sort(), ['id', 'line', 'rule_key', 'text']); assert.deepEqual(Object.keys(e).sort(), ['id', 'line', 'rule_key', 'rule_keys', 'text']);
assert.match(e.id, /^[0-9a-f]{40}$/); assert.match(e.id, /^[0-9a-f]{40}$/);
assert.equal(e.rule_key, 'SECURITY_RISK'); assert.equal(e.rule_key, 'SECURITY_RISK');
assert.deepEqual(e.rule_keys, ['SECURITY_RISK']);
assert.equal(e.text, S1); assert.equal(e.text, S1);
assert.equal(e.line, OUTSIDE); assert.equal(e.line, OUTSIDE);
assert.equal(v.brief_path, 'brief.md'); assert.equal(v.brief_path, 'brief.md');
@ -246,7 +247,7 @@ test('gate guard: a planted known_outside entry that is not a Voyage security fi
assert.equal(r.verdict.known_outside?.length, 1, 'round 1 lists the backed finding'); assert.equal(r.verdict.known_outside?.length, 1, 'round 1 lists the backed finding');
const planted = r.text.replace(/```json\n[\s\S]*?\n```/, () => { const planted = r.text.replace(/```json\n[\s\S]*?\n```/, () => {
const v = extractVerdict(r.text).verdict; const v = extractVerdict(r.text).verdict;
return '```json\n' + JSON.stringify({ ...v, known_outside: [{ ...v.known_outside[0], rule_key: 'MISSING_TEST' }] }, null, 2) + '\n```'; return '```json\n' + JSON.stringify({ ...v, known_outside: [{ ...v.known_outside[0], rule_key: 'MISSING_TEST', rule_keys: ['MISSING_TEST'] }] }, null, 2) + '\n```';
}); });
assert.equal(extractVerdict(planted).error, null); assert.equal(extractVerdict(planted).error, null);
const g = gatePlan(planted, { threatModel: TM }); const g = gatePlan(planted, { threatModel: TM });
@ -313,6 +314,35 @@ test('CLI: --record --brief lists a backed finding as a known issue; the gate wa
} }
}); });
test('fix 6: brief_path is stored relative to the repo top level, and the gate run from a subdirectory reads the same brief', () => {
const dir = realpathSync(mkdtempSync(join(tmpdir(), 'plan-gate-tm-')));
try {
assert.equal(spawnSync('git', ['init', '-q'], { cwd: dir, env: childEnv(GIT_CLEAN) }).status, 0);
const b = join(dir, 'brief.md');
const s = stampIntentApproval(BRIEF, new Date(NOW));
writeFileSync(b, s.text);
const repo = resolveRepo(dir);
assert.equal(appendApproval({ repo, kind: 'intent', subject: normalizeSubject(repo, b), sha: normalizeHex(s.hash) }).appended, true);
const sub = join(dir, 'sub');
mkdirSync(sub);
const p = join(dir, 'plan.md');
for (const [cwd, briefArg] of [[dir, 'brief.md'], [sub, '../brief.md'], [sub, b]]) {
writeFileSync(p, buildPlan());
const t1 = readFileSync(p, 'utf-8');
const payload = dd({ pc: [{ ...f('blocker', at(t1, R1_CHANGES), S1, 'SECURITY_RISK', { threat_model_ref: OUTSIDE }), file: p }] });
const c1 = run(cwd, ['--record', p, '--brief', briefArg, '--reviewed', planHash(t1)], JSON.stringify(payload));
assert.equal(c1.code, 0, c1.out + c1.err);
assert.equal(lastJson(c1.out).known_outside, 1, `${cwd} ${briefArg}: ${c1.out}`);
assert.equal(extractVerdict(readFileSync(p, 'utf-8')).verdict.brief_path, 'brief.md', `stored relative to the top level (${briefArg})`);
const g = run(sub, [p]);
assert.equal(g.code, 0, `the gate from a subdirectory: ${g.out}`);
assert.match(g.out, /WARN {2}\[PLAN_GATE_KNOWN_OUTSIDE\]/);
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
test('CLI: --brief without a value is a usage error; a threat model that is not usable is said in the output', () => { test('CLI: --brief without a value is a usage error; a threat model that is not usable is said in the output', () => {
const dir = realpathSync(mkdtempSync(join(tmpdir(), 'plan-gate-tm-'))); const dir = realpathSync(mkdtempSync(join(tmpdir(), 'plan-gate-tm-')));
try { try {
@ -337,6 +367,73 @@ test('CLI: --brief without a value is a usage error; a threat model that is not
} }
}); });
// ---- Part 2 fix round ---------------------------------------------------------------
test('fix 1: a string entry in a reviewer\'s findings still counts as a blocker through dedup and --record', () => {
const t1 = buildPlan();
const d = dedupFindings([
{ agent: 'plan-critic', payload: { findings: ['BLOCKER: step 2 runs rm -rf on an unquoted user path'] } },
{ agent: 'scope-guardian', payload: { findings: [] } },
]);
assert.equal(d.findings.length, 1, 'the string entry is kept as a finding with no valid severity');
const r = rec(t1, d);
assert.equal(r.verdict.counts.blocker, 1);
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', () => {
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 })],
pcPrev: [{ id, resolved: false }], sgPrev: [{ id, resolved: false }],
}), { base: baseOf(r1.text), threatModel: TM, briefPath: 'brief.md' });
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);
});
test('fix 3: a merged finding whose security key is only in rule_keys is backed at record and in the bare gate alike', () => {
const t1 = buildPlan();
const l = at(t1, R1_CHANGES);
const d = dd({
pc: [f('blocker', l, S1, 'injection/shell', { threat_model_ref: OUTSIDE })],
sg: [f('blocker', l, S1, 'SECURITY_RISK', { threat_model_ref: OUTSIDE })],
});
assert.equal(d.findings.length, 1, 'dedup merged the two');
const r = rec(t1, d, { threatModel: TM, briefPath: 'brief.md' });
assert.equal(r.verdict.known_outside?.length, 1);
assert.deepEqual(r.verdict.known_outside[0].rule_keys, ['injection/shell', 'SECURITY_RISK']);
const g = gatePlan(r.text, { threatModel: TM });
assert.equal(g.code, 0, JSON.stringify(g.reasons));
assert.ok(g.warnings.some(w => w.code === 'PLAN_GATE_KNOWN_OUTSIDE'), JSON.stringify(g.warnings));
// extractVerdict accepts rule_keys and refuses a malformed one.
const v = extractVerdict(r.text).verdict;
const bad = r.text.replace(/```json\n[\s\S]*?\n```/, () => '```json\n' + JSON.stringify({ ...v, known_outside: [{ ...v.known_outside[0], rule_keys: [7] }] }, null, 2) + '\n```');
assert.ok(extractVerdict(bad).error, 'a rule_keys that is not a list of strings is malformed');
});
test('fix 4: a reviewer\'s mark on a known_outside id passes dedup and the count, and resolved: true removes the entry (diff mode and full count)', () => {
const r1 = backedRound1();
const id = r1.verdict.known_outside?.[0]?.id;
assert.ok(id, 'round 1 lists the backed finding');
const t2 = editA(r1.text);
const d = dd({ pcPrev: [{ id, resolved: true }], sgPrev: [{ id, resolved: true }] });
assert.deepEqual(d.marks, [{ agent: 'plan-critic', id, resolved: true }, { agent: 'scope-guardian', id, resolved: true }]);
for (const opts of [{ base: baseOf(r1.text) }, { baseNote: 'no snapshot; full counting' }]) {
const r2 = recordVerdict(t2, d, { reviewed: planHash(t2), planPaths: PATHS, now: NOW, ...opts });
assert.equal(r2.code, 0, r2.message);
assert.equal(r2.verdict.known_outside, undefined, JSON.stringify(opts));
}
});
// ---- Handover 4 --------------------------------------------------------------------- // ---- Handover 4 ---------------------------------------------------------------------
test('Handover 4 names --brief, known_outside, brief_path and the two new codes', () => { test('Handover 4 names --brief, known_outside, brief_path and the two new codes', () => {