open/voyage
1
0
Fork 0
Code Issues Releases 13 Activity

merge: wave 1 order 5 (review) into main

Browse source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-06 20:35:45 +02:00
commit 32dafb585f
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
29 changed files with 2615 additions and 112 deletions
Download patch file Download diff file Expand all files Collapse all files

9
CLAUDE.md
View file

@ -30,7 +30,7 @@ Voyage — a contract-driven Claude Code pipeline: brief, research, plan, execut
| `/trekresearch` | Research — deep local + external research, produces structured research brief. Opt-in `--engine {swarm\|deep-research}` delegates the external phase to Claude Code's built-in `/deep-research` workflow (swarm default) | 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 | session |
| `/trekexecute` | Execute — disciplined plan/session-spec executor with failure recovery | session |
| `/trekreview` | Review — independent post-hoc review of delivered code against the brief. Produces `review.md` with severity-tagged findings (Handover 6) | session |
| `/trekreview` | Review — a gate: Claude Code's `/code-review` + `/security-review` as engine, then Voyage's brief control (which stops); writes `review.md` with a required verdict that `lib/review/verdict-gate.mjs` decides on (0 ALLOW/WARN · 1 BLOCK · 2 never green); proposes recurring finding classes as project memory, line by line (Handover 6) | session |
| `/trekcontinue` | Continue — resumes the next session of a multi-session voyage project. Reads `.session-state.local.json` (Handover 7) and immediately begins executing | session |
| `/trekendsession` | End-session — mark the current session complete and write session-state pointing at the next session. Helper for informal multi-session flows | session |
@ -52,6 +52,13 @@ Full flag reference for each command (modes, `--gates`, `--profile`, breaking ch
> pre-registered measurement in `docs/storm-measurement.md` — decline is a
> no-op, adopt is one constant.
> **Review is a gate (review_version 1.1).** `verdict:` is required in
> `review.md` (K7); exit 2 from the verdict gate is never green. No finding is
> struck for length — only MINOR/SUGGESTION text is trimmed, and counted.
> Project-memory lines are written only between Voyage's markers in the target
> repo's `.claude/rules/voyage-review-lessons.md`, each one operator-approved.
> Contract → `docs/HANDOVER-CONTRACTS.md` §Handover 6.
## Agents
| Agent | Model | Role |

39
README.md
View file

@ -410,13 +410,33 @@ and a contracted **Handover 6 (review → plan)** so findings can be fed back in
`/trekplan --brief review.md` to produce a remediation plan — closing
the iteration loop without ad-hoc conventions.
**The review is a gate.** In this order: Claude Code's own `/code-review` and
`/security-review` find correctness and security defects (started through the
Skill tool, never with `--fix`); Voyage's control against the brief runs after
them and is the part that stops. The verdict is written in `review.md`
(`verdict: ALLOW | WARN | BLOCK`, required) and one command decides on it:
```bash
node lib/review/verdict-gate.mjs <dir>/review.md
# 0 = ALLOW or WARN · 1 = BLOCK · 2 = could not decide (never green)
```
A WARN that exists only because a check did not run is a 2, not a 0: a
review that was not performed in full stops a merge.
No finding is struck for length: only MINOR/SUGGESTION text is trimmed, and
counted; what the coordinator set aside is listed under `## Declined to
Judge`. An engine that could not run (older Claude Code, no `origin` remote)
keeps the review from coming back `ALLOW` — and from passing the gate — and
the summary says why.
### Modes
| Mode | Command | Description |
|------|---------|-------------|
| **Default** | `/trekreview --project <dir>` | brief-conformance + code-correctness reviewers in parallel, coordinator dedup + verdict, write `{dir}/review.md` |
| **Default** | `/trekreview --project <dir>` | Claude Code `/code-review` + `/security-review`, then brief-conformance + code-correctness reviewers in parallel, coordinator dedup + verdict, write `{dir}/review.md`, verdict gate |
| **Since ref** | `/trekreview --project <dir> --since <ref>` | Override "before" SHA for the diff range. Validated via `git rev-parse --verify` |
| **Quick** | `/trekreview --project <dir> --quick` | Skip brief-conformance reviewer; skip coordinator's reasonableness filter — fast correctness-only pass |
| **Quick** | `/trekreview --project <dir> --quick` | `/code-review` + code-correctness reviewer only; skip brief-conformance reviewer, `/security-review` and the coordinator's reasonableness filter — fast correctness-only pass |
| **Validate** | `/trekreview --project <dir> --validate` | Schema-only check on existing `review.md`. No LLM calls |
| **Dry run** | `/trekreview --project <dir> --dry-run` | Print discovered scope + triage map; skip writes |
| **Profile** | `/trekreview --profile <name> --project <dir>` | (v4.1.0) Pin model profile for the review phase. See [Profile system](#profile-system-v410). |
@ -430,7 +450,18 @@ the iteration loop without ad-hoc conventions.
- `## Findings (MINOR)` — nice-to-fix
- `## Findings (SUGGESTION)` — opinion-only
Required body sections: `## Executive Summary`, `## Coverage`, `## Remediation Summary`. The Coverage section enumerates which files were deep-reviewed, summary-only, or skipped (with reason) — explicit triage to avoid silent skips.
Required body sections: `## Executive Summary`, `## Coverage`, `## Remediation Summary`. The Coverage section enumerates which files were deep-reviewed, summary-only, or skipped (with reason) — explicit triage to avoid silent skips. Every BLOCKER is also written as a numbered plan step (`### Step R1: …` with Files, Source finding, Test first, Verify) under `## Remediation Steps`, ready to append to a plan.
### Project memory (recurring findings)
A finding class (the same rule) that shows up in two reviews in your repo
becomes one proposed line — **mistake · fix · how to check** — with both
findings as evidence. Mechanical classes name a check; judgement classes go
to `.claude/rules/voyage-review-lessons.md`, which Claude Code loads at
session start; `CLAUDE.md` only gets a one-line pointer. You approve every
line (Voyage asks). Voyage writes only between its own markers and stops at a
size budget — past it, the lessons belong in a skill, and that is your call.
The history is the `review.md` files your repo already has; no new store.
### Triage gate (deterministic)
@ -444,7 +475,7 @@ A path-pattern classifier produces `{file → deep-review|summary-only|skip}` be
/trekplan --brief <dir>/review.md
# → plan.md with `source_findings: [<id>, ...]` audit trail
# (BLOCKER + MAJOR findings become plan goals; MINOR + SUGGESTION skipped for v1.0)
# (BLOCKER + MAJOR findings become plan goals; MINOR + SUGGESTION skipped)
```
The plan's optional `source_findings:` frontmatter list is the audit trail back to consumed findings. See `docs/HANDOVER-CONTRACTS.md` for the full Handover 6 contract.

76
agents/review-coordinator.md
View file

@ -46,7 +46,18 @@ trailing JSON block) ready to write to disk.
You will receive a prompt containing:
- **Reviewer outputs** — JSON-block payloads from
`brief-conformance-reviewer` and `code-correctness-reviewer` (in `quick`
mode, only the latter).
mode, only the latter), plus the engine payloads from Claude Code's own
review: `claude-code-review` (`/code-review`) and `claude-security-review`
(`/security-review`, not in `quick`). Engine findings carry the catalogue
keys `CODE_REVIEW_DEFECT` / `SECURITY_REVIEW_VULNERABILITY` (or a narrower
key) and are treated exactly like any reviewer's findings.
- **Expected reviewers** — the names that were due, engines included, and
each engine recorded as not run with its reason. A name that did not report
forbids `ALLOW` (Pass 4).
- **Engine findings outside the review scope** — the `out_of_scope` list from
`lib/review/engine-findings.mjs`. Each one is `unverified`
(`engine:out-of-scope`): a real claim on a file this review did not cover.
List it under `## Declined to Judge`; it forbids `ALLOW`.
- **Triage map** — `{file → deep-review|summary-only|skip, reason}` from
the /trekreview triage gate.
- **Brief metadata** — `task`, `slug`, `project_dir`, `brief_path` from
@ -89,7 +100,7 @@ is load-bearing** — see *Suppression is two-valued* below:
| Filter | Test | Fails if | Removed as |
|--------|------|----------|------------|
| Succinctness | `title.length ≤ 100` and `detail.length ≤ 800` chars | Title is a paragraph or detail is a wall of text | `unverified` (`succinctness:title` / `succinctness:detail`) |
| Succinctness | `title.length ≤ 100` and `detail.length ≤ 800` chars | Title is a paragraph or detail is a wall of text | **never removed.** MINOR/SUGGESTION (catalogue tier): the text is trimmed (ending `[…trimmed]`), the finding is kept and counted. BLOCKER/MAJOR: kept whole, every character. |
| Accuracy | `file` resolves under the repo root AND `line` is plausible (≥ 0; ≤ file line count when known) | Path traversal escape, negative line, or impossibly large line number | **dropped** (`accuracy:refuted`) |
| Actionability | `recommended_action` is non-empty AND begins with an imperative verb | Empty action, "consider …" hedges, or restating the title | `unverified` (`actionability:empty`) |
@ -98,6 +109,15 @@ examines the claim, so neither can establish the finding is unreal. Accuracy
does: a citation that escapes the repo root refutes the finding as a claim
about this codebase.
**No finding is struck for length** (ordre 5). Before review_version 1.1 an
over-long `detail` sent a finding to `unverified`, and S118's most serious
finding fell out of the Handover 6 list that way. Length is packaging, never
a claim: only small findings are trimmed, and the trims are counted
(`trimmed` in the JSON block). The trim decision reads the **catalogue** tier,
so a mislabelled MAJOR is never cut. Borrowed: "report at most five nits …
summarize the rest as a count" — cap only the nits (ai-native-sdlc
`assets/REVIEW.md:7-23`, MIT).
When removing a finding, preserve a one-line note in the
`Suppressed Findings` body section so the user knows why the count
shrank.
@ -136,7 +156,7 @@ reasonableness filtering is bypassed. To prevent unknown rule_keys
from polluting downstream plans, the coordinator MUST substitute any
rule_key not exported from `lib/review/rule-catalogue.mjs:RULE_KEYS`
with the literal string `PLAN_EXECUTE_DRIFT` (the most general drift
category from the 12-entry catalogue). The original rule_key is
category in the catalogue). The original rule_key is
preserved in the finding's `original_rule_key` field for diagnostic
purposes. This normalization happens BEFORE writing review.md,
ensuring all `rule_key` values in the final review match the
@ -186,12 +206,17 @@ BLOCKER.
**When `ALLOW` is withheld, the Executive Summary's FIRST sentence must say
so and name why** — e.g. "WARN: no blocking findings survived, but 1 finding
could not be verified (succinctness:title) and brief-conformance-reviewer did
not report." A withheld ALLOW that the reader cannot see is the same defect
could not be verified (actionability:empty) and claude-security-review did
not run (no origin remote)." A withheld ALLOW that the reader cannot see is the same defect
in a new place.
Verdict is mechanical — never override. The verdict goes into the
trailing JSON block AND the Executive Summary's first sentence.
frontmatter `verdict:` field (REQUIRED, K7), the trailing JSON block AND the
Executive Summary's first sentence — the same value in all three.
`lib/review/verdict-gate.mjs` reads the frontmatter and refuses (exit 2,
never green) a file whose JSON block or BLOCKER findings contradict it, and
a WARN whose `allow_blocked_by` names a reviewer or engine that did not run.
Always write `allow_blocked_by` (an empty list when nothing withheld ALLOW).
## Output: review.md content
@ -203,7 +228,7 @@ Produce the full review.md content as your output. The
```yaml
---
type: trekreview
review_version: "1.0"
review_version: "1.1"
created: {YYYY-MM-DD}
task: "{from brief frontmatter}"
slug: {from brief frontmatter}
@ -212,6 +237,7 @@ brief_path: {brief_path from input}
scope_sha_start: {scope_sha_start or null if mtime fallback}
scope_sha_end: {scope_sha_end}
reviewed_files_count: {N}
verdict: {BLOCK | WARN | ALLOW}
findings:
- {finding-id-1-40-char-hex}
- {finding-id-2-40-char-hex}
@ -219,7 +245,9 @@ findings:
```
The `findings:` field MUST use block-style YAML (one ID per line, ` - `
prefix). Flow-style `findings: [a, b]` breaks the frontmatter parser.
prefix). Flow-style `findings: [a, b]` breaks the frontmatter parser. With
zero findings write `findings: []` — the empty list is the one inline form
the parser reads; a bare `findings:` is null and fails validation.
### Body sections (in order)
@ -240,7 +268,27 @@ prefix). Flow-style `findings: [a, b]` breaks the frontmatter parser.
Pass 2 or Pass 3, with the reason AND its fate, tagged `[dropped]` or
`[unverified]`. Unverified lines come first: they are the ones that
withheld `ALLOW`.
9. `## Remediation Summary` — bullet count per severity + 1 sentence on
9. `## Declined to Judge` — REQUIRED. One line per finding you set aside
without ruling on it (every `[unverified]` one: `file:line rule_key —
reason`), then one line `Trimmed: N` (MINOR/SUGGESTION text cut, findings
kept). Write `None.` plus `Trimmed: 0` when empty. Borrowed: "Declined to
judge … nothing you set aside is dropped silently" (Superpowers
`skills/requesting-code-review/code-reviewer.md:43-48`, MIT).
10. `## Remediation Steps` — REQUIRED when there is at least one BLOCKER. One
numbered step per BLOCKER, in severity-then-file order, written so
Handover 6 can append it to a plan unchanged:
```
### Step R1: {recommended_action, imperative}
- **Files:** `{file}`
- **Source finding:** `{40-char id}`
- **Test first:** {a test that reproduces the finding and fails today}
- **Verify:** {command that proves it fixed; default: re-run /trekreview → finding `{id}` absent}
- **On failure:** escalate — the finding needs the operator, not improvisation
```
Steps are appended, numbered and never reworded across iterations
(Spec Kit `/converge`, MIT). The strict validator fails a BLOCKER without
its step (`REVIEW_MISSING_REMEDIATION_STEP`).
11. `## Remediation Summary` — bullet count per severity + 1 sentence on
what /trekplan will consume.
Each Findings subsection uses the `### {finding-id-40-char-hex}` heading
@ -261,7 +309,9 @@ The LAST fenced block in the file is a `json` block:
{
"verdict": "BLOCK | WARN | ALLOW",
"counts": { "BLOCKER": N, "MAJOR": N, "MINOR": N, "SUGGESTION": N },
"allow_blocked_by": ["unverified:succinctness:title (1)", "missing-reviewer:brief-conformance-reviewer", "unattributable-payload (1)"],
"allow_blocked_by": ["unverified:actionability:empty (1)", "missing-reviewer:claude-security-review", "unattributable-payload (1)"],
"declined_to_judge": [{ "file": "lib/x.mjs", "line": 3, "rule_key": "MISSING_TEST", "reason": "actionability:empty" }],
"trimmed": 0,
"findings": [
{
"id": "<40-char-hex>",
@ -303,8 +353,10 @@ for the ID list.
`findings:` list and the JSON block IDs must match.
- **Suppressed findings are accountable.** When you remove a finding via
Pass 2 or Pass 3, log it in `## Suppressed Findings` with the reason and
its fate (`[dropped]` / `[unverified]`). Silent drops break the audit
trail.
its fate (`[dropped]` / `[unverified]`), and list every `[unverified]` one
again under `## Declined to Judge`. Silent drops break the audit trail.
- **No finding is struck for length.** Trim only MINOR/SUGGESTION text and
count it; a long BLOCKER or MAJOR keeps every character.
- **Never spend an unexamined finding as evidence of a clean review.** If a
removal did not refute the finding, it is `unverified` and `ALLOW` is off
the table. This is the one place where you may not be minimal: when in

44
agents/review-orchestrator.md
View file

@ -11,10 +11,12 @@ tools: ["Read", "Glob", "Grep", "Write", "Edit", "Bash"]
Orchestrator Phase 2 = Command Phase 2 (Validate brief)
Orchestrator Phase 3 = Command Phase 3 (Discover scope SHA range)
Orchestrator Phase 4 = Command Phase 4 (Triage gate — path classifier)
Orchestrator Phase 4.7 = Command Phase 4.7 (Claude Code engine: /code-review, /security-review)
Orchestrator Phase 5 = Command Phase 5 (Parallel reviewers)
Orchestrator Phase 6 = Command Phase 6 (Coordinator dedup + verdict)
Orchestrator Phase 7 = Command Phase 7 (Write review.md)
Orchestrator Phase 8 = Command Phase 8 (Validate + stats)
Orchestrator Phase 8 = Command Phase 8 (Validate + verdict gate + stats)
Orchestrator Phase 8.7 = Command Phase 8.7 (Project memory: recurring finding classes)
As of v3.2.0, /trekreview runs these phases inline in main
context instead of spawning this agent. Keep this file as the canonical
reference for what those phases do. -->
@ -135,6 +137,18 @@ Record the treatment for every file. Files marked `skip` MUST appear in
the Coverage section of review.md — never silently drop them. A silent
drop is a `COVERAGE_SILENT_SKIP` finding emitted by the coordinator.
### Phase 4.7 — Claude Code's review engine
Start Claude Code's own `/code-review` (target: the review's SHA range,
level `high`) and `/security-review` through the Skill tool — never with
`--fix`, `--comment`, `--post` or `ultra`. Wait for both to deliver (a
background `/code-review` delivers later in the conversation), transcribe
their findings, and convert each with `lib/review/engine-findings.mjs` into a
reviewer payload (`claude-code-review`, `claude-security-review`). An engine
that did not run stays in the expected reviewer set and forbids `ALLOW`. The
Phase 5 reviewers never see the engine payloads. Full procedure:
`commands/trekreview.md` § Phase 4.7.
### Phase 5 — Launch parallel reviewers
Launch **two reviewer agents in parallel** using the Agent tool — one
@ -163,20 +177,24 @@ review.md but it is structural, not behavioral.
### Phase 6 — Coordinator dedup + verdict
Launch `review-coordinator` with the merged findings array from Phase 5.
Launch `review-coordinator` with the merged findings array from Phase 5
plus the Phase 4.7 engine payloads and the expected reviewer set.
The coordinator runs a 4-pass process:
1. **Dedup** by `(file, line, rule_key)` triplet — keep highest severity.
2. **HubSpot Judge filters** — drop findings failing Succinctness,
Accuracy, or Actionability.
2. **HubSpot Judge filters** — Accuracy refutes; Actionability sets aside
as unverified; Succinctness never removes a finding (only MINOR/SUGGESTION
text is trimmed, and counted).
3. **Cloudflare reasonableness** — drop speculative findings without a
`file:line` citation; drop findings whose `rule_key` is not in
`RULE_CATALOGUE`.
4. **Compute verdict** — `BLOCK` if `BLOCKER ≥ 1`, `WARN` if `MAJOR ≥ 1`,
else `ALLOW`.
The coordinator's output is the full review.md content — frontmatter +
body sections + trailing JSON block — ready to write.
The coordinator's output is the full review.md content — frontmatter
(with the required `verdict:`) + body sections (`## Declined to Judge`,
and `## Remediation Steps` with one numbered step per BLOCKER) + trailing
JSON block — ready to write.
In `quick` mode, skip pass 3 (reasonableness filter). Passes 1, 2, 4
still run.
@ -205,11 +223,25 @@ If validation fails, repair the file (most failures are fixable in place
finding-ID). Do NOT proceed if any REVIEW_REQUIRED_FRONTMATTER field is
missing.
Then decide on the verdict with the gate (K7):
```
node ${CLAUDE_PLUGIN_ROOT}/lib/review/verdict-gate.mjs {project_dir}/review.md
```
0 = ALLOW/WARN, 1 = BLOCK, 2 = could not decide (never green).
Append a stats line to `${CLAUDE_PLUGIN_DATA}/trekreview-stats.jsonl`:
```json
{"ts":"...","slug":"...","verdict":"BLOCK|WARN|ALLOW","counts":{"BLOCKER":N,"MAJOR":N,"MINOR":N,"SUGGESTION":N},"reviewed_files_count":N,"mode":"default|quick|validate|dry-run","duration_ms":N}
```
### Phase 8.7 — Project memory
`lib/review/recurrence.mjs propose` finds finding classes seen in two
review.md files in the repo; the operator approves each proposed line
(AskUserQuestion) before `recurrence.mjs apply` writes it between Voyage's
markers in `.claude/rules/voyage-review-lessons.md`. Full procedure:
`commands/trekreview.md` § Phase 8.7.
## Hard rules
- **Currently runs inline, not in background.** This orchestrator file is

262
commands/trekreview.md
View file

@ -5,7 +5,7 @@ description: |
review.md with severity-tagged findings (BLOCKER/MAJOR/MINOR/SUGGESTION)
per Handover 6 (review → plan).
argument-hint: "--project <dir> [--since <ref>] [--quick] [--validate] [--dry-run]"
allowed-tools: Agent, Read, Glob, Grep, Write, Edit, Bash, AskUserQuestion
allowed-tools: Agent, Read, Glob, Grep, Write, Edit, Bash, AskUserQuestion, Skill
---
# Ultrareview Local v1.0
@ -29,6 +29,17 @@ The review is **independent**: each reviewer runs without cross-feeding,
and the coordinator applies BOUNDED operations only. Synthesis-level
inference across files is forbidden in v1.0 (Judge Agent pattern).
**The review is a gate (review_version 1.1, ordre 5).** In this order:
Claude Code's own `/code-review` and `/security-review` are the engine
(Phase 4.7); Voyage's control against the brief runs after them and is the
part that stops (Phase 5–6); the verdict is written in `review.md` and one
command decides on it (`lib/review/verdict-gate.mjs`, Phase 8: 0 = ALLOW/WARN,
1 = BLOCK, 2 = could not decide, never green). No finding is struck for
length. A finding class seen in two reviews becomes one proposal for the
project's memory, approved line by line (Phase 8.7). Order credited to Boris
Cherny's public workflow (engine → brief control); the gate to Kiro and Spec
Kit — see `docs/HANDOVER-CONTRACTS.md` § Handover 6.
See `agents/review-orchestrator.md` for the canonical workflow this
command executes inline.
@ -150,9 +161,18 @@ The combined file list is the review scope. Note that the
Assumptions section declares this is allowed; the review surfaces it
explicitly in the Coverage table.
If the file count is `0`, write a one-line review.md noting "No diff
between {before_sha} and {after_sha}; nothing to review." Verdict: ALLOW.
Skip Phases 4–7. Continue to Phase 8 (validate + stats).
If the file count is `0`, write a minimal review.md per the template:
`verdict: ALLOW`, `findings: []` (the empty list is the one inline form the
parser reads — a bare `findings:` is null and fails validation), the
required sections (`## Executive Summary`, `## Coverage`, `## Declined to
Judge` with `None.` and `Trimmed: 0`, `## Remediation Summary`), each saying
"No diff between {before_sha} and {after_sha}; nothing to review.", and the
trailing block `{"verdict": "ALLOW", "counts": {"BLOCKER": 0, "MAJOR": 0,
"MINOR": 0, "SUGGESTION": 0}, "allow_blocked_by": [], "findings": []}` —
without it the verdict gate
cannot back the ALLOW and exits 2. There is nothing for the Phase 4.7 engine
to review, so its absence does not withhold ALLOW here. Skip Phases 4–7.
Continue to Phase 8 (validate + stats).
## Phase 4 — Triage gate (deterministic path-pattern classifier)
@ -253,6 +273,81 @@ prompt in Phase 5, and its summary line goes into the Coverage section of
built by `formatCriteriaEvidence` precisely so the orchestrator cannot narrate a
pass that never happened.
## Phase 4.7 — Claude Code's review engine (`/code-review`, `/security-review`)
**Skipped in `dry-run` and `validate`.** In `quick` mode only `/code-review`
runs. Wrap, not a new engine: Claude Code's built-in reviews find the
correctness and security defects; Voyage's brief control (Phase 5) runs after
them and is what stops.
Verified in Claude Code's live documentation (2026-10-06):
- Claude may start `/code-review` itself (`code-review.md`: "Claude can start
`/code-review` on its own", unconditionally from **v2.1.246**) and
`/security-review` (`skills.md`: "A few built-in commands are also available
through the Skill tool, including `/init` and `/security-review`").
- `/code-review` runs as a background subagent by default and its findings
"arrive in your conversation when the review completes". It accepts a target
such as a ref range.
- `/security-review` "Reviews the diff between your branch and origin's
default branch" and "Needs an `origin` remote". The docs do not state its
output format or whether it runs in the background.
Run both through the **Skill tool**, one after the other:
1. `Skill("code-review", args: "high {before_sha}...{after_sha}")` — the
review's own SHA range from Phase 3. `high` is set explicitly; note in the
Executive Summary that this also becomes the level later `/code-review`
runs in this session reuse.
2. `Skill("security-review")` — no arguments; its base is fixed.
**Never** pass `--fix`, `--comment`, `--post` or `ultra`: a review never edits
code, never posts anywhere, and never uploads the repo to a cloud review.
**Wait for the findings.** A background `/code-review` delivers its findings
later in this conversation. Do not start Phase 6 before both engines have
delivered or have been recorded as not run. For each engine that delivered,
transcribe every finding — file, line (0 when none), the one-sentence summary,
the failure scenario, the category if given, and a narrower catalogue
`rule_key` only when one plainly fits — into
`{project_dir}/.review-engine-{code-review|security-review}.json` as
`{"engine": "...", "findings": [...]}`. Transcribe; never re-judge, merge or
drop an engine finding on the way. Then convert it:
```bash
git diff --name-only "$before_sha".."$after_sha" > "{project_dir}/.review-scope.txt"
git diff --name-only HEAD >> "{project_dir}/.review-scope.txt"
node ${CLAUDE_PLUGIN_ROOT}/lib/review/engine-findings.mjs \
--scope "{project_dir}/.review-scope.txt" --repo-root "$(git rev-parse --show-toplevel)" \
"{project_dir}/.review-engine-code-review.json"
```
Exit 0 prints a reviewer payload (`claude-code-review` /
`claude-security-review`) for Phase 6 plus `out_of_scope`: engine findings on
files outside the review's SHA range (`/security-review` diffs against
origin's default branch, which can be wider). Pass `out_of_scope` to the
coordinator as well: each one is `unverified` (`engine:out-of-scope`), is
listed under `## Declined to Judge` and in Coverage, and forbids `ALLOW` —
never silently dropped. Absolute paths under the repo root are made relative
before the scope match (`--repo-root`). Exit 1 = the transcription is
malformed, or the scope list is empty: fix it from the engine's own text (or
the scope from Phase 3) and re-run; never feed a payload that did not pass.
Exit 2 = could not run — record that engine as not run.
A "narrower" `rule_key` you transcribe is used only when its catalogue tier is
at least the engine's own (MAJOR for `/code-review`, BLOCKER for
`/security-review`); the adapter refuses to let a transcription lower an
engine finding.
**An engine that did not run** (Claude Code older than v2.1.246, the Skill
call refused, no `origin` remote, an `ambiguous argument` error, or no findings
ever delivered) is recorded as not run with its reason. It stays in the
expected reviewer set, so it forbids `ALLOW` (fail-closed); the Executive
Summary's first sentence names it. An engine that ran and found nothing
delivers an empty payload — that is a report, not an absence.
The two engine payloads are NOT shown to the Phase 5 reviewers: reviewers stay
independent, and the coordinator is the only place the outputs meet.
## Phase 5 — Launch parallel reviewers
**Substrate routing (opt-in `--workflow`).** When `workflow_substrate == true`, run
@ -322,8 +417,11 @@ Summary will note the brief-conformance pass was skipped.
### Reviewer accounting — every expected reviewer MUST report
Write down the expected reviewer set BEFORE the spawn: both reviewers in
default mode, `code-correctness-reviewer` alone in `quick` mode. After the
spawn, account for each one by name.
default mode, `code-correctness-reviewer` alone in `quick` mode — plus, in
every mode, the Phase 4.7 engines that were due (`claude-code-review`, and
`claude-security-review` outside `quick`). After the spawn, account for each
one by name. An engine recorded as not run is passed on as missing, never
left out of the set.
**Zero findings from a silent reviewer is indistinguishable from zero findings
from a clean diff** — unless you check. A reviewer is *accounted for* only when
@ -353,13 +451,18 @@ forbids `ALLOW` (`lib/review/coordinator-contract.mjs`, `missing_reviewers`).
## Phase 6 — Coordinator dedup + verdict
Launch `review-coordinator` (Agent tool) with the merged findings array
from Phase 5 plus the triage map, brief metadata, and SHA range.
from Phase 5 AND the Phase 4.7 engine payloads, plus the triage map, brief
metadata, SHA range, the expected reviewer set (engines included), and each
not-run engine with its reason.
The coordinator runs the 4-pass process documented in
`agents/review-coordinator.md`:
1. **Dedup** by `(file, line, rule_key)` triplet.
2. **HubSpot Judge filters** — Succinctness, Accuracy, Actionability.
Succinctness never removes a finding: only MINOR/SUGGESTION text is
trimmed, and every trim is counted. A long BLOCKER or MAJOR keeps every
character.
3. **Cloudflare reasonableness** — remove speculative or catalogue-violating
findings (skipped in `quick` mode).
4. **Verdict** — BLOCK / WARN / ALLOW per the threshold table.
@ -374,9 +477,10 @@ vocabulary, and the `allow_blocked_by` field: `agents/review-coordinator.md`
§*Suppression is two-valued*, mirrored deterministically in
`lib/review/coordinator-contract.mjs`.
The coordinator's output is the full review.md content — frontmatter +
body sections + trailing JSON block. Do NOT re-run the reviewers based
on the coordinator's output.
The coordinator's output is the full review.md content — frontmatter
(with the REQUIRED `verdict:`) + body sections (including `## Declined to
Judge` and, when there is a BLOCKER, `## Remediation Steps`) + trailing JSON
block. Do NOT re-run the reviewers based on the coordinator's output.
## Phase 5–6 via the Workflow substrate (opt-in `--workflow`)
@ -398,11 +502,16 @@ Workflow({
briefPath: "{brief_path}",
diffPath: "{path to the unified diff file from Phase 3}",
triage: "{triage map as 'path → treatment' lines from Phase 4}",
quick: {true if mode == quick, else false}
quick: {true if mode == quick, else false},
enginePayloads: [{the Phase 4.7 payloads from engine-findings.mjs}],
engineNotRun: [{each engine reviewer name recorded as not run}]
}
})
```
A reviewer that returns nothing, or an engine listed in `engineNotRun`,
forbids `ALLOW` on this path too (`missing_reviewers` in the result).
Contract (verified in S10 part B — follow exactly):
- **Pass `args` as a JSON object.** The script defensively re-parses a JSON *string*, but
the object form is the contract.
@ -439,6 +548,18 @@ write to a temp file, then rename. The frontmatter `findings:` field
must use **block-style YAML** (one ID per line, ` - ` prefix). The
parser at `lib/util/frontmatter.mjs` does not support flow-style arrays.
**The verdict is in the file (K7).** The frontmatter carries
`verdict: ALLOW | WARN | BLOCK`, equal to the trailing JSON block's
`verdict`. It is required at write time; a review without it fails Phase 8.
**A BLOCKER is written as a plan step.** For every BLOCKER the body carries,
under `## Remediation Steps`, one numbered step `### Step R<n>: <imperative
action>` with `**Files:**`, `**Source finding:** <40-char id>`, `**Test
first:**` (a test that reproduces the finding and fails today), `**Verify:**`
(the command that proves it fixed; default: re-run `/trekreview` and the
finding is absent) and `**On failure:** escalate`. Handover 6 can append these
to a plan without rewording them; earlier steps are never rewritten.
If `mode == dry-run`: skip the write; print the would-be path and the
first 60 lines of the rendered output.
@ -455,10 +576,34 @@ If validation fails:
finding-ID, REVIEW_VERSION_FORMAT warning): repair in place — re-emit
the missing section, recompute the finding-ID, fix the version
string. Re-validate.
- `REVIEW_MISSING_VERDICT` / `REVIEW_MISSING_REMEDIATION_STEP`: repair
from the coordinator's own output — copy the JSON block's `verdict` into the
frontmatter; write the missing `### Step R<n>` from that BLOCKER's
`recommended_action`, `file` and `id`. Never invent a verdict the JSON block
does not carry. Re-validate.
- For unrepairable errors (REVIEW_WRONG_TYPE, malformed frontmatter):
stop and ask the user to re-run; do not silently produce an invalid
review.md.
**Decide on the verdict with the gate (K7).** Once the validator passes:
```bash
node ${CLAUDE_PLUGIN_ROOT}/lib/review/verdict-gate.mjs "{review_path}"
```
Exit 0 = ALLOW or WARN; exit 1 = BLOCK; exit 2 = the gate could not decide
(missing verdict, a verdict the file contradicts — in its JSON block, its
`## Findings (BLOCKER)` section or its finding-id list — a JSON block that is
not the last fence or has the wrong shape, two verdict lines, unreadable
file) — **never green**. A WARN that exists because a reviewer or engine
did not run (`missing-reviewer:` / `unattributable-payload` in
`allow_blocked_by`) is also exit 2: a review not performed in full stops a
merge (operator decision 2026-10-06). It takes exactly one file. On exit 0
with WARN, `--json` also returns `warn_reasons` (the JSON block's
`allow_blocked_by`); print them. Capture the exit code as `gate_exit` and print it in Phase 9 exactly
as returned. This is the command that later stops a merge or a release on
BLOCK; `/trekreview` reports it and never overrides it.
Append a stats line to `${CLAUDE_PLUGIN_DATA}/trekreview-stats.jsonl`
(create the file if it does not exist):
@ -488,8 +633,76 @@ HTML is a convenience, not a gate.
When `mode == validate`:
1. Skip Phases 3–7 entirely.
2. Run the strict validator on `{project_dir}/review.md`.
3. Print a one-line PASS/FAIL summary plus the JSON output on FAIL.
4. Exit 0 on PASS, 1 on FAIL. Never write to disk. Never call any agent.
3. Print a one-line PASS/FAIL summary plus the JSON output on FAIL. A
`review_version` 1.0 review without `verdict:` FAILS here (K7 is a
write-time rule); it still seeds a plan, because `/trekplan` reads in soft
mode.
4. Run `lib/review/verdict-gate.mjs` on the same file and print its line and
exit code.
5. Exit 0 on PASS, 1 on FAIL. Never write to disk. Never call any agent.
Skip Phase 8.7.
## Phase 8.7 — The project's memory (recurring finding classes)
**Skipped in `dry-run` and `validate`.** A finding class (the same catalogue
`rule_key`) that shows up in two review.md files in this repo becomes ONE
proposal; one sighting is no proposal. The history is the review.md files the
repo already has (every `.claude/projects/*/review.md`, plus earlier committed
versions of them) — no new store.
```bash
node ${CLAUDE_PLUGIN_ROOT}/lib/review/recurrence.mjs propose --repo "$PWD"
```
`--repo` is the session's working directory — the directory `project_dir`
paths are relative to, and the project Claude Code loads `.claude/rules/`
from. Not the git toplevel: for a project in a subdirectory that would read
no reviews and write the lessons in the wrong place.
It prints `{"proposals": [...]}` (exit 0) or exits 2 (could not run — say so
in the summary; never read 2 as "nothing recurred"). Each proposal names its
`rule_key`, its route and its evidence (the finding IDs from two reviews), and
carries one line in the format **mistake · fix · how to check**:
- route `check` (mechanical) — the line names the deterministic check that
catches the class;
- route `rule` (judgement) — the line is for the repo's `.claude/rules/`
file, which Claude Code loads at session start;
- review-hygiene classes are never proposed.
**The operator approves every line.** For each proposal, use
`AskUserQuestion` (up to 4 proposals per call), in plain language, showing
the line and its two pieces of evidence. Options: **"Skriv linja inn"**
(recommended: the class has recurred) and **"Ikke nå"**; the operator may
answer with an edited line through "Other". Never write a line the operator
did not approve, and never treat "sounds good" or "do as you like" as a yes
to a line it was not shown.
On yes:
```bash
node ${CLAUDE_PLUGIN_ROOT}/lib/review/recurrence.mjs apply --repo "$PWD" \
--rule-key <RULE_KEY> [--line "<the operator's edited line>"]
```
It writes ONLY between Voyage's own markers in
`.claude/rules/voyage-review-lessons.md` and, if the repo has a `CLAUDE.md`,
gives it a one-line pointer once (never the lesson itself). Exit 1 with
`over-budget` means the managed block is full: tell the operator the lessons
belong in a skill now — that is the operator's decision, not this command's.
Exit 1 with `invalid-line` means the line was refused by the schema check
(missing one of the three parts, a line break or control character, marker
text, or over 700 characters) — show the `detail` and ask again. Exit 1 with
`markers-damaged` means Voyage's markers in the lessons file are not one
start before one end: say so; the operator fixes the file by hand. A
finding class only counts when two reviews hold two DIFFERENT findings of it
— one unfixed finding seen again is a persisting defect, not a lesson — and
the two catch-all engine keys are never proposed. Report every outcome in the summary; when no class
recurred, say "no recurring finding class" — do not pad.
Borrowed: routing (Pocock `retro`), the three-part line (ai-native-sdlc),
the managed block (Spec Kit `agent-context`), evidence per proposal and
schema-check before write (Kiro), review → memory and the size budget
(Cherny). Credits in `lib/review/recurrence.mjs`.
## Phase 9 — Present summary
@ -506,12 +719,20 @@ After the write succeeds, print:
**Annotation HTML:** file://{$ANNOT_HTML}
**Scope:** {before_sha}..{after_sha} ({reviewed_files_count} files)
**Verdict:** {BLOCK | WARN | ALLOW}
**Gate:** verdict-gate exit {gate_exit} ({0 = ALLOW/WARN | 1 = BLOCK | 2 = could not decide, or a control did not run — not green})
**Engine:** /code-review {delivered N | not run: reason} · /security-review {delivered N | not run: reason}
### Counts
- BLOCKER: {N}
- MAJOR: {N}
- MINOR: {N}
- SUGGESTION: {N}
- Trimmed (MINOR/SUGGESTION text only, never removed): {N}
- Declined to judge: {N} — {one line each: file:line rule_key — reason}
### Project memory
{each proposal: rule_key → written | declined by operator | over-budget | invalid-line;
or "no recurring finding class"; or "recurrence check could not run"}
### Top findings
- [{severity}] {title} ({file}:{line})
@ -613,7 +834,7 @@ unknown `rule_key` values from polluting downstream remediation plans
(Handover 6), the coordinator applies its v5.1.1 high-effort
normalization rule — substituting unknown `rule_key` values with the
literal string `PLAN_EXECUTE_DRIFT` (the most general drift category
in the 12-entry catalogue) and preserving the original in
in the catalogue) and preserving the original in
`original_rule_key`. See `agents/review-coordinator.md` § Pass 3
"High-effort normalization (v5.1.1)" for the full normalization spec.
@ -652,4 +873,15 @@ Low effort: skip the brief-conformance reviewer entirely (existing
- **Honesty.** If the diff is trivially small or all-skip, say so. Do
not pad findings to make the review look thorough.
- **No production code.** This command never runs production code, never
writes to anything outside `{project_dir}` and `${CLAUDE_PLUGIN_DATA}`.
writes to anything outside `{project_dir}` and `${CLAUDE_PLUGIN_DATA}` —
with one exception, only after the operator approved that exact line in
Phase 8.7: Voyage's own marked block in `.claude/rules/voyage-review-lessons.md`
and a one-line pointer in `CLAUDE.md`.
- **The engine never edits code.** `/code-review` and `/security-review` run
without `--fix`, `--comment`, `--post` or `ultra`. A review that changes
code is not a review.
- **No finding is struck for length.** Only MINOR/SUGGESTION text is trimmed,
and every trim is counted; what the coordinator set aside is listed under
`## Declined to Judge`.
- **The verdict is in the file, and a command decides on it.** `verdict:` is
required (K7); `lib/review/verdict-gate.mjs` exit 2 is never green.

24
docs/HANDOVER-CONTRACTS.md
View file

@ -14,7 +14,7 @@ Each artifact carries an explicit version field. Schema bumps are coordinated:
| `research/*.md` | (implicit; tracked via `type: trekresearch-brief`) | unversioned |
| `plan.md` | `plan_version` (plan header line) | `1.7` |
| `progress.json` | `schema_version` (top-level) | `"1"` |
| `review.md` | `review_version` (frontmatter) | `1.0` |
| `review.md` | `review_version` (frontmatter) | `1.1` |
| `.session-state.local.json` | `schema_version` (top-level) | `1` (number) |
## Breaking-change protocol
@ -334,7 +334,9 @@ The strongest validation in the entire pipeline. Phase 5.5 (planning-orchestrato
**Producer:** `/trekreview` Phase 7 (write `review.md` after coordinator dedup + verdict).
**Consumer:** `/trekplan` Phase 1 when `--brief review.md` is supplied and the consumer detects `type: trekreview` in frontmatter. The plan command branches into a remediation-plan path: BLOCKER + MAJOR findings become plan goals, the produced `plan.md` carries a `source_findings: [<id>, ...]` frontmatter list as the audit trail back to the consumed findings. MINOR + SUGGESTION are skipped for v1.0 plan-input.
**Consumer:** `/trekplan` Phase 1 when `--brief review.md` is supplied and the consumer detects `type: trekreview` in frontmatter. The plan command branches into a remediation-plan path: BLOCKER + MAJOR findings become plan goals, the produced `plan.md` carries a `source_findings: [<id>, ...]` frontmatter list as the audit trail back to the consumed findings. MINOR + SUGGESTION are skipped for plan-input. Since 1.1 every BLOCKER is also written as a numbered `### Step R<n>` under `## Remediation Steps` (Files · Source finding · Test first · Verify · On failure), so a consumer can append it as a plan step without rewording it.
**Second consumer — the verdict gate (K7, 1.1):** `lib/review/verdict-gate.mjs <review.md>` reads the frontmatter `verdict` and exits 0 = ALLOW/WARN, 1 = BLOCK, 2 = could not decide. Exit 2 is never green: a missing file, a missing or unknown verdict, two verdict lines, a trailing JSON block that is not the last fence or lacks `verdict`/`counts`/`findings`, and a file whose JSON block, `## Findings (BLOCKER)` section or finding-id list contradicts a greener verdict all land there — and so does a WARN that exists because a reviewer or engine did not run (`missing-reviewer:` / `unattributable-payload` in the JSON block's `allow_blocked_by`, which a 1.1 review must carry): a review not performed in full stops a merge (operator decision 2026-10-06). Exactly one file per call. Whatever stops a merge or a release on a review reads this exit code, never a summary. Borrowed: the verdict goes to a file and a command gates on it (Kiro `feature-pipeline`); the executor reads the verdict but never writes it (Spec Kit).
**Path conventions:**
- Project-dir mode (recommended): `{project_dir}/review.md` (one per review iteration; subsequent runs overwrite atomically).
@ -352,10 +354,10 @@ The strongest validation in the entire pipeline. Phase 5.5 (planning-orchestrato
| `brief_path` | string | yes | path to consumed `brief.md` | Audit trail back to brief |
| `scope_sha_end` | string | yes | git sha of HEAD at review time | Defines "after" boundary |
| `reviewed_files_count` | number | yes | ≥ 0 | From triage gate Coverage |
| `findings` | list | yes | block-style YAML list of 40-char hex IDs | Flat array; full objects in body |
| `findings` | list | yes | block-style YAML list of 40-char hex IDs; `[]` when empty | Flat array; full objects in body. A bare `findings:` is null and fails |
| `verdict` | string | yes (1.1; strict at write, warning at read) | `BLOCK \| WARN \| ALLOW` | Coordinator output; equals the trailing JSON block's `verdict` (K7) |
| `created` | date | optional | YYYY-MM-DD | |
| `scope_sha_start` | string | optional | git sha at review start | `null` if mtime fallback used |
| `verdict` | string | optional | `BLOCK \| WARN \| ALLOW` | Coordinator output |
`findings:` is a flat array of finding-IDs (40-char hex from `lib/parsers/finding-id.mjs`). The full finding objects (severity, location, message, evidence, fix) live in the body as `### <id>` subsections under per-severity `## Findings (...)` headings — same pattern as brief-reviewer to avoid frontmatter-parser fragility on lists of dicts.
@ -364,7 +366,12 @@ The strongest validation in the entire pipeline. Phase 5.5 (planning-orchestrato
- `## Coverage`
- `## Remediation Summary`
Optional but standard sections: `## Findings (BLOCKER)`, `## Findings (MAJOR)`, `## Findings (MINOR)`, `## Findings (SUGGESTION)`. The `## Coverage` section enumerates which files were deep-reviewed, summary-only, or skipped (with reason) — this is how the triage gate stays honest and avoids Copilot-style silent skips.
Optional but standard sections: `## Findings (BLOCKER)`, `## Findings (MAJOR)`, `## Findings (MINOR)`, `## Findings (SUGGESTION)`. The `## Coverage` section enumerates which files were deep-reviewed, summary-only, or skipped (with reason) — this is how the triage gate stays honest and avoids Copilot-style silent skips; engine findings on files outside the review's SHA range are listed there too.
Since 1.1 (ordre 5):
- `## Remediation Steps` — required at `review_version` ≥ 1.1 when there is a BLOCKER: one `### Step R<n>` per BLOCKER carrying `Source finding: <id>` (strict error `REVIEW_MISSING_REMEDIATION_STEP`, soft warning). Steps are appended and numbered, never reworded across iterations (Spec Kit `/converge`).
- `## Declined to Judge` — required at `review_version` ≥ 1.1 (strict error, soft warning; 1.0 reviews exempt): every finding set aside without a ruling, with its reason (including engine findings outside the review scope, `engine:out-of-scope`), plus `Trimmed: N`. **No finding is struck for length**: only MINOR/SUGGESTION text is trimmed, and counted (`trimmed` in the JSON block); a long BLOCKER/MAJOR keeps every character.
- Findings from Claude Code's own `/code-review` and `/security-review` enter under the catalogue keys `CODE_REVIEW_DEFECT` (MAJOR) and `SECURITY_REVIEW_VULNERABILITY` (BLOCKER), or a narrower key, with `reviewer` `claude-code-review` / `claude-security-review`.
**Validation strategy:**
@ -374,12 +381,14 @@ Optional but standard sections: `## Findings (BLOCKER)`, `## Findings (MAJOR)`,
| Required fields | every read | All `REVIEW_REQUIRED_FRONTMATTER` present |
| Type discriminator | every read | `type === "trekreview"` |
| Findings shape | every read | Array of strings, each matching `^[0-9a-f]{40}$` |
| Verdict present | strict: error · soft: warning | `verdict` ∈ `ALLOW \| WARN \| BLOCK`; an out-of-enum value is an error in both modes |
| Remediation step per BLOCKER | strict: error · soft: warning | `### Step R<n>` with `Source finding: <id>` under `## Remediation Steps` |
| Body sections | strict only | `Executive Summary`, `Coverage`, `Remediation Summary` |
| Version format | every read | `review_version` matches `N.M`; warning otherwise |
The validator (`lib/validators/review-validator.mjs`) exposes the same CLI as the others: `node lib/validators/review-validator.mjs --json <review.md>`. Strict mode is the default; `--soft` downgrades section-missing errors to warnings. `/trekreview` Phase 8 runs `--strict`. `/trekplan` Phase 1 (when consuming `--brief review.md`) runs `--soft` so a partially-valid review can still seed a plan.
**Versioning:** current is `1.0`. There are no live `0.x` reviews. Future schema changes follow the breaking-change protocol above.
**Versioning:** current is `1.1` (ordre 5: `verdict` required, remediation steps, Declined to Judge, two engine rule keys). N-1 window: a `1.0` review without `verdict` still validates in soft mode (warning) and still seeds a plan; it fails strict mode and the verdict gate (exit 2), because a missing verdict is never read as ALLOW. Future schema changes follow the breaking-change protocol above.
**Failure modes:**
- `REVIEW_NOT_FOUND` → consumer halts with usage message
@ -391,6 +400,9 @@ The validator (`lib/validators/review-validator.mjs`) exposes the same CLI as th
- `REVIEW_BAD_FINDING_ID` → an ID is not 40-char hex; halt
- `REVIEW_MISSING_SECTION` → strict halt; soft-mode warning
- `REVIEW_VERSION_FORMAT` → warning only; review_version not in `N.M` form
- `REVIEW_MISSING_VERDICT` → strict halt; soft-mode warning (1.0 reviews)
- `REVIEW_BAD_VERDICT` → halt; verdict outside `ALLOW | WARN | BLOCK`
- `REVIEW_MISSING_REMEDIATION_STEP` → strict halt; soft-mode warning
---

3
docs/architecture.md
View file

@ -8,7 +8,8 @@ Imported from `CLAUDE.md` via pointer.
- `lib/util/{frontmatter,result,atomic-write,autonomy-gate}.mjs` — shared YAML-frontmatter parser + Result helpers + `atomicWriteJson(path, obj)` for tmp+rename writes + autonomy-gate state machine (v3.4.0)
- `lib/parsers/{plan-schema,manifest-yaml,project-discovery,arg-parser,bash-normalize,jaccard,finding-id}.mjs` — pure parsers (no I/O), unit-tested. `manifest-yaml` extended in v3.4.0 with additive `skip_commit_check` + `memory_write` flags (forward-compat: unknown keys ignored)
- `lib/review/{rule-catalogue,plan-review-dedup}.mjs` — version-pinned rule catalogue (12 keys) + Phase 9 inline dedup helpers (v3.4.0)
- `lib/review/{rule-catalogue,plan-review-dedup}.mjs` — version-pinned rule catalogue (14 keys) + Phase 9 inline dedup helpers (v3.4.0)
- `lib/review/{verdict-gate,engine-findings,recurrence}.mjs` — the review gate (K7: 0 ALLOW/WARN · 1 BLOCK · 2 never green), Claude Code `/code-review` + `/security-review` findings → reviewer payload, recurring finding classes → operator-approved project-memory lines (review_version 1.1)
- `lib/stats/event-emit.mjs` — single-source stats event emitter for autonomy-gate transitions and main-merge-gate (v3.4.0)
- `lib/validators/{brief,research,plan,progress,session-state}-validator.mjs` — schema validators with CLI shims (`node lib/validators/X.mjs --json <path>`)
- `lib/validators/architecture-discovery.mjs` — drift-WARN external-contract discovery for `architecture/overview.md`

6
docs/command-modes.md
View file

@ -85,11 +85,11 @@ If `{project_dir}/architecture/overview.md` exists (typically produced by an opt
| Flag | Behavior |
|------|----------|
| _(default)_ | Run brief-conformance + code-correctness reviewers in parallel, coordinator dedup + verdict, write `{project_dir}/review.md` |
| _(default)_ | Claude Code `/code-review` + `/security-review` (engine, via the Skill tool, never `--fix`), then brief-conformance + code-correctness reviewers in parallel, coordinator dedup + verdict, write `{project_dir}/review.md` with the required `verdict:`, decide with `lib/review/verdict-gate.mjs` (0 ALLOW/WARN · 1 BLOCK · 2 never green), propose recurring finding classes as project memory (operator approves each line) |
| `--project <dir>` | **Required.** Path to trekplan project folder containing `brief.md`. Review is written to `{dir}/review.md` |
| `--since <ref>` | Override "before" SHA for the diff range. Validated via `git rev-parse --verify` |
| `--quick` | Skip brief-conformance reviewer; skip coordinator's reasonableness filter — fast correctness-only pass |
| `--validate` | Schema-only check on existing `{dir}/review.md`. No LLM calls |
| `--quick` | `/code-review` + code-correctness reviewer only; skip brief-conformance reviewer, `/security-review` and the coordinator's reasonableness filter — fast correctness-only pass |
| `--validate` | Schema-only check on existing `{dir}/review.md` plus the verdict gate. No LLM calls. A 1.0 review without `verdict:` fails here (it still seeds a plan in soft mode) |
| `--dry-run` | Print discovered scope + triage map; skip writes |
| `--fg` | No-op alias (foreground is default) |
| `--workflow` | (opt-in, NW2) Run Phase 5–6 on the bake-off-validated Workflow substrate (`scripts/trekreview-armB.workflow.mjs`) instead of the default prose path. Default stays prose; requires **Claude Code 2.1.154+** (raises the consumer floor — opt-in for portability). Fidelity-equivalent per `docs/T2-bakeoff-results.md` |

147
lib/review/coordinator-contract.mjs
View file

@ -18,7 +18,8 @@
//
// What IS implemented, purely: Pass 1 (triplet dedup → highest-severity-wins
// survivor + conformance tiebreak + detail concat + raised_by provenance),
// Pass 2 succinctness + actionability-presence, Pass 3 reasonableness
// Pass 2 succinctness (trims MINOR/SUGGESTION text, never removes) +
// actionability-presence, Pass 3 reasonableness
// (citation / unknown-rule_key suppression, severity-mismatch correction),
// Pass 4 verdict thresholds — fail-closed: a suppression that did not REFUTE
// the finding, and a reviewer that never reported, forbid ALLOW (see
@ -33,6 +34,8 @@
import { SEVERITY_VALUES, RULE_KEYS, getRule } from './rule-catalogue.mjs';
import { computeFindingId } from '../parsers/finding-id.mjs';
import { validateFindings } from './findings-schema.mjs';
import { realpathSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
export const JUDGE_TITLE_MAX = 100;
export const JUDGE_DETAIL_MAX = 800;
@ -64,15 +67,17 @@ export const REFUTING_REASONS = Object.freeze(new Set([
]));
/**
* The reason vocabulary on the unverified side. `file-existence:indeterminate`
* The reason vocabulary on the unverified side. Length is not in it: since
* ordre 5 (P11) succinctness trims MINOR/SUGGESTION text and never removes a
* finding, so `succinctness:*` is no longer emitted (a stray one still fails
* closed through `classifySuppression`). `file-existence:indeterminate`
* is emitted by the LLM coordinator's Pass 3 (which runs the fs Glob this
* module deliberately excludes); the vocabulary is owned here so prose and lib
* cannot drift.
*/
export const UNVERIFIED_REASONS = Object.freeze([
'succinctness:title',
'succinctness:detail',
'actionability:empty',
'engine:out-of-scope',
'unknown-rule_key',
'file-existence:indeterminate',
]);
@ -185,36 +190,91 @@ export function dedupByTriplet(findings) {
}
/**
* Pass 2 — HubSpot Judge (deterministic subset): drop on succinctness
* (title > 100 or detail > 800 chars) and actionability (recommended_action,
* when present, must be a non-empty string). The imperative-verb test is
* excluded (LLM judgement).
* The tiers whose over-long text may be trimmed. Length is packaging, never a
* claim: a BLOCKER or MAJOR keeps every character, and nothing is ever removed
* for length. Borrowed: "cap only the nits, and count what was cut"
* (ai-native-sdlc `assets/REVIEW.md:7-23`, MIT).
*/
export const TRIMMABLE_SEVERITIES = Object.freeze(['MINOR', 'SUGGESTION']);
export const TRIM_MARKER = ' […trimmed]';
/**
* The tier a finding will be judged at: the catalogue tier when the rule_key is
* known (Pass 3 resets to it anyway), else the reviewer-asserted severity. The
* trim decision reads this, so a reviewer cannot get a MAJOR trimmed by
* mislabelling it MINOR.
* @param {object} f
* @returns {string}
*/
function effectiveSeverity(f) {
return getRule(f.rule_key)?.severity ?? f.severity;
}
function trimText(text, max) {
return text.slice(0, max - TRIM_MARKER.length) + TRIM_MARKER;
}
/**
* Trim a detail but keep Pass 1's "Also flagged by …" provenance lines: the
* reviewer's own text is what gets cut. When the provenance alone is too long
* it is cut too — `raised_by` still names every reviewer.
*/
function trimDetail(detail, max) {
const at = detail.indexOf('\nAlso flagged by ');
if (at === -1) return trimText(detail, max);
const head = detail.slice(0, at);
const tail = detail.slice(at);
const room = max - tail.length - TRIM_MARKER.length;
if (room < 40) return trimText(detail, max);
return head.slice(0, room) + TRIM_MARKER + tail;
}
/**
* Pass 2 — HubSpot Judge (deterministic subset): succinctness and
* actionability (recommended_action, when present, must be a non-empty
* string). The imperative-verb test is excluded (LLM judgement).
*
* Both tests read a `.length`; neither examines the claim, so neither can
* establish the finding is unreal. Both therefore route to `unverified`.
* `dropped` stays in the signature for the refuting Pass-2 filter this subset
* excludes (Accuracy: a path-traversal escape IS a refutation).
* Succinctness never removes a finding (ordre 5, P11 — S118's most serious
* finding was struck for a long `detail`). A MINOR/SUGGESTION whose title is
* over 100 or detail over 800 characters is TRIMMED, kept, and returned in
* `trimmed` so the count is visible; a BLOCKER/MAJOR keeps its full text.
*
* Actionability reads a `.length`, never the claim, so it cannot establish the
* finding is unreal: it routes to `unverified`. `dropped` stays in the
* signature for the refuting Pass-2 filter this subset excludes (Accuracy: a
* path-traversal escape IS a refutation).
* @param {object[]} findings
* @returns {{ kept: object[], dropped: object[], unverified: object[] }}
* @returns {{ kept: object[], dropped: object[], unverified: object[], trimmed: object[] }}
*/
export function judgeFilter(findings) {
const kept = [];
const dropped = [];
const unverified = [];
const trimmed = [];
for (const f of findings) {
const titleLen = (f.title ?? '').length;
const detailLen = (f.detail ?? '').length;
let reason = null;
if (titleLen > JUDGE_TITLE_MAX) reason = 'succinctness:title';
else if (detailLen > JUDGE_DETAIL_MAX) reason = 'succinctness:detail';
else if ('recommended_action' in f &&
if ('recommended_action' in f &&
(typeof f.recommended_action !== 'string' || f.recommended_action.trim().length === 0)) {
reason = 'actionability:empty';
suppress(f, 'actionability:empty', dropped, unverified);
continue;
}
const title = f.title ?? '';
const detail = f.detail ?? '';
const longTitle = title.length > JUDGE_TITLE_MAX;
const longDetail = detail.length > JUDGE_DETAIL_MAX;
if ((longTitle || longDetail) && TRIMMABLE_SEVERITIES.includes(effectiveSeverity(f))) {
const out = { ...f };
const fields = [];
if (longTitle) { out.title = trimText(title, JUDGE_TITLE_MAX); fields.push('title'); }
if (longDetail) { out.detail = trimDetail(detail, JUDGE_DETAIL_MAX); fields.push('detail'); }
out.trimmed_fields = fields;
kept.push(out);
trimmed.push(out);
} else {
kept.push(f);
}
if (reason) suppress(f, reason, dropped, unverified);
else kept.push(f);
}
return { kept, dropped, unverified };
return { kept, dropped, unverified, trimmed };
}
/**
@ -313,15 +373,38 @@ export function computeVerdict(findings, options = {}) {
* forbids ALLOW. Do not iterate both and count twice.
*
* @param {Array<{reviewer?: string, findings: object[]}>} reviewerPayloads
* @param {{ expectedReviewers?: string[] }} [options]
* @returns {{ verdict: string, counts: Record<string, number>, findings: object[], suppressed: object[], unverified: object[], skipped: object[], missing_reviewers: string[], unattributable_payloads: number, allow_blocked_by: string[] }}
* @param {{ expectedReviewers?: string[], engineOutOfScope?: object[] }} [options]
* `declined_to_judge` lists every unverified finding with its reason, and
* `trimmed_count` counts the MINOR/SUGGESTION findings whose text was trimmed
* (kept, judged, and counted — never removed for length).
*
* @returns {{ verdict: string, counts: Record<string, number>, findings: object[], suppressed: object[], unverified: object[], skipped: object[], missing_reviewers: string[], unattributable_payloads: number, allow_blocked_by: string[], declined_to_judge: object[], trimmed_count: number }}
*/
export function runContract(reviewerPayloads, options = {}) {
const { findings: ingested, skipped } = ingest(reviewerPayloads);
const deduped = dedupByTriplet(ingested);
const judged = judgeFilter(deduped);
const reasoned = reasonablenessFilter(judged.kept);
const unverified = [...judged.unverified, ...reasoned.unverified];
// Engine findings on files outside the review's SHA range (engine-findings.mjs
// `out_of_scope`): real claims nobody here judged, so they forbid ALLOW.
const outOfScope = (options.engineOutOfScope ?? [])
.map((f) => ({ ...f, suppressed_reason: 'engine:out-of-scope' }));
const unverified = [...judged.unverified, ...reasoned.unverified, ...outOfScope];
// What the coordinator set aside without ruling on it, one line each, so a
// reader sees every finding that did not reach a verdict tier. Borrowed:
// "Declined to judge … nothing you set aside is dropped silently"
// (Superpowers `skills/requesting-code-review/code-reviewer.md:43-48`, MIT).
const declined_to_judge = unverified.map((f) => ({
file: f.file ?? null,
line: f.line ?? null,
rule_key: f.rule_key ?? null,
severity: f.severity ?? null,
title: f.title ?? null,
reason: f.suppressed_reason,
}));
// A trimmed finding that Pass 3 later removed is accounted for there, not here.
const keptSet = new Set(reasoned.kept.map((f) => tripletKey(f)));
const trimmed_count = judged.trimmed.filter((f) => keptSet.has(tripletKey(f))).length;
// A reviewer counts as REPORTED only when a payload carrying its name
// validated. `validateFindings` merely warns on a missing `reviewer`, so a
@ -360,12 +443,22 @@ export function runContract(reviewerPayloads, options = {}) {
missing_reviewers,
unattributable_payloads,
allow_blocked_by,
declined_to_judge,
trimmed_count,
};
}
// ---- CLI shim ----------------------------------------------------------------
if (import.meta.url === `file://${process.argv[1]}`) {
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
if (isMain()) {
const args = process.argv.slice(2);
const filePath = args.find((a) => !a.startsWith('--'));
if (!filePath) {

159
lib/review/engine-findings.mjs Normal file
View file

@ -0,0 +1,159 @@
// lib/review/engine-findings.mjs
// Claude Code's review as the engine (ordre 5, P11 — wrap, not a new engine).
//
// /trekreview Phase 4.7 starts Claude Code's own `/code-review` and
// `/security-review` through the Skill tool (never with `--fix`, never anything
// that edits code). Their findings come back as text: per finding a file
// location, a one-sentence summary, a failure scenario and an optional category
// slug (code.claude.com/docs/en/tools-reference.md, `ReportFindings`). The
// orchestrator transcribes them into a small JSON list; THIS module turns that
// list into a reviewer payload the coordinator already understands, so the
// engine's findings meet Voyage's brief control in the same dedup + verdict.
//
// Input (one engine per call):
// { "engine": "code-review" | "security-review",
// "findings": [ { "file": "...", "line": 12, "summary": "...",
// "failure_scenario": "...", "category": "correctness",
// "rule_key": "<optional narrower catalogue key>" } ] }
//
// Output: { reviewer, findings: [...catalogue-conformant...], out_of_scope: [...] }
// - rule_key: the given one when it is in the catalogue AND at least as
// severe as the engine's own key, else CODE_REVIEW_DEFECT (MAJOR) /
// SECURITY_REVIEW_VULNERABILITY (BLOCKER). A "narrower" key can never
// lower an engine finding's tier.
// - File paths are made repo-relative (an absolute path under `repoRoot`
// loses that prefix) before they are compared with the scope.
// - A finding on a file outside the review scope is NOT dropped silently: it
// lands in `out_of_scope`, which the coordinator counts as unverified
// (`engine:out-of-scope`) — it forbids ALLOW and is listed under
// Declined to Judge (`/security-review` diffs against origin's default
// branch, which can be wider than the review's SHA range).
// - An empty scope list is an error, never "everything is out of scope".
//
// CLI: node lib/review/engine-findings.mjs [--scope <file-list.txt>] [--repo-root <dir>] <engine-findings.json>
// exit 0 = payload on stdout; 1 = input invalid (errors on stderr); 2 = could not run.
import { readFileSync, existsSync, realpathSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { computeFindingId } from '../parsers/finding-id.mjs';
import { RULE_KEYS, getRule, SEVERITY_VALUES } from './rule-catalogue.mjs';
import { validateFindings } from './findings-schema.mjs';
export const ENGINES = Object.freeze({
'code-review': Object.freeze({
reviewer: 'claude-code-review',
rule_key: 'CODE_REVIEW_DEFECT',
brief_ref: 'NFR — code correctness (Claude Code /code-review)',
}),
'security-review': Object.freeze({
reviewer: 'claude-security-review',
rule_key: 'SECURITY_REVIEW_VULNERABILITY',
brief_ref: 'NFR — security (Claude Code /security-review)',
}),
});
function normPath(p, repoRoot) {
let out = String(p ?? '').trim().replace(/\\/g, '/');
if (repoRoot) {
const root = String(repoRoot).replace(/\\/g, '/').replace(/\/+$/, '') + '/';
if (out.startsWith(root)) out = out.slice(root.length);
}
return out.replace(/^(\.\/)+/, '');
}
function parseLine(v) {
if (Number.isInteger(v) && v >= 0) return v;
if (typeof v === 'string' && /^\d+$/.test(v.trim())) return Number(v.trim());
return 0;
}
const tier = (key) => SEVERITY_VALUES.indexOf(getRule(key).severity);
/**
* Convert one engine's transcribed findings into a coordinator payload.
* @param {{ engine: string, findings: object[] }} input
* @param {{ scopeFiles?: string[], repoRoot?: string }} [opts]
* @returns {{ ok: boolean, errors: string[], payload?: { reviewer: string, findings: object[] }, out_of_scope?: object[] }}
*/
export function toReviewerPayload(input, opts = {}) {
const errors = [];
const spec = ENGINES[input?.engine];
if (!spec) return { ok: false, errors: [`engine must be one of ${Object.keys(ENGINES).join('|')}, got ${JSON.stringify(input?.engine)}`] };
if (!Array.isArray(input.findings)) return { ok: false, errors: ['findings must be an array'] };
if (opts.scopeFiles && opts.scopeFiles.length === 0) {
return { ok: false, errors: ['scope list is empty — the review scope is unknown, so no engine finding can be placed'] };
}
const scope = opts.scopeFiles ? new Set(opts.scopeFiles.map((f) => normPath(f, opts.repoRoot))) : null;
const findings = [];
const out_of_scope = [];
input.findings.forEach((f, i) => {
const file = normPath(f?.file, opts.repoRoot);
const summary = String(f?.summary ?? '').trim();
if (!file) errors.push(`findings[${i}].file is empty`);
if (!summary) errors.push(`findings[${i}].summary is empty`);
if (!file || !summary) return;
const line = parseLine(f.line);
const narrower = typeof f.rule_key === 'string' && RULE_KEYS.has(f.rule_key)
&& tier(f.rule_key) <= tier(spec.rule_key);
const rule_key = narrower ? f.rule_key : spec.rule_key;
const out = {
id: computeFindingId(file, line, rule_key),
severity: getRule(rule_key).severity,
rule_key,
file,
line,
brief_ref: spec.brief_ref,
title: summary,
detail: [String(f.failure_scenario ?? '').trim(), f.category ? `(engine category: ${f.category})` : '']
.filter(Boolean).join(' ') || summary,
recommended_action: `Resolve: ${summary}`,
engine: input.engine,
};
if (scope && !scope.has(file)) out_of_scope.push(out);
else findings.push(out);
});
if (errors.length > 0) return { ok: false, errors };
const payload = { reviewer: spec.reviewer, findings };
const v = validateFindings(payload);
if (!v.valid) return { ok: false, errors: v.errors.map((e) => `${e.code}: ${e.message}`) };
return { ok: true, errors: [], payload, out_of_scope };
}
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
if (isMain()) {
const args = process.argv.slice(2);
const valueOf = (name) => { const i = args.indexOf(name); return i === -1 ? null : args[i + 1]; };
const scopePath = valueOf('--scope');
const repoRoot = valueOf('--repo-root');
const flagValues = new Set([scopePath, repoRoot].filter(Boolean));
const filePath = args.find((a) => !a.startsWith('--') && !flagValues.has(a));
if (!filePath || !existsSync(filePath) || (scopePath && !existsSync(scopePath))) {
process.stderr.write('Usage: engine-findings.mjs [--scope <file-list.txt>] [--repo-root <dir>] <engine-findings.json>\n');
process.exit(2);
}
let input;
let scopeFiles;
try {
input = JSON.parse(readFileSync(filePath, 'utf-8'));
if (scopePath) scopeFiles = readFileSync(scopePath, 'utf-8').split('\n').map((l) => l.trim()).filter(Boolean);
} catch (e) {
process.stderr.write(`engine-findings: could not read input: ${e.message}\n`);
process.exit(2);
}
const r = toReviewerPayload(input, { scopeFiles, repoRoot: repoRoot ?? undefined });
if (!r.ok) {
for (const e of r.errors) process.stderr.write(` ERROR ${e}\n`);
process.exit(1);
}
process.stdout.write(JSON.stringify({ ...r.payload, out_of_scope: r.out_of_scope }, null, 2) + '\n');
process.exit(0);
}

13
lib/review/findings-schema.mjs
View file

@ -21,7 +21,8 @@
//
// 3-layer pattern (Content → Raw-text → CLI shim) mirroring the other validators.
import { readFileSync, existsSync } from 'node:fs';
import { readFileSync, existsSync, realpathSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { issue, fail } from '../util/result.mjs';
import { RULE_KEYS, SEVERITY_VALUES } from './rule-catalogue.mjs';
@ -152,7 +153,15 @@ export function validateReviewerOutput(rawText) {
// ---- CLI shim ----------------------------------------------------------------
if (import.meta.url === `file://${process.argv[1]}`) {
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
if (isMain()) {
const args = process.argv.slice(2);
const filePath = args.find((a) => !a.startsWith('--'));
if (!filePath) {

392
lib/review/recurrence.mjs Normal file
View file

@ -0,0 +1,392 @@
// lib/review/recurrence.mjs
// P5 — the project's memory, fed by review (ordre 5).
//
// A finding class (the same catalogue `rule_key`) that shows up in TWO review.md
// files in the target repo, as two DIFFERENT findings (distinct finding IDs),
// becomes ONE proposal; one sighting is no proposal, and one unfixed finding
// seen again in the next iteration is a persisting defect, not a recurrence.
// Catch-all keys (route null) and findings whose key was normalised from an
// unknown one (`original_rule_key`) are never counted.
// The history is the review.md files the repo already has — the current
// `.claude/projects/*/review.md` plus, where git tracks them, their earlier
// committed versions. No new store is created.
//
// Routing (the catalogue's `memory_route`): mechanical → the lesson names a
// deterministic check; judgement → a line in the target repo's `.claude/rules/`
// file; CLAUDE.md gets only a pointer. Review-hygiene classes (route null) are
// never proposed. Format of every line: mistake · fix · how to check.
//
// Writes: only between Voyage's own markers, under a size budget; past the
// budget the write is refused and the lesson belongs in a skill (operator's
// call). Every line is approved by the operator first (/trekreview Phase 8.7,
// AskUserQuestion) — this module never decides that on its own.
//
// Borrowed (credited, MIT where it applies):
// - routing check / standard / pointer: Pocock `skills/in-progress/retro/SKILL.md:19,41`
// - the three-part line "mistake · fix · how to check": ai-native-sdlc `assets/CLAUDE.md:27-29`
// - the managed block between START/END markers: Spec Kit `extensions/agent-context/README.md:5`
// - evidence (two finding IDs) per proposal, schema-check before writing:
// Kiro https://kiro.dev/blog/trust-agent-triage/ (docs only, closed product)
// - review → memory, and the size budget ("or make a skill"): Boris Cherny's
// public talks/posts (sources A, B; see docs/praksisanalyse)
//
// CLI:
// node lib/review/recurrence.mjs propose --repo <dir>
// → {"proposals":[…], "reviews_read": N, "skipped": [...]} on stdout, exit 0; exit 2 = could not run
// node lib/review/recurrence.mjs apply --repo <dir> --rule-key <KEY> [--line "<edited line>"]
// → {"written":true|false, "reason":…} on stdout; exit 0 written, 1 refused, 2 could not run
import {
readFileSync, writeFileSync, renameSync, existsSync, mkdirSync, readdirSync, statSync, realpathSync, lstatSync,
} from 'node:fs';
import { join, relative, dirname } from 'node:path';
import { execFileSync } from 'node:child_process';
import { fileURLToPath } from 'node:url';
import { validateReviewContent } from '../validators/review-validator.mjs';
import { extractFindingsBlock } from './findings-schema.mjs';
import { getRule } from './rule-catalogue.mjs';
export const LESSONS_DIR = join('.claude', 'rules');
export const LESSONS_FILE_NAME = 'voyage-review-lessons.md';
export const MARK_START = '<!-- voyage:review-lessons:start -->';
export const MARK_END = '<!-- voyage:review-lessons:end -->';
export const POINTER_START = '<!-- voyage:review-lessons-pointer:start -->';
export const POINTER_END = '<!-- voyage:review-lessons-pointer:end -->';
export const POINTER_LINE =
`Recurring review lessons (operator-approved, managed by Voyage /trekreview): \`${LESSONS_DIR}/${LESSONS_FILE_NAME}\`.`;
/** Size budget for the managed block. Past it, a lesson belongs in a skill. */
export const DEFAULT_BUDGET_ENTRIES = 20;
export const DEFAULT_BUDGET_CHARS = 6000;
export const LINE_MAX = 700;
export const RECURRENCE_THRESHOLD = 2;
const BLOCK_HEADER = 'Each line: a finding class /trekreview saw in two reviews, approved by the operator. Format: mistake · fix · how to check.';
/** How to check a mechanical class. Judgement classes get the review itself. */
const CHECK_HINTS = Object.freeze({
MISSING_TEST: 'a coverage check on the changed files with the repo\'s own test tool — red when a changed source file has no test that exercises it.',
PLACEHOLDER_IN_CODE: '`git grep -nE "TODO|FIXME|XXX|debugger" -- <changed files>` returns nothing.',
UNDECLARED_DEPENDENCY: 'every import in the changed files resolves to a declared dependency (the repo\'s own dependency checker).',
BROKEN_SUCCESS_CRITERION: 'run each Success Criterion\'s verification command before the step is called done; only exit 0 counts.',
});
// ---- reading the history ----------------------------------------------------
/**
* Pull the kept findings out of one review.md text.
* @param {string} text
* @returns {{ ok: boolean, identity?: string, findings?: object[], reason?: string }}
*/
export function parseReview(text) {
const r = validateReviewContent(text, { strict: false });
if (!r.valid) return { ok: false, reason: (r.errors ?? []).map((e) => e.code).join(',') || 'invalid' };
const fm = r.parsed.frontmatter ?? {};
const block = extractFindingsBlock(r.parsed.body ?? '');
if (block === null) return { ok: false, reason: 'no-json-block' };
let payload;
try { payload = JSON.parse(block); } catch { return { ok: false, reason: 'json-parse-error' }; }
const findings = (Array.isArray(payload?.findings) ? payload.findings : [])
.map((f) => ({ ...f, rule_key: f?.rule_key ?? f?.rule }))
.filter((f) => typeof f.rule_key === 'string');
const identity = `${fm.project_dir ?? fm.slug ?? '?'}@${fm.scope_sha_end ?? '?'}`;
return { ok: true, identity, findings };
}
function git(repo, args) {
return execFileSync('git', ['-C', repo, ...args], { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] });
}
/**
* Every review.md the repo already holds: current files under
* `.claude/projects/<dir>/review.md`, plus earlier committed versions of those
* files when git tracks them. Deduplicated by review identity
* (project_dir @ scope_sha_end).
* @param {string} repo
* @returns {{ reviews: Array<{ identity: string, source: string, findings: object[] }>, skipped: Array<{ source: string, reason: string }> }}
*/
export function collectReviews(repo) {
const sources = [];
const projects = join(repo, '.claude', 'projects');
if (existsSync(projects)) {
for (const name of readdirSync(projects).sort()) {
const p = join(projects, name, 'review.md');
if (existsSync(p) && statSync(p).isFile()) {
sources.push({ source: relative(repo, p), text: readFileSync(p, 'utf-8') });
}
}
}
// Earlier versions (review.md is overwritten per iteration; git keeps the rest).
for (const { source } of [...sources]) {
let shas = [];
try { shas = git(repo, ['log', '--format=%H', '--', source]).split('\n').filter(Boolean); } catch { shas = []; }
for (const sha of shas) {
try {
// `./` makes the path relative to --repo, not to the enclosing git root.
sources.push({ source: `${source}@${sha.slice(0, 12)}`, text: git(repo, ['show', `${sha}:./${source}`]) });
} catch { /* not in that commit's tree */ }
}
}
const seen = new Set();
const reviews = [];
const skipped = [];
for (const s of sources) {
const parsed = parseReview(s.text);
if (!parsed.ok) { skipped.push({ source: s.source, reason: parsed.reason }); continue; }
if (seen.has(parsed.identity)) continue;
seen.add(parsed.identity);
reviews.push({ identity: parsed.identity, source: s.source, findings: parsed.findings });
}
return { reviews, skipped };
}
// ---- the managed block ------------------------------------------------------
function lessonsPath(repo) {
return join(repo, LESSONS_DIR, LESSONS_FILE_NAME);
}
function count(text, needle) {
return text.split(needle).length - 1;
}
/** The managed block, null when absent, or { damaged: true } when the markers are not exactly one START before one END. */
function readBlock(text) {
const starts = count(text, MARK_START);
const ends = count(text, MARK_END);
if (starts === 0 && ends === 0) return null;
const start = text.indexOf(MARK_START);
const end = text.indexOf(MARK_END);
if (starts !== 1 || ends !== 1 || end < start) return { damaged: true };
return { start, end, inner: text.slice(start + MARK_START.length, end) };
}
function blockEntries(inner) {
return inner.split('\n').filter((l) => l.startsWith('- ['));
}
/** The rule_keys already written as lessons. */
export function writtenClasses(repo) {
const p = lessonsPath(repo);
if (!existsSync(p)) return new Set();
const block = readBlock(readFileSync(p, 'utf-8'));
if (!block || block.damaged) return new Set();
const keys = new Set();
for (const line of blockEntries(block.inner)) {
const m = line.match(/^- \[([A-Z_]+)\]/);
if (m) keys.add(m[1]);
}
return keys;
}
// ---- proposals --------------------------------------------------------------
function clip(s, n) {
const t = String(s ?? '').replace(/<!--|-->/g, '').replace(/[\u0000-\u001f\u007f\u2028\u2029\s]+/g, ' ').trim();
return t.length > n ? `${t.slice(0, n - 1)}…` : t;
}
function composeLine(rule, evidence) {
const seen = evidence.map((e) => `"${clip(e.title ?? e.rule_key, 80)}" (${clip(e.file, 60)})`).join(', ');
const fix = clip(evidence[evidence.length - 1].recommended_action
?? evidence.find((e) => e.recommended_action)?.recommended_action
?? 'Address the class before the step is called done.', 160);
const how = rule.memory_route === 'check'
? CHECK_HINTS[rule.rule_key] ?? `a deterministic check for ${rule.rule_key}.`
: `/trekreview flags it as ${rule.rule_key}; read the diff for it before the step is called done.`;
const ids = evidence.map((e) => clip(String(e.id ?? '').slice(0, 8), 8)).join(', ');
const body = `Mistake: ${clip(rule.description, 160)} Seen: ${seen}. · Fix: ${fix} · How to check: ${how} (evidence: ${ids})`;
const line = `- [${rule.rule_key}] (${rule.memory_route}) ${body}`;
if (line.length <= LINE_MAX) return line;
// Shorten the free-text parts, never the three labels.
const short = evidence.map((e) => `"${clip(e.title ?? e.rule_key, 40)}" (${clip(e.file, 40)})`).join(', ');
return `- [${rule.rule_key}] (${rule.memory_route}) Mistake: ${clip(rule.description, 120)} Seen: ${short}. · Fix: ${clip(fix, 120)} · How to check: ${clip(how, 160)} (evidence: ${ids})`;
}
/**
* Validate a lesson line (generated or operator-edited). Schema-check before
* any write (Kiro's lesson-capture fix).
* @param {string} line
* @returns {{ valid: boolean, reason?: string }}
*/
export function validateLessonLine(line) {
if (typeof line !== 'string' || line.trim().length === 0) return { valid: false, reason: 'empty' };
if (/[\u0000-\u001f\u007f\u2028\u2029]/.test(line)) return { valid: false, reason: 'control or line-break character' };
if (line.length > LINE_MAX) return { valid: false, reason: `longer than ${LINE_MAX}` };
if (line.includes('<!--') || line.includes('-->')) return { valid: false, reason: 'marker text' };
const a = line.indexOf('Mistake:');
const b = line.indexOf('Fix:');
const c = line.indexOf('How to check:');
if (a === -1 || b === -1 || c === -1 || !(a < b && b < c)) {
return { valid: false, reason: 'needs "Mistake:", "Fix:", "How to check:" in that order' };
}
return { valid: true };
}
/**
* Find recurring finding classes in the repo's review history.
* @param {string} repo
* @returns {{ proposals: object[], reviews_read: number, skipped: object[] }}
*/
export function proposeLessons(repo) {
const { reviews, skipped } = collectReviews(repo);
const already = writtenClasses(repo);
const byKey = new Map();
for (const review of reviews) {
const perKey = new Map();
for (const f of review.findings) {
if (f.original_rule_key !== undefined) continue; // normalised from an unknown key: not a class
if (!perKey.has(f.rule_key)) perKey.set(f.rule_key, []);
perKey.get(f.rule_key).push(f);
}
for (const [key, fs] of perKey) {
if (!byKey.has(key)) byKey.set(key, []);
byKey.get(key).push({ review: review.identity, source: review.source, findings: fs });
}
}
const proposals = [];
for (const [key, sightings] of [...byKey].sort(([a], [b]) => a.localeCompare(b))) {
// Two reviews, and two DIFFERENT findings: pick, per review, a finding whose
// id has not been used yet. The same unfixed finding twice is one defect.
const usedIds = new Set();
const evidence = [];
for (const s of sightings) {
const f = s.findings.find((x) => !usedIds.has(x.id ?? `${x.file}:${x.line}`));
if (!f) continue;
usedIds.add(f.id ?? `${f.file}:${f.line}`);
evidence.push({ ...f, review: s.review, source: s.source });
}
if (evidence.length < RECURRENCE_THRESHOLD) continue;
const rule = getRule(key);
if (!rule || rule.memory_route === null || rule.memory_route === undefined) continue;
if (already.has(key)) continue;
const ev = evidence.slice(0, RECURRENCE_THRESHOLD);
const line = composeLine(rule, ev);
if (!validateLessonLine(line).valid) continue; // never propose a line apply would refuse
proposals.push({
rule_key: key,
route: rule.memory_route,
target: `${LESSONS_DIR}/${LESSONS_FILE_NAME}`,
line,
evidence: evidence.map((e) => ({ id: e.id ?? null, review: e.review, source: e.source, file: e.file ?? null, title: e.title ?? null })),
});
}
return { proposals, reviews_read: reviews.length, skipped };
}
// ---- writing an approved lesson ---------------------------------------------
function atomicWriteText(path, text) {
// Write THROUGH a symlink (CLAUDE.md -> AGENTS.md is common): rename onto the
// real target, so the link survives and the target gets the change.
if (existsSync(path) && lstatSync(path).isSymbolicLink()) path = realpathSync(path);
mkdirSync(dirname(path), { recursive: true });
const tmp = `${path}.tmp-${process.pid}`;
writeFileSync(tmp, text);
renameSync(tmp, path);
}
function withPointer(claudeText) {
if (claudeText.includes(POINTER_START) || claudeText.includes(POINTER_END)) return null;
const sep = claudeText.endsWith('\n') ? '\n' : '\n\n';
return `${claudeText}${sep}${POINTER_START}\n${POINTER_LINE}\n${POINTER_END}\n`;
}
/**
* Write ONE operator-approved lesson. Call only after the operator said yes to
* this exact line.
* @param {string} repo
* @param {string} ruleKey
* @param {{ line?: string, budgetEntries?: number, budgetChars?: number }} [opts]
* @returns {{ written: boolean, reason: string, path?: string, pointer?: string }}
*/
export function applyLesson(repo, ruleKey, opts = {}) {
const budgetEntries = opts.budgetEntries ?? DEFAULT_BUDGET_ENTRIES;
const budgetChars = opts.budgetChars ?? DEFAULT_BUDGET_CHARS;
if (writtenClasses(repo).has(ruleKey)) return { written: false, reason: 'already-present' };
const proposal = proposeLessons(repo).proposals.find((p) => p.rule_key === ruleKey);
if (!proposal) return { written: false, reason: 'no-recurrence' };
let line = proposal.line;
if (opts.line !== undefined) {
const body = String(opts.line).replace(/^- \[[A-Z_]+\]\s*(\((check|rule)\)\s*)?/, '');
line = `- [${ruleKey}] (${proposal.route}) ${body.trim()}`;
}
const check = validateLessonLine(line);
if (!check.valid) return { written: false, reason: 'invalid-line', detail: check.reason };
const path = lessonsPath(repo);
const current = existsSync(path) ? readFileSync(path, 'utf-8') : '';
let block = readBlock(current);
if (block && block.damaged) {
return { written: false, reason: 'markers-damaged', detail: `${LESSONS_DIR}/${LESSONS_FILE_NAME} must hold exactly one start marker before one end marker — fix it by hand` };
}
let base = current;
if (!block) {
const sep = current.length === 0 || current.endsWith('\n') ? '' : '\n';
base = `${current}${sep}${MARK_START}\n${BLOCK_HEADER}\n${MARK_END}\n`;
block = readBlock(base);
}
const entries = blockEntries(block.inner);
const innerAfter = `${block.inner.replace(/\n*$/, '')}\n${line}\n`;
if (entries.length + 1 > budgetEntries || innerAfter.length > budgetChars) {
return { written: false, reason: 'over-budget', detail: 'the managed block is full — the lessons belong in a skill now (operator decision)' };
}
const next = base.slice(0, block.start + MARK_START.length) + innerAfter + base.slice(block.end);
atomicWriteText(path, next);
let pointer = 'no-claude-md';
const claudePath = join(repo, 'CLAUDE.md');
if (existsSync(claudePath)) {
const updated = withPointer(readFileSync(claudePath, 'utf-8'));
if (updated === null) pointer = 'present';
else { atomicWriteText(claudePath, updated); pointer = 'written'; }
}
return { written: true, reason: 'written', path: relative(repo, path), pointer };
}
// ---- CLI --------------------------------------------------------------------
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
function flag(args, name) {
const i = args.indexOf(name);
return i === -1 ? undefined : args[i + 1];
}
if (isMain()) {
const [cmd, ...args] = process.argv.slice(2);
const repo = flag(args, '--repo');
const out = (o) => process.stdout.write(JSON.stringify(o, null, 2) + '\n');
if (!['propose', 'apply'].includes(cmd) || !repo) {
process.stderr.write('Usage: recurrence.mjs propose --repo <dir> | apply --repo <dir> --rule-key <KEY> [--line "<text>"]\n');
process.exit(2);
}
if (!existsSync(repo) || !statSync(repo).isDirectory()) {
process.stderr.write(`recurrence: repo not found: ${repo}\n`);
process.exit(2);
}
try {
if (cmd === 'propose') {
out(proposeLessons(repo));
process.exit(0);
}
const key = flag(args, '--rule-key');
if (!key) {
process.stderr.write('recurrence apply: --rule-key is required\n');
process.exit(2);
}
const r = applyLesson(repo, key, { line: flag(args, '--line') });
out(r);
process.exit(r.written ? 0 : 1);
} catch (e) {
process.stderr.write(`recurrence: could not run: ${e.message}\n`);
process.exit(2);
}
}

45
lib/review/rule-catalogue.mjs
View file

@ -1,8 +1,21 @@
// lib/review/rule-catalogue.mjs
// Canonical rule catalogue for /trekreview v1.0.
// Canonical rule catalogue for /trekreview.
//
// 12 rule keys, 4-tier severity (matches brief contract).
// 14 rule keys, 4-tier severity (matches brief contract).
// llm-security 5-tier alignment is a v1.1 candidate.
//
// Ordre 5 (review_version 1.1) added the two engine keys: findings from Claude
// Code's own `/code-review` and `/security-review` enter the coordinator under
// them when no narrower key fits (wrap, not a new engine).
//
// `memory_route` decides where a finding class that recurs across two
// review.md files is proposed as project memory (P5, lib/review/recurrence.mjs):
// 'check' - mechanical: a deterministic check can catch it;
// 'rule' - judgement: a line in the target repo's `.claude/rules/` file;
// null - never proposed: review hygiene (about the review itself, not the
// target code), or a catch-all key (the two engine keys) whose
// members share nothing but their source.
// Routing borrowed from Pocock `skills/in-progress/retro/SKILL.md:19` (MIT).
export const SEVERITY_VALUES = Object.freeze(['BLOCKER', 'MAJOR', 'MINOR', 'SUGGESTION']);
@ -21,75 +34,103 @@ export const RULE_CATALOGUE = Object.freeze([
severity: 'MAJOR',
category: 'conformance',
description: 'Finding lacks brief_ref pointing to the brief section it traces back to.',
memory_route: null,
}),
Object.freeze({
rule_key: 'UNIMPLEMENTED_CRITERION',
severity: 'BLOCKER',
category: 'conformance',
description: 'A brief Success Criterion has no corresponding implementation in the delivered code.',
memory_route: 'rule',
}),
Object.freeze({
rule_key: 'SCOPE_CREEP_BUILT',
severity: 'MAJOR',
category: 'scope',
description: 'Code implements features beyond what the brief requested.',
memory_route: 'rule',
}),
Object.freeze({
rule_key: 'NON_GOAL_VIOLATED',
severity: 'BLOCKER',
category: 'scope',
description: 'Code implements something the brief explicitly listed as a Non-Goal.',
memory_route: 'rule',
}),
Object.freeze({
rule_key: 'MISSING_TEST',
severity: 'MAJOR',
category: 'tests',
description: 'Delivered behavior has no automated test coverage.',
memory_route: 'check',
}),
Object.freeze({
rule_key: 'SECURITY_INJECTION',
severity: 'BLOCKER',
category: 'security',
description: 'Code path constructs commands, queries, or templates from untrusted input without sanitization.',
memory_route: 'rule',
}),
Object.freeze({
rule_key: 'PLACEHOLDER_IN_CODE',
severity: 'MAJOR',
category: 'maintenance',
description: 'Committed code contains TBD/TODO/FIXME/XXX/console.log/debugger placeholders.',
memory_route: 'check',
}),
Object.freeze({
rule_key: 'MISSING_ERROR_HANDLING',
severity: 'MINOR',
category: 'correctness',
description: 'Code path can fail silently (uncaught promise, unchecked return, missing try/catch on I/O).',
memory_route: 'rule',
}),
Object.freeze({
rule_key: 'UNDECLARED_DEPENDENCY',
severity: 'MAJOR',
category: 'maintenance',
description: 'Code imports or invokes something not declared in package.json / not bundled / not present in PATH.',
memory_route: 'check',
}),
Object.freeze({
rule_key: 'PLAN_EXECUTE_DRIFT',
severity: 'MAJOR',
category: 'conformance',
description: 'Delivered code diverges from what the plan said would be built (different file, different approach, different API).',
memory_route: 'rule',
}),
Object.freeze({
rule_key: 'BROKEN_SUCCESS_CRITERION',
severity: 'BLOCKER',
category: 'conformance',
description: 'A brief Success Criterion is implemented but the verification command/test fails or is structurally incorrect.',
memory_route: 'check',
}),
Object.freeze({
rule_key: 'COVERAGE_SILENT_SKIP',
severity: 'MAJOR',
category: 'tests',
description: 'Triage gate skipped a file without recording it in the Coverage section of review.md (hidden truncation).',
memory_route: null,
}),
Object.freeze({
rule_key: 'CODE_REVIEW_DEFECT',
severity: 'MAJOR',
category: 'correctness',
description: "A defect reported by Claude Code's /code-review that no narrower rule_key covers.",
memory_route: null,
}),
Object.freeze({
rule_key: 'SECURITY_REVIEW_VULNERABILITY',
severity: 'BLOCKER',
category: 'security',
description: "A vulnerability reported by Claude Code's /security-review that no narrower rule_key covers.",
memory_route: null,
}),
]);
export const MEMORY_ROUTES = Object.freeze(['check', 'rule']);
export const RULE_KEYS = Object.freeze(new Set(RULE_CATALOGUE.map((r) => r.rule_key)));
/**

201
lib/review/verdict-gate.mjs Normal file
View file

@ -0,0 +1,201 @@
// lib/review/verdict-gate.mjs
// K7 — the review verdict gate (ordre 5, P11).
//
// One command decides on the verdict written in review.md, so whatever stops a
// merge or a release reads a file and an exit code, never a model's summary:
//
// node lib/review/verdict-gate.mjs [--json] <review.md>
// 0 = ALLOW or WARN
// 1 = BLOCK
// 2 = could not decide: missing/unreadable file, not a review, no verdict,
// an unknown verdict, a file that contradicts its own verdict, or a
// WARN because a reviewer or engine did not run (`missing-reviewer:` /
// `unattributable-payload` in allow_blocked_by — operator decision
// 2026-10-06: a review not performed in full stops a merge).
// Exit 2 is NEVER green.
//
// Fail-closed reading: a missing `verdict` is exit 2, never ALLOW. A verdict
// that is GREENER than the file's own findings (ALLOW/WARN over a BLOCKER, or a
// frontmatter verdict the trailing JSON block disagrees with) is exit 2 as well
// — the file cannot be trusted for the direction that would let work through.
// BLOCK always exits 1: a stricter-than-supported verdict is still a stop.
//
// Borrowed: the verdict goes to a file and a command check gates on it
// (Kiro `feature-pipeline`, https://kiro.dev/docs/workflows/patterns); the
// executor may read the verdict but never write it (Spec Kit
// `templates/checklist-template.md:8-9`, MIT). The blocking itself is Voyage's
// own design — neither source blocks on its own.
import { readFileSync, existsSync, realpathSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { validateReviewContent, VERDICT_VALUES } from '../validators/review-validator.mjs';
import { extractFindingsBlock } from './findings-schema.mjs';
export const EXIT = Object.freeze({ PASS: 0, BLOCK: 1, UNDECIDED: 2 });
function undecided(reason, verdict = null) {
return { exit: EXIT.UNDECIDED, verdict, reason };
}
/**
* Decide on review.md text. Pure: no I/O.
* @param {string} text
* @returns {{ exit: 0|1|2, verdict: string|null, reason: string }}
*/
export function decideVerdict(text) {
const r = validateReviewContent(text, { strict: false });
const fm = r.parsed?.frontmatter ?? {};
if (!r.valid) {
const codes = (r.errors ?? []).map((e) => e.code).join(', ');
return undecided(`not a readable review.md (${codes || 'parse failure'})`);
}
const verdict = fm.verdict;
if (verdict === undefined || verdict === null || verdict === '') {
return undecided('review.md carries no verdict (K7: required) — never read as ALLOW');
}
if (!VERDICT_VALUES.includes(verdict)) {
return undecided(`unknown verdict ${JSON.stringify(verdict)}`);
}
if (verdict === 'BLOCK') {
return { exit: EXIT.BLOCK, verdict, reason: 'verdict BLOCK' };
}
// ALLOW / WARN: the file must back the greener direction, from every place a
// BLOCKER can be written. A shape the gate cannot read is exit 2, never 0.
const body = r.parsed.body ?? '';
const fmText = text.slice(0, Math.max(0, text.indexOf('\n---', 3)));
if ((fmText.match(/^verdict\s*:/gm) ?? []).length > 1) {
return undecided('frontmatter carries more than one verdict line', verdict);
}
const lastFence = lastFenceLanguage(body);
if (lastFence === null) {
return undecided(`verdict ${verdict} has no trailing JSON block to back it`, verdict);
}
if (lastFence !== 'json') {
return undecided(`the last fenced block is "${lastFence || '(no language)'}", not the trailing \`json\` block`, verdict);
}
const block = extractFindingsBlock(body);
let payload;
try {
payload = JSON.parse(block);
} catch (e) {
return undecided(`trailing JSON block does not parse: ${e.message}`, verdict);
}
const shape = checkPayloadShape(payload);
if (shape) return undecided(`trailing JSON block: ${shape}`, verdict);
if (payload.verdict !== verdict) {
return undecided(`frontmatter verdict ${verdict} disagrees with the JSON block verdict ${payload.verdict}`, verdict);
}
const blockerFindings = payload.findings.filter((f) => String(f.severity).toUpperCase() === 'BLOCKER').length;
const blockerCount = Object.entries(payload.counts)
.filter(([k]) => k.toUpperCase() === 'BLOCKER')
.reduce((n, [, v]) => n + Number(v || 0), 0);
const blockerSection = blockerSectionEntries(body);
if (blockerFindings > 0 || blockerCount > 0 || blockerSection > 0) {
return undecided(`verdict ${verdict} over BLOCKER finding(s) (json: ${blockerFindings}, counts: ${blockerCount}, ## Findings (BLOCKER): ${blockerSection})`, verdict);
}
const fmIds = new Set(Array.isArray(fm.findings) ? fm.findings : []);
const jsonIds = new Set(payload.findings.map((f) => f.id));
if (fmIds.size !== jsonIds.size || [...fmIds].some((id) => !jsonIds.has(id))) {
return undecided('frontmatter findings: list and the JSON block finding ids differ', verdict);
}
// A review in which a reviewer or engine never reported was not performed in
// full: that is "could not decide", not a passable warning (operator
// decision 2026-10-06, ordre 5). A 1.1 review must carry the list.
if (atLeastVersion(fm.review_version, 1, 1) && !Array.isArray(payload.allow_blocked_by)) {
return undecided('trailing JSON block lacks allow_blocked_by (required from review_version 1.1)', verdict);
}
const warnReasons = Array.isArray(payload.allow_blocked_by) ? payload.allow_blocked_by.map(String) : [];
const notRun = warnReasons.filter((w) => w.startsWith('missing-reviewer:') || w.startsWith('unattributable-payload'));
if (notRun.length > 0) {
return undecided(`a control did not run: ${notRun.join(', ')}`, verdict);
}
return { exit: EXIT.PASS, verdict, reason: `verdict ${verdict}`, warn_reasons: warnReasons };
}
function atLeastVersion(v, major, minor) {
const m = typeof v === 'string' ? v.match(/^(\d+)\.(\d+)$/) : null;
return Boolean(m) && (Number(m[1]) > major || (Number(m[1]) === major && Number(m[2]) >= minor));
}
const FENCE_RE = /^```[ \t]*([A-Za-z0-9_+-]*)[^\n]*\n[\s\S]*?^```[ \t]*$/gm;
/** Language tag of the LAST fenced block in `body` (lower-cased), '' for none, null when no fence. */
function lastFenceLanguage(body) {
let last = null;
let m;
FENCE_RE.lastIndex = 0;
while ((m = FENCE_RE.exec(body)) !== null) last = m[1].toLowerCase();
return last;
}
/** The JSON block shape the gate needs; returns an error string or null. */
function checkPayloadShape(p) {
if (p === null || typeof p !== 'object' || Array.isArray(p)) return 'not an object';
if (!VERDICT_VALUES.includes(p.verdict)) return `verdict must be one of ${VERDICT_VALUES.join('|')}`;
if (p.counts === null || typeof p.counts !== 'object' || Array.isArray(p.counts)) return 'counts must be an object';
if (!Array.isArray(p.findings)) return 'findings must be an array';
for (const [i, f] of p.findings.entries()) {
if (f === null || typeof f !== 'object' || typeof f.severity !== 'string' || typeof f.id !== 'string') {
return `findings[${i}] needs a string id and severity`;
}
}
return null;
}
/** Number of `### ` entries under `## Findings (BLOCKER)`. */
function blockerSectionEntries(body) {
const start = body.search(/^##\s+Findings\s*\(BLOCKER\)/im);
if (start === -1) return 0;
const rest = body.slice(start).replace(/^[^\n]*\n/, '');
const next = rest.search(/^##\s+/m);
const section = next === -1 ? rest : rest.slice(0, next);
return (section.match(/^###\s+\S/gm) ?? []).length;
}
/**
* Decide on a review.md path.
* @param {string} filePath
*/
export function decideVerdictFile(filePath) {
if (!filePath || !existsSync(filePath)) return undecided(`file not found: ${filePath}`);
let text;
try {
text = readFileSync(filePath, 'utf-8');
} catch (e) {
return undecided(`cannot read ${filePath}: ${e.message}`);
}
return decideVerdict(text);
}
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
if (isMain()) {
const args = process.argv.slice(2);
const files = args.filter((a) => !a.startsWith('--'));
const filePath = files[0];
if (files.length !== 1) {
// Exactly one file: a glob would otherwise gate on its first match only.
process.stderr.write('Usage: verdict-gate.mjs [--json] <review.md> (exactly one file)\n');
process.exit(EXIT.UNDECIDED);
}
let result;
try {
result = decideVerdictFile(filePath);
} catch (e) {
result = undecided(`gate crashed: ${e.message}`);
}
if (args.includes('--json')) {
process.stdout.write(JSON.stringify({ path: filePath, ...result }) + '\n');
} else {
const label = result.exit === EXIT.UNDECIDED ? 'UNDECIDED (not green)' : result.verdict;
process.stdout.write(`verdict-gate: ${label} — ${result.reason} — ${filePath}\n`);
}
process.exit(result.exit);
}

98
lib/validators/review-validator.mjs
View file

@ -6,8 +6,15 @@
// tolerated silently. Strict-key checks are intentionally avoided so new
// optional fields (jf. the source_findings precedent) can be added without
// a review_version bump.
//
// K7 (ordre 5, review_version 1.1): `verdict: ALLOW | WARN | BLOCK` is REQUIRED.
// Strict (write-time) mode rejects a review without it; soft (read-time) mode
// warns, so a 1.0 review can still seed a plan (N-1 window, Handover 6). The
// gate that decides on the verdict is lib/review/verdict-gate.mjs, which never
// reads a missing verdict as ALLOW.
import { readFileSync, existsSync } from 'node:fs';
import { readFileSync, existsSync, realpathSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { parseDocument } from '../util/frontmatter.mjs';
import { issue, ok, fail } from '../util/result.mjs';
@ -23,8 +30,42 @@ export const REVIEW_REQUIRED_FRONTMATTER = [
'findings',
];
export const REVIEW_BODY_SECTIONS = ['Executive Summary', 'Coverage', 'Remediation Summary'];
export const VERDICT_VALUES = Object.freeze(['ALLOW', 'WARN', 'BLOCK']);
const HEX_ID_RE = /^[0-9a-f]{40}$/;
const JSON_FENCE_GLOBAL = /```json[ \t]*\r?\n([\s\S]*?)```/gi;
/** IDs of BLOCKER findings in the trailing JSON block ([] when absent/unparseable). */
function blockerIdsFromBody(body) {
let last = null;
let m;
JSON_FENCE_GLOBAL.lastIndex = 0;
while ((m = JSON_FENCE_GLOBAL.exec(body)) !== null) last = m[1];
if (last === null) return [];
try {
const payload = JSON.parse(last);
return (Array.isArray(payload?.findings) ? payload.findings : [])
.filter((f) => f?.severity === 'BLOCKER' && typeof f.id === 'string')
.map((f) => f.id);
} catch {
return [];
}
}
/** Finding IDs named by `Source finding:` lines under `## Remediation Steps` step headings. */
function remediationStepSources(body) {
const start = body.search(/^##\s+Remediation Steps\b/m);
if (start === -1) return new Set();
const rest = body.slice(start).replace(/^##\s+Remediation Steps[^\n]*\n/, '');
const next = rest.search(/^##\s+/m);
const section = next === -1 ? rest : rest.slice(0, next);
const ids = new Set();
for (const step of section.split(/^###\s+Step\s+R\d+\b/m).slice(1)) {
const m = step.match(/Source finding:\**\s*`?([0-9a-f]{40})`?/);
if (m) ids.add(m[1]);
}
return ids;
}
export function validateReviewContent(text, opts = {}) {
const strict = opts.strict !== false;
@ -66,6 +107,51 @@ export function validateReviewContent(text, opts = {}) {
}
}
if (!('verdict' in fm) || fm.verdict === null || fm.verdict === '') {
const issueObj = issue(
'REVIEW_MISSING_VERDICT',
'Required frontmatter field missing: verdict (ALLOW | WARN | BLOCK)',
'Copy the coordinator verdict from the trailing JSON block into the frontmatter.',
);
if (strict) errors.push(issueObj);
else warnings.push(issueObj);
} else if (!VERDICT_VALUES.includes(fm.verdict)) {
errors.push(issue(
'REVIEW_BAD_VERDICT',
`frontmatter.verdict must be one of ${VERDICT_VALUES.join('|')}, got ${JSON.stringify(fm.verdict)}`,
));
}
// Every BLOCKER is written as a numbered remediation step that Handover 6 can
// append to a plan unchanged: `### Step R<n>: …` with `Source finding: <id>`.
// Borrowed: review gaps become appended, numbered tasks with a source-ref,
// never rewriting earlier ones (Spec Kit `templates/commands/converge.md:156-163`, MIT).
// 1.1+ only (N-1 window): a 1.0 review never carried the section.
const v = typeof fm.review_version === 'string' ? fm.review_version.match(/^(\d+)\.(\d+)$/) : null;
const atLeast11 = v && (Number(v[1]) > 1 || (Number(v[1]) === 1 && Number(v[2]) >= 1));
const blockerIds = atLeast11 ? blockerIdsFromBody(body) : [];
if (blockerIds.length > 0) {
const steps = remediationStepSources(body);
for (const id of blockerIds) {
if (!steps.has(id)) {
const issueObj = issue(
'REVIEW_MISSING_REMEDIATION_STEP',
`BLOCKER ${id} has no "### Step R<n>" with "Source finding: ${id}" under ## Remediation Steps`,
'Write one numbered remediation step per BLOCKER (Handover 6).',
);
if (strict) errors.push(issueObj);
else warnings.push(issueObj);
}
}
}
// 1.1+: what the coordinator set aside is listed, never left out (ordre 5).
if (atLeast11 && !/^##\s+Declined to Judge\b/m.test(body)) {
const issueObj = issue('REVIEW_MISSING_SECTION', 'Required body section missing: ## Declined to Judge (review_version ≥ 1.1)');
if (strict) errors.push(issueObj);
else warnings.push(issueObj);
}
for (const section of REVIEW_BODY_SECTIONS) {
const re = new RegExp(`^##\\s+${section}\\b`, 'm');
if (!re.test(body)) {
@ -94,7 +180,15 @@ export function validateReview(filePath, opts = {}) {
return { ...r, parsed: { ...r.parsed, filePath } };
}
if (import.meta.url === `file://${process.argv[1]}`) {
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
if (isMain()) {
const args = process.argv.slice(2);
const strict = !args.includes('--soft');
const filePath = args.find(a => !a.startsWith('--'));

BIN
scripts/trekreview-armB.workflow.mjs
View file

Binary file not shown.

33
templates/trekreview-template.md
View file

@ -1,6 +1,6 @@
---
type: trekreview
review_version: "1.0"
review_version: "1.1"
created: {YYYY-MM-DD}
task: "{Task description from brief.md}"
slug: {project-slug}
@ -9,6 +9,7 @@ brief_path: .claude/projects/{YYYY-MM-DD}-{slug}/brief.md
scope_sha_start: {sha-from-progress.json/session_start_sha-OR-null-if-mtime-fallback}
scope_sha_end: {sha-of-HEAD-at-review-time}
reviewed_files_count: {N}
verdict: BLOCK
findings:
- 0123456789abcdef0123456789abcdef01234567
- fedcba9876543210fedcba9876543210fedcba98
@ -94,6 +95,30 @@ should look at first.
bug but worth noting for v1.1 cleanup.
- recommended_action: Track in TODO.md if not already.
## Declined to Judge
None.
Trimmed: 0
> One line per finding the coordinator set aside without ruling on it
> (`file:line RULE_KEY — reason`), then `Trimmed: N` — MINOR/SUGGESTION text
> cut, findings kept. No finding is struck for length.
## Remediation Steps
### Step R1: Update the template to use block-style YAML for source_findings
- **Files:** `lib/foo.mjs`
- **Source finding:** `0123456789abcdef0123456789abcdef01234567`
- **Test first:** a test in `tests/lib/source-findings.test.mjs` that feeds a
flow-style `source_findings` and fails today
- **Verify:** re-run `/trekreview` → finding `0123456789abcdef0123456789abcdef01234567` absent
- **On failure:** escalate — the finding needs the operator, not improvisation
> One numbered step per BLOCKER (required when there is one), so Handover 6
> can append it to a plan unchanged.
## Remediation Summary
- 1 BLOCKER → must address before next plan iteration
@ -103,13 +128,17 @@ should look at first.
If running `/trekplan --brief review.md`, the planner will consume
the BLOCKER + MAJOR findings as plan goals (their `recommended_action`
becomes the step intent). MINOR + SUGGESTION are skipped for v1.0
becomes the step intent; each BLOCKER's `## Remediation Steps` entry is
ready to append as a plan step). MINOR + SUGGESTION are skipped for
plan-input.
```json
{
"verdict": "BLOCK",
"counts": { "BLOCKER": 1, "MAJOR": 1, "MINOR": 1, "SUGGESTION": 1 },
"allow_blocked_by": [],
"declined_to_judge": [],
"trimmed": 0,
"findings": [
{
"id": "0123456789abcdef0123456789abcdef01234567",

4
tests/fixtures/trekreview/review-run-A.md vendored
View file

@ -9,7 +9,7 @@ brief_path: .claude/projects/2026-05-01-jwt-auth/brief.md
scope_sha_start: 0123456789abcdef0123456789abcdef01234567
scope_sha_end: fedcba9876543210fedcba9876543210fedcba98
reviewed_files_count: 3
verdict: WARN
verdict: BLOCK
findings:
- d2d0e27875ae9ef0d818cb08bb6f14e6d33c4232
- 7861519c326c207aabf17072db51c469bebc217b
@ -23,7 +23,7 @@ findings:
## Executive Summary
Implementation hits the brief's core success criteria (login + refresh + logout) but
has one BLOCKER and four MAJOR/MINOR issues. Verdict: **WARN** — fix the BLOCKER
has one BLOCKER and four MAJOR/MINOR issues. Verdict: **BLOCK** — fix the BLOCKER
before merge; the MAJORs should land in a follow-up plan.
This is a SYNTHETIC v1.0 fixture for testing the Jaccard determinism pipeline. It is

4
tests/fixtures/trekreview/review-run-B.md vendored
View file

@ -9,7 +9,7 @@ brief_path: .claude/projects/2026-05-01-jwt-auth/brief.md
scope_sha_start: 0123456789abcdef0123456789abcdef01234567
scope_sha_end: fedcba9876543210fedcba9876543210fedcba98
reviewed_files_count: 3
verdict: WARN
verdict: BLOCK
findings:
- d2d0e27875ae9ef0d818cb08bb6f14e6d33c4232
- 7861519c326c207aabf17072db51c469bebc217b
@ -25,7 +25,7 @@ findings:
Same diff as Run A, re-reviewed independently to test determinism. This run found
the same 5 findings plus one extra (a placeholder TODO in logout.mjs that Run A
missed). Verdict: **WARN** — same as Run A; the extra finding is MAJOR but does
missed). Verdict: **BLOCK** — same as Run A; the extra finding is MAJOR but does
not change the merge gate.
This is a SYNTHETIC v1.0 fixture for testing the Jaccard determinism pipeline.

100
tests/lib/coordinator-contract.test.mjs
View file

@ -127,20 +127,53 @@ test('reasonablenessFilter — citation-less is REFUTED, unknown rule_key is UNV
// ---- Pass 2 — judge --------------------------------------------------------
test('judgeFilter — over-long title and empty recommended_action are UNVERIFIED, not dropped', () => {
// Contract change (fail-closed): both implemented Pass 2 tests read a
// `.length` and never examine the claim, so neither refutes the finding.
test('judgeFilter — an over-long MAJOR title is kept whole; an empty recommended_action is UNVERIFIED', () => {
// Length never removes a finding (ordre 5, P11). Actionability reads a
// `.length` and never examines the claim, so it cannot refute the finding.
// `dropped` is empty here on purpose — the refuting Pass 2 filter (Accuracy)
// is the one this deterministic subset excludes.
const longTitle = 'x'.repeat(101);
const j = judgeFilter([
{ file: 'x.mjs', line: 1, rule_key: 'MISSING_TEST', severity: 'MAJOR', title: 'x'.repeat(101) }, // too long → unverified
{ file: 'x.mjs', line: 1, rule_key: 'MISSING_TEST', severity: 'MAJOR', title: longTitle }, // MAJOR → kept whole
{ file: 'x.mjs', line: 2, rule_key: 'MISSING_TEST', severity: 'MAJOR', title: 'ok', recommended_action: ' ' }, // empty action → unverified
{ file: 'x.mjs', line: 3, rule_key: 'MISSING_TEST', severity: 'MAJOR', title: 'ok' }, // keep (no action field is fine)
]);
assert.equal(j.kept.length, 1);
assert.equal(j.kept.length, 2);
assert.equal(j.kept[0].title, longTitle, 'a MAJOR keeps every character');
assert.equal(j.dropped.length, 0);
assert.equal(j.unverified.length, 2);
assert.deepEqual(j.unverified.map((f) => f.suppressed_reason), ['succinctness:title', 'actionability:empty']);
assert.equal(j.trimmed.length, 0);
assert.deepEqual(j.unverified.map((f) => f.suppressed_reason), ['actionability:empty']);
});
test('judgeFilter — only MINOR/SUGGESTION text is trimmed, and every trim is counted', () => {
const j = judgeFilter([
{ file: 'x.mjs', line: 1, rule_key: 'MISSING_ERROR_HANDLING', severity: 'MINOR', title: 't'.repeat(150), detail: 'd'.repeat(1200) },
{ file: 'x.mjs', line: 2, rule_key: 'MISSING_ERROR_HANDLING', severity: 'MINOR', title: 'short', detail: 'short' },
]);
assert.equal(j.kept.length, 2, 'a trimmed finding is still kept');
assert.equal(j.trimmed.length, 1);
assert.ok(j.kept[0].title.length <= 100 && j.kept[0].detail.length <= 800);
assert.deepEqual(j.kept[0].trimmed_fields, ['title', 'detail']);
assert.ok(j.kept[0].detail.endsWith('[…trimmed]'), 'the cut is visible in the text');
});
test('judgeFilter — a trimmed MINOR keeps its "Also flagged by" provenance', () => {
const [f] = dedupByTriplet([
{ file: 'x.mjs', line: 1, rule_key: 'MISSING_ERROR_HANDLING', severity: 'MINOR', title: 'a', detail: 'q'.repeat(900), reviewer: 'code-correctness-reviewer' },
{ file: 'x.mjs', line: 1, rule_key: 'MISSING_ERROR_HANDLING', severity: 'MINOR', title: 'b', detail: 'r', reviewer: 'claude-code-review' },
]);
const j = judgeFilter([f]);
assert.ok(j.kept[0].detail.length <= 800);
assert.match(j.kept[0].detail, /Also flagged by claude-code-review: b\.$/);
});
test('judgeFilter — the trim decision reads the catalogue tier, not the reviewer label', () => {
// MISSING_TEST is MAJOR in the catalogue; a reviewer calling it MINOR must
// not get its text cut.
const detail = 'd'.repeat(900);
const j = judgeFilter([{ file: 'x.mjs', line: 1, rule_key: 'MISSING_TEST', severity: 'MINOR', title: 't', detail }]);
assert.equal(j.trimmed.length, 0);
assert.equal(j.kept[0].detail, detail);
});
// ---- ingest ----------------------------------------------------------------
@ -229,19 +262,34 @@ test('computeVerdict — a reviewer that did not report forbids ALLOW', () => {
assert.ok(r.allow_blocked_by.some((x) => x.includes('brief-conformance-reviewer')));
});
test('runContract — a BLOCKER dropped for an over-long title cannot yield ALLOW', () => {
// Pass 2 succinctness reads `.length`. It never examines the claim, so it
// cannot establish the finding is unreal -- it is unverified, not refuted.
test('runContract — a BLOCKER with an over-long title is kept and yields BLOCK', () => {
// Before ordre 5 this finding went to `unverified` on `succinctness:title`
// (the fail-closed floor kept it from ALLOW, but it fell out of the list).
const result = runContract([
{ reviewer: 'code-correctness-reviewer', findings: [
{ file: 'lib/auth/jwt.mjs', line: 19, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER', title: 'x'.repeat(101), detail: 'algo taken from the JWT header' },
] },
]);
assert.notEqual(result.verdict, 'ALLOW', 'an unsubstantiated BLOCKER must never clear the review');
assert.equal(result.findings.length, 0, 'it is still not a kept finding');
assert.equal(result.unverified.length, 1);
assert.equal(result.unverified[0].suppressed_reason, 'succinctness:title');
assert.equal(result.suppressed.length, 1, 'suppressed stays the union of dropped + unverified');
assert.equal(result.verdict, 'BLOCK');
assert.equal(result.findings.length, 1);
assert.equal(result.unverified.length, 0);
assert.equal(result.suppressed.length, 0);
});
test('runContract — declined_to_judge lists every unverified finding with its reason; trimmed_count counts trims', () => {
const result = runContract([
{ reviewer: 'code-correctness-reviewer', findings: [
{ file: 'a.mjs', line: 1, rule_key: 'MISSING_TEST', severity: 'MAJOR', title: 'empty action', recommended_action: '' },
{ file: 'b.mjs', line: 2, rule_key: 'MISSING_ERROR_HANDLING', severity: 'MINOR', title: 'long', detail: 'z'.repeat(850), recommended_action: 'Guard it.' },
] },
]);
assert.equal(result.declined_to_judge.length, 1);
assert.deepEqual(
{ file: result.declined_to_judge[0].file, reason: result.declined_to_judge[0].reason },
{ file: 'a.mjs', reason: 'actionability:empty' },
);
assert.equal(result.trimmed_count, 1);
assert.equal(result.counts.MINOR, 1, 'the trimmed MINOR still counts in its tier');
});
test('runContract — a schema-invalid payload cannot yield ALLOW (an unread reviewer is an absent one)', () => {
@ -333,3 +381,25 @@ test('runContract — an anonymous invalid payload forbids ALLOW on its own, wit
assert.equal(result.unattributable_payloads, 1);
assert.notEqual(result.verdict, 'ALLOW');
});
// ---- No finding is struck for length (ordre 5, P11) --------------------------
//
// The defect (STATE, Åpne funn; S118): a finding whose `detail` ran past 800
// characters went to `unverified` on `succinctness:detail` alone, and S118's most
// serious finding fell out of the Handover 6 list that way. Length is packaging,
// not a claim: only MINOR/SUGGESTION text is trimmed, and every trim is counted.
test('runContract — a BLOCKER with a 900-character detail survives and yields BLOCK', () => {
const detail = 'The refresh token is accepted after logout because the revocation list is never consulted. '.repeat(10).slice(0, 900);
assert.equal(detail.length, 900);
const result = runContract([
{ reviewer: 'code-correctness-reviewer', findings: [
{ file: 'lib/auth/refresh.mjs', line: 31, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER',
title: 'Revoked refresh token still accepted', detail, recommended_action: 'Check the revocation list before issuing.' },
] },
]);
assert.equal(result.verdict, 'BLOCK');
assert.equal(result.findings.length, 1, 'the long BLOCKER is a kept finding');
assert.equal(result.findings[0].detail, detail, 'its detail is not cut');
assert.equal(result.unverified.length, 0);
});

13
tests/lib/doc-consistency.test.mjs
View file

@ -209,11 +209,12 @@ test('HANDOVER-CONTRACTS.md contains Handover 7 section (session-state)', () =>
});
test('review-validator has CLI shim', () => {
// Ordre 5: the realpath form (no-op-free under a symlink or a space); the run
// through a symlink is proven in tests/validators/review-validator.test.mjs.
const text = read('lib/validators/review-validator.mjs');
assert.ok(
text.includes('import.meta.url === '),
'lib/validators/review-validator.mjs should expose the standard CLI shim ' +
'(if (import.meta.url === `file://${process.argv[1]}`)) so commands can call it from Bash',
text.includes('realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url))'),
'lib/validators/review-validator.mjs should expose the realpath CLI shim so commands can call it from Bash',
);
});
@ -268,12 +269,12 @@ test('HANDOVER-CONTRACTS.md Handover 7 § Lifecycle names --cleanup and produced
// file" (commands/trekcontinue.md exists, so that loop already asserts the
// /trekcontinue reference). Single-command existence pin → redundant.
test('rule-catalogue has exactly 12 entries', async () => {
test('rule-catalogue has exactly 14 entries', async () => {
const mod = await import('../../lib/review/rule-catalogue.mjs');
assert.strictEqual(
mod.RULE_CATALOGUE.length,
12,
'lib/review/rule-catalogue.mjs RULE_CATALOGUE size invariant: must be 12 (v1.0 baseline)',
14,
'lib/review/rule-catalogue.mjs RULE_CATALOGUE size invariant: must be 14 (12 v1.0 baseline + 2 engine keys, ordre 5)',
);
});

111
tests/lib/engine-findings.test.mjs Normal file
View file

@ -0,0 +1,111 @@
// tests/lib/engine-findings.test.mjs
// Claude Code's /code-review and /security-review as the engine (ordre 5, P11):
// their transcribed findings become a coordinator payload, and meet Voyage's
// brief control in the same dedup + verdict.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { toReviewerPayload, ENGINES } from '../../lib/review/engine-findings.mjs';
import { runContract } from '../../lib/review/coordinator-contract.mjs';
import { validateFindings } from '../../lib/review/findings-schema.mjs';
test('engine-findings — a /code-review finding becomes a MAJOR CODE_REVIEW_DEFECT', () => {
const r = toReviewerPayload({ engine: 'code-review', findings: [
{ file: './lib/a.mjs', line: 12, summary: 'Off-by-one drops the last page', failure_scenario: '10 items, page size 5 → page 2 empty', category: 'correctness' },
] });
assert.equal(r.ok, true, r.errors.join('\n'));
assert.equal(r.payload.reviewer, ENGINES['code-review'].reviewer);
const [f] = r.payload.findings;
assert.deepEqual([f.rule_key, f.severity, f.file, f.line], ['CODE_REVIEW_DEFECT', 'MAJOR', 'lib/a.mjs', 12]);
assert.match(f.detail, /page 2 empty/);
assert.equal(validateFindings(r.payload).valid, true);
});
test('engine-findings — a /security-review finding becomes a BLOCKER SECURITY_REVIEW_VULNERABILITY', () => {
const r = toReviewerPayload({ engine: 'security-review', findings: [
{ file: 'lib/q.mjs', line: 4, summary: 'SQL built from request body' },
] });
assert.equal(r.ok, true);
assert.equal(r.payload.findings[0].severity, 'BLOCKER');
});
test('engine-findings — a narrower catalogue key given by the orchestrator is kept; an unknown one falls back', () => {
const r = toReviewerPayload({ engine: 'code-review', findings: [
{ file: 'a.mjs', line: 1, summary: 's', rule_key: 'MISSING_TEST' },
{ file: 'b.mjs', line: 1, summary: 's', rule_key: 'MADE_UP' },
] });
assert.deepEqual(r.payload.findings.map((f) => f.rule_key), ['MISSING_TEST', 'CODE_REVIEW_DEFECT']);
});
test('engine-findings — a file outside the review scope is listed, never dropped silently', () => {
const r = toReviewerPayload({ engine: 'security-review', findings: [
{ file: 'in.mjs', line: 1, summary: 'a' },
{ file: 'old/out.mjs', line: 1, summary: 'b' },
] }, { scopeFiles: ['in.mjs'] });
assert.equal(r.payload.findings.length, 1);
assert.equal(r.out_of_scope.length, 1);
assert.equal(r.out_of_scope[0].file, 'old/out.mjs');
});
test('engine-findings — empty file or summary is an input error, unknown engine too', () => {
assert.equal(toReviewerPayload({ engine: 'code-review', findings: [{ file: '', summary: 'x' }] }).ok, false);
assert.equal(toReviewerPayload({ engine: 'code-review', findings: [{ file: 'a', summary: '' }] }).ok, false);
assert.equal(toReviewerPayload({ engine: 'simplify', findings: [] }).ok, false);
});
test('engine-findings — engine and brief control meet in one verdict; engine + Voyage on the same triplet collapse', () => {
const engine = toReviewerPayload({ engine: 'code-review', findings: [
{ file: 'lib/a.mjs', line: 12, summary: 'No test covers the new branch', rule_key: 'MISSING_TEST' },
] }).payload;
const conformance = { reviewer: 'brief-conformance-reviewer', findings: [
{ file: 'lib/a.mjs', line: 12, rule_key: 'MISSING_TEST', severity: 'MAJOR', title: 'SC2 untested' },
{ file: 'lib/b.mjs', line: 3, rule_key: 'UNIMPLEMENTED_CRITERION', severity: 'BLOCKER', title: 'SC3 missing' },
] };
const result = runContract([engine, conformance],
{ expectedReviewers: [engine.reviewer, 'brief-conformance-reviewer'] });
assert.equal(result.verdict, 'BLOCK', 'the brief control stops');
assert.equal(result.findings.length, 2);
const a = result.findings.find((f) => f.file === 'lib/a.mjs');
assert.deepEqual([...a.raised_by].sort(), ['brief-conformance-reviewer', 'claude-code-review']);
});
test('engine-findings — a "narrower" key can never lower a security BLOCKER', () => {
const r = toReviewerPayload({ engine: 'security-review', findings: [
{ file: 'a.mjs', line: 1, summary: 's', rule_key: 'MISSING_ERROR_HANDLING' }, // MINOR → refused
{ file: 'b.mjs', line: 1, summary: 's', rule_key: 'SECURITY_INJECTION' }, // BLOCKER → kept
] });
assert.deepEqual(r.payload.findings.map((f) => [f.rule_key, f.severity]),
[['SECURITY_REVIEW_VULNERABILITY', 'BLOCKER'], ['SECURITY_INJECTION', 'BLOCKER']]);
});
test('engine-findings — absolute paths under the repo root match the scope; a numeric-string line is kept', () => {
const r = toReviewerPayload({ engine: 'code-review', findings: [
{ file: '/abs/repo/lib/a.mjs', line: '12', summary: 's' },
] }, { scopeFiles: ['lib/a.mjs'], repoRoot: '/abs/repo' });
assert.equal(r.payload.findings.length, 1);
assert.equal(r.payload.findings[0].file, 'lib/a.mjs');
assert.equal(r.payload.findings[0].line, 12);
});
test('engine-findings — an empty scope list is an error, never "everything out of scope"', () => {
const r = toReviewerPayload({ engine: 'security-review', findings: [{ file: 'a.mjs', summary: 's' }] }, { scopeFiles: [] });
assert.equal(r.ok, false);
});
test('engine-findings — an out-of-scope engine finding forbids ALLOW and is declined to judge', () => {
const r = toReviewerPayload({ engine: 'security-review', findings: [{ file: 'old/x.mjs', line: 2, summary: 'leak' }] },
{ scopeFiles: ['in.mjs'] });
const result = runContract(
[r.payload, { reviewer: 'brief-conformance-reviewer', findings: [] }],
{ expectedReviewers: [r.payload.reviewer, 'brief-conformance-reviewer'], engineOutOfScope: r.out_of_scope },
);
assert.equal(result.verdict, 'WARN');
assert.deepEqual(result.declined_to_judge.map((d) => d.reason), ['engine:out-of-scope']);
});
test('engine-findings — an engine that was expected but never reported forbids ALLOW', () => {
const result = runContract([{ reviewer: 'brief-conformance-reviewer', findings: [] }],
{ expectedReviewers: ['brief-conformance-reviewer', ENGINES['security-review'].reviewer] });
assert.equal(result.verdict, 'WARN');
assert.deepEqual(result.missing_reviewers, ['claude-security-review']);
});

5
tests/lib/fidelity-diff.test.mjs
View file

@ -25,7 +25,7 @@ const RUN_B = fixture('review-run-B.md');
test('parseReviewArtifact — extracts verdict + finding IDs from frontmatter', () => {
const a = parseReviewArtifact(RUN_A);
assert.equal(a.verdict, 'WARN');
assert.equal(a.verdict, 'BLOCK', 'one BLOCKER → BLOCK (the fixture said WARN until ordre 5; the threshold table never allowed that)');
assert.ok(Array.isArray(a.findingIds));
assert.equal(a.findingIds.length, 5);
for (const id of a.findingIds) assert.match(id, /^[0-9a-f]{40}$/);
@ -71,7 +71,8 @@ test('fidelityDiff — A vs B (A ⊂ B): same verdict, jaccard 5/6, equivalent a
test('fidelityDiff — divergent verdict makes it non-equivalent even at jaccard 1.0', () => {
const a = parseReviewArtifact(RUN_A);
const flipped = RUN_A.replace('verdict: WARN', 'verdict: BLOCK');
const flipped = RUN_A.replace('verdict: BLOCK', 'verdict: WARN');
assert.notEqual(flipped, RUN_A, 'fixture control: the verdict line was flipped');
const d = fidelityDiff(RUN_A, flipped);
assert.equal(d.verdictMatch, false);
assert.equal(d.jaccard, 1);

191
tests/lib/review-prose-pins.test.mjs Normal file
View file

@ -0,0 +1,191 @@
// tests/lib/review-prose-pins.test.mjs
// Prose pins for the review process (ordre 5, bølge 1: P11 + P5). Counted as
// doc-pins by lib/util/test-census.mjs. Each pin names the behaviour it guards
// so a failing pin says what broke, not just which string moved.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFileSync } from 'node:fs';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
import { RULE_KEYS } from '../../lib/review/rule-catalogue.mjs';
import { UNVERIFIED_REASONS } from '../../lib/review/coordinator-contract.mjs';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const read = (rel) => readFileSync(join(ROOT, rel), 'utf-8');
function section(text, heading) {
const start = text.indexOf(heading);
assert.ok(start >= 0, `heading missing: ${heading}`);
const next = text.indexOf('\n## ', start + heading.length);
return text.slice(start, next === -1 ? undefined : next);
}
const CMD = read('commands/trekreview.md');
const COORD = read('agents/review-coordinator.md');
// ---- P11: Claude Code's review as the engine --------------------------------
test('trekreview — frontmatter allows the Skill tool (the engine is started through it)', () => {
const fm = CMD.slice(0, CMD.indexOf('\n---', 4));
assert.match(fm, /allowed-tools:.*\bSkill\b/);
});
test('trekreview — Phase 4.7 starts /code-review and /security-review through the Skill tool', () => {
const p = section(CMD, '## Phase 4.7');
assert.ok(p.includes('Skill("code-review"'), 'code-review via Skill');
assert.ok(p.includes('Skill("security-review")'), 'security-review via Skill');
assert.ok(p.includes('lib/review/engine-findings.mjs'), 'engine findings go through the adapter');
});
test('trekreview — the engine never edits code, posts or uploads (no --fix/--comment/--post/ultra)', () => {
const p = section(CMD, '## Phase 4.7');
assert.match(p, /\*\*Never\*\* pass `--fix`, `--comment`, `--post` or `ultra`/);
const call = p.match(/Skill\("code-review", args: "([^"]*)"\)/);
assert.ok(call, 'the code-review call is written out');
for (const banned of ['--fix', '--comment', '--post', 'ultra']) {
assert.ok(!call[1].includes(banned), `the engine call must not carry ${banned}`);
}
});
test('trekreview — an engine that did not run forbids ALLOW (fail-closed), and engine payloads stay out of the reviewers', () => {
const p = section(CMD, '## Phase 4.7');
assert.match(p, /forbids `ALLOW`/);
assert.match(p, /NOT shown to the Phase 5\s+reviewers/);
});
test('trekreview — the engine claims cite the live docs they were verified in', () => {
const p = section(CMD, '## Phase 4.7');
assert.ok(p.includes('v2.1.246'), 'model-invocation floor for /code-review');
assert.ok(p.includes("origin's"), '/security-review base is origin default branch');
});
// ---- P11: no finding struck for length; declined-to-judge ----------------------
test('coordinator — succinctness never removes a finding; only MINOR/SUGGESTION text is trimmed', () => {
assert.match(COORD, /\| Succinctness \|[^\n]*never removed/);
assert.ok(!UNVERIFIED_REASONS.includes('succinctness:detail'), 'length is no longer an unverified reason');
assert.ok(!UNVERIFIED_REASONS.includes('succinctness:title'));
});
test('coordinator — writes ## Declined to Judge with the trim count', () => {
assert.ok(COORD.includes('`## Declined to Judge` — REQUIRED'));
assert.ok(COORD.includes('Trimmed: N'));
});
// ---- K7: the verdict in the file, and the gate -------------------------------
test('coordinator — review_version 1.1 frontmatter carries verdict', () => {
assert.ok(COORD.includes('review_version: "1.1"'));
assert.match(COORD, /\nverdict: \{BLOCK \| WARN \| ALLOW\}\n/);
});
test('trekreview — Phase 8 decides with lib/review/verdict-gate.mjs and calls exit 2 not green', () => {
const p = section(CMD, '## Phase 8 —');
assert.ok(p.includes('lib/review/verdict-gate.mjs'));
assert.match(p, /never\s+green/);
});
test('gate — a control that did not run stops a merge (operator decision 2026-10-06) is written down', () => {
assert.match(section(CMD, '## Phase 8 —'), /a review not performed in full stops a\s+merge/);
assert.match(section(read('docs/HANDOVER-CONTRACTS.md'), '## Handover 6'), /did not run/);
});
test('trekreview — a BLOCKER is written as a numbered Step R<n> with Source finding (Handover 6)', () => {
assert.ok(CMD.includes('### Step R<n>'));
assert.ok(COORD.includes('**Source finding:**'));
const h6 = section(read('docs/HANDOVER-CONTRACTS.md'), '## Handover 6');
assert.ok(h6.includes('Remediation Steps'), 'Handover 6 documents the remediation steps');
assert.match(h6, /\| `verdict` \| string \| yes/, 'Handover 6 lists verdict as required');
assert.ok(h6.includes('verdict-gate.mjs'), 'Handover 6 names the gate');
});
test('template — templates/trekreview-template.md validates strictly and the gate reads it as BLOCK', async () => {
const { validateReview } = await import('../../lib/validators/review-validator.mjs');
const { decideVerdictFile } = await import('../../lib/review/verdict-gate.mjs');
const path = join(ROOT, 'templates', 'trekreview-template.md');
const v = validateReview(path, { strict: true });
assert.equal(v.valid, true, JSON.stringify(v.errors));
assert.equal(decideVerdictFile(path).exit, 1);
});
test('trekreview — the zero-diff review the prose prescribes passes the gate as ALLOW', async () => {
const { decideVerdict } = await import('../../lib/review/verdict-gate.mjs');
const p3 = section(CMD, '## Phase 3');
assert.ok(p3.includes('findings: []') && p3.includes('"findings": []') && p3.includes('"allow_blocked_by": []'),
'Phase 3 prescribes the empty list, the JSON block and allow_blocked_by');
const text = `---
type: trekreview
review_version: "1.1"
task: "t"
slug: s
project_dir: p/
brief_path: p/brief.md
scope_sha_end: abc
reviewed_files_count: 0
verdict: ALLOW
findings: []
---
## Executive Summary
No diff.
## Coverage
No diff.
## Declined to Judge
None.
Trimmed: 0
## Remediation Summary
No diff.
\`\`\`json
{"verdict": "ALLOW", "counts": {"BLOCKER": 0, "MAJOR": 0, "MINOR": 0, "SUGGESTION": 0}, "allow_blocked_by": [], "findings": []}
\`\`\`
`;
assert.equal(decideVerdict(text).exit, 0, decideVerdict(text).reason);
});
test('trekreview — Phase 8.7 points recurrence at the session directory, not the git toplevel', () => {
const p = section(CMD, '## Phase 8.7');
assert.ok(p.includes('--repo "$PWD"'));
assert.ok(!p.includes('--repo "$(git rev-parse --show-toplevel)"'));
});
// ---- P5: project memory --------------------------------------------------------
test('trekreview — Phase 8.7 asks the operator per line (AskUserQuestion) before recurrence.mjs apply', () => {
const p = section(CMD, '## Phase 8.7');
const ask = p.indexOf('AskUserQuestion');
const apply = p.indexOf('recurrence.mjs apply');
assert.ok(ask >= 0 && apply > ask, 'approval comes before the write');
assert.match(p, /over-budget/);
assert.match(p, /belong in a skill/);
});
test('trekreview — the write-outside-project_dir exception names only the marked block and the pointer', () => {
const rules = section(CMD, '## Hard rules');
assert.ok(rules.includes('.claude/rules/voyage-review-lessons.md'));
assert.ok(rules.includes('only after the operator approved that exact line'));
});
// ---- the Workflow substrate stays in step -----------------------------------------
test('armB workflow — its inlined rule-key enum equals the catalogue (no drift)', () => {
const src = read('scripts/trekreview-armB.workflow.mjs');
const block = src.slice(src.indexOf('const RULE_KEYS = ['), src.indexOf('];', src.indexOf('const RULE_KEYS = [')));
const keys = [...block.matchAll(/'([A-Z_]+)'/g)].map((m) => m[1]).sort();
assert.deepEqual(keys, [...RULE_KEYS].sort());
});
test('armB workflow — a missing reviewer or engine forbids ALLOW on the Workflow path too', () => {
const src = read('scripts/trekreview-armB.workflow.mjs');
assert.ok(src.includes("if (verdict === 'ALLOW' && missingReviewers > 0) verdict = 'WARN';"));
assert.ok(src.includes('enginePayloads'));
});

309
tests/lib/review-recurrence.test.mjs Normal file
View file

@ -0,0 +1,309 @@
// tests/lib/review-recurrence.test.mjs
// P5 — the project's memory (ordre 5). A finding class (the same rule_key) that
// shows up in two review.md files in the target repo becomes ONE proposal; one
// sighting is no proposal. Proposals are read from the review.md files the repo
// already has (no new store), and an approved one is written only between
// Voyage's own markers, under a size budget, with CLAUDE.md getting a pointer.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, existsSync, symlinkSync, lstatSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const CLI = join(ROOT, 'lib', 'review', 'recurrence.mjs');
const mod = existsSync(CLI) ? await import('../../lib/review/recurrence.mjs') : null;
function reviewText({ slug, sha, findings }) {
const json = JSON.stringify({
verdict: 'WARN',
counts: { BLOCKER: 0, MAJOR: findings.length, MINOR: 0, SUGGESTION: 0 },
findings,
}, null, 2);
return `---
type: trekreview
review_version: "1.1"
task: "${slug}"
slug: ${slug}
project_dir: .claude/projects/2026-10-0${sha.length % 9}-${slug}/
brief_path: .claude/projects/x/brief.md
scope_sha_end: ${sha}
reviewed_files_count: 1
verdict: WARN
findings:
${findings.map((f) => ` - ${f.id}`).join('\n')}
---
# Review: ${slug}
## Executive Summary
x
## Coverage
x
## Remediation Summary
x
\`\`\`json
${json}
\`\`\`
`;
}
function finding(id, rule_key, extra = {}) {
return {
id: id.padEnd(40, '0'), severity: 'MAJOR', rule_key, file: 'lib/a.mjs', line: 3,
title: `${rule_key} title ${id}`, detail: 'd', recommended_action: `Fix it (${id}).`, ...extra,
};
}
function repoWith(reviews) {
const dir = mkdtempSync(join(tmpdir(), 'recurrence-'));
reviews.forEach((r, i) => {
const p = join(dir, '.claude', 'projects', `2026-10-0${i + 1}-${r.slug}`);
mkdirSync(p, { recursive: true });
writeFileSync(join(p, 'review.md'), reviewText(r));
});
return dir;
}
test('recurrence — the module exists', () => {
assert.ok(mod, 'lib/review/recurrence.mjs must exist (P5)');
});
test('recurrence — two review.md with the same finding class → exactly one proposal', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST', { file: 'lib/b.mjs' })] },
]);
try {
const { proposals } = mod.proposeLessons(repo);
assert.equal(proposals.length, 1);
assert.equal(proposals[0].rule_key, 'MISSING_TEST');
assert.equal(proposals[0].evidence.length, 2, 'the proposal carries both findings as its evidence');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — one review.md with the class → no proposal', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'PLACEHOLDER_IN_CODE')] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — the same class twice inside ONE review is still one sighting → no proposal', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST'), finding('a2', 'MISSING_TEST', { line: 9 })] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — routing: mechanical → a check, judgement → a .claude/rules/ file', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'PLACEHOLDER_IN_CODE'), finding('a2', 'PLAN_EXECUTE_DRIFT')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'PLACEHOLDER_IN_CODE'), finding('b2', 'PLAN_EXECUTE_DRIFT')] },
]);
try {
const byKey = Object.fromEntries(mod.proposeLessons(repo).proposals.map((p) => [p.rule_key, p]));
assert.equal(byKey.PLACEHOLDER_IN_CODE.route, 'check');
assert.equal(byKey.PLAN_EXECUTE_DRIFT.route, 'rule');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — the proposed line has the three parts: mistake · fix · how to check', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
const [p] = mod.proposeLessons(repo).proposals;
assert.match(p.line, /Mistake:/);
assert.match(p.line, /Fix:/);
assert.match(p.line, /How to check:/);
assert.equal(mod.validateLessonLine(p.line).valid, true);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — review-hygiene classes never become target-repo lessons', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_BRIEF_REF'), finding('a2', 'COVERAGE_SILENT_SKIP')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_BRIEF_REF'), finding('b2', 'COVERAGE_SILENT_SKIP')] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — apply writes only between Voyage markers and gives CLAUDE.md a pointer only', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
writeFileSync(join(repo, 'CLAUDE.md'), '# Project\n\nOperator text stays.\n');
mkdirSync(join(repo, '.claude', 'rules'), { recursive: true });
writeFileSync(join(repo, '.claude', 'rules', mod.LESSONS_FILE_NAME), 'Operator preface.\n');
const r = mod.applyLesson(repo, 'MISSING_TEST');
assert.equal(r.written, true, JSON.stringify(r));
const rules = readFileSync(join(repo, '.claude', 'rules', mod.LESSONS_FILE_NAME), 'utf-8');
assert.ok(rules.startsWith('Operator preface.\n'), 'text outside the markers is untouched');
const start = rules.indexOf(mod.MARK_START);
const end = rules.indexOf(mod.MARK_END);
assert.ok(start >= 0 && end > start);
assert.match(rules.slice(start, end), /MISSING_TEST/);
const claude = readFileSync(join(repo, 'CLAUDE.md'), 'utf-8');
assert.ok(claude.startsWith('# Project\n\nOperator text stays.\n'));
assert.ok(claude.includes(mod.LESSONS_FILE_NAME), 'CLAUDE.md gets a pointer to the rules file');
assert.ok(!claude.includes('Mistake:'), 'CLAUDE.md gets ONLY the pointer, never the lesson');
assert.deepEqual(mod.proposeLessons(repo).proposals, [], 'an applied class is not proposed again');
const again = mod.applyLesson(repo, 'MISSING_TEST');
assert.equal(again.written, false, 'applying twice writes nothing');
const claude2 = readFileSync(join(repo, 'CLAUDE.md'), 'utf-8');
assert.equal(claude2, claude, 'the pointer is written once');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — apply refuses past the size budget (the lesson belongs in a skill then)', () => {
const keys = ['MISSING_TEST', 'PLACEHOLDER_IN_CODE', 'PLAN_EXECUTE_DRIFT', 'SCOPE_CREEP_BUILT'];
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: keys.map((k, i) => finding(`a${i}`, k)) },
{ slug: 'two', sha: 'bbb', findings: keys.map((k, i) => finding(`b${i}`, k)) },
]);
try {
const r = mod.applyLesson(repo, 'MISSING_TEST', { budgetEntries: 0 });
assert.equal(r.written, false);
assert.equal(r.reason, 'over-budget');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — an edited line without the three parts is refused', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
const r = mod.applyLesson(repo, 'MISSING_TEST', { line: 'just do better' });
assert.equal(r.written, false);
assert.equal(r.reason, 'invalid-line');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — an earlier committed version of the same review.md counts as history (git, no new store)', () => {
// review.md is overwritten per iteration; git keeps the earlier one.
const repo = repoWith([{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'PLAN_EXECUTE_DRIFT')] }]);
const g = (...a) => spawnSync('git', ['-C', repo, ...a], { encoding: 'utf-8' });
try {
assert.equal(g('init', '-q').status, 0);
g('config', 'user.email', 't@example.invalid');
g('config', 'user.name', 't');
g('add', '.claude');
assert.equal(g('commit', '-q', '-m', 'review 1').status, 0);
assert.deepEqual(mod.proposeLessons(repo).proposals, [], 'one iteration is one sighting');
const p = join(repo, '.claude', 'projects', '2026-10-01-one', 'review.md');
writeFileSync(p, reviewText({ slug: 'one', sha: 'bbbb', findings: [finding('b1', 'PLAN_EXECUTE_DRIFT', { line: 40 })] }));
const { proposals } = mod.proposeLessons(repo);
assert.equal(proposals.length, 1, 'the committed iteration plus the current one recur');
assert.equal(proposals[0].rule_key, 'PLAN_EXECUTE_DRIFT');
} finally { rmSync(repo, { recursive: true, force: true }); }
});
// ---- Independent review of d88245c: each of these was wrong before ----------
test('recurrence — the SAME unfixed finding in two iterations is one defect, not a recurring class', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('cc', 'MISSING_TEST')] },
{ slug: 'one', sha: 'bbbb', findings: [finding('cc', 'MISSING_TEST')] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — catch-all engine keys and keys normalised from unknown ones are never proposed', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'CODE_REVIEW_DEFECT'), finding('a2', 'SECURITY_REVIEW_VULNERABILITY'), finding('a3', 'PLAN_EXECUTE_DRIFT', { original_rule_key: 'AD_HOC' })] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'CODE_REVIEW_DEFECT'), finding('b2', 'SECURITY_REVIEW_VULNERABILITY'), finding('b3', 'PLAN_EXECUTE_DRIFT', { original_rule_key: 'AD_HOC' })] },
]);
try {
assert.deepEqual(mod.proposeLessons(repo).proposals, []);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — a proposal built from long titles, actions and marker text is still a line apply accepts', () => {
const long = 'x'.repeat(300);
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST', { title: `<!-- ${long}`, file: `lib/${long}.mjs`, recommended_action: long })] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST', { title: long, recommended_action: `${long} -->` })] },
]);
try {
const [p] = mod.proposeLessons(repo).proposals;
assert.ok(p, 'the proposal is not lost');
assert.equal(mod.validateLessonLine(p.line).valid, true, p.line);
assert.equal(mod.applyLesson(repo, 'MISSING_TEST').written, true);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — an edited line with \\r or U+2028 (a hidden second lesson) is refused', () => {
for (const sep of ['\r', '
']) {
const line = `Mistake: x${sep}- [SCOPE_CREEP_BUILT] (rule) Mistake: y Fix: z How to check: w`;
assert.equal(mod.validateLessonLine(line).valid, false, JSON.stringify(sep));
}
});
test('recurrence — damaged markers refuse the write instead of crashing', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
mkdirSync(join(repo, '.claude', 'rules'), { recursive: true });
writeFileSync(join(repo, '.claude', 'rules', mod.LESSONS_FILE_NAME), `${mod.MARK_END}\n${mod.MARK_START}\n`);
const r = mod.applyLesson(repo, 'MISSING_TEST');
assert.deepEqual([r.written, r.reason], [false, 'markers-damaged']);
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — a symlinked CLAUDE.md keeps its link and the target gets the pointer', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
writeFileSync(join(repo, 'AGENTS.md'), '# Agents\n');
symlinkSync('AGENTS.md', join(repo, 'CLAUDE.md'));
assert.equal(mod.applyLesson(repo, 'MISSING_TEST').written, true);
assert.ok(lstatSync(join(repo, 'CLAUDE.md')).isSymbolicLink(), 'the link survives');
assert.ok(readFileSync(join(repo, 'AGENTS.md'), 'utf-8').includes(mod.LESSONS_FILE_NAME));
} finally { rmSync(repo, { recursive: true, force: true }); }
});
test('recurrence — CLI propose prints JSON and exits 0; a missing repo exits 2', () => {
const repo = repoWith([
{ slug: 'one', sha: 'aaa', findings: [finding('a1', 'MISSING_TEST')] },
{ slug: 'two', sha: 'bbb', findings: [finding('b1', 'MISSING_TEST')] },
]);
try {
const r = spawnSync(process.execPath, [CLI, 'propose', '--repo', repo], { encoding: 'utf-8' });
assert.equal(r.status, 0, r.stderr);
assert.equal(JSON.parse(r.stdout).proposals.length, 1);
const missing = spawnSync(process.execPath, [CLI, 'propose', '--repo', join(repo, 'nope')], { encoding: 'utf-8' });
assert.equal(missing.status, 2);
} finally { rmSync(repo, { recursive: true, force: true }); }
});

20
tests/lib/rule-catalogue.test.mjs
View file

@ -6,6 +6,7 @@ import {
SEVERITY_VALUES,
CATEGORY_VALUES,
getRule,
MEMORY_ROUTES,
} from '../../lib/review/rule-catalogue.mjs';
test('RULE_CATALOGUE — every entry has all 4 required fields', () => {
@ -38,9 +39,24 @@ test('RULE_CATALOGUE — all category values within enum', () => {
}
});
test('RULE_KEYS.size === RULE_CATALOGUE.length (== 12) — pinned by doc-consistency', () => {
test('RULE_KEYS.size === RULE_CATALOGUE.length (== 14) — pinned by doc-consistency', () => {
assert.equal(RULE_KEYS.size, RULE_CATALOGUE.length);
assert.equal(RULE_CATALOGUE.length, 12);
assert.equal(RULE_CATALOGUE.length, 14);
});
test('RULE_CATALOGUE — the two Claude Code engine keys exist at their tiers (ordre 5)', () => {
assert.equal(getRule('CODE_REVIEW_DEFECT')?.severity, 'MAJOR');
assert.equal(getRule('SECURITY_REVIEW_VULNERABILITY')?.severity, 'BLOCKER');
});
test('RULE_CATALOGUE — every entry declares a memory_route (check | rule | null)', () => {
for (const entry of RULE_CATALOGUE) {
assert.ok('memory_route' in entry, `${entry.rule_key} has no memory_route`);
assert.ok(entry.memory_route === null || MEMORY_ROUTES.includes(entry.memory_route),
`${entry.rule_key} has invalid memory_route: ${entry.memory_route}`);
}
assert.equal(getRule('MISSING_BRIEF_REF').memory_route, null, 'review hygiene is never project memory');
assert.equal(getRule('COVERAGE_SILENT_SKIP').memory_route, null, 'review hygiene is never project memory');
});
test('getRule — returns frozen entry on hit, null on miss, null on bad input', () => {

238
tests/lib/verdict-gate.test.mjs Normal file
View file

@ -0,0 +1,238 @@
// tests/lib/verdict-gate.test.mjs
// K7 — the review verdict gate (ordre 5, P11). One command decides on the
// verdict written in review.md: 0 = ALLOW/WARN, 1 = BLOCK, 2 = could not read.
// Exit 2 is never green: a missing file, a missing verdict, or a verdict the
// file contradicts all land there.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, rmSync, existsSync, symlinkSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const GATE = join(ROOT, 'lib', 'review', 'verdict-gate.mjs');
const { decideVerdict } = existsSync(GATE) ? await import('../../lib/review/verdict-gate.mjs') : {};
function review({ verdict, jsonVerdict, severities = [], allowBlockedBy = [] }) {
const ids = severities.map((_, i) => String(i + 1).padStart(40, 'a'));
const fmVerdict = verdict === undefined ? '' : `verdict: ${verdict}\n`;
const findings = severities.map((s, i) => ({
id: ids[i], severity: s, rule_key: 'MISSING_TEST', file: 'a.mjs', line: i + 1, title: 't', detail: 'd', recommended_action: 'Add a test.',
}));
const counts = { BLOCKER: 0, MAJOR: 0, MINOR: 0, SUGGESTION: 0 };
for (const s of severities) counts[s] += 1;
const json = JSON.stringify({ verdict: jsonVerdict ?? verdict, counts, allow_blocked_by: allowBlockedBy, findings }, null, 2);
return `---
type: trekreview
review_version: "1.1"
task: "t"
slug: s
project_dir: .claude/projects/2026-10-06-s/
brief_path: .claude/projects/2026-10-06-s/brief.md
scope_sha_end: abc
reviewed_files_count: 1
${fmVerdict}findings:
${ids.map((id) => ` - ${id}`).join('\n')}
---
# Review: t
## Executive Summary
x
## Coverage
| File | Treatment | Reason |
|------|-----------|--------|
| a.mjs | summary-only | default |
## Remediation Summary
x
\`\`\`json
${json}
\`\`\`
`;
}
function runGate(text, { name = 'review.md', write = true } = {}) {
const dir = mkdtempSync(join(tmpdir(), 'verdict-gate-'));
try {
const p = join(dir, name);
if (write) writeFileSync(p, text);
const r = spawnSync(process.execPath, [GATE, p], { encoding: 'utf-8' });
return r;
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
test('verdict-gate — the CLI exists', () => {
assert.ok(existsSync(GATE), 'lib/review/verdict-gate.mjs must exist (K7)');
});
test('verdict-gate — BLOCK → exit 1', () => {
const r = runGate(review({ verdict: 'BLOCK', severities: ['BLOCKER'] }));
assert.equal(r.status, 1, r.stdout + r.stderr);
});
test('verdict-gate — ALLOW → exit 0', () => {
const r = runGate(review({ verdict: 'ALLOW', severities: ['MINOR'] }));
assert.equal(r.status, 0, r.stdout + r.stderr);
});
test('verdict-gate — WARN → exit 0', () => {
const r = runGate(review({ verdict: 'WARN', severities: ['MAJOR'] }));
assert.equal(r.status, 0, r.stdout + r.stderr);
});
test('verdict-gate — missing file → exit 2', () => {
const r = runGate('', { write: false });
assert.equal(r.status, 2, r.stdout + r.stderr);
});
test('verdict-gate — a review.md without verdict → exit 2, never ALLOW', () => {
const r = runGate(review({ verdict: undefined, jsonVerdict: 'ALLOW', severities: ['MINOR'] }));
assert.equal(r.status, 2, r.stdout + r.stderr);
});
test('verdict-gate — an unknown verdict value → exit 2', () => {
const r = runGate(review({ verdict: 'PASS', severities: ['MINOR'] }));
assert.equal(r.status, 2, r.stdout + r.stderr);
});
test('verdict-gate — frontmatter ALLOW over a BLOCKER finding is a contradiction → exit 2', () => {
const r = runGate(review({ verdict: 'ALLOW', severities: ['BLOCKER'] }));
assert.equal(r.status, 2, r.stdout + r.stderr);
});
test('verdict-gate — frontmatter and trailing JSON disagree → exit 2', () => {
const r = runGate(review({ verdict: 'WARN', jsonVerdict: 'BLOCK', severities: ['MAJOR'] }));
assert.equal(r.status, 2, r.stdout + r.stderr);
});
test('verdict-gate — not a review (wrong type) → exit 2', () => {
const r = runGate(review({ verdict: 'ALLOW', severities: ['MINOR'] }).replace('type: trekreview', 'type: trekbrief'));
assert.equal(r.status, 2, r.stdout + r.stderr);
});
test('verdict-gate — --json prints the decision as JSON', () => {
const dir = mkdtempSync(join(tmpdir(), 'verdict-gate-'));
try {
const p = join(dir, 'review.md');
writeFileSync(p, review({ verdict: 'BLOCK', severities: ['BLOCKER'] }));
const r = spawnSync(process.execPath, [GATE, '--json', p], { encoding: 'utf-8' });
assert.equal(r.status, 1);
const out = JSON.parse(r.stdout);
assert.equal(out.verdict, 'BLOCK');
assert.equal(out.exit, 1);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
// ---- Adversarial inputs (independent review of d88245c): each was exit 0 ----
function swapJson(text, jsonSrc) {
return text.replace(/```json\n[\s\S]*?```\n$/, `\`\`\`json\n${jsonSrc}\n\`\`\`\n`);
}
test('verdict-gate — a greener verdict over a malformed or evasive JSON block is exit 2', () => {
const base = review({ verdict: 'WARN', severities: ['MAJOR'] });
assert.equal(decideVerdict(base).exit, 0, 'control: the untouched WARN review passes');
const id = String(1).padStart(40, 'a');
for (const json of [
'{}',
'null',
`{"verdict":"WARN","counts":{"BLOCKER":0},"findings":[{"id":"${id}","severity":"blocker"}]}`,
`{"verdict":"WARN","counts":{"blocker":1},"findings":[{"id":"${id}","severity":"MAJOR"}]}`,
`{"verdict":"WARN","counts":{},"findings":{"${id}":{"severity":"BLOCKER"}}}`,
]) {
assert.equal(decideVerdict(swapJson(base, json)).exit, 2, json);
}
});
test('verdict-gate — the trailing json block must be the last fence', () => {
const base = review({ verdict: 'WARN', severities: ['MAJOR'] });
assert.equal(decideVerdict(`${base}\n\`\`\`json\n{"verdict":"WARN","counts":{},"findings":[]}\n\`\`\`\n`).exit, 2,
'a later json fence cannot replace the real block');
assert.equal(decideVerdict(base.replace(/```json\n(?=[\s\S]*```\n$)/, '```jsonc\n')).exit, 2,
'a jsonc last fence is not the json block');
});
test('verdict-gate — a ## Findings (BLOCKER) entry under a WARN verdict is exit 2', () => {
const base = review({ verdict: 'WARN', severities: ['MAJOR'] });
const t = base.replace('## Coverage', '## Findings (BLOCKER)\n\n### deadbeef\n\n- file: a.mjs\n\n## Coverage');
assert.equal(decideVerdict(t).exit, 2);
});
test('verdict-gate — two verdict lines in the frontmatter are exit 2', () => {
const base = review({ verdict: 'ALLOW', severities: ['MINOR'] });
assert.equal(decideVerdict(base.replace('verdict: ALLOW\n', 'verdict: BLOCK\nverdict: ALLOW\n')).exit, 2);
});
test('verdict-gate — frontmatter finding ids that differ from the JSON block are exit 2', () => {
const base = review({ verdict: 'WARN', severities: ['MAJOR'] });
const t = base.replace(` - ${String(1).padStart(40, 'a')}`, ` - ${'b'.repeat(40)}`);
assert.equal(decideVerdict(t).exit, 2);
});
// ---- Operator decision 2026-10-06: a control that did not run stops a merge ----
test('verdict-gate — WARN because a reviewer or engine did not run is exit 2, not a passable warning', () => {
for (const reason of ['missing-reviewer:claude-security-review', 'unattributable-payload (1)']) {
const t = review({ verdict: 'WARN', severities: ['MINOR'], allowBlockedBy: [reason] });
const d = decideVerdict(t);
assert.equal(d.exit, 2, reason);
assert.match(d.reason, /did not run/);
}
});
test('verdict-gate — WARN for a set-aside finding or a MAJOR still passes (exit 0) with its reasons', () => {
const d = decideVerdict(review({ verdict: 'WARN', severities: ['MAJOR'], allowBlockedBy: ['unverified:actionability:empty (1)'] }));
assert.equal(d.exit, 0);
assert.deepEqual(d.warn_reasons, ['unverified:actionability:empty (1)']);
});
test('verdict-gate — a 1.1 review whose JSON block lacks allow_blocked_by cannot pass', () => {
const base = review({ verdict: 'WARN', severities: ['MAJOR'] });
const t = base.replace(/\s*"allow_blocked_by": \[\],?/, '');
assert.notEqual(t, base, 'fixture control');
assert.equal(decideVerdict(t).exit, 2);
});
test('verdict-gate — more than one file argument is refused (exit 2), never a first-match decision', () => {
const dir = mkdtempSync(join(tmpdir(), 'verdict-gate-'));
try {
const a = join(dir, 'a.md');
const b = join(dir, 'b.md');
writeFileSync(a, review({ verdict: 'ALLOW', severities: ['MINOR'] }));
writeFileSync(b, review({ verdict: 'BLOCK', severities: ['BLOCKER'] }));
const r = spawnSync(process.execPath, [GATE, a, b], { encoding: 'utf-8' });
assert.equal(r.status, 2);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
test('verdict-gate — run through a symlink and a path with a space it still decides (realpath main guard)', () => {
// The old `file://${argv[1]}` guard is a no-op under a symlink or a space:
// the CLI would exit 0 without reading anything — a silent green.
const dir = mkdtempSync(join(tmpdir(), 'verdict gate '));
try {
const link = join(dir, 'gate link.mjs');
symlinkSync(GATE, link);
const p = join(dir, 'my review.md');
writeFileSync(p, review({ verdict: 'BLOCK', severities: ['BLOCKER'] }));
const r = spawnSync(process.execPath, [link, p], { encoding: 'utf-8' });
assert.equal(r.status, 1, r.stdout + r.stderr);
assert.match(r.stdout, /BLOCK/);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

71
tests/scripts/trekreview-armb-workflow.test.mjs Normal file
View file

@ -0,0 +1,71 @@
// tests/scripts/trekreview-armb-workflow.test.mjs
// The opt-in Workflow substrate for /trekreview Phase 5–6, run for real with
// stubbed Workflow globals (agent/parallel/phase/log). Ordre 5: a reviewer or
// engine that did not report forbids ALLOW, and the verdict's floor is
// recomputed from the findings the coordinator returns.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { readFileSync } from 'node:fs';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
const SRC = readFileSync(join(ROOT, 'scripts', 'trekreview-armB.workflow.mjs'), 'utf-8')
.replace('export const meta', 'const meta');
const AsyncFunction = Object.getPrototypeOf(async () => {}).constructor;
const run = new AsyncFunction('args', 'agent', 'parallel', 'phase', 'log', SRC);
const BASE = { briefPath: 'b.md', diffPath: 'd.diff', triage: '' };
const engine = (reviewer, findings = []) => ({ reviewer, findings });
function stubs({ reviewers = [], coordinator = null } = {}) {
let i = 0;
return {
agent: async (_prompt, opts) => (opts.label === 'coordinator' ? coordinator : reviewers[i++] ?? null),
parallel: async (thunks) => Promise.all(thunks.map((t) => t())),
phase: () => {},
log: () => {},
};
}
async function exec(args, s) {
return run(args, s.agent, s.parallel, s.phase, s.log);
}
test('armB — both reviewers and both engines clean → ALLOW (known-positive control)', async () => {
const s = stubs({ reviewers: [{ reviewer: 'c', findings: [] }, { reviewer: 'k', findings: [] }] });
const r = await exec({ ...BASE, enginePayloads: [engine('claude-code-review'), engine('claude-security-review')] }, s);
assert.equal(r.verdict, 'ALLOW');
assert.deepEqual(r.missing_engines, []);
});
test('armB — engines the caller never mentioned are missing → not ALLOW', async () => {
const s = stubs({ reviewers: [{ reviewer: 'c', findings: [] }, { reviewer: 'k', findings: [] }] });
const r = await exec({ ...BASE }, s);
assert.equal(r.verdict, 'WARN');
assert.deepEqual(r.missing_engines, ['claude-code-review', 'claude-security-review']);
});
test('armB — a reviewer that returned nothing → not ALLOW', async () => {
const s = stubs({ reviewers: [null, { reviewer: 'k', findings: [] }] });
const r = await exec({ ...BASE, enginePayloads: [engine('claude-code-review'), engine('claude-security-review')] }, s);
assert.equal(r.verdict, 'WARN');
});
test('armB — a coordinator ALLOW over a BLOCKER finding is recomputed to BLOCK', async () => {
const f = { severity: 'BLOCKER', rule_key: 'UNIMPLEMENTED_CRITERION', file: 'a.mjs', line: 1, title: 't', detail: 'd' };
const s = stubs({
reviewers: [{ reviewer: 'c', findings: [f] }, { reviewer: 'k', findings: [] }],
coordinator: { verdict: 'ALLOW', findings: [f] },
});
const r = await exec({ ...BASE, enginePayloads: [engine('claude-code-review'), engine('claude-security-review')] }, s);
assert.equal(r.verdict, 'BLOCK');
});
test('armB — quick mode expects only the /code-review engine', async () => {
const s = stubs({ reviewers: [{ reviewer: 'k', findings: [] }] });
const r = await exec({ ...BASE, quick: true, enginePayloads: [engine('claude-code-review')] }, s);
assert.equal(r.verdict, 'ALLOW');
assert.deepEqual(r.missing_engines, []);
});

110
tests/validators/review-validator.test.mjs
View file

@ -1,7 +1,14 @@
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, writeFileSync, rmSync, symlinkSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
import { validateReviewContent } from '../../lib/validators/review-validator.mjs';
const VALIDATOR = join(dirname(fileURLToPath(import.meta.url)), '..', '..', 'lib', 'validators', 'review-validator.mjs');
const GOOD_REVIEW = `---
type: trekreview
review_version: "1.0"
@ -13,6 +20,7 @@ brief_path: .claude/projects/2026-04-30-jwt-auth/brief.md
scope_sha_start: abc123
scope_sha_end: def456
reviewed_files_count: 7
verdict: ALLOW
findings:
- 0123456789abcdef0123456789abcdef01234567
- fedcba9876543210fedcba9876543210fedcba98
@ -112,3 +120,105 @@ test('validateReview — empty findings array is acceptable (no findings = ALLOW
const r = validateReviewContent(t);
assert.equal(r.valid, true, JSON.stringify(r.errors));
});
// ---- K7: the verdict is required in review.md (ordre 5) ----------------------
const NO_VERDICT = GOOD_REVIEW.replace(/^verdict: .*\n/m, '');
test('validateReview — strict mode rejects a review.md without verdict (REVIEW_MISSING_VERDICT)', () => {
assert.ok(!/^verdict:/m.test(NO_VERDICT), 'fixture control: the verdict line is gone');
const r = validateReviewContent(NO_VERDICT, { strict: true });
assert.equal(r.valid, false, 'a review without a verdict must not pass the write-time check');
assert.ok(r.errors.find(e => e.code === 'REVIEW_MISSING_VERDICT'), JSON.stringify(r.errors));
});
test('validateReview — soft (read-time) mode only warns on a missing verdict (N-1 window for 1.0 reviews)', () => {
const r = validateReviewContent(NO_VERDICT, { strict: false });
assert.equal(r.valid, true, JSON.stringify(r.errors));
assert.ok(r.warnings.find(w => w.code === 'REVIEW_MISSING_VERDICT'));
});
test('validateReview — a verdict outside ALLOW|WARN|BLOCK is an error in both modes', () => {
const t = GOOD_REVIEW.replace('verdict: ALLOW', 'verdict: PASS');
for (const strict of [true, false]) {
const r = validateReviewContent(t, { strict });
assert.equal(r.valid, false);
assert.ok(r.errors.find(e => e.code === 'REVIEW_BAD_VERDICT'));
}
});
// ---- A BLOCKER is written as a plan step Handover 6 can append (ordre 5) ----
const BLOCKER_ID = '763d174e6c519fafbadcba5d1706708479e36e61';
const WITH_BLOCKER = GOOD_REVIEW.replace('verdict: ALLOW', 'verdict: BLOCK')
.replace('review_version: "1.0"', 'review_version: "1.1"')
.replace('## Remediation Summary', '## Declined to Judge\n\nNone.\n\nTrimmed: 0\n\n## Remediation Summary') + `
\`\`\`json
{"verdict": "BLOCK", "counts": {"BLOCKER": 1, "MAJOR": 0, "MINOR": 0, "SUGGESTION": 0},
"findings": [{"id": "${BLOCKER_ID}", "severity": "BLOCKER", "rule_key": "UNIMPLEMENTED_CRITERION", "file": "lib/login.mjs", "line": 23}]}
\`\`\`
`;
const STEP = `
## Remediation Steps
### Step R1: Return 401 on a bad password
- **Files:** \`lib/login.mjs\`
- **Source finding:** \`${BLOCKER_ID}\`
- **Verify:** re-run /trekreview → finding \`${BLOCKER_ID}\` absent
`;
test('validateReview — strict: a BLOCKER without a numbered remediation step is an error', () => {
const r = validateReviewContent(WITH_BLOCKER, { strict: true });
assert.equal(r.valid, false);
assert.ok(r.errors.find(e => e.code === 'REVIEW_MISSING_REMEDIATION_STEP'), JSON.stringify(r.errors));
});
test('validateReview — strict: a BLOCKER with its "Step R1 … Source finding" passes', () => {
const t = WITH_BLOCKER.replace('## Remediation Summary', `${STEP}\n## Remediation Summary`);
const r = validateReviewContent(t, { strict: true });
assert.equal(r.valid, true, JSON.stringify(r.errors));
});
test('validateReview — a 1.0 review with a BLOCKER is not held to the 1.1 remediation-step rule (N-1)', () => {
const v10 = WITH_BLOCKER.replace('review_version: "1.1"', 'review_version: "1.0"');
assert.equal(validateReviewContent(v10, { strict: true }).valid, true);
});
test('validateReview — soft: a missing remediation step only warns', () => {
const r = validateReviewContent(WITH_BLOCKER, { strict: false });
assert.equal(r.valid, true);
assert.ok(r.warnings.find(w => w.code === 'REVIEW_MISSING_REMEDIATION_STEP'));
});
test('validateReview — a clean review with zero findings validates with `findings: []` (a bare `findings:` does not)', () => {
const ids = /findings:\n(?: - [0-9a-f]{40}\n)+/;
assert.match(GOOD_REVIEW, ids, 'fixture control');
assert.equal(validateReviewContent(GOOD_REVIEW.replace(ids, 'findings: []\n'), { strict: true }).valid, true);
assert.equal(validateReviewContent(GOOD_REVIEW.replace(ids, 'findings:\n'), { strict: true }).valid, false);
});
test('validateReview — strict: a 1.1 review needs ## Declined to Judge; a 1.0 review does not', () => {
const v11 = GOOD_REVIEW.replace('review_version: "1.0"', 'review_version: "1.1"');
const r = validateReviewContent(v11, { strict: true });
assert.equal(r.valid, false);
assert.ok(r.errors.find(e => e.code === 'REVIEW_MISSING_SECTION' && /Declined to Judge/.test(e.message)));
const fixed = v11.replace('## Remediation Summary', '## Declined to Judge\n\nNone.\n\nTrimmed: 0\n\n## Remediation Summary');
assert.equal(validateReviewContent(fixed, { strict: true }).valid, true);
assert.equal(validateReviewContent(GOOD_REVIEW, { strict: true }).valid, true, '1.0 stays valid');
});
test('review-validator CLI — run through a symlink and a path with a space it still validates (exit 1, not a silent 0)', () => {
const dir = mkdtempSync(join(tmpdir(), 'review validator '));
try {
const link = join(dir, 'validator link.mjs');
symlinkSync(VALIDATOR, link);
const p = join(dir, 'no verdict.md');
writeFileSync(p, NO_VERDICT);
const r = spawnSync(process.execPath, [link, p], { encoding: 'utf-8' });
assert.equal(r.status, 1, r.stdout + r.stderr);
assert.match(r.stderr, /REVIEW_MISSING_VERDICT/);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});