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

feat(review): the review is a gate - verdict in the file, CC engine, project memory

Browse source 
Ordre 5 (wave 1, P11 + P5), review_version 1.1.

- K7: `verdict: ALLOW | WARN | BLOCK` is required in review.md (strict
  error, soft warning for 1.0 reviews). New CLI lib/review/verdict-gate.mjs:
  0 = ALLOW/WARN, 1 = BLOCK, 2 = could not decide (never green); a verdict
  greener than the file's own BLOCKERs or JSON block is exit 2. Realpath
  main guard (the old file:// guard exits 0 under a symlink - measured).
- No finding is struck for length: succinctness trims only MINOR/SUGGESTION
  text (catalogue tier), keeps BLOCKER/MAJOR whole, counts trims; the
  coordinator lists what it declined to judge.
- Claude Code's /code-review and /security-review run as the engine via the
  Skill tool (never --fix/--comment/--post/ultra); lib/review/engine-findings.mjs
  turns their findings into a reviewer payload; two catalogue keys
  (CODE_REVIEW_DEFECT, SECURITY_REVIEW_VULNERABILITY), catalogue 12 -> 14.
  An engine that did not run forbids ALLOW.
- Every BLOCKER is written as a numbered `### Step R<n>` with its source
  finding (strict error REVIEW_MISSING_REMEDIATION_STEP).
- P5: lib/review/recurrence.mjs - a finding class in two review.md files
  (current + git history, no new store) becomes one proposal, routed by the
  catalogue's memory_route; written only between Voyage markers in
  .claude/rules/voyage-review-lessons.md under a size budget, CLAUDE.md gets
  a pointer only; the operator approves every line (Phase 8.7).
- Workflow substrate: catalogue enum in step; a missing reviewer or engine
  forbids ALLOW there too (it returned ALLOW on zero findings before).
- Fixtures review-run-A/B said WARN over a BLOCKER; now BLOCK.

Borrowed and credited in code: Superpowers (declined-to-judge), ai-native-sdlc
(cap only nits; mistake/fix/check), Spec Kit (managed block; converge steps),
Pocock (retro routing), Kiro (verdict file + gate; evidence per lesson),
Cherny (engine -> brief control order; review -> memory; budget).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-06 19:45:52 +02:00
commit d88245c06e
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
28 changed files with 2052 additions and 110 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 |

35
README.md
View file

@ -410,13 +410,29 @@ 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)
```
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 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 +446,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 +471,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.

70
agents/review-coordinator.md
View file

@ -46,7 +46,14 @@ 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).
- **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 +96,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 +105,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 +152,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 +202,15 @@ 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.
## Output: review.md content
@ -203,7 +222,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 +231,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 +239,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 +262,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 +303,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 +347,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

228
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 9.5). 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,12 @@ 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), and the
three required sections, each saying "No diff between {before_sha} and
{after_sha}; nothing to review." Skip Phases 4–7. Continue to Phase 8
(validate + stats).
## Phase 4 — Triage gate (deterministic path-pattern classifier)
@ -253,6 +267,71 @@ 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" "{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). Those go in the Coverage
section, one line each — never silently dropped. Exit 1 = the transcription is
malformed: fix it from the engine's own text and re-run. Exit 2 = could not
run.
**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 +401,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 +435,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 +461,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 +486,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 +532,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 +560,27 @@ 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, unreadable file) — **never
green**. 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 +610,65 @@ 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 "$(git rev-parse --show-toplevel)"
```
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 "$(git rev-parse --show-toplevel)" \
--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 an edited line lacks the three parts: show
why and ask again. 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 +685,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 — 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 +800,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 +839,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, and a file whose trailing JSON block or BLOCKER findings contradict a greener verdict all land there. 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 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` — written by the coordinator: every finding set aside without a ruling, with its reason, 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` |

123
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,14 +67,15 @@ 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',
'unknown-rule_key',
'file-existence:indeterminate',
@ -185,36 +189,76 @@ 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;
}
/**
* 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 = trimText(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 };
}
/**
@ -314,7 +358,11 @@ export function computeVerdict(findings, options = {}) {
*
* @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[] }}
* `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);
@ -322,6 +370,21 @@ export function runContract(reviewerPayloads, options = {}) {
const judged = judgeFilter(deduped);
const reasoned = reasonablenessFilter(judged.kept);
const unverified = [...judged.unverified, ...reasoned.unverified];
// 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 +423,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) {

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

@ -0,0 +1,133 @@
// 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, else
// CODE_REVIEW_DEFECT (MAJOR) / SECURITY_REVIEW_VULNERABILITY (BLOCKER).
// - A finding on a file outside the review scope is NOT dropped silently: it
// lands in `out_of_scope` for the Coverage section (`/security-review`
// diffs against origin's default branch, which can be wider than the
// review's SHA range).
//
// CLI: node lib/review/engine-findings.mjs [--scope <file-list.txt>] <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 } 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) {
return String(p ?? '').replace(/^\.\//, '').trim();
}
/**
* Convert one engine's transcribed findings into a coordinator payload.
* @param {{ engine: string, findings: object[] }} input
* @param {{ scopeFiles?: 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'] };
const scope = opts.scopeFiles ? new Set(opts.scopeFiles.map(normPath)) : null;
const findings = [];
const out_of_scope = [];
input.findings.forEach((f, i) => {
const file = normPath(f?.file);
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 = Number.isInteger(f.line) && f.line >= 0 ? f.line : 0;
const rule_key = typeof f.rule_key === 'string' && RULE_KEYS.has(f.rule_key) ? 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 scopeIdx = args.indexOf('--scope');
const scopePath = scopeIdx === -1 ? null : args[scopeIdx + 1];
const filePath = args.find((a, i) => !a.startsWith('--') && (scopeIdx === -1 || i !== scopeIdx + 1));
if (!filePath || !existsSync(filePath) || (scopePath && !existsSync(scopePath))) {
process.stderr.write('Usage: engine-findings.mjs [--scope <file-list.txt>] <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 });
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) {

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

@ -0,0 +1,356 @@
// 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 becomes ONE proposal; one sighting is no proposal.
// 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 9.5,
// 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,
} 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 readBlock(text) {
const start = text.indexOf(MARK_START);
const end = text.indexOf(MARK_END);
if (start === -1 || end === -1 || end < start) return null;
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) 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(/\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) => String(e.id ?? '').slice(0, 8)).join(', ');
const body = `Mistake: ${clip(rule.description, 160)} Seen: ${seen}. · Fix: ${fix} · How to check: ${how} (evidence: ${ids})`;
return `- [${rule.rule_key}] (${rule.memory_route}) ${body}`;
}
/**
* 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 (line.includes('\n')) return { valid: false, reason: 'multi-line' };
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 firstPerKey = new Map();
for (const f of review.findings) {
if (!firstPerKey.has(f.rule_key)) firstPerKey.set(f.rule_key, f);
}
for (const [key, f] of firstPerKey) {
if (!byKey.has(key)) byKey.set(key, []);
byKey.get(key).push({ ...f, review: review.identity, source: review.source });
}
}
const proposals = [];
for (const [key, evidence] of [...byKey].sort(([a], [b]) => a.localeCompare(b))) {
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);
proposals.push({
rule_key: key,
route: rule.memory_route,
target: `${LESSONS_DIR}/${LESSONS_FILE_NAME}`,
line: composeLine(rule, ev),
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) {
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)) 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);
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);
}
}

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

@ -1,8 +1,20 @@
// 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 - review hygiene (about the review itself, not the target code):
// never proposed.
// Routing borrowed from Pocock `skills/in-progress/retro/SKILL.md:19` (MIT).
export const SEVERITY_VALUES = Object.freeze(['BLOCKER', 'MAJOR', 'MINOR', 'SUGGESTION']);
@ -21,75 +33,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: 'rule',
}),
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: 'rule',
}),
]);
export const MEMORY_ROUTES = Object.freeze(['check', 'rule']);
export const RULE_KEYS = Object.freeze(new Set(RULE_CATALOGUE.map((r) => r.rule_key)));
/**

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

@ -0,0 +1,127 @@
// 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, or a file that contradicts its own verdict.
// 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.
const block = extractFindingsBlock(r.parsed.body ?? '');
if (block === null) {
return undecided(`verdict ${verdict} has no trailing JSON block to back it`, verdict);
}
let payload;
try {
payload = JSON.parse(block);
} catch (e) {
return undecided(`trailing JSON block does not parse: ${e.message}`, verdict);
}
if (payload?.verdict !== undefined && payload.verdict !== verdict) {
return undecided(`frontmatter verdict ${verdict} disagrees with the JSON block verdict ${payload.verdict}`, verdict);
}
const blockerFindings = Array.isArray(payload?.findings)
? payload.findings.filter((f) => f?.severity === 'BLOCKER').length
: 0;
const blockerCount = Number(payload?.counts?.BLOCKER ?? 0);
if (blockerFindings > 0 || blockerCount > 0) {
return undecided(`verdict ${verdict} over ${Math.max(blockerFindings, blockerCount)} BLOCKER finding(s)`, verdict);
}
return { exit: EXIT.PASS, verdict, reason: `verdict ${verdict}` };
}
/**
* 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 filePath = args.find((a) => !a.startsWith('--'));
if (!filePath) {
process.stderr.write('Usage: verdict-gate.mjs [--json] <review.md>\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);
}

88
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,41 @@ 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).
const blockerIds = 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);
}
}
}
for (const section of REVIEW_BODY_SECTIONS) {
const re = new RegExp(`^##\\s+${section}\\b`, 'm');
if (!re.test(body)) {
@ -94,7 +170,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.

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

@ -127,20 +127,43 @@ 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 — 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 +252,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 +371,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)',
);
});

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

@ -0,0 +1,77 @@
// 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 — 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);

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

@ -0,0 +1,128 @@
// 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('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');
});
// ---- 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'));
});

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

@ -0,0 +1,239 @@
// 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 } 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 }); }
});
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', () => {

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

@ -0,0 +1,153 @@
// 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');
function review({ verdict, jsonVerdict, severities = [] }) {
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, 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 });
}
});
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 });
}
});

93
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,88 @@ 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') + `
\`\`\`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 — 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('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 });
}
});