The review of v6.2.0..f669b87 (BLOCK: 1 BLOCKER, 12 MAJOR, 1 MINOR), each fixed behind a test that failed first, or, for the two missing tests, a test shown to fail on a stub of the branch it covers. - Review steps appended by /trekexecute Phase 7.2 are held to the step rules on any plan version: plan-validator --steps-from <N> (on a plan below 1.8 a template review step validated and was executed). - The guard refuses where no one can be asked also when its rules will not load; the wave merge path names that refusal instead of MERGE_REFUSED. Tests for the classifier-error refusal and post-approval-log's own check. - release-check: the version files may change only in their version after the review (JSON compared without it), README diff lines are read inside hunks only, an unreadable diff is exit 2, and npm test runs without NODE_TEST_CONTEXT. - What counts as a step of only tests: a note in parentheses is not a path, and a data file under specs/ is not a test. End-to-end: rides on the last step that builds behaviour (validator hint, trekplan, template). - A removed line is cited as <rev>:<path>:<line> (findings-schema hint, trekreview re-ask). - skill-lint counts a trailing newline as no line; regression-check takes commandEnv from lib/util/command-env.mjs; CLAUDE.md names four mode-read exemptions. - /security-review, older than v6.2.0: the criteria runner refuses git --output, which writes a file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
932 lines
46 KiB
Markdown
932 lines
46 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]"
|
||
allowed-tools: Agent, Read, Glob, Grep, Write, Edit, Bash, AskUserQuestion, Skill
|
||
---
|
||
|
||
# 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.1, ordre 5).** In this order:
|
||
Claude Code's own `/code-review` and `/security-review` are the engine
|
||
(Phase 4.7); Voyage's control against the brief runs after them and is the
|
||
part that stops (Phase 5–6); the verdict is written in `review.md` and one
|
||
command decides on it (`lib/review/verdict-gate.mjs`, Phase 8: 0 = ALLOW/WARN,
|
||
1 = BLOCK, 2 = could not decide, never green). No finding is struck for
|
||
length. A finding class seen in two reviews becomes one proposal for the
|
||
project's memory, approved line by line (Phase 8.7). Order credited to Boris
|
||
Cherny's public workflow (engine → brief control); the gate to Kiro and Spec
|
||
Kit — see `docs/HANDOVER-CONTRACTS.md` § Handover 6.
|
||
|
||
See `agents/review-orchestrator.md` for the canonical workflow this
|
||
command executes inline.
|
||
|
||
## Phase 1 — Parse mode and validate input
|
||
|
||
Parse the arguments with the shared arg-parser. Run it first, exactly like
|
||
this, before anything else:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/parsers/arg-parser.mjs --command trekreview -- $ARGUMENTS
|
||
```
|
||
|
||
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. |
|
||
| `--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). |
|
||
| `--workflow` | boolean | **(opt-in, NW2)** Run Phase 5–6 on the bake-off-validated Workflow substrate (`scripts/trekreview-armB.workflow.mjs`) instead of the default prose Agent-tool path. Requires **Claude Code 2.1.154+**. Combines with `--quick`. See *§ Phase 5–6 via the Workflow substrate*. |
|
||
|
||
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).
|
||
- `dry-run` if `--dry-run` is set.
|
||
- `quick` if `--quick` is set.
|
||
- `default` otherwise.
|
||
|
||
Set `workflow_substrate` (orthogonal to `mode` — a substrate choice, not a behavior mode):
|
||
- `true` if `--workflow` is set — Phase 5–6 run on the Workflow substrate (see the Phase 5
|
||
routing gate). The Workflow tool requires **Claude Code 2.1.154+**; if it is unavailable,
|
||
fall back to the prose path and note the fallback in the Executive Summary.
|
||
- `false` otherwise. **Default stays prose**: the substrate is opt-in, so the lower
|
||
portability floor of the prose path is preserved unless the operator opts in.
|
||
|
||
## 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}"
|
||
|
||
# v5.9 — composed phase-model resolution (brief > profile > default) for the
|
||
# review phase. ONE call returns {model, source}, plus
|
||
# effort only when the brief's phase signal sets one; without it the phase
|
||
# runs at effort `standard` (no change). Captured as
|
||
# phase_signal_result and used in Phase 7 at the reviewer-launch site to
|
||
# inject the resolved model. Append --profile {profile} when the operator
|
||
# passed --profile.
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/profiles/resolver.mjs --resolve-phase-model --phase review --brief-path "{brief_path}" [--profile {profile}] --json
|
||
```
|
||
|
||
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)`.
|
||
|
||
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 + stats).
|
||
|
||
## 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` | Matches `auth/**`, `crypto/**`, `**/security/**`, `hooks/**`. |
|
||
| `summary-only` | Default treatment for everything else. |
|
||
|
||
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 reaches a 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 — 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 (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"
|
||
```
|
||
|
||
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.
|
||
|
||
**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
|
||
Phases 5–6 via the Workflow substrate documented in *§ Phase 5–6 via the Workflow
|
||
substrate* (below Phase 6), then resume at Phase 7 with the returned
|
||
`{verdict, findings}`. When `false` (the default), run the prose Agent-tool path
|
||
described in the rest of this phase.
|
||
|
||
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.
|
||
|
||
| 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 | 7-dimension code review. Emits findings tagged with rule_keys from the correctness/security/maintenance/tests categories. |
|
||
|
||
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`.
|
||
|
||
`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
|
||
`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
|
||
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, 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.
|
||
|
||
**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 5–6 via the Workflow substrate (opt-in `--workflow`)
|
||
|
||
Runs **only** when `workflow_substrate == true`. This is the **NW2 port**: it expresses
|
||
the SAME Phase 5–6 pipeline (parallel reviewers → triplet-dedup → coordinator verdict) as
|
||
a single Workflow, reusing the NW1 findings schema. The S10 bake-off found it
|
||
**fidelity-equivalent** to the prose path — see `docs/T2-bakeoff-results.md` (verdict
|
||
**POSITIVE**: verdict-match 1.0, issue-coverage 100%, `(file,rule_key)` jaccard ≥
|
||
within-arm, tokens +4.4%). It stays **opt-in**, not the default, because the Workflow tool
|
||
raises the consumer floor to **Claude Code 2.1.154+** (outward-facing; the prose path
|
||
keeps the lower floor).
|
||
|
||
Invoke the port via the **Workflow** tool with the Phase 1–4 output pinned into `args`:
|
||
|
||
```
|
||
Workflow({
|
||
scriptPath: "${CLAUDE_PLUGIN_ROOT}/scripts/trekreview-armB.workflow.mjs",
|
||
args: {
|
||
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},
|
||
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.
|
||
- **Reviewers are `StructuredOutput`-schema-forced** — `rule_key` is enum-enforced at the
|
||
tool layer (stronger than the prose path's post-hoc NW1 check), so there is no
|
||
`JSON.parse`/re-ask dance.
|
||
- **Recover the result from the `RESULT_JSON:{…}` line** inside the workflow output logs.
|
||
The script returns `{verdict, findings, ...}` AND logs it as that line; the
|
||
notification's `<result>` may be truncated, so parse the logged line.
|
||
- The reviewer/coordinator agentTypes are namespaced inside the script
|
||
(`voyage:brief-conformance-reviewer`, `voyage:code-correctness-reviewer`,
|
||
`voyage:review-coordinator`).
|
||
|
||
Then continue at **Phase 7** exactly as the prose path does — Phase 7 rendering, Phase 8
|
||
validation, and the operator gate are **shared** and substrate-independent (both paths
|
||
return the same `{verdict, findings}` shape).
|
||
|
||
**Known limitation (per bake-off §Posture, surfaced not hidden).** Classifier interference
|
||
was measured **0** at 9-agent concurrency in the session's default permission mode; an
|
||
explicit `auto`/`bypass`-mode re-run was not performed (the permission mode is operator-set,
|
||
not settable from within a session). trekreview's small fan-out showed 0 interference in S8
|
||
and S10. The large fan-out case (the trekplan swarm) is out of NW2 scope.
|
||
|
||
## 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.
|
||
|
||
**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 + stats
|
||
|
||
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; exit 1 = BLOCK; exit 2 = the gate could not decide
|
||
(missing verdict, a verdict the file contradicts — in its JSON block, its
|
||
`## Findings (BLOCKER)` section or its finding-id list — a JSON block that is
|
||
not the last fence or has the wrong shape, two verdict lines, unreadable
|
||
file) — **never green**. A WARN that exists because a reviewer or engine
|
||
did not run (`missing-reviewer:` / `unattributable-payload` in
|
||
`allow_blocked_by`) is also exit 2: a review not performed in full stops a
|
||
merge (operator decision 2026-10-06). It takes exactly one file. On exit 0
|
||
with WARN, `--json` also returns `warn_reasons` (the JSON block's
|
||
`allow_blocked_by`); print them. Capture the exit code as `gate_exit` and print it in Phase 9 exactly
|
||
as returned. This is the command that later stops a merge or a release on
|
||
BLOCK; `/trekreview` reports it and never overrides it.
|
||
|
||
Then append one record to `trekreview-stats.jsonl` in the plugin data directory.
|
||
Run it exactly like this. The record goes on stdin, and the writer resolves
|
||
the directory: `CLAUDE_PLUGIN_DATA`, which the Bash tool env does not carry,
|
||
else the directory `scripts/yardstick.mjs` reads.
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/stats/stats-append.mjs trekreview <<'JSON'
|
||
{"ts":"{ISO-8601}","slug":"{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}
|
||
JSON
|
||
```
|
||
|
||
The writer prints `{"written": true, ...}`. On `"written": false` (exit 1),
|
||
print its `reason` in one line and continue. A tracking failure never blocks
|
||
the main workflow, but it is reported, not skipped silently.
|
||
|
||
**Build the operator-annotation HTML.** After stats land, 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 HTML renders
|
||
`review.md` with line numbers, lets the operator click any line to attach
|
||
their own note (not Claude-generated suggestions — the operator drives
|
||
every annotation), keeps a sidebar of all notes, persists state in
|
||
localStorage, and exposes a "Copy Prompt" button. 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: **"Skriv linja inn"**
|
||
(recommended: the class has recurred) and **"Ikke nå"**; the operator may
|
||
answer with an edited line through "Other". Never write a line the operator
|
||
did not approve, and never treat "sounds good" or "do as you like" as a yes
|
||
to a line it was not shown.
|
||
|
||
On yes:
|
||
|
||
```bash
|
||
node ${CLAUDE_PLUGIN_ROOT}/lib/review/recurrence.mjs apply --repo "$PWD" \
|
||
--rule-key <RULE_KEY> [--line "<the operator's edited line>"]
|
||
```
|
||
|
||
It writes ONLY between Voyage's own markers in
|
||
`.claude/rules/voyage-review-lessons.md` and, if the repo has a `CLAUDE.md`,
|
||
gives it a one-line pointer once (never the lesson itself). Exit 1 with
|
||
`over-budget` means the managed block is full: tell the operator the lessons
|
||
belong in a skill now — that is the operator's decision, not this command's.
|
||
Exit 1 with `invalid-line` means the line was refused by the schema check
|
||
(missing one of the three parts, a line break or control character, marker
|
||
text, or over 700 characters) — show the `detail` and ask again. Exit 1 with
|
||
`markers-damaged` means Voyage's markers in the lessons file are not one
|
||
start before one end: say so; the operator fixes the file by hand. A
|
||
finding class only counts when two reviews hold two DIFFERENT findings of it
|
||
— one unfixed finding seen again is a persisting defect, not a lesson — and
|
||
the two catch-all engine keys are never proposed. Report every outcome in the summary; when no class
|
||
recurred, say "no recurring finding class" — do not pad.
|
||
|
||
Borrowed: routing (Pocock `retro`), the three-part line (ai-native-sdlc),
|
||
the managed block (Spec Kit `agent-context`), evidence per proposal and
|
||
schema-check before write (Kiro), review → memory and the size budget
|
||
(Cherny). Credits in `lib/review/recurrence.mjs`.
|
||
|
||
## Phase 9 — Present summary
|
||
|
||
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/WARN | 1 = BLOCK | 2 = could not decide, or a control did not run — not green})
|
||
**Engine:** /code-review {delivered N | not run: reason} · /security-review {delivered N | not run: reason}
|
||
|
||
### Counts
|
||
- BLOCKER: {N}
|
||
- MAJOR: {N}
|
||
- MINOR: {N}
|
||
- SUGGESTION: {N}
|
||
- Trimmed (MINOR/SUGGESTION text only, never removed): {N}
|
||
- Declined to judge: {N} — {one line each: file:line rule_key — reason}
|
||
|
||
### Project memory
|
||
{each proposal: rule_key → written | declined by operator | over-budget | invalid-line;
|
||
or "no recurring finding class"; or "recurrence check could not run"}
|
||
|
||
### Top findings
|
||
- [{severity}] {title} ({file}:{line})
|
||
...
|
||
{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 and Claude revises review.md
|
||
from your 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`).
|
||
|
||
## Profile (v4.1)
|
||
|
||
Accepts `--profile <name>` where `<name>` is `economy`, `balanced`, `premium`, `fable`,
|
||
or a custom profile under `voyage-profiles/`. Default: `premium`.
|
||
|
||
Resolution order (per `lib/profiles/resolver.mjs`):
|
||
1. `--profile` flag (source: `flag`)
|
||
2. `VOYAGE_PROFILE` env-var (source: `env`)
|
||
3. `premium` default (source: `default`)
|
||
|
||
The selected profile drives `phase_models.review` — `economy` uses sonnet
|
||
for the brief-conformance + code-correctness reviewers; `balanced` and
|
||
`premium` use opus (review benefits from deeper reasoning).
|
||
|
||
Examples:
|
||
```
|
||
/trekreview --profile balanced --project .claude/projects/2026-05-09-add-auth
|
||
VOYAGE_PROFILE=premium /trekreview --project ...
|
||
```
|
||
|
||
Stats records emit `profile` and `profile_source`.
|
||
|
||
## Composition rule (v5.1)
|
||
|
||
Independent of the profile system. When `brief.md` carries
|
||
`phase_signals` (brief_version ≥ 2.1), each downstream phase resolves
|
||
effort + model as:
|
||
|
||
```
|
||
effort_for_phase = brief.phase_signals[<phase>]?.effort ?? 'standard'
|
||
model_for_phase = brief.phase_signals[<phase>]?.model ?? profile.phase_models[<phase>]
|
||
```
|
||
|
||
The brief signal wins per-phase when present; the profile fills any
|
||
gaps. Both fields are mechanically resolved by the single composed CLI
|
||
`node ${CLAUDE_PLUGIN_ROOT}/lib/profiles/resolver.mjs --resolve-phase-model`
|
||
invoked in Phase 2; the resolved JSON is `{model, source}`, plus
|
||
`effort` only when the brief's phase signal sets one (without it the
|
||
phase runs at effort `standard`: no change). It is captured as
|
||
`phase_signal_result` and passed to `Agent` tool calls explicitly. The
|
||
resolver controls the `model` parameter at Agent-spawn sites only — the
|
||
orchestrator's own model is fixed at invocation time (command frontmatter
|
||
omits `model:`, so it follows the session model) and cannot be switched
|
||
mid-turn. Sub-agents fall back to `model:` in their own `agents/*.md`
|
||
frontmatter when no spawn-site injection happens.
|
||
|
||
For `/trekreview` specifically: `effort == 'low'` activates the existing
|
||
`--quick`-equivalent code-path (skip the brief-conformance reviewer; run
|
||
correctness-only). `effort == 'standard'` (or absent) → no change.
|
||
`effort == 'high'` activates the high-effort behavior documented under
|
||
`### High-effort behavior (v5.1.1)` below.
|
||
|
||
### Sequencing gate surface
|
||
|
||
Phase 1 already calls `brief-validator.mjs --soft` against `{brief_path}`.
|
||
If the validator returns `BRIEF_V51_MISSING_SIGNALS` in `errors`
|
||
(brief_version ≥ 2.1 without `phase_signals` or `phase_signals_partial:
|
||
true`), halt with: `Brief is brief_version 2.1 but does not carry
|
||
phase_signals — re-run /trekbrief to commit them (Phase 3.5).`
|
||
Enforcement is validator-only; this surface just makes the friendly hint
|
||
readable.
|
||
|
||
### High-effort behavior (v5.1.1)
|
||
|
||
When `phase_signal_result.effort == 'high'` for the `review` phase,
|
||
skip Pass 3 (Cloudflare reasonableness filter) in
|
||
`agents/review-coordinator.md`. Passes 1, 2, and 4 still run.
|
||
Rationale: high-effort review trusts the operator to weigh borderline
|
||
findings rather than have the coordinator drop them. To prevent
|
||
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 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.
|
||
|
||
Standard effort (or absent): run all 4 passes as usual.
|
||
Low effort: skip the brief-conformance reviewer entirely (existing
|
||
`--quick`-equivalent code-path).
|
||
|
||
## 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.
|
||
- **Cost.** Model resolution at Agent-spawn sites is a three-layer fallback:
|
||
brief `phase_signals[<phase>].model` > `profile.phase_models[<phase>]` >
|
||
agent frontmatter `model:`. The composed resolver returns the first two
|
||
layers as `phase_signal_result.model`; spawn sites inject it, and agent
|
||
frontmatter is the fallback when no injection happens.
|
||
- **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}` 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.
|