- tests/lib/review-rounds.test.mjs: round 2 whose Claude Security payload holds the round-1 BLOCKER only in out_of_scope keeps it counted through the real finalize and verdict gate (BLOCK, gate exit 1, not under Resolved since round 1); a clean run resolves it (F-V4; red under the mutation that drops out_of_scope from raisedOutside). - README.md, SECURITY.md: "blocks only at confidence high" names the exception of decision 222 (counted at high in an earlier round, it stays counted when it comes back below high or outside the scope) (F-V5). - commands/trekreview.md Phase 5: an engine finding is held by a matching finding in review.md (engine MAJOR/BLOCKER put back first) or a matching below_threshold / out_of_scope entry; an engine MINOR or SUGGESTION the coordinator left out holds nothing, a known issue (decision 226). - tests/lib/review-prose-pins.test.mjs: both sentences pinned; red on the old prose. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1058 lines
56 KiB
Markdown
1058 lines
56 KiB
Markdown
---
|
||
name: trekreview
|
||
description: |
|
||
Independent post-hoc review of delivered code against the brief. Produces
|
||
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] [--answers]"
|
||
allowed-tools: Agent, Read, Glob, Grep, Write, Edit, Bash, AskUserQuestion, Skill, Workflow
|
||
---
|
||
|
||
# Ultrareview Local v1.0
|
||
|
||
Independent post-hoc review of code delivered by `/trekexecute`
|
||
against the contract in `brief.md`. Produces `review.md` — a structured
|
||
artifact with severity-tagged findings that `/trekplan --brief
|
||
review.md` can consume as plan input (Handover 6).
|
||
|
||
Pipeline position:
|
||
|
||
```
|
||
/trekbrief → brief.md
|
||
/trekresearch → research/*.md
|
||
/trekplan → plan.md
|
||
/trekexecute → progress.json (+ commits)
|
||
/trekreview → review.md (this command)
|
||
```
|
||
|
||
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.2, ordre 5).** In this order:
|
||
Claude Code's own `/code-review` and one security engine (Claude Security
|
||
where it is installed, else Claude Code's `/security-review`; with neither,
|
||
security is "not measured", never 0) 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, or
|
||
WARN with every counting MAJOR answered, 1 = BLOCK, 2 = could not decide,
|
||
never green). No finding is struck for
|
||
length. A finding class seen in two reviews becomes one proposal for the
|
||
project's memory, approved line by line (Phase 8.7). Order credited to Boris
|
||
Cherny's public workflow (engine → brief control); the gate to Kiro and Spec
|
||
Kit — see `docs/HANDOVER-CONTRACTS.md` § Handover 6.
|
||
|
||
## Phase 1 — Parse mode and validate input
|
||
|
||
Parse the arguments with the shared arg-parser. Run it first, exactly like
|
||
this, before anything else (the arguments reach the parser as the one line
|
||
inside the quoted heredoc, on stdin, never as shell words):
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/parsers/arg-parser.mjs --command trekreview --stdin <<'VOYAGE_ARGS_EOF_7F3C9E'
|
||
$ARGUMENTS
|
||
VOYAGE_ARGS_EOF_7F3C9E
|
||
```
|
||
|
||
It prints the parse as JSON on stdout. It also checks the required flag
|
||
itself: on exit 1 it has printed the error and usage lines on stderr.
|
||
|
||
The parser recognizes these flags (see `lib/parsers/arg-parser.mjs`
|
||
FLAG_SCHEMA `trekreview` entry):
|
||
|
||
| Flag | Type | Purpose |
|
||
|------|------|---------|
|
||
| `--project <dir>` | valued | Required. Path to trekplan project folder containing `brief.md`. |
|
||
| `--since <ref>` | valued | Optional. Override "before" SHA for the diff. Validated via `git rev-parse --verify`. |
|
||
| `--quick` | boolean | Skip the brief-conformance pass; run only the code-correctness reviewer; skip the coordinator's reasonableness filter except the brief-conflict rule. |
|
||
| `--validate` | boolean | Schema-only check on existing `{project_dir}/review.md`. No LLM calls. |
|
||
| `--dry-run` | boolean | Print the discovered scope and triage map. Skip writes. |
|
||
| `--fg` | boolean | No-op alias (foreground is default). |
|
||
| `--answers` | boolean | Run only Phase 8.8 (the operator's answers to the counting MAJORs) on the existing `{project_dir}/review.md`, then the Phase 8 gate line. No `prepare`, no reviewer, no write of findings: it opens no new round and counts nothing against the cap (decision 93). |
|
||
|
||
Resolution:
|
||
1. If the parser exits 1 (for example `--project` is missing), print its
|
||
stderr lines verbatim, exactly as the parser wrote them, and stop. Do not
|
||
compose the error message yourself.
|
||
2. Trim trailing slash from `{dir}`. Set:
|
||
- `project_dir = {dir}`
|
||
- `brief_path = {dir}/brief.md`
|
||
- `review_path = {dir}/review.md`
|
||
3. If `{dir}` does not exist or `{dir}/brief.md` is missing:
|
||
```
|
||
Error: project directory not initialized. Run /trekbrief first.
|
||
Missing: {dir}/brief.md
|
||
```
|
||
|
||
Set `mode`:
|
||
- `validate` if `--validate` is set (overrides everything else; skip to Phase 8.5).
|
||
- `answers` if `--answers` is set: skip to Phase 8.8.
|
||
- `dry-run` if `--dry-run` is set.
|
||
- `quick` if `--quick` is set.
|
||
- `default` otherwise.
|
||
|
||
## Phase 2 — Validate brief
|
||
|
||
Run the brief validator in soft mode — the brief is upstream context, not
|
||
something this command produces, so partial grades are acceptable as long
|
||
as the file is parseable:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/validators/brief-validator.mjs --soft --json "{brief_path}"
|
||
```
|
||
|
||
Read the JSON output. If `valid: false` AND any error has code
|
||
`BRIEF_MISSING_REQUIRED_FIELD` or `FRONTMATTER_PARSE_ERROR`: stop and
|
||
ask the user to re-run `/trekbrief`. Other soft errors become
|
||
warnings in the review's Executive Summary.
|
||
|
||
Read the brief frontmatter. Capture for review.md:
|
||
- `task` → review frontmatter `task`
|
||
- `slug` → review frontmatter `slug`
|
||
- `project_dir` → review frontmatter `project_dir` (defaults to the
|
||
CLI `--project` value when missing)
|
||
|
||
## Phase 3 — Discover scope SHA range
|
||
|
||
Determine the "before" SHA that bounds the review:
|
||
|
||
1. **`--since <ref>` override** — if set, validate via:
|
||
```bash
|
||
git rev-parse --verify "$since_ref"
|
||
```
|
||
On failure: print `Error: --since ref is not a valid git revision: {ref}` and stop.
|
||
Set `before_sha = $(git rev-parse --verify "$since_ref")`.
|
||
|
||
2. **Preferred path** — read `{project_dir}/progress.json` if it exists.
|
||
Extract `session_start_sha`. Validate it via `git rev-parse --verify`.
|
||
Set `before_sha = session_start_sha`.
|
||
|
||
3. **Fallback** — no `progress.json`. Use the brief's mtime to find the
|
||
most recent commit at or before the brief was written:
|
||
```bash
|
||
brief_mtime=$(stat -f %m "{brief_path}") # macOS; on Linux use stat -c %Y
|
||
before_sha=$(git log --until="@$brief_mtime" -n 1 --format=%H)
|
||
```
|
||
Emit a clear warning that gets surfaced in the review's Executive
|
||
Summary: "scope_sha_start unavailable — falling back to brief mtime
|
||
({timestamp}). Coverage may include unrelated commits."
|
||
|
||
Compute the "after" SHA: `after_sha=$(git rev-parse HEAD)`.
|
||
|
||
**Then the round.** Rounds converge, at most `MAX_REVIEW_ROUNDS` (**3**, in
|
||
`lib/review/review-rounds.mjs`) per run:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-rounds.mjs prepare --project "{project_dir}" --since {before_sha} --json
|
||
```
|
||
|
||
Add `--dry-run` when
|
||
`mode == dry-run`. It prints `{round, round_base, previous}`; keep all three for
|
||
Phases 5 and 7. A later round is one where `{project_dir}/review.md` has this
|
||
`scope_sha_start` (the same run): `prepare` counts the round from the run's
|
||
archives, keeps the previous review as `{project_dir}/{previous}`
|
||
(`review-round-<run>-<n>.md`, `<run>` = the first 12 hex of `before_sha`), and
|
||
`round_base` is its `scope_sha_end`. Every round it deletes the previous
|
||
round's `.review-out-*.md` and `.review-engine-payload-*.json` files, so each
|
||
such file finalize reads was written this round. Exit 1 stops, for one of two
|
||
reasons, told apart by the start of `reason`: `review round cap reached` (the
|
||
operator re-plans), or `review-stop-rule:` — the stop rule (decisions 217,
|
||
220; `lib/review/stop-rule.mjs`), judged when round 3 would open: the
|
||
previous round and this one brought new BLOCKER/MAJOR findings of the same
|
||
class, or the blocking findings (BLOCKERs and counting MAJORs) did not get
|
||
fewer. Its line gives the class and the counts per round with the two
|
||
recommendations, Known issue or New mechanism, never another round; the
|
||
operator decides (`/trekexecute` Phase 7.2 asks it). Exit 2 stops "not measured" (the round base
|
||
is not an ancestor of HEAD, the round count does not match the archives, or an
|
||
archive differs). Print the reason in one line; no review is written in either
|
||
case.
|
||
|
||
Capture working-tree changes (uncommitted at review time):
|
||
```bash
|
||
git diff --name-only "$before_sha".."$after_sha"
|
||
git diff --name-only HEAD # uncommitted (annotated [uncommitted])
|
||
```
|
||
|
||
The combined file list is the review scope. Note that the
|
||
`[uncommitted]` annotation is a **brief-level contract** — the brief's
|
||
Assumptions section declares this is allowed; the review surfaces it
|
||
explicitly in the Coverage table.
|
||
|
||
If the file count is `0`, write a minimal review.md per the template:
|
||
`verdict: ALLOW`, `findings: []` (the empty list is the one inline form the
|
||
parser reads — a bare `findings:` is null and fails validation), the
|
||
required sections (`## Executive Summary`, `## Coverage`, `## Declined to
|
||
Judge` with `None.` and `Trimmed: 0`, `## Remediation Summary`), each saying
|
||
"No diff between {before_sha} and {after_sha}; nothing to review.", and the
|
||
trailing block `{"verdict": "ALLOW", "counts": {"BLOCKER": 0, "MAJOR": 0,
|
||
"MINOR": 0, "SUGGESTION": 0}, "allow_blocked_by": [], "findings": []}` —
|
||
without it the verdict gate
|
||
cannot back the ALLOW and exits 2. There is nothing for the Phase 4.7 engine
|
||
to review, so its absence does not withhold ALLOW here. Skip Phases 4–7.
|
||
Continue to Phase 8 (validate).
|
||
|
||
## Phase 4 — Triage gate (deterministic path-pattern classifier)
|
||
|
||
The triage gate is **deterministic** — no LLM judgment. It classifies
|
||
every file from Phase 3 into a treatment bucket:
|
||
|
||
| Treatment | When |
|
||
|-----------|------|
|
||
| `skip` | Matches `*.lock`, `*.svg`, `dist/**`, `build/**`, `node_modules/**`, OR the file's first 3 lines contain a generated-file marker (`@generated`, `Code generated by`, `DO NOT EDIT`). |
|
||
| `deep-review` | Default for every changed file not in the other two rows — code, tests, `commands/`, `agents/`, `skills/` Markdown, and the instruction files `CLAUDE.md`, `AGENTS.md` and `.claude/rules/**`. |
|
||
| `summary-only` | `docs/**`, Markdown files at the repo root other than `CLAUDE.md` and `AGENTS.md` (README, CHANGELOG), `tests/fixtures/**`. |
|
||
|
||
A changed instruction file (`CLAUDE.md`, `AGENTS.md`, `.claude/rules/*.md`) at any depth, `docs/**` included, gets deep review: `instruction-scope.mjs --triage` lists them.
|
||
|
||
```bash
|
||
git -c core.quotePath=false diff --name-only -z {before_sha}..{after_sha} | xargs -0 node ${CLAUDE_PLUGIN_ROOT}/lib/review/instruction-scope.mjs --triage
|
||
```
|
||
|
||
Hard refuse-with-suggestion gates — use `AskUserQuestion`:
|
||
|
||
```
|
||
if (reviewed_files_count > 100) → ask user
|
||
if (estimated_diff_tokens > 100000) → ask user
|
||
```
|
||
|
||
Token estimation: `wc -c "$diff_file" / 4` (rough proxy). Use
|
||
`AskUserQuestion` with the prompt:
|
||
|
||
> The diff under review is large (`{N}` files / `~{T}` tokens). Continue
|
||
> with the full scope, narrow with `--since <closer-ref>`, or stop?
|
||
|
||
Options:
|
||
1. **Continue** — proceed at this scope.
|
||
2. **Narrow** — print suggested `git log --oneline {before}..HEAD` so the
|
||
user can pick a closer ref, then stop.
|
||
3. **Stop** — cancel.
|
||
|
||
Record the treatment for every file. Files marked `skip` MUST appear in
|
||
the Coverage section of `review.md` — never silently drop them. Silent
|
||
drops are `COVERAGE_SILENT_SKIP` (MAJOR) per the rule catalogue.
|
||
|
||
If `mode == dry-run`: print the triage map and exit.
|
||
|
||
## Phase 4.5 — Run the brief's success-criteria checks
|
||
|
||
**Skipped in `quick` mode** (that mode does not launch
|
||
`brief-conformance-reviewer`, so there is nobody to hand the result to) and in
|
||
`dry-run`.
|
||
|
||
`brief-conformance-reviewer` is asked to decide whether each Success Criterion's
|
||
verification command passes. Its tools are `Read`, `Glob`, `Grep` — it cannot
|
||
run anything, and it stays that way: a reviewer that executes the code it
|
||
reviews is not an independent reviewer. So THIS command runs the commands, and
|
||
the reviewer judges the RESULT.
|
||
|
||
```bash
|
||
# Resolve the plugin root ONCE. ${CLAUDE_PLUGIN_ROOT} is substituted in this
|
||
# command's text but is EMPTY in the Bash tool's process env, and a bare
|
||
# `node ${CLAUDE_PLUGIN_ROOT}/lib/…` then runs `node /lib/…`, which exits 1 —
|
||
# indistinguishable from a criterion that failed.
|
||
# Only the literal form is substituted (a `:-` default is never replaced); the
|
||
# fallback takes the highest installed version under cache/<market>/voyage/.
|
||
VOYAGE_ROOT="${CLAUDE_PLUGIN_ROOT}"
|
||
case "$VOYAGE_ROOT" in
|
||
/*) ;;
|
||
*) VOYAGE_ROOT="$(ls -d "$HOME"/.claude/plugins/cache/*/voyage/[0-9]*/ 2>/dev/null \
|
||
| awk -F/ '{ split($(NF-1), v, "."); printf "%09d%09d%09d %s\n", v[1], v[2], v[3], $(0) }' \
|
||
| sort | tail -1 | cut -d' ' -f2-)"
|
||
VOYAGE_ROOT="${VOYAGE_ROOT%/}" ;;
|
||
esac
|
||
if [ ! -f "$VOYAGE_ROOT/lib/verification/criteria-runner.mjs" ]; then
|
||
echo "[voyage] success-criteria checks could not run - plugin root unresolved."
|
||
echo " NOT a pass: hand the reviewer NO results and say so."
|
||
exit 2
|
||
fi
|
||
|
||
# Every command is screened twice before it runs (with no shell): the runner's own
|
||
# ALLOWLIST of test runners (npm test, node --test, pytest, bash tests/<script>,
|
||
# a read-only git subcommand) reports anything else as NOT RUN, and the executor
|
||
# denylist (catastrophe) reports BLOCKED. Neither is ever run. Foreground only.
|
||
# The working tree the criteria run in - the same resolution trekexecute
|
||
# Phase 7 uses, so one criterion cannot resolve two ways in the two phases.
|
||
CRITERIA_CWD="$(git rev-parse --show-toplevel 2>/dev/null || pwd)"
|
||
|
||
node "$VOYAGE_ROOT/lib/verification/criteria-runner.mjs" \
|
||
--brief "{brief_path}" --evidence --cwd "$CRITERIA_CWD"
|
||
```
|
||
|
||
Exit 0 means every criterion passed; exit 1 means at least one failed or was
|
||
blocked by the executor denylist; a criterion with no command, or one outside
|
||
the allowlist of test runners, is reported as NOT RUN;
|
||
exit 2 means the runner could not run. **The exit
|
||
code does not stop the review** — a failing criterion is exactly what the review
|
||
exists to find. Capture stdout as `sc_evidence_block`.
|
||
|
||
If the runner exits 2 (or the root could not be resolved), set
|
||
`sc_evidence_block` to a single line naming the failure and stating that no
|
||
criterion was checked. An unrun check is never a pass, and the reviewer must be
|
||
told which of the two it is looking at.
|
||
|
||
`sc_evidence_block` is pasted verbatim into the `brief-conformance-reviewer`
|
||
prompt in Phase 5, and its summary line goes into the Coverage section of
|
||
`review.md`. Do NOT summarise, re-word or re-judge it on the way — the block is
|
||
built by `formatCriteriaEvidence` precisely so the orchestrator cannot narrate a
|
||
pass that never happened.
|
||
|
||
## Phase 4.7 — The engine: Claude Code's `/code-review` and the security engine
|
||
|
||
**Skipped in `dry-run` and `validate`.** In `quick` mode only `/code-review`
|
||
runs: no security engine. Wrap, not a new engine: `/code-review` and the
|
||
security engine 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.
|
||
|
||
**Choose the security engine first** (not in `quick`; E 3.1, decisions 175,
|
||
196). Claude Security is `available` when the `claude-security:claude-security`
|
||
skill is in this session's skill list AND the Workflow tool is among the tools
|
||
you can call right now (look at your tools, do not assume); otherwise it is
|
||
`missing`.
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/security-engine.mjs choose --repo . --base {before_sha} --head {after_sha} --claude-security {available|missing}
|
||
```
|
||
|
||
It prints `{engine, reason}`: `claude-security` when Claude Security is
|
||
available and `{before_sha}..{after_sha}` has committed changes; else
|
||
`security-review` when origin's HEAD resolves and `origin/HEAD...HEAD` is
|
||
non-empty; else `null`, with `no-engine` (no `origin` remote and no Claude
|
||
Security), `no-origin` (an `origin` remote whose HEAD does not resolve) or
|
||
`empty-diff` (the engine's range holds no committed change). Exit 2 = it could
|
||
not run: print the reason in one line and record the security engine as not
|
||
run. Keep `engine` and `reason` for Phases 5 and 9.
|
||
|
||
Then run, 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. The security engine `choose` picked, and only that one:
|
||
- `claude-security` —
|
||
`Skill("claude-security:claude-security", args: "scan changes --base {before_sha} --effort high")`.
|
||
Claude Security asks one question of its own before the scan starts (the
|
||
scan's time and token cost): put it to the operator as it stands and never
|
||
answer it yourself. "No", a question nobody can answer (no operator
|
||
present), the Workflow tool missing after all, or a scan that ends without
|
||
a report: record Claude Security as not run, with the reason. **Never fall
|
||
back to `/security-review`**: the review is then not measured for
|
||
security.
|
||
- `security-review` — `Skill("security-review")`, no arguments; its base is
|
||
fixed (origin's default branch). `/security-review` runs only when
|
||
`choose` picked it.
|
||
- `null` — no security engine runs: record it as not run with `choose`'s
|
||
reason.
|
||
|
||
**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, and Claude Security's scan runs as a workflow
|
||
until it writes its report. Do not start Phase 6 before both engines have
|
||
delivered or have been recorded as not run. For `/code-review` and
|
||
`/security-review`, transcribe every finding — file, line (0 when none), the one-sentence summary,
|
||
the failure scenario, the category if given (it decides the tier:
|
||
`simplification`, `efficiency`, `reuse`, `style` and `readability` enter as MINOR), the
|
||
fix the engine proposes (`fix`) only when its text states one — never write one
|
||
the engine did not give — and a narrower catalogue `rule_key` only when one
|
||
plainly fits — into
|
||
`{project_dir}/.review-engine-{code-review|security-review}.json` as
|
||
`{"engine": "...", "findings": [...]}`. Transcribe; never re-judge, merge or
|
||
drop an engine finding on the way. Then convert it:
|
||
|
||
```bash
|
||
git diff --name-only "$before_sha".."$after_sha" > "{project_dir}/.review-scope.txt"
|
||
git diff --name-only HEAD >> "{project_dir}/.review-scope.txt"
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/engine-findings.mjs \
|
||
--scope "{project_dir}/.review-scope.txt" --repo-root "$(git rev-parse --show-toplevel)" \
|
||
"{project_dir}/.review-engine-code-review.json" \
|
||
> "{project_dir}/.review-engine-payload-code-review.json" \
|
||
&& cat "{project_dir}/.review-engine-payload-code-review.json"
|
||
```
|
||
|
||
The converted payload is saved to `{project_dir}/.review-engine-payload-{engine}.json`
|
||
(`code-review`, `security-review` or `claude-security`), the same stdout, also written: Phase 7's finalize
|
||
reads it from there. Never edit that file by hand.
|
||
Exit 0 prints a reviewer payload (`claude-code-review` /
|
||
`claude-security-review`) for Phase 6 plus `out_of_scope`: engine findings on
|
||
files outside the review's SHA range (`/security-review` diffs against
|
||
origin's default branch, which can be wider). Pass `out_of_scope` to the
|
||
coordinator as well: each one is listed under `## Declined to Judge`
|
||
(`engine:out-of-scope`) and in Coverage — never silently dropped. It is
|
||
`unverified` and forbids `ALLOW` — with one exception: a MINOR or SUGGESTION
|
||
from `/code-review`, which was given exactly this range, is an improvement to
|
||
code this review did not change and does not hold `ALLOW` back. A MAJOR one
|
||
still does: a change that breaks an unchanged caller is cited at the caller. Absolute paths under the repo root are made relative
|
||
before the scope match (`--repo-root`). Exit 1 = the transcription is
|
||
malformed, or the scope list is empty: fix it from the engine's own text (or
|
||
the scope from Phase 3) and re-run; never feed a payload that did not pass.
|
||
Exit 2 = could not run — record that engine as not run.
|
||
|
||
A "narrower" `rule_key` you transcribe is used only when its catalogue tier is
|
||
at least the engine's own (MAJOR for `/code-review`, BLOCKER for
|
||
`/security-review`); the adapter refuses to let a transcription lower an
|
||
engine finding.
|
||
|
||
**Claude Security's findings are converted, never transcribed.** When its
|
||
scan has finished it has written `CLAUDE-SECURITY-<ts>/` in the repo root
|
||
(behind its own `.gitignore`); convert that report, after the scope file
|
||
above exists:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/security-engine.mjs convert --report-dir . \
|
||
--base {before_sha} --head {after_sha} --scope "{project_dir}/.review-scope.txt" \
|
||
> "{project_dir}/.review-engine-payload-claude-security.json" \
|
||
&& cat "{project_dir}/.review-engine-payload-claude-security.json"
|
||
```
|
||
|
||
Exit 0 prints the `claude-security` payload: `findings` and `out_of_scope` as
|
||
for any engine, and `below_threshold`, the findings Claude Security marked
|
||
below confidence `high` (decision 219). Pass `below_threshold` to the
|
||
coordinator as well: each one is listed under `## Declined to Judge`
|
||
(`engine:below-threshold`), never counted, and it forbids `ALLOW`. Exit 2 =
|
||
not measured (no report stamp of a changes scan of exactly
|
||
`{before_sha}..{after_sha}` — an empty diff writes none —, a scan not
|
||
verified, researchers that did not all return, an empty diff or scope, a
|
||
scan limited by `--scope`, a malformed record, fewer or more records than
|
||
the stamp's `findings.total`): print the reason and record Claude Security
|
||
as not run;
|
||
the empty file the redirect leaves is no payload. Exit 1 = the scope list is
|
||
empty: rebuild it from Phase 3 and re-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, Claude
|
||
Security's question answered "No" or left unanswered, a report `convert`
|
||
reads as not measured, `choose` picking no engine, 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) and the verdict gate exits
|
||
2; the Executive Summary's first sentence names it. For security that
|
||
sentence says "security: not measured" and why (`choose`'s reason, or why
|
||
the engine did not run): never 0, never "no findings". 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
|
||
|
||
Launch two reviewer agents **in parallel** via the Agent tool — one
|
||
message, multiple tool calls.
|
||
|
||
> **Never pass the Agent tool's `name` parameter — at this or any other spawn
|
||
> site in this command.** `name` does not label a subagent, it changes its kind:
|
||
> the spawn is recorded as `taskKind: "in_process_teammate"` (`spawnDepth: 0`)
|
||
> instead of a subagent (`spawnDepth: 1`). A teammate's final assistant text is
|
||
> not a return value — it reaches the orchestrator only if the teammate itself
|
||
> calls `SendMessage(to: "main")`, and every agent in `agents/` declares a
|
||
> `tools:` allowlist without SendMessage. The agent still runs and still
|
||
> produces correct output; the result is simply never delivered, so the phase
|
||
> presents as a hung agent that no re-prompting can revive. Measured
|
||
> 2026-08-17: named 0/5 returned, unnamed 3/3. Mechanism, denominators, and the
|
||
> on-disk recovery path: `docs/agent-return-channel-defect.md`.
|
||
|
||
Reviewers run independently. Do NOT pre-feed findings between them.
|
||
|
||
**Reviewer stance: the `reviewing-adversarially` skill.** Both reviewers and
|
||
the coordinator have `voyage:reviewing-adversarially` preloaded through their
|
||
`skills:` line. Name it in every reviewer prompt anyway, so the rules hold even
|
||
where preloading did not happen: "Follow the reviewing-adversarially skill: no
|
||
praise; every finding cites file:line or quotes the line; no downgrade without
|
||
a reason; say what you read in full and what only as a summary; list what you
|
||
did not judge under ### Declined to Judge; check claims against the diff, not
|
||
a report."
|
||
|
||
| Agent | Mode-gated | Purpose |
|
||
|-------|------------|---------|
|
||
| `brief-conformance-reviewer` | Skipped in `quick` | Trace each Success Criterion + Non-Goal to delivered code. Emits findings tagged with rule_keys from the conformance/scope categories. |
|
||
| `code-correctness-reviewer` | Always runs | 8-dimension code review. Emits findings tagged with rule_keys from the correctness/security/maintenance/tests categories, plus the conformance-category instruction-file keys `CLAUDE_MD_DEVIATION` and `CLAUDE_MD_STALE`. |
|
||
|
||
Each reviewer prompt includes:
|
||
- **Diff context** — the unified diff from Phase 3, truncated per file
|
||
for files marked `summary-only`.
|
||
- **Triage map** — full file list with treatments. Reviewers must
|
||
respect `skip` decisions.
|
||
- **Brief path** — `{brief_path}` (read on demand; do not inline).
|
||
- **Rule catalogue** — reference to `lib/review/rule-catalogue.mjs`.
|
||
|
||
**The instruction set.** The `code-correctness-reviewer` prompt also gets the
|
||
repo's tracked instruction files for this diff, from `instruction-scope.mjs --set`
|
||
at `{after_sha}`:
|
||
|
||
```bash
|
||
git -c core.quotePath=false diff --name-only {before_sha}..{after_sha} > "{project_dir}/.review-changed.txt"
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/instruction-scope.mjs --set --repo . --sha {after_sha} --changed "{project_dir}/.review-changed.txt"
|
||
```
|
||
|
||
On exit 0, list its `files` under **Instruction files** and tell the reviewer
|
||
to check the diff against exactly these files and no other instruction file
|
||
(dimension 8, `CLAUDE_MD_DEVIATION` / `CLAUDE_MD_STALE`, MINOR). On exit 2,
|
||
tell it that none could be read; finalize (Phase 7) then sets every finding
|
||
under those two keys aside under Declined to Judge (`instruction-out-of-set`),
|
||
as it does any such finding whose `instruction_ref` is not in the set.
|
||
|
||
**From round 2** (`round` ≥ 2 from Phase 3), each reviewer prompt also gets
|
||
the round input:
|
||
- the round diff, `git diff {round_base}..{after_sha}`, beside the run's diff;
|
||
- the previous findings, from the trailing JSON block of
|
||
`{project_dir}/{previous}`, each with its id, to mark in the reviewer's own
|
||
JSON block as `previous: [{id, resolved}]` (`resolved: true` only when the
|
||
round diff fixes it);
|
||
- the line "you may read the files around it; a previous finding is never
|
||
re-raised as new; flag every occurrence of a finding class".
|
||
|
||
Finalize (Phase 7) carries a previous finding until every reviewer that ran in
|
||
its round marks it resolved, and an engine finding until its engine ran this
|
||
round and nothing of this round holds it. Two things hold it: a matching
|
||
finding in `review.md` (an engine MAJOR or BLOCKER the coordinator left out
|
||
is put back into `review.md` first, at its own severity), or a matching entry
|
||
of that engine's `below_threshold` or `out_of_scope` (decision 222: a Claude
|
||
Security finding counted at confidence `high` that comes back below it stays
|
||
counted). An engine MINOR or SUGGESTION the coordinator left out of
|
||
`review.md` holds nothing, so the previous finding resolves (a known issue,
|
||
decision 226). The engines (Phase 4.7) stay on the run range
|
||
(`/security-review` takes no range; Claude Security scans from `{before_sha}`).
|
||
|
||
`brief-conformance-reviewer` additionally receives `sc_evidence_block` from
|
||
Phase 4.5, pasted verbatim — the command, exit code and first line of output for
|
||
every Success Criterion. It is the ONLY evidence that agent has about whether a
|
||
criterion's verification passes, because it cannot run one.
|
||
|
||
Save each reviewer's **whole answer**, verbatim — its prose and its trailing
|
||
JSON block — to `{project_dir}/.review-out-{reviewer}.md`, not only the JSON
|
||
block: the check reads the prose around the block too (wave 2, order 9: the
|
||
saved file held the block alone, 76 bytes, so no prose was ever checked).
|
||
Then **validate it against the reviewer-output schema** rather than merely
|
||
parsing it. Run:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/findings-schema.mjs --json --repo-root "$(git rev-parse --show-toplevel)" "{project_dir}/.review-out-{reviewer}.md"
|
||
```
|
||
|
||
`validateReviewerOutput` in `lib/review/findings-schema.mjs` extracts the
|
||
last fenced `json` block, parses it, and schema-checks every finding
|
||
(load-bearing fields: `file`, `rule_key` ∈ catalogue, `severity` ∈ enum,
|
||
`line` integer ≥ 0). With `--repo-root`, a cited line past the end of a file
|
||
that exists is `FINDING_LINE_OUT_OF_RANGE`: the reviewer cited the line in the
|
||
diff, not in the file, and is re-asked like any other schema failure (the
|
||
voyage-lekerepo trial, 2026-10-06, lost its only brief-anchored MAJOR that way). Parse failure and schema failure surface through the
|
||
same stable error codes (`FINDINGS_NO_JSON_BLOCK`, `FINDINGS_PARSE_ERROR`,
|
||
`FINDING_*`).
|
||
|
||
`FINDING_PROSE_LINE_OUT_OF_RANGE` is the same check on a `path:line` written
|
||
in prose. In a finding's own fields (`title`, `detail`, `recommended_action`,
|
||
`brief_ref`) it is an **error**: re-ask that reviewer like any other schema
|
||
failure, and quote the way a removed line is cited, `<rev>:<path>:<line>`
|
||
(`v6.2.0:hooks/x.mjs:120`, not judged against today's file), so a finding
|
||
about deleted code keeps its line instead of losing it. In the prose around the JSON block it is a **warning**: the re-ask
|
||
asks for the block only and cannot correct that prose, so do not re-ask —
|
||
report each such warning (reviewer, citation, the file's line count) in the
|
||
Coverage section of `review.md`.
|
||
|
||
On any failure, re-ask **that reviewer** to re-emit a conforming JSON
|
||
block only — quote the reported error codes/locations so the fix is
|
||
targeted. **Bounded retries: N=2.** If the output still fails after 2
|
||
re-asks, stop and report which reviewer produced non-conforming output;
|
||
do not feed unvalidated findings to the coordinator.
|
||
|
||
In `quick` mode, launch only `code-correctness-reviewer`. The Executive
|
||
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 — plus, in
|
||
every mode, the Phase 4.7 engines that were due: `claude-code-review`, and
|
||
outside `quick` the security engine `choose` picked (`claude-security` for
|
||
Claude Security, `claude-security-review` for `/security-review`), or
|
||
`security-engine` when it picked none or could not run. `security-engine` never reports, so the
|
||
review carries `missing-reviewer:security-engine` and the verdict gate exits 2:
|
||
security not measured, never 0. 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
|
||
it returned a payload that validated. Three ways it fails to:
|
||
|
||
| Failure | Handling |
|
||
|---------|----------|
|
||
| Output fails the schema after the 2 bounded re-asks | STOP (already specified above) |
|
||
| Returned no final message at all | Re-ask that reviewer **once**. Still nothing → STOP. |
|
||
| Was never launched (spawn error, wrong mode) | STOP. |
|
||
|
||
**On STOP: name the reviewer and the failure, and do not proceed to Phase 6.**
|
||
Do not let the coordinator compute a verdict over a review one of whose
|
||
reviewers never spoke — the count would be complete-looking and wrong. This is
|
||
the same shape as the schema branch above ("do not feed unvalidated findings to
|
||
the coordinator"), applied to the other two ways a reviewer can go missing.
|
||
|
||
A reviewer that ran but never delivered is most often the return-channel
|
||
defect: check `~/.claude/projects/<proj>/<session>/subagents/agent-*.jsonl` for
|
||
its final assistant block before re-asking, and confirm no `name` parameter was
|
||
passed at the spawn (see the warning at the top of this phase).
|
||
|
||
If you proceed anyway under an explicit operator instruction, pass the expected
|
||
set to the coordinator as `expectedReviewers` so the missing reviewer at least
|
||
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 AND the Phase 4.7 engine payloads (Claude Security's
|
||
`below_threshold` list with them), plus the triage map, brief
|
||
metadata, SHA range, the expected reviewer set (engines included), and each
|
||
not-run engine with its reason. Name `voyage:reviewing-adversarially` in the
|
||
coordinator's prompt as in Phase 5, and pass each reviewer's
|
||
`### Declined to Judge` lines verbatim from its saved
|
||
`{project_dir}/.review-out-{reviewer}.md`: the coordinator copies them into
|
||
`## Declined to Judge`, so nothing a reviewer set aside disappears.
|
||
|
||
**Brief choices.** Next to the brief metadata, pass the brief's choices
|
||
verbatim: extract the `## Non-Goals`, `## Constraints` and `## Preferences`
|
||
sections and the operator decisions in `## Open Questions / Assumptions` from
|
||
the brief, word for word (a section the brief does not carry is passed as
|
||
absent, never written for it). The coordinator reads them for the
|
||
brief-conflict rule only (`agents/review-coordinator.md` Pass 3): an engine
|
||
finding whose recommended action contradicts one of them is kept and marked
|
||
`contradicts_brief`, never removed.
|
||
|
||
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** (skipped in `quick` mode) — drop a finding
|
||
with no `file:line` citation (`no-citation`) or whose file is in neither
|
||
the working tree nor the diff (`file-existence:refuted`); route one whose
|
||
`rule_key` is not in `RULE_CATALOGUE` (`unknown-rule_key`) or whose file
|
||
cannot be checked (`file-existence:indeterminate`) to `unverified`, which
|
||
forbids `ALLOW`; reset a severity that does not match its catalogue tier.
|
||
The brief-conflict rule still applies in every mode.
|
||
4. **Verdict** — BLOCK / WARN / ALLOW per the threshold table.
|
||
|
||
**Fail-closed.** Every removal in Pass 2 and Pass 3 is either
|
||
**dropped** (the test refuted the finding as a claim about this codebase) or
|
||
**unverified** (the finding was removed without its claim ever being settled).
|
||
A non-empty `unverified` bucket forbids `ALLOW`; the verdict becomes `WARN` and
|
||
the Executive Summary's first sentence must say why. The fail-closed rule never
|
||
raises a verdict — it only withholds the clean one. Fate table, reason
|
||
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
|
||
(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 7 — Write review.md
|
||
|
||
Write the coordinator's output verbatim to:
|
||
|
||
```
|
||
{project_dir}/review.md
|
||
```
|
||
|
||
Create parent directories if they do not exist. Atomic write pattern:
|
||
write to a temp file, then rename.
|
||
|
||
**Then compute the finding ids** — the coordinator has no shell and writes
|
||
`@@ID_<n>@@` placeholders (`agents/review-coordinator.md`, *IDs are computed
|
||
by code*):
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/fill-finding-ids.mjs {project_dir}/review.md
|
||
```
|
||
|
||
Exit 0 = every placeholder filled (or none were left); 1 = a placeholder with
|
||
no finding behind it, or a JSON id that is not the computed one — re-ask the
|
||
coordinator once with the errors quoted, then stop and report; 2 = could not
|
||
run (no JSON block) — stop. Never fill an id by hand. 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.
|
||
|
||
**Then finalize the review** — one deterministic pass after `fill-finding-ids`:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-finalize.mjs --review "{project_dir}/review.md" --repo . --project "{project_dir}" --round {round}
|
||
```
|
||
|
||
From round 2 it also gets the round base and the previous review, as `prepare`
|
||
printed them, and the carried answers follow it:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-finalize.mjs --review "{project_dir}/review.md" --repo . --project "{project_dir}" --round {round} --round-base {round_base} --previous "{project_dir}/{previous}"
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-answers.mjs carry --review "{project_dir}/review.md" --previous "{project_dir}/{previous}" --repo .
|
||
```
|
||
|
||
In round 1 too, finalize exit 1 or 2 stops the review: print the reason and "not measured", and stop. The file stays
|
||
as `fill-finding-ids` wrote it, and the verdict gate reads an engine MAJOR or BLOCKER missing from it as exit 2
|
||
(`review-not-measured: engine finding not in review`).
|
||
From round 2, finalize exit 1 or 2 stops the review: print the reason and "not measured", and stop. The previous
|
||
findings reach review.md only through finalize's carry, so the verdict gate reads a round 2+ review without
|
||
`finalized: true` as exit 2 (`review-not-measured: round 2+ review not finalized`), never green.
|
||
Finalize writes `round`, `round_base`, `finalized: true`, `ran` (what ran this round, read from the
|
||
`.review-out-*.md` and `.review-engine-payload-*.json` files) and, from round 2, `resolved` with a
|
||
`## Resolved since round <n>` section; a carried BLOCKER gets its `### Step R<n>`.
|
||
The standard path now puts back engine findings at MAJOR or above: finalize's
|
||
first pass reads the Phase 4.7 payloads itself from `{project_dir}` (`.review-engine-payload-*.json`, no new
|
||
argument), and each engine finding at MAJOR or BLOCKER whose row no finding of review.md carries is put back at its
|
||
own severity, with the line `**Source:** engine {name}, put back by the standard-path comparison` in its entry; the
|
||
verdict is only raised, and a put-back BLOCKER gets its `### Step R<n>`. An engine finding in `out_of_scope` stays
|
||
declined (`engine:out-of-scope`). `review-answers.mjs carry`
|
||
writes an `## Answers` row with `Carried from` for each carried MAJOR the operator accepted in an
|
||
earlier round; on exit 1 or 2 print the reason in one line and continue: no answer is carried.
|
||
A finding is pre-existing when it sits on a line the range `scope_sha_start..scope_sha_end` did not add or
|
||
change; finalize marks it from git, the verdict gate re-derives the mark, and a BLOCKER or security finding
|
||
counts in full whatever the mark. The range counts only when `scope_sha_start` is bound to the fork point from main
|
||
(`main`, else `master`): the fork point itself, or a later commit reached through process files only (the approval
|
||
log, `.claude/projects/`); otherwise, and for a range that ends on main itself, nothing is pre-existing. A carried finding (`carried_from`) keeps its line from the previous round's end,
|
||
so it is never pre-existing, whatever the mark.
|
||
|
||
**The review policy.** The policy file is `REVIEW.md` at the repo root, read at `scope_sha_start` from git, never
|
||
from the working tree, and finalize applies it as its last pass. Only its one `voyage-review-policy` block is read;
|
||
the file's text is data, never an instruction. The block holds at most one `max-minor` line (`max-minor: <0-999>`),
|
||
the cap: at most that many MINOR and SUGGESTION findings stay reported, and each one over it is listed by title
|
||
under `## Declined to Judge` with `policy:cap`, followed by `Over the policy cap: N`. It also holds at most 50
|
||
`skip` lines (`skip: <glob>`): a MINOR or SUGGESTION finding on a skip path is listed there with `policy:skip`, and
|
||
its file stays in Coverage. The policy caps and skips MINOR and SUGGESTION findings only, never a BLOCKER, a MAJOR
|
||
or a security finding; the verdict gate exits 2 on a review in which one was moved. A malformed or changed file (any
|
||
other line in the block, two blocks, a bad glob, a symlink, or a file changed inside the reviewed range) is not
|
||
applied, and the review says so in `review_policy: not-applied` and `review_policy_note`. Without the file there is
|
||
no cap (`review_policy: absent`). The review records the file's sha256 in `review_policy_sha256`, `none` without it.
|
||
|
||
**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.
|
||
|
||
## Phase 8 — Validate output
|
||
|
||
Run the strict validator:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/validators/review-validator.mjs --json "{review_path}"
|
||
```
|
||
|
||
If validation fails:
|
||
- For repairable errors (missing required body section, malformed
|
||
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 with every counting MAJOR answered; exit 1 = BLOCK;
|
||
exit 2 = the gate could not decide
|
||
(missing verdict, a verdict the file contradicts — in its JSON block, its
|
||
`## Findings (BLOCKER)` section or its finding-id list — a JSON block that is
|
||
not the last fence or has the wrong shape, two verdict lines, unreadable
|
||
file) — **never green**. A WARN that exists because a reviewer or engine
|
||
did not run (`missing-reviewer:` / `unattributable-payload` in
|
||
`allow_blocked_by`) is also exit 2: a review not performed in full stops a
|
||
merge (operator decision 2026-10-06). An exit 2 with a reason that starts
|
||
`review-unanswered:` before Phase 8.8 is expected: the operator's answers come
|
||
there. It takes exactly one file. On exit 0
|
||
with WARN, `--json` also returns `warn_reasons` (the JSON block's
|
||
`allow_blocked_by`); print them. Capture the exit code as `gate_exit` and print it in Phase 9 exactly
|
||
as returned. This is the command that later stops a merge or a release on
|
||
BLOCK; `/trekreview` reports it and never overrides it.
|
||
|
||
**Build the operator-annotation HTML.** After the gate, run:
|
||
|
||
```bash
|
||
ANNOT_HTML=$(node ${CLAUDE_PLUGIN_ROOT}/scripts/annotate.mjs "{review_path}" 2>&1)
|
||
```
|
||
|
||
`stdout` is the absolute path to the `.html` on success: the page where the
|
||
operator writes their own notes on `review.md`. The loop that follows is the
|
||
skill `voyage:annotating`: call it when the operator pastes the notes back.
|
||
If `annotate.mjs` exits non-zero, surface a one-line warning and continue —
|
||
the annotation HTML is a convenience, not a gate.
|
||
|
||
## Phase 8.5 — Validate-only mode (`--validate`)
|
||
|
||
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. A
|
||
`review_version` 1.0 review without `verdict:` FAILS here (K7 is a
|
||
write-time rule); it still seeds a plan, because `/trekplan` reads in soft
|
||
mode.
|
||
4. Run `lib/review/verdict-gate.mjs` on the same file and print its line and
|
||
exit code.
|
||
5. Exit 0 on PASS, 1 on FAIL. Never write to disk. Never call any agent.
|
||
Skip Phase 8.7.
|
||
|
||
## Phase 8.7 — The project's memory (recurring finding classes)
|
||
|
||
**Skipped in `dry-run` and `validate`.** A finding class (the same catalogue
|
||
`rule_key`) that shows up in two review.md files in this repo becomes ONE
|
||
proposal; one sighting is no proposal. The history is the review.md files the
|
||
repo already has (every `.claude/projects/*/review.md`, plus earlier committed
|
||
versions of them) — no new store.
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/recurrence.mjs propose --repo "$PWD"
|
||
```
|
||
|
||
`--repo` is the session's working directory — the directory `project_dir`
|
||
paths are relative to, and the project Claude Code loads `.claude/rules/`
|
||
from. Not the git toplevel: for a project in a subdirectory that would read
|
||
no reviews and write the lessons in the wrong place.
|
||
|
||
It prints `{"proposals": [...]}` (exit 0) or exits 2 (could not run — say so
|
||
in the summary; never read 2 as "nothing recurred"). Each proposal names its
|
||
`rule_key`, its route and its evidence (the finding IDs from two reviews), and
|
||
carries one line in the format **mistake · fix · how to check**:
|
||
- route `check` (mechanical) — the line names the deterministic check that
|
||
catches the class;
|
||
- route `rule` (judgement) — the line is for the repo's `.claude/rules/`
|
||
file, which Claude Code loads at session start;
|
||
- review-hygiene classes are never proposed.
|
||
|
||
**The operator approves every line.** For each proposal, use
|
||
`AskUserQuestion` (up to 4 proposals per call), in plain language, showing
|
||
the line and its two pieces of evidence. Options: **"Add the line"**
|
||
(recommended: the class has recurred) and **"Not now"**; the operator may
|
||
answer with an edited line through "Other". Never write a line the operator
|
||
did not approve, and never treat "sounds good" or "do as you like" as a yes
|
||
to a line it was not shown.
|
||
|
||
On yes:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/recurrence.mjs apply --repo "$PWD" \
|
||
--rule-key <RULE_KEY> [--line "<the operator's edited line>"]
|
||
```
|
||
|
||
It writes ONLY between Voyage's own markers in
|
||
`.claude/rules/voyage-review-lessons.md` and, if the repo has a `CLAUDE.md`,
|
||
gives it a one-line pointer once (never the lesson itself). Exit 1 with
|
||
`over-budget` means the managed block is full: tell the operator the lessons
|
||
belong in a skill now — that is the operator's decision, not this command's.
|
||
Exit 1 with `invalid-line` means the line was refused by the schema check
|
||
(missing one of the three parts, a line break or control character, marker
|
||
text, or over 700 characters) — show the `detail` and ask again. Exit 1 with
|
||
`markers-damaged` means Voyage's markers in the lessons file are not one
|
||
start before one end: say so; the operator fixes the file by hand. A
|
||
finding class only counts when two reviews hold two DIFFERENT findings of it
|
||
— one unfixed finding seen again is a persisting defect, not a lesson — and
|
||
the two catch-all engine keys are never proposed. Report every outcome in the summary; when no class
|
||
recurred, say "no recurring finding class" — do not pad.
|
||
|
||
Borrowed: routing (Pocock `retro`), the three-part line (ai-native-sdlc),
|
||
the managed block (Spec Kit `agent-context`), evidence per proposal and
|
||
schema-check before write (Kiro), review → memory and the size budget
|
||
(Cherny). Credits in `lib/review/recurrence.mjs`.
|
||
|
||
## Phase 8.8 — Answers
|
||
|
||
**Skipped in `dry-run` and `validate`.** Every counting MAJOR needs the
|
||
operator's answer before a WARN opens the merge or the release; the verdict
|
||
gate decides that, not this command. List the open ones:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-answers.mjs pending --review {project_dir}/review.md --json
|
||
```
|
||
|
||
- Exit 0: nothing to ask.
|
||
- Exit 2: say "answers not measured" in one line; the merge stays closed.
|
||
- Exit 1: it lists the open MAJORs (`id`, `title`, `detail`, `security`,
|
||
`fixed_pending`). Ask about each one with `AskUserQuestion`, at most four per
|
||
call and one finding per question, in plain words with no file or function
|
||
names. Each question gives the consequence of each option:
|
||
- **Fix it**: a plan step is written, and the merge stays closed until a
|
||
later review round no longer finds it.
|
||
- **Accept: <a reason in plain words>**: the merge can open with this
|
||
finding left as it is. Never offered for a finding `pending` marks
|
||
`security: true`. That finding gets one option, Fix it, and its question says a security finding cannot be accepted.
|
||
|
||
The recommendation is the session's honest judgement per finding. It is
|
||
never "accept" by default. The operator may type their own reason; a hollow
|
||
answer ("sounds good", "do as you like") is not a reason.
|
||
|
||
For each answer, a "fixed" first gets its `### Step R<n>` under
|
||
`## Remediation Steps`, in the form Phase 7 writes for a BLOCKER (Files,
|
||
Source finding, Test first, Verify, On failure). Then record it, with the
|
||
reason on stdin:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-answers.mjs record --review "{project_dir}/review.md" --finding {id} --answer {accepted|fixed} --repo . --round {R} --store {artifact_store} --trailer "Co-Authored-By: Claude {model} <noreply@anthropic.com>"
|
||
```
|
||
|
||
`{R}` is this review's round (Phase 3's `{round}`), `{artifact_store}` the
|
||
brief's `artifact_store` (absent = `local`), and `{model}` this session's model
|
||
name. It appends a K2 `review` record, writes the `## Answers` row and commits
|
||
the log as `docs(voyage): review answers (round {R})`; show the operator the
|
||
stored reason it returns. On `"committed": false`, print its reason in one
|
||
line and continue: the answer is logged but not committed. On exit 1 (a
|
||
hollow or empty reason, a "fixed" without its R-step, an accept on a security
|
||
finding), say the refusal in one line and ask that finding again once. A
|
||
second refusal leaves it unanswered. On exit 2, say "answer not recorded" with
|
||
its reason in one line.
|
||
|
||
Then run the Phase 8 gate line again and report its result as `gate_exit`.
|
||
|
||
**Known issues outside the threat model** (E, decisions 164, 218). List each
|
||
entry of the review's `## Outside the Threat Model` (the trailing JSON block's
|
||
`known_outside`) for the operator in plain words, with the brief's `Outside:`
|
||
line it falls under: Voyage's own security findings the brief's threat model
|
||
puts outside. They are not answered and do not count. The verdict gate checks
|
||
each against the brief on every run: one whose line is gone, or whose intent
|
||
approval went stale, makes the gate exit 2 (not measured) until the review
|
||
runs again. To lift one back in, the operator changes that `Outside:` line and
|
||
approves the brief's intent again.
|
||
|
||
Run on its own, outside `/trekexecute`, "fixed" leaves the finding open until a
|
||
later `/trekreview` round marks it resolved.
|
||
|
||
**`--answers` mode.** `/trekreview --answers --project <dir>` runs only this
|
||
phase on the existing `{project_dir}/review.md`, and then the Phase 8 gate
|
||
line. It runs no `prepare`, no reviewer and no write of findings, so it opens
|
||
no new round and counts nothing against the cap (decision 93: accepting needs
|
||
no new review round). When `{project_dir}/review.md` is missing it stops with
|
||
one line: "no review to answer: {project_dir}/review.md is missing". In this
|
||
mode `--round` is the review's own `round` field (1 when absent), and
|
||
`--store` is the brief's `artifact_store` (absent = `local`), read from the
|
||
brief as Phase 1 finds it.
|
||
|
||
## Phase 9 — Present summary
|
||
|
||
After the write succeeds, print:
|
||
|
||
```
|
||
## Ultrareview Complete
|
||
|
||
**Task:** {task}
|
||
**Mode:** {default | quick | dry-run}
|
||
**Brief:** {brief_path}
|
||
**Project:** {project_dir}
|
||
**Review:** {review_path}
|
||
**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, or WARN with every counting MAJOR answered | 1 = BLOCK | 2 = could not decide, a control did not run, or a MAJOR without an answer or answered "fixed" and not yet confirmed — not green})
|
||
**Engine:** /code-review {delivered N | not run: reason} · security {Claude Security delivered N, M below threshold | /security-review delivered N | not measured: reason | none in quick mode}
|
||
|
||
### 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})
|
||
...
|
||
{up to 5 highest-severity findings}
|
||
|
||
────────────────────────────────────────────────────────────────────
|
||
To review and annotate the review, open it in a browser:
|
||
|
||
open file://{$ANNOT_HTML}
|
||
|
||
Click any line to add YOUR OWN note. The sidebar collects every note,
|
||
the "Copy Prompt" button gathers them into one structured prompt.
|
||
Paste that prompt back into this chat: Claude answers each Question
|
||
and revises review.md from your Fix and Change notes. Annotations
|
||
persist in your browser if you close the tab and reopen the same file.
|
||
────────────────────────────────────────────────────────────────────
|
||
|
||
You can also:
|
||
- Feed BLOCKER + MAJOR findings into a follow-up plan:
|
||
/trekplan --brief {review_path}
|
||
- Re-run with `--quick` for a faster correctness-only pass
|
||
- Re-run with `--since <ref>` to narrow scope
|
||
```
|
||
|
||
Per **Handover 6**, BLOCKER and MAJOR findings are consumed by
|
||
`/trekplan --brief review.md` to produce a remediation plan. The
|
||
review's frontmatter `findings:` list and the trailing JSON block are
|
||
the contract for that handover (see `docs/HANDOVER-CONTRACTS.md`).
|
||
|
||
## Hard rules
|
||
|
||
- **Brief is the contract.** Every finding in the review traces to a
|
||
brief section via `brief_ref`, except `SCOPE_CREEP_BUILT` (which
|
||
traces to "no anchor"). Conformance is the conformance reviewer's
|
||
job — code-correctness findings carry generic anchors like
|
||
`"NFR — code correctness"`.
|
||
- **Independent reviewers.** Do NOT cross-feed findings between
|
||
brief-conformance-reviewer and code-correctness-reviewer. The
|
||
coordinator is the only place where outputs combine.
|
||
- **Bounded coordination.** Synthesis-level inference across files is
|
||
forbidden in v1.0. The coordinator dedups, filters, and computes the
|
||
verdict — nothing more.
|
||
- **Triage map respected.** Files marked `skip` MUST appear in the
|
||
Coverage section. Silent drops are `COVERAGE_SILENT_SKIP` (MAJOR).
|
||
- **Block-style YAML for findings list.** The frontmatter parser does
|
||
not support flow-style arrays. `findings: [a, b]` is broken; use
|
||
`findings:\n - a\n - b`.
|
||
- **Refuse-with-suggestion above 100 files / 100K tokens.** Never run
|
||
blind on a giant diff. Use AskUserQuestion to surface the gate.
|
||
- **Privacy.** Never log secrets, tokens, or credentials in review.md.
|
||
Findings citing files with secret-like content must redact the secret
|
||
in the `detail` field.
|
||
- **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}` —
|
||
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`. Claude Security writes its own report
|
||
folder `CLAUDE-SECURITY-<ts>/` in the repo root, behind its own `.gitignore`;
|
||
`/trekreview` only reads it.
|
||
- **The engine never edits code.** `/code-review`, `/security-review` and Claude Security run
|
||
without `--fix`, `--comment`, `--post` or `ultra`; Claude Security runs
|
||
`scan changes` only, never its patch job. 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.
|