Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ 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.7.0

#### Changed
- **GitHub-mode verifier fleet (`stage_23_verify_findings`) now dispatches synchronously, and a missing BLOCKING/HIGH verifier output raises a loud coverage-gap signal (FEA-3154).** Applies the v3.6.0 reviewer hardening to the verifier fleet. Previously `stage_23` (skill `code-review:verify-findings`) spawned every verifier with `run_in_background: true` + blocking `TaskOutput` in **both** modes — the same pattern v3.6.0 proved unsafe for the reviewer fleet, where a headless `claude -p` turn can end before the blocking collection is issued, so the process exits and verifier verdicts never land. That failure is worse for verifiers than for reviewers: `stage_23` is `on_failure: continue`, so a missing output degrades silently to `pending_verification[]`, which `_compute_canonical_verdict` does not read — a BLOCKING finding whose verifier died could ship `APPROVED`. GitHub mode now dispatches verifiers **synchronously** one-at-a-time (`run_in_background: false`, wait for each Task response before the next), which keeps the headless turn alive by construction; local mode keeps parallel background dispatch + blocking `TaskOutput`. A matching walker-level guard in `commands/start.md` bans watcher files / sleep loops / polling loops / turn-ending waits at `stage_23`. As defense-in-depth (a genuine verifier crash still degrades silently), `cmd_verify_consolidate` now emits — in GitHub mode only, when one or more BLOCKING/HIGH findings had no verifier output — one aggregate `coverage:verifier-missing-output` coverage-gap finding (`severity: HIGH`, `required: false`) that escalates the canonical verdict to `NEEDS_ATTENTION` (mirroring the existing "verifier uncertain → human triages, not silent approval" semantics), so an unverified high-severity finding cannot pass silently. Threaded via a new optional `--mode` on the `verify-consolidate` subcommand (wired at `stage_24a_verify_consolidate`, the same `{mode}` token the adjacent `finalize-result` already uses). Net-new github-synchronous verifier behavior — not a revert (pre-v3.5.0 verifiers were background in both modes). Adds the `TestFEA3154VerifierDispatchContracts` prose-contract class plus behavioral `cmd_verify_consolidate`/`cmd_finalize_result` tests.

### code-review v3.6.1

#### Fixed
Expand Down
2 changes: 1 addition & 1 deletion plugins/code-review/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "code-review",
"description": "Code review plugin",
"version": "3.6.1",
"version": "3.7.0",
"author": {
"name": "ClosedLoop",
"email": "support@closedloop.ai"
Expand Down
6 changes: 5 additions & 1 deletion plugins/code-review/commands/start.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ These notes annotate the run-plan stages with anything not obvious from the plan
- **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.
- **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.
- **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.
- **stage_23_verify_findings** (PLN-722): agent_fleet stage. Invoke the `code-review:verify-findings` skill. Each spawned agent reads its `verifier_inputs/<finding_id>.json` (containing the finding + the `verifier_prompt_path` + the canonical `output_path`) and emits one verdict file at `<CR_DIR>/agent_verifier_<finding_id>.json`. `on_failure: continue` so a single agent crash never aborts review.
- **stage_23_verify_findings** (PLN-722): agent_fleet stage. Invoke the `code-review:verify-findings` skill. Each spawned agent reads its `verifier_inputs/<finding_id>.json` (containing the finding + the `verifier_prompt_path` + the canonical `output_path`) and emits one verdict file at `<CR_DIR>/agent_verifier_<finding_id>.json`. `on_failure: continue` so a single agent crash never aborts review. In `MODE=github`, the walker must follow the skill's synchronous verifier branch: do not use `TaskOutput`, watcher files, sleep loops, polling loops, or turn-ending waits as replacements for synchronous verifier completion. `stage_23` must complete every GitHub synchronous verifier, leaving no verifier task still running, or fail before `stage_24a_verify_consolidate`.
- **stage_24a_verify_consolidate** (PLN-722, extended in PLN-721): merges all `agent_verifier_*.json` outputs back into the validated set, applies sensitive-path escalation from `.closedloop-ai/settings/verification-gates.json` (rules: REJECTED on `sensitive_paths` + BLOCKING/HIGH → TENTATIVE with severity capped at HIGH; any finding on `tentative_on_paths` → TENTATIVE; any finding on `mandatory_human_review_paths` → TENTATIVE + `force_human_review: true`), routes JUSTIFIED-VALID verdicts to a new `justified[]` bucket and JUSTIFIED-INVALID verdicts back into `verified[]` (the audited justification was refuted; the original concern stands), and writes `<CR_DIR>/findings_verified.json` with the bucket-split shape `{verified[], rejected[], pending_verification[], justified[], force_human_review}`. `tentative_on_paths` lifts JUSTIFIED-VALID/INVALID to TENTATIVE on the same operator-policy contract as the other verdicts. When `--cache-dir` is set, fresh verifier outputs are written back to the `verifications/` namespace (30-day TTL) for re-use on subsequent runs. Missing fleet outputs degrade to `pending_verification[]`; `on_failure: continue`.
- **stage_25_finalize_result** (PLN-722 + PLN-721): writes `<CR_DIR>/review_result.json` (the canonical envelope) BEFORE running schema validation. PLN-722: prefers `<CR_DIR>/findings_verified.json` (verify-consolidate output) when present and honors its `force_human_review` flag in the verdict computation; falls back to `findings_validated.json` (everything to `verified[]`) when verify-consolidate didn't run. PLN-721: pipes the consolidate `justified[]` bucket into the envelope, and loads operator-overridable thresholds from `.closedloop-ai/settings/verdict-thresholds.json` (defaults to `impact_cumulative=2`; absent/malformed → built-in default) so `_compute_canonical_verdict`'s cumulative Impact gate (FEA-1401 / PLN-726 OQ#6) can fire (≥ 2 BLOCKING/HIGH `ImpactAnalysis` findings in `verified[]` → NEEDS_ATTENTION). A non-zero exit signals reviewer-emitted category/field drift (e.g. a category not in the canonical enum) but does not block the pipeline — `on_failure: continue` lets `stage_28_verdict` read the structurally complete envelope. Surface the stderr text in the present step so operators can correct prompts/schema; do not abort.
- **stage_26_cache_update**: gated by **Gate C**.
Expand All @@ -421,6 +421,10 @@ Decomposition rationale: ~470 lines of reviewer-fleet dispatch content was extra

When the walker reaches `stage_23_verify_findings`, invoke the `code-review:verify-findings` skill. The skill owns the full finding-verifier dispatch: reading `verify_manifest.json`, spawning one falsify-oriented verifier Task per `to_verify[]` entry (skipping `cache_hits[]`), the no-retry collection contract, and the `pending_verification[]` degradation when a verifier output is missing.

The skill is invoked for both `MODE=local` and `MODE=github`, but Task scheduling is mode-specific: GitHub mode dispatches verifiers synchronously, while local mode preserves parallel background dispatch plus blocking collection. In `MODE=github` a missing verifier output for a BLOCKING/HIGH finding also raises a durable coverage-gap signal at `stage_24a_verify_consolidate` so an unverified high-severity finding cannot pass silently to an approved verdict.

GitHub headless mode has a walker-level guard in addition to the skill contract: verifiers and retries must be dispatched synchronously, and the walker must not proceed to `stage_24a_verify_consolidate`, emit a final summary, mark the review complete, or end the assistant turn while any GitHub verifier 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 verifier response.

<!-- replaced-by-skill: code-review:verify-findings — DO NOT add inline verifier-fleet dispatch content here -->

---
Expand Down
Loading
Loading