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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ 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.8.1

#### Fixed
- `/code-review --github <PR>` run outside GitHub Actions no longer reviews whatever tree happens to be checked out. `cmd_resolve_scope` skipped every PR-head check when the mode was github, but `--github` selects file-based handoff output and is a supported invocation from a developer machine — so such a run left `review_root` empty and every reviewer read the operator's branch while holding a diff computed from `origin/<base>...origin/<head>`. Worst on the deep tier, whose cross-file reviewers grep for callsites the wrong tree does not have, and silent in both directions: nothing in the output said which tree was read. GitHub mode now VERIFIES the tree it was handed and REFUSES with a non-zero exit when it cannot establish it as the PR's source. Two checkout shapes are accepted, because both occur in GitHub Actions: HEAD is the PR head (`actions/checkout` with an explicit `ref: <head sha>`), or HEAD is the PR's merge ref `refs/pull/N/merge` — the DEFAULT checkout for a `pull_request` event. Either way the tree must have no uncommitted changes to tracked files. The check costs at most five local git calls and changes nothing else: github mode still creates no worktree, still emits an empty `review_root`, and still emits an empty `head_sha`, so no consumer of those fields moves. Local mode is untouched — it continues to isolate the PR head into a detached worktree, which github mode deliberately does not do because `review_root` redirects a whole agent fleet and `github-review.md`'s own steps do not resolve it.
- GitHub mode refuses when the PR head did not come from the PR's own metadata. When `gh pr view` fails or returns no head branch, `_resolve_pr_scope` falls back to guessing the head from the checked-out branch, and a clean checkout of that branch would pass any tree check and be reviewed against the requested PR's diff. `_resolve_pr_scope` now reports `head_ref_from_metadata`, and `cmd_resolve_scope` refuses in github mode before verifying the checkout whenever it is false. This also covers a CI runner on a detached HEAD, where the guess degrades to `origin/HEAD`.
- GitHub mode accepts a merge checkout only when it is shaped like the PR's merge ref. Checking that HEAD's second parent is the PR head was not enough: a merge of the head into an unrelated branch, or an octopus merge that folds a third branch in, passed it and put code the PR's diff does not contain in front of reviewers. `_head_is_pr_merge` now requires exactly two parents, the second being the PR head and the first being on the PR's base branch (an ancestor of `origin/<baseRefName>`, taken from the PR metadata rather than a `--base` override). Ancestry rather than equality keeps a genuine merge ref accepted after the base branch advances past the commit GitHub merged into. Any git error during these checks refuses.

### code-review v3.8.0

#### Changed
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.8.0",
"version": "3.8.1",
"author": {
"name": "ClosedLoop",
"email": "support@closedloop.ai"
Expand Down
2 changes: 1 addition & 1 deletion plugins/code-review/commands/start.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ These notes annotate the run-plan stages with anything not obvious from the plan

- **stage_01_setup**: already executed in stage 0b (which captured stdout and wrote `setup.json` itself). The walker treats this as a no-op; the run plan's `stdout` field is `None` for this stage because no shell redirect is correct here.
- **stage_02_prep_assets**: copies `shared_prompt.txt` and `bha_suffix.txt` from `<PLUGIN_ROOT>/tools/prompts/` to `<CR_DIR>`. Both cache and non-cache paths use these assets.
- **stage_03_resolve_scope**: writes `<CR_DIR>/scope.json` with `diff_scope`, `base_ref`, `head_ref`, `review_branch`, `diff_tip`, `pr_number`, `path_filter`, `scope_kind`, `pr_auto_detected`, `head_sha`, `review_root`, `worktree_path`. After this stage, run `finalize-cache` to populate `<CR_DIR>/cache_config.json`. The walker uses these for token resolution downstream. **PR-head worktree (local PR review):** for `MODE=local` PR reviews (`scope_kind == "pr"`, not hygiene-only) resolve-scope isolates source reads so reviewer/verifier agents read the PR head, not the operator's working tree. The diff is computed from the fetched remote refs (`origin/<base>...origin/<head>`); reading the working tree is safe **only** when it already IS the PR head with a clean tree. Otherwise resolve-scope materializes a detached git worktree at the PR head SHA under `<CR_DIR>/pr_head_worktree` and records its absolute path in `review_root`/`worktree_path`; reviewer prompts and verifier inputs then read **source** under `<REVIEW_ROOT>`. **Fail-closed:** if isolation is required but cannot be established (PR head unresolvable, or `git worktree add` fails), resolve-scope returns non-zero and the run aborts (`on_failure: abort`) rather than silently review the wrong branch — the operator is told to check out the PR branch or fix the git error. `review_root` is empty (agents read the working tree) only for the already-at-head-and-clean case, staged/file/branch scope, hygiene-only, and GitHub mode (where the runner already checks out the head). **Worktree lifecycle:** resolve-scope runs a startup GC (`_gc_stale_pr_head_worktrees`) that reclaims orphaned `cr-*/pr_head_worktree` checkouts from prior runs that aborted before teardown; `stage_30_footer` tears down the current run's worktree (validating the path equals the canonical `<CR_DIR>/pr_head_worktree` before the destructive removal). Because the walker can abort before the footer, the next run's startup GC is the backstop — a leaked worktree is never silently reused. Code-intelligence-aware reviewers run grep-only (`CODE_INTEL_ALLOWED=false`) whenever `review_root` is set, since any external index covers the operator checkout, not the PR head.
- **stage_03_resolve_scope**: writes `<CR_DIR>/scope.json` with `diff_scope`, `base_ref`, `head_ref`, `review_branch`, `diff_tip`, `pr_number`, `path_filter`, `scope_kind`, `pr_auto_detected`, `head_sha`, `review_root`, `worktree_path`. After this stage, run `finalize-cache` to populate `<CR_DIR>/cache_config.json`. The walker uses these for token resolution downstream. **PR-head worktree (local PR review):** for `MODE=local` PR reviews (`scope_kind == "pr"`, not hygiene-only) resolve-scope isolates source reads so reviewer/verifier agents read the PR head, not the operator's working tree. The diff is computed from the fetched remote refs (`origin/<base>...origin/<head>`); reading the working tree is safe **only** when it already IS the PR head with a clean tree. Otherwise resolve-scope materializes a detached git worktree at the PR head SHA under `<CR_DIR>/pr_head_worktree` and records its absolute path in `review_root`/`worktree_path`; reviewer prompts and verifier inputs then read **source** under `<REVIEW_ROOT>`. **Fail-closed:** if isolation is required but cannot be established (PR head unresolvable, or `git worktree add` fails), resolve-scope returns non-zero and the run aborts (`on_failure: abort`) rather than silently review the wrong branch — the operator is told to check out the PR branch or fix the git error. `review_root` is empty (agents read the working tree) only for the already-at-head-and-clean case, staged/file/branch scope, hygiene-only, and GitHub mode. **GitHub mode verifies instead of isolating (ISS-8769):** for `MODE=github` PR reviews (`scope_kind == "pr"`, not hygiene-only) resolve-scope first requires the PR head to come from the PR's own metadata — when `gh pr view` fails, the only head left is a guess (the checked-out branch), and a tree matching a guess proves nothing, so it refuses — then checks that the checked-out tree actually holds that head: either HEAD is that commit, or HEAD is the PR's merge ref (`refs/pull/N/merge`, the default `actions/checkout` for a `pull_request` event), a merge with exactly two parents whose second parent is that commit and whose first parent is on the PR's base branch (an ancestor of `origin/<baseRefName>`, which tolerates the base advancing after GitHub computed the merge) — with no uncommitted changes to tracked files. Both runner shapes pass and cost at most five local git calls; nothing else changes, and `review_root`/`head_sha` stay empty so no downstream consumer moves. Anything else **refuses** with a non-zero exit. `--github` is a supported invocation from a developer machine (it selects file-based handoff output; it does not declare "I am inside GitHub Actions"), and before this check such a run silently reviewed the operator's branch against the PR's diff. It refuses rather than isolating because `review_root` redirects a whole agent fleet and `github-review.md`'s own steps do not resolve it; the remedy is to check out the PR branch, or drop `--github` and use the local flow, which isolates for you. **Worktree lifecycle:** resolve-scope runs a startup GC (`_gc_stale_pr_head_worktrees`) that reclaims orphaned `cr-*/pr_head_worktree` checkouts from prior runs that aborted before teardown; `stage_30_footer` tears down the current run's worktree (validating the path equals the canonical `<CR_DIR>/pr_head_worktree` before the destructive removal). Because the walker can abort before the footer, the next run's startup GC is the backstop — a leaked worktree is never silently reused. Code-intelligence-aware reviewers run grep-only (`CODE_INTEL_ALLOWED=false`) whenever `review_root` is set, since any external index covers the operator checkout, not the PR head.
- **stage_07_auto_incremental**: runs **before** `stage_05_parse_diff` (its array position is between `stage_04_finalize_cache` and `stage_05_parse_diff`). This ordering matters: any `diff_scope` override must be applied to the cached `<DIFF_SCOPE>` token BEFORE parse-diff and extract-patches materialize `diff_data.json` and `patches_all.txt`, otherwise downstream stages see full-PR diff data alongside a narrowed token. The stage retains its `_07_` id as a stable label; execution order follows array position. Writes `<CR_DIR>/auto_incremental.json` with optional `diff_scope` (override) and `review_mode_line`. If `diff_scope` is non-null, update the cached `<DIFF_SCOPE>` token. Print `review_mode_line` (always) and, if `pr_auto_detected` was true in `scope.json`, print `"Auto-detected PR #<PR_NUMBER> for branch <REVIEW_BRANCH>."`.
- **stage_08_fetch_intent**: the helper writes `intent_context.json` into `cr_dir` itself; its stdout is a small `{path, source}` summary that the walker discards. The run plan's `stdout` field is `None` here because redirecting stdout to `intent_context.json` would corrupt the file by overwriting the helper's structured payload with the summary.
- **stage_09_detect_injection** (PLN-720): scores PR title/body/commits against the canonical 9-pattern catalogue and writes `<CR_DIR>/injection_report.json`. On severity ≥ Medium (score ≥ 30), rewrites `<CR_DIR>/intent_context.json` in place with `quarantine: true` and redacted fields. On severity ≥ High (score ≥ 70), also writes `<CR_DIR>/agent_injection-detector.json` containing a canonical `InjectionAttempt` finding — the `agent_*.json` naming makes `cmd_collect_findings` pick it up via the standard glob with no extra wiring. Always appends one JSONL entry to `.closedloop-ai/injection-log.jsonl` (90-day TTL, swept on read). `on_failure: continue` is intentional — a detector crash must never abort the pipeline.
Expand Down
Loading
Loading