Skip to content

Commit 7ad4427

Browse files
wongkclaude
andcommitted
feat(code-review): parallelize GitHub-mode reviewer/verifier fleets
GitHub/headless reviews dispatched the reviewer fleet (stage_20) and verifier fleet (stage_23) synchronously one agent at a time, so CI wall-clock was the SUM of every agent — a shallow review with 3 serial reviewers + 2 serial verifiers took ~30 min. The synchronous rule existed because headless `claude -p` used to terminate before background subagents finished. As of Claude Code v2.1.182+ (shipped by claude-code-action@v1), `claude -p` waits for background subagents to complete because their output is part of the final result, so a blocking TaskOutput keeps the turn alive until reviewers finish. Unify both modes on the local parallel pattern: spawn every standard-flow reviewer/verifier with run_in_background: true and collect all with blocking TaskOutput before advancing. Fleet stages now take the SLOWEST agent, not the sum — roughly halving shallow-tier CI review time. A synchronous one-at-a-time fallback stays documented for Claude Code < v2.1.182, and CLAUDE_CODE_PRINT_BG_WAIT_CEILING_MS can raise the 10-min per-agent wait cap for long reviewers. Updates spawn-reviewers + verify-findings skills and start.md; replaces the FEA-2162 synchronous-GitHub test class with the parallel-dispatch contract. Full code-review suite green (1308 passed), ruff + pyright clean. Bump 3.4.1 -> 3.5.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7b4bb42 commit 7ad4427

6 files changed

Lines changed: 144 additions & 173 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ All notable changes to the claude-plugins project will be documented in this fil
44

55
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`.
66

7+
### code-review v3.5.0
8+
9+
#### Changed
10+
- GitHub-mode reviews now dispatch the reviewer fleet (`stage_20`) and verifier fleet (`stage_23`) as parallel background tasks collected with a blocking `TaskOutput`, the same as local mode, instead of running each agent synchronously one at a time. Headless `claude -p` waits for background subagents to finish before exiting (Claude Code v2.1.182+), so a code-review CI run's fleet stages now take the wall-clock of the slowest agent rather than the sum of all agents — roughly halving shallow-tier CI review time. A synchronous one-at-a-time fallback stays documented for Claude Code older than v2.1.182, the per-agent background wait can be raised past its 10-minute default via `CLAUDE_CODE_PRINT_BG_WAIT_CEILING_MS`, and runs that background reviewers must allow the `TaskOutput` tool. Updates the `spawn-reviewers` and `verify-findings` skills and the `start.md` walker contract.
11+
712
### code-review v3.4.1
813

914
#### Fixed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "code-review",
33
"description": "Code review plugin",
4-
"version": "3.4.1",
4+
"version": "3.5.0",
55
"author": {
66
"name": "ClosedLoop",
77
"email": "support@closedloop.ai"

plugins/code-review/commands/start.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ These notes annotate the run-plan stages with anything not obvious from the plan
390390
- **stage_17_partition**: positioned in the run plan array after `stage_19_cache_check` so Gate B's `route` invocation runs first and supplies `--max-bha-agents`. The stage id retains its `_17_` prefix as a stable label (stage ids are not strict ordinals; execution order follows array position). Reads `partitions.json` afterward; entries shape `{id, files, total_loc, is_test_only}` with `files[].file` (NOT `path`), `files[].loc`, `files[].is_test`, optional `files[].line_range`. **PLN-774**: top-level keys also carry `partition_mode` (`"unified"` | `"partitioned"`), `partition_count`, `total_changed_loc`, and `unified_threshold_loc`. When total changed LOC ≤ `BHA_UNIFIED_THRESHOLD_LOC` (default 5000, settable via `.closedloop-ai/settings/code-review.json:bha_unified_threshold_loc`; `0` = always partition), the partitioner emits a single unified partition holding every file so cross-region invariants stay visible to one BHA reviewer's context. `cmd_verify_prepare` propagates `partition_mode` + `partition_count` into `verify_manifest.json` for the presenter footer. The `stats.verification.by_reviewer` block naturally labels BHA by partition via the filename-derived `reviewer` field (`agent_bha_p0.json` → `reviewer='bha_p0'`) — no extra split logic is needed; under unified mode only a single `bha_p0` bucket exists because there is only one partition.
391391
- **stage_19_cache_check**: writes `<CR_DIR>/cache_result.json` (stats), `<CR_DIR>/agent_cached_bha.json` (cached BHA findings, glob-compatible with `agent_*`), `<CR_DIR>/uncached_diff_data.json` (filtered diff_data for uncached files). Do NOT print the cache status here — it is printed in Gate A (hygiene exit) or Gate B (after route).
392392
- **stage_19b_derive_spawn_spec** (PLN-725): runs `derive-spawn-spec`. Reads `<CR_DIR>/coverage.json` (`final` section — post-arbitrate), `<CR_DIR>/partitions.json`, and `<CR_DIR>/spawn.json` (`route` section, written by Gate B's `cmd_route --cr-dir`) and writes `<CR_DIR>/spawn.json` (`spec` section) — a flat list of agent descriptors keyed by `agent_id` (e.g. `bha_p0`, `bhb`, `auditor`, `domain_0`, `fast`) carrying `reviewer`, `model`, `partitioned`, `patches_file`, `source`, `bucket`, and (for BHA) `partition_id` + `is_test_only`. The fast-path branch from Gate B is honored (`fast_path: true` → single `fast` agent, bucket walk skipped). BHA descriptors are capped at `coverage_plan.budget.bha_partitions` (the post-arbitrate cap, which may be < the partitioner's output count); the excess partitions land in `skipped[]` with `reason: "budget_capped"`. A BLOCKING verify verdict (`budget.gated_by_verify: true`) drives **plan sanitization**: only `source: "core"` reviewers survive; every `rule` or `critic` entry is moved to `skipped[]` with `reason: "gated_by_verify"` (the canonical BLOCKING finding from stage_15c remains the operator-facing signal). Required-bucket skips with non-benign reasons (everything except `deferred_pln723`, `no_partitions`, `gated_by_verify`) generate coverage-gap findings appended to `<CR_DIR>/coverage_gaps.json` so finalize-result picks them up — the spec-driven dispatch never silently drops a required reviewer. `on_failure: continue` — a derive failure writes a sentinel spec with `arbitrate_status: "fallback"` (`fallback_reason` ∈ {`coverage_plan_missing_or_malformed`, `partitions_missing_or_malformed`}), which the stage_20 orchestrator interprets as "ignore the spec, use the static reviewer table fallback in the `code-review:spawn-reviewers` skill." Note: stage_19b depends only on `stage_16_arbitrate_budget`, NOT on `stage_17_partition`, so Gate B's fast-path branch (which skips stage_17) can still reach stage_20 with a fast descriptor.
393-
- **stage_20_spawn_reviewers**: agent_fleet stage. Invoke the `code-review:spawn-reviewers` skill. The skill reads `<CR_DIR>/spawn.json` (`spec` section) first and dispatches one Task per agent descriptor (using the `agent_id`, `reviewer`, `model`, and `patches_file` from the spec). If `spawn.json` is missing, its `spec` section is absent, or it marks `arbitrate_status: "fallback"`, the skill walks its static reviewer table fallback instead — a derive failure must never block review. In `MODE=github`, the walker must follow the skill's synchronous standard-flow branch: do not use `TaskOutput`, watcher files, sleep loops, polling loops, or turn-ending waits as replacements for synchronous reviewer completion. `stage_20` must complete every GitHub synchronous reviewer and retry, leaving no reviewer task still running, or fail before `stage_21_collect_findings`.
393+
- **stage_20_spawn_reviewers**: agent_fleet stage. Invoke the `code-review:spawn-reviewers` skill. The skill reads `<CR_DIR>/spawn.json` (`spec` section) first and dispatches one Task per agent descriptor (using the `agent_id`, `reviewer`, `model`, and `patches_file` from the spec). If `spawn.json` is missing, its `spec` section is absent, or it marks `arbitrate_status: "fallback"`, the skill walks its static reviewer table fallback instead — a derive failure must never block review. In **both** `MODE=local` and `MODE=github` the skill dispatches the fleet as parallel background tasks and collects every one with a blocking `TaskOutput` (headless `claude -p` awaits background subagents on Claude Code v2.1.182+). `stage_20` must collect every reviewer and its retries — leaving no reviewer task still runningbefore proceeding to `stage_21_collect_findings`; watcher files, sleep loops, polling loops, and turn-ending "wait" turns are forbidden substitutes for the blocking `TaskOutput` collection.
394394
- **stage_20b_verify_spawn** (PLN-725): runs `verify-spawn`. Reads `<CR_DIR>/spawn.json` (`spec` section) and globs `<CR_DIR>/agent_*.json`; for every descriptor with `bucket: "required"` that has no on-disk output, appends a coverage-gap finding to `<CR_DIR>/coverage_gaps.json` (reason `spawn_missing_required_agent`) and records the omission in `<CR_DIR>/spawn.json` (`verification` section). Missing best-effort descriptors are recorded for telemetry but emit no finding — best-effort omissions are budget-driven, not coverage gaps. No-ops cleanly when the spec is missing (`spec_missing`), marks fallback (`spec_fallback`), or contains no agents (`spec_empty`). `on_failure: continue` — a verification bug must never block review; worst case is missing telemetry, not a halted pipeline. Wired before `stage_21_collect_findings` so the gap findings land in `coverage_gaps.json` in time for `cmd_finalize_result` to merge them into the canonical envelope.
395395
- **stage_22_validate**: writes `<CR_DIR>/findings_validated.json` via `> <CR_DIR>/findings_validated.json` redirection. Validates finding scope and applies the out-of-hunk confidence gate. P2+ findings whose `line` falls outside the file's changed range survive when `confidence > out_of_hunk_confidence_floor` (default `0.80`, operator-tunable via `.closedloop-ai/settings/code-review.json:out_of_hunk_confidence_floor`, range `[0.0, 1.0]`) — this admits legitimate companion-change findings (e.g. a signature change in the diff window leaving stale sibling call sites just outside it) while still filtering low-confidence noise. Survivors get tagged `out_of_hunk_kept: true` so presenters can label them as companion-change without re-deriving hunk membership; the validate-stats block exposes `kept_out_of_hunk` and `discarded_out_of_hunk_low_confidence`. The comparison is strict `>`, so setting the floor to `1.0` is a kill switch (nothing can clear); setting it to `0.0` lets every out-of-hunk P2+ through (lean on the PLN-722 verifier downstream). Per-finding verification (stage_23) still applies on top, so noise that surfaces here gets a second-pass CONFIRMED/REJECTED verdict.
396396
- **stage_22b_verify_prepare** (PLN-722): tier-selects findings for verification per the canonical table — BLOCKING/HIGH always; MEDIUM with confidence < 0.85 yes; MEDIUM with confidence ≥ 0.85 no; LOW (P3) no; `category: "Hygiene"` no; `source: "injection-detector"` no. Ranks the eligible set by `severity_weight × confidence`, caps at `VERIFY_MAX_VERIFICATIONS = 50`, and writes (a) `<CR_DIR>/verify_manifest.json` with `to_verify[]` + `skipped_no_verification[]` + `deferred_budget[]` + `cache_hits[]`, and (b) `<CR_DIR>/verifier_inputs/<finding_id>.json` per eligible finding. When `--cache-dir` is set, fresh verifier outputs from a prior run for the same `(finding_id, code_snippet_hash, model, prompt_hash)` tuple are pre-materialized at `agent_verifier_<finding_id>.json` and skipped from `to_verify[]` (logged under `cache_hits[]`). `on_failure: continue` is intentional — verify-prepare failure degrades to "no verifier this run", not a pipeline abort.
@@ -407,9 +407,9 @@ These notes annotate the run-plan stages with anything not obvious from the plan
407407

408408
When the walker reaches `stage_20_spawn_reviewers`, invoke the `code-review:spawn-reviewers` skill. The skill owns the full reviewer-fleet dispatch: spawn-spec consumption (`spawn.json.spec`, the authoritative path), GRAPH_PROJECT resolution, the per-agent prompt template and role suffixes (Bug Hunter A/B, Unified Auditor, Domain Critics, Design Critic, Impact Analyzer), the context-budget constraints, the standard / fast-path / all-cached-BHA / gated-by-verify branches, the static-table fallback (`arbitrate_status: "fallback"`), the spawn + collection contract, and agent-failure recovery.
409409

410-
The skill is invoked for both `MODE=local` and `MODE=github`, but standard-flow Task scheduling is mode-specific: GitHub mode dispatches reviewers synchronously, while local mode preserves parallel background dispatch plus blocking collection. The verifier fleet (`stage_23`) and the PLN-725 single-agent dispatch (`stage_11` / `stage_15`) are **not** in this skill; they are owned by the `code-review:verify-findings` and `code-review:singleton-dispatch` skills respectively.
410+
The skill is invoked for both `MODE=local` and `MODE=github` with the **same** parallel Task scheduling: spawn every standard-flow reviewer as a background task and collect them all with blocking `TaskOutput`, so `stage_20` wall-clock is the slowest reviewer rather than the sum. The verifier fleet (`stage_23`) and the PLN-725 single-agent dispatch (`stage_11` / `stage_15`) are **not** in this skill; they are owned by the `code-review:verify-findings` and `code-review:singleton-dispatch` skills respectively.
411411

412-
GitHub headless mode has a walker-level guard in addition to the skill contract: standard-flow reviewers and retries must be dispatched synchronously, and the walker must not proceed to `stage_21_collect_findings`, emit a final summary, mark the review complete, or end the assistant turn while any GitHub reviewer remains outstanding. Watcher files, sleep loops, polling loops, background `TaskOutput` waits, and "I'll continue when notified" turns are forbidden substitutes for completing the synchronous reviewer response.
412+
GitHub headless mode carries one walker-level guard: the turn stays alive only while a pending synchronous tool call is outstanding, so the blocking `TaskOutput` collection is what keeps `claude -p` running until reviewers finish (headless awaits background subagents on Claude Code v2.1.182+). The walker must not proceed to `stage_21_collect_findings`, emit a final summary, mark the review complete, or end the assistant turn while any reviewer remains outstanding. Watcher files, sleep loops, polling loops, and "I'll continue when notified" turns are forbidden — they end the turn with no pending call and the process exits before the tail stages run. On Claude Code < v2.1.182, background subagents are not awaited; use the skill's legacy synchronous fallback there.
413413

414414
Decomposition rationale: ~470 lines of reviewer-fleet dispatch content was extracted as a skill so the orchestration spine stays lean and the content no longer loads into orchestrator context during the deterministic prefix (stages 0-19) or on hygiene-only / full-cache-hit runs that never reach `stage_20`.
415415

0 commit comments

Comments
 (0)