feat(code-review): wire start.md to the run-prefix loop (PLN-1229 Phase 3) (#175)

* feat(code-review): wire start.md to the run-prefix loop (PLN-1229 Phase 3)

Rewrite the orchestrator's deterministic-prefix walk into a run-prefix loop —
the payoff phase that collapses ~19 helper stages (and their serial model turns)
into a handful of orchestrator turns.

start.md:
- New "Deterministic Prefix — run-prefix loop" section: invoke run-prefix and
  dispatch on next_action — needs_singleton (singleton-dispatch skill, then
  re-invoke --resume-from), hygiene_exit (Gate A present + EXIT),
  ready_for_reviewers (print cache/fast-path notices, read CACHE_DIR from
  cache_config.json, hand off to the Walker Contract at stage_20), and error
  (fall back to the per-stage walk from failed_stage).
- Reframe the Walker Contract as (a) the always-walked reviewer/verify/present
  tail from stage_20 onward and (b) the labeled per-stage fallback for the
  prefix. Gate A/B now execute inside run-prefix (their prose is the fallback
  recipe); Gate C/D still fire in the tail. Update the "Batch deterministic
  helper stages" note to point at run-prefix.
- Keep the operator todo list accurate across the now-silent prefix.

singleton-dispatch skill: step 4 now branches on caller — return to start.md for
--resume-from in the run-prefix flow, advance the walker only in the fallback.

SCHEMA.md: correct the route-failure failed_stage to stage_19_cache_check
(matching the merged runner) and note the shallow-tier derive-static-spec stage.

Additive with a live per-stage fallback (no behavior lost); MINOR bump. Prose
change validated by an expert review of the rewire (no HIGH findings; all flagged
items fixed) — the runner itself is parity-proven by Phases 1-2. Full code-review
suite 1306 passed / 3 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(code-review): changelog + README for run-prefix orchestrator wiring (PLN-1229 Phase 3)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(code-review): remove stray subagent hook debug log from plugin tree

The Phase 3 feat commit accidentally swept in
plugins/code-review/.closedloop-ai/subagent-start-hook-debug.log, a
runtime artifact written by the SubagentStart hook. The existing
.gitignore patterns for .closedloop-ai/ are root-anchored, so they never
covered plugin-local runtime dirs. Remove the file and add a
plugins/**/.closedloop-ai/ guard so hook/review artifacts can't be
committed under the plugin tree again.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: wongk

.gitignore

@@ -38,6 +38,9 @@ __pycache__/
 .closedloop-ai/*
 !.closedloop-ai/settings/
 !.closedloop-ai/bootstrap-metadata.json
+# Plugin-local runtime working dirs must never be committed (the patterns above
+# are root-anchored; hook/review artifacts can otherwise land under plugins/**).
+plugins/**/.closedloop-ai/
 
 # Generated route knowledge files
 .claude/docs/routes/*

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to the claude-plugins project will be documented in this fil
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Entries are listed newest-first; each plugin section is treated as released when merged to `main`.
 
+### code-review v3.4.0
+
+#### Changed
+- The `/code-review` orchestrator now runs the deterministic review prefix via the `run-prefix` helper loop instead of walking those stages one at a time. After session setup it invokes `run-prefix` and acts on the returned status: dispatching a signal-extraction or coverage-critic agent and resuming when one is needed, presenting hygiene findings and exiting on a hygiene-only run, or — once the whole prefix (including model routing and partitioning) is done — printing the cache/fast-path notices and handing off to the reviewer fleet. This collapses roughly nineteen helper stages and their serial model turns into a handful of orchestrator turns, the single biggest turn-count reduction in a review. The per-stage walk remains fully documented as the labeled fallback, used only when `run-prefix` reports an error or is unavailable (e.g. an older plugin cache), so no behavior is lost.
+- The stage-by-stage "Walker Contract" is now scoped to the reviewer/verification/presentation tail (spawn-reviewers onward) plus that prefix fallback; the hygiene-only exit and the routing/partition gate are performed inside `run-prefix` and surfaced through its result. The single-agent dispatch skill and `SCHEMA.md` were updated to match (the skill returns control for `run-prefix` to resume in the default flow, and `SCHEMA.md` documents the routing-failure recovery anchor and the shallow-tier spawn-spec stage).
+
 ### code-review v3.3.0
 
 #### Changed

plugins/code-review/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "code-review",
   "description": "Code review plugin",
-  "version": "3.3.0",
+  "version": "3.4.0",
   "author": {
     "name": "ClosedLoop",
     "email": "support@closedloop.ai"

plugins/code-review/README.md

@@ -56,7 +56,9 @@ canonical schema documented in [SCHEMA.md](SCHEMA.md). Key contracts:
   (default cap=20, BHA floor=1 waived for docs-only PRs, required overflow
   fails closed and emits coverage gaps).
 - **`prepare-run`** emits a declarative `run_plan.json` describing the 30-stage
-  pipeline. (The orchestrator rewrite over this plan is a follow-up.)
+  pipeline; the orchestrator runs its deterministic prefix in one process via
+  `run-prefix` and walks the reviewer/verification/presentation tail
+  stage-by-stage (PLN-1229).
 - **Canonical `prompt_hash`** folds in `schema_version`: a MAJOR schema bump
   invalidates every cache namespace at once.
 
@@ -66,7 +68,7 @@ The terminal artifact of every review run is `review_result.json` (PLN-722 envel
 
 | Component | Role |
 |---|---|
-| `start.md` | Orchestrator command. Parses flags, sets up the session, invokes the helper CLI subcommands in sequence, spawns reviewer sub-agents, collects results, and presents findings |
+| `start.md` | Orchestrator command. Parses flags, sets up the session, runs the deterministic prefix in one process via `run-prefix` (then walks the reviewer tail stage-by-stage), spawns reviewer sub-agents, collects results, and presents findings |
 | `github-review.md` | Loaded by the orchestrator only in GitHub mode. Contains PR metadata resolution, file-based handoff format for CI, and summary format |
 | `code_review_helpers.py` | Python CLI that handles all deterministic work: git diff parsing, hygiene pattern matching, file partitioning, risk scoring/model routing, finding validation, cache management, and GitHub comment posting |
 | `shared_prompt.txt` | Constraints injected into every reviewer agent prompt: file assignment rules, evidence standards, severity definitions, and output format |
@@ -196,7 +198,7 @@ The orchestrator executes these steps in order:
 13. **Review state write** — persists the current diff tip so future `--since-last-review` runs can narrow the scope
 14. **Footer** — prints elapsed time, token usage stats, and writes the deterministic verdict JSON to `<CR_DIR>/verdict.json` (consumed by the `code` plugin's `run-loop.sh`)
 
-(Step numbers in this list are illustrative; the canonical 30-stage ordering lives in `prepare-run`'s `run_plan.json`.)
+(Step numbers in this list are illustrative; the canonical 30-stage ordering lives in `prepare-run`'s `run_plan.json`. Steps 2–8 — the deterministic prefix through routing and partitioning — run in a single process via the `run-prefix` helper; the orchestrator walks the reviewer/validation/presentation tail from step 9 onward.)
 
 ## Helper CLI (`code_review_helpers.py`)
 

plugins/code-review/SCHEMA.md

@@ -550,8 +550,10 @@ The exit code is `0` for every well-formed result (including `error`) — the
 honored exactly as the Walker Contract prescribes: `abort` → `error`; `continue`
 → proceed; `continue_with_coverage_gap` → proceed after writing an
 `agent-failure` system finding to `agent_<stage>-failed.json` (collected by
-`collect-findings`). A `route` failure is surfaced as `error` with
-`failed_stage: "route"`.
+`collect-findings`). A Gate B `route` failure is surfaced as `error` with
+`failed_stage: "stage_19_cache_check"` (route is not a plan stage, so the error
+anchors on the stage a per-stage fallback resumes from — re-running cache-check
+→ route → partition).
 
 ---
 

plugins/code-review/commands/start.md

@@ -60,10 +60,10 @@ The walk is hybrid:
 
 **Turn & context discipline (cost).** Cache cost scales with carried context × turn count, so keep both small:
 - **Never read large artifacts into the orchestrator's context.** `diff_data.json`, `patches_*.txt`, and per-file diffs are passed to helpers and reviewers as **file-path arguments**, never `cat`/`Read` into the walk. Reviewers read patches themselves (see the spawn skill's anti-inline rule). The only large file the orchestrator reads is `review_result.json` at the present stage, once, with the per-section display caps the present skill already applies.
-- **Batch deterministic helper stages.** As a turn-count optimization you MAY chain a run of consecutive `helper`-kind stages into one `Bash` call (`cmd1 && cmd2 && …`, each redirecting stdout per its `stdout` field) — but ONLY a run in which **every** stage declares `on_failure: abort`, **none** has a `GATES` entry firing after it, **none** is a branching-gate boundary (A/B/C/D) or an `agent_fleet`/`present`/singleton-dispatch stage, and no stage's args depend on a value an in-batch predecessor printed to stdout. Those constraints make recovery unambiguous: there is no gate to interleave, and no `continue` stage whose successors the `&&` short-circuit would wrongly skip. After the chain returns, confirm each chained stage's `expected_outputs`; if the chain exited non-zero, an `abort` stage failed, so **abort** (do not run any gate against the partial batch). Any stage with an associated gate, a non-`abort` `on_failure` (`continue` / `continue_with_coverage_gap`), or a stdout dependency runs solo under the normal one-stage-at-a-time walk. When in doubt, don't batch.
+- **Batch the deterministic prefix with `run-prefix`.** The entire deterministic prefix (stages 01→19b) runs in ONE process via the `run-prefix` helper — see the **Deterministic Prefix — `run-prefix` loop** section below — collapsing ~19 helper stages (and their ~4 serial model turns each) into a handful of orchestrator turns. This is the default and the single biggest turn-count saving. Only in the per-stage **fallback walk** (when `run-prefix` errored or is unavailable) may you additionally chain a run of consecutive `helper`-kind stages into one `Bash` call (`cmd1 && cmd2 && …`, each redirecting stdout per its `stdout` field) — but ONLY a run in which **every** stage declares `on_failure: abort`, **none** has a `GATES` entry firing after it, **none** is a branching-gate boundary (A/B/C/D) or an `agent_fleet`/`present`/singleton-dispatch stage, and no stage's args depend on a value an in-batch predecessor printed to stdout. Those constraints make recovery unambiguous: there is no gate to interleave, and no `continue` stage whose successors the `&&` short-circuit would wrongly skip. After the chain returns, confirm each chained stage's `expected_outputs`; if the chain exited non-zero, an `abort` stage failed, so **abort** (do not run any gate against the partial batch). Any stage with an associated gate, a non-`abort` `on_failure` (`continue` / `continue_with_coverage_gap`), or a stdout dependency runs solo under the normal one-stage-at-a-time walk. When in doubt, don't batch.
 - **Narrate sparingly.** Emit only the operator-essential lines the per-stage notes mark for printing (review-mode line, cache status, fast-path notice, verdict). Do not echo intermediate stage progress as prose.
 
-Four runtime gates modify walker default behavior (they are runtime-driven and either replace the default walk or add a condition on top of a plan stage):
+Four runtime gates modify walker default behavior (they are runtime-driven and either replace the default walk or add a condition on top of a plan stage). **Gate A and Gate B fire inside the prefix — `run-prefix` performs them and surfaces the result to you (as `hygiene_exit` / `ready_for_reviewers`); you only execute their mechanics in the per-stage fallback walk. Gate C and Gate D fire in the walked tail (after `stage_20`), so you always apply them.**
 1. **Gate A** — after `stage_12_hygiene`, if `flags.hygiene_only` is true: present hygiene findings and **EXIT** (no further stages, no verdict, no footer).
 2. **Gate B** — after `stage_19_cache_check`, invoke `route` (model routing) to compute `fast_path` and `max_bha_agents`. `fast_path == true` skips `stage_17_partition` entirely and drives a single fast-path reviewer in `stage_20`.
 3. **Gate C** — before `stage_26_cache_update`, skip if `fast_path == true` OR `CACHE_DIR` is empty.
@@ -200,8 +200,36 @@ If MODE=github, also Read `${CLAUDE_PLUGIN_ROOT}/prompts/github-review.md` now.
 
 ---
 
+## Deterministic Prefix — `run-prefix` loop
+
+The deterministic prefix — every stage from `stage_01_setup` through Gate B (`route`), `stage_17_partition`, and `stage_19b_derive_spawn_spec` (or `stage_19c_derive_static_spec` in `--depth shallow`) — is run **in one process** by the `run-prefix` helper instead of one orchestrator turn per stage. This is the default path. Do **not** walk these stages one at a time (the per-stage **Walker Contract** below is the labeled fallback, used only when `run-prefix` returns `error` or is unavailable, e.g. an old marketplace cache with no `run-prefix` subcommand).
+
+`run-prefix` reads `run_plan.json` + `setup.json` from `<CR_DIR>` (both written in stage 0), resolves each stage's placeholder tokens from prior-stage artifacts, honors every stage's `on_failure` policy and validation gate exactly as the Walker Contract prescribes, and pauses only at genuine decision points — emitting a status JSON. See `SCHEMA.md` §7b for the full result contract.
+
+Invoke it, then dispatch on the result's `next_action` (authoritative — read the field, not the exit code, which is `0` for every well-formed result):
+
+```bash
+python3 <HELPERS> run-prefix --cr-dir <CR_DIR> --plugin-root <PLUGIN_ROOT>
+```
+
+Read the status JSON from stdout and act:
+
+1. **`needs_singleton`** — a PLN-725 singleton needs an agent (`singleton` is `"extract_signals"` or `"coverage_critic"`). Invoke the `code-review:singleton-dispatch` skill for that stage — it reads the prepare manifest `run-prefix` just wrote and spawns one synchronous Task, writing `pln725_<singleton>.json`. Then **re-invoke** `run-prefix` with `--resume-from <resume_stage>` (the `resume_stage` from the result — the sibling consolidate stage) and dispatch on the new result. Both singletons fire on most runs, so expect up to two such pauses per review.
+
+2. **`hygiene_exit`** — **Gate A** (hygiene-only). Mark the pre-review todos `run-prefix` completed (`Parse scope and get diff data`, `Run deterministic hygiene checks`) `completed`. If `cache_status_message` is non-null, print it. Render `<CR_DIR>/hygiene.json` using the **Hygiene Findings Format (Gate A render target)** section below. If `MODE=github`, do the Gate A GitHub write (`.closedloop-ai/code-review-summary.md` + `.closedloop-ai/code-review-findings.json`). Then mark "Present hygiene findings" `completed` and **EXIT** — no route, partition, agents, validate, finalize, verdict, or footer.
+
+3. **`ready_for_reviewers`** — the whole deterministic prefix is done; `run-prefix` has already run Gate B `route`, partitioned (or skipped partition in fast-path), and derived the spawn spec. Mark the pre-review todos `run-prefix` completed (`Parse scope and get diff data`, `Run deterministic hygiene checks`, `Assess scope and route models`) `completed`. Cache `FAST_PATH` (`fast_path`) and `MAX_BHA_AGENTS` (`max_bha_agents`) from the result. **Read `CACHE_DIR` from `<CR_DIR>/cache_config.json` (`cache_dir`, empty when no cache)** — the run-prefix loop skipped the walk where the fallback would have cached it, and Gate C, Gate D, and the notices below all need it. If `cache_status_message` is non-null, print it. If `FAST_PATH` is true, read `<CR_DIR>/spawn.json` (`route.models.fast_path_reviewer`) and print `"Fast path selected: 1 reviewer (<fast_path_reviewer>)."` (matching the Gate B fallback notice) and — when `CACHE_DIR` is set — `"BHA Cache: bypassed in fast-path mode."`, and replace the "Spawn reviewer agents in parallel" todo with "Run fast-path review". Then continue with the **Walker Contract** below **starting at `stage_20_spawn_reviewers`** — the reviewer fleet and everything after it are still walked one stage at a time. (Any `<CACHE_DIR>` / `<REVIEW_ROOT>` / other tokens the tail stages need are resolved from the on-disk artifacts `run-prefix` wrote, per the token table.)
+
+4. **`error`** — a stage aborted or a validation gate failed (`failed_stage` names the stage; `message` carries the diagnostic). Partial artifacts on disk are preserved. **Fall back** to the per-stage **Walker Contract** below, resuming the walk from `failed_stage` (re-run only that stage forward). If a downstream stage keeps failing, surface `message` to the operator.
+
+The routing/cache notices (Gate A cache line, Gate B fast-path + cache line) are the operator-essential output of this loop — emit them and nothing else; do not narrate the individual prefix stages `run-prefix` ran.
+
+---
+
 ## Walker Contract
 
+**When this applies.** The Walker Contract governs the **reviewer/verify/present tail** — `stage_20_spawn_reviewers` onward — which is always walked one stage at a time. In the normal flow the `run-prefix` loop above has already run the deterministic prefix (stages 01→19b) and handed off at `stage_20_spawn_reviewers`, so **begin the walk there**. The Contract is ALSO the **per-stage fallback for the prefix**: if `run-prefix` returned `error` (or is unavailable), walk the prefix stages one at a time from `failed_stage`, applying the same steps 1-8 and the Branching Gates (A/B) below. Everything in this section — token resolution, `on_failure`, gates, singleton dispatch — is exactly what `run-prefix` reproduces internally; it is documented here as the canonical contract and the recovery path.
+
 **Reading `<CR_DIR>/*.json` artifacts.** The walker reads run-plan output JSON to resolve placeholder tokens (`<DIFF_SCOPE>`, `<CACHE_DIR>`, etc.). If your session has a hook that intercepts the `Read` tool on generated artifacts (e.g. a code-discovery gate that demands codebase-memory-mcp lookups), fall back to `cat` via `Bash` — these are pipeline artifacts, not source code.
 
 Walk `STAGES` in array order. For each stage:
@@ -263,6 +291,8 @@ If a token's source file does not exist yet (a prior stage that produces it was
 
 Four runtime gates modify walker default behavior. Each is documented below with the exact stage boundary it fires at.
 
+**Gate A and Gate B fire inside the prefix, so `run-prefix` performs them for you.** In the normal flow you never execute the mechanics below — `run-prefix` runs Gate A's hygiene-only exit (surfaced as `next_action: "hygiene_exit"`) and Gate B's `route` + partition (surfaced as `next_action: "ready_for_reviewers"` with `fast_path` / `max_bha_agents`), and the **Deterministic Prefix — `run-prefix` loop** section tells you what to print and where to hand off. The Gate A/B detail below is the canonical spec and the recipe for the **per-stage fallback walk** (when `run-prefix` errored). **Gate C and Gate D fire after `stage_20`, in the walked tail, so you always apply them yourself** as described.
+
 ### Gate A — After `stage_12_hygiene`: Hygiene-only early exit
 
 If `FLAGS.hygiene_only` is true (or the equivalent `--hygiene-only` was passed):
@@ -397,7 +427,7 @@ When the walker reaches `stage_23_verify_findings`, invoke the `code-review:veri
 
 ## PLN-725 Single-Agent Dispatch
 
-Walker-contract step 6 points here. When the stage just finished is `stage_11_extract_signals` or `stage_15_coverage_critic`, invoke the `code-review:singleton-dispatch` skill. The skill owns the full protocol: reading the prepare manifest's `status` (`cache_hit` / `skipped` → no dispatch; `needs_agent` → spawn one synchronous singleton Task), the by-convention `pln725_*.json` agent write target, and the fail-closed semantics the sibling consolidate stage relies on.
+The `run-prefix` loop's `next_action: "needs_singleton"` handler points here (and, in the per-stage fallback walk, so does Walker-Contract step 6). When `run-prefix` reports a singleton — or, in the fallback walk, when the stage just finished is `stage_11_extract_signals` or `stage_15_coverage_critic` — invoke the `code-review:singleton-dispatch` skill. The skill owns the full protocol: reading the prepare manifest's `status` (`cache_hit` / `skipped` → no dispatch; `needs_agent` → spawn one synchronous singleton Task), the by-convention `pln725_*.json` agent write target, and the fail-closed semantics the sibling consolidate stage relies on. In the `run-prefix` flow the manifest already exists on disk (the runner wrote it); after the skill writes `pln725_<singleton>.json`, re-invoke `run-prefix --resume-from <resume_stage>` per the loop.
 
 <!-- replaced-by-skill: code-review:singleton-dispatch — DO NOT add inline singleton-dispatch content here -->
 

plugins/code-review/skills/singleton-dispatch/SKILL.md

@@ -77,7 +77,7 @@ Spawn one synchronous `Task` (do **not** set `run_in_background: true`). Unlike
    | `{INPUT_PATH}` | `manifest.input_path` |
    | `{OUTPUT_PATH}` | The by-convention agent write target from the table above — `<CR_DIR>/pln725_extract_signals.json` (stage_11) or `<CR_DIR>/pln725_coverage_critic.json` (stage_15). **NOT** `manifest.output_path`. |
    | `{STAGE_LABEL}` | `"signal-extraction"` for stage_11, `"coverage-critic"` for stage_15. |
-4. After the Task returns, advance the walker to the sibling consolidate stage. No `TaskOutput` call — that's for background tasks; synchronous Tasks complete before control returns to the walker.
+4. After the Task returns, hand control back to the caller. **In the default `run-prefix` flow** (the caller is `start.md`'s Deterministic Prefix loop): do NOT advance the walker — return, and `start.md` re-invokes `run-prefix --resume-from <resume_stage>`, which runs the sibling consolidate stage. **In the per-stage fallback walk** (the caller is walking stages one at a time): advance the walker to the sibling consolidate stage yourself. Either way, no `TaskOutput` call — that's for background tasks; synchronous Tasks complete before control returns.
 
 ### Failure semantics