change-review

Change review

Produce a focused review of a changeset against a fixed checklist, dispatching the deep passes to /code-review (correctness), /security-review (security), and /ponytail-review (over-engineering). Read-only by default — modify or comment only when --fix/--comment is passed.

0. Parse flags

From $ARGUMENTS, separate the target from the flags:

• --fix — after reviewing, apply high-confidence, mechanically-fixable findings (lint/style on new files, stale-doc edits, and correctness via /code-review --fix). Never auto-fix lower-confidence findings — surface them as suggestions.
• --comment — post all findings (every checklist point + both hand-offs) as comments, each with its confidence score.
• --effort low|medium|high|max — forwarded to /code-review (and used to scale the security pass). Defaults to the reviewer's own default tier when omitted.
• --ci — CI/machine mode: emit findings as change-review-findings.json (schema in §6) instead of a prose report, and stay strictly read-only for side effects — apply any --fix edits to the working tree only, and never commit, push, or comment (a separate privileged CI job performs those). Without --ci, output is the normal prose report.
• No flags → strictly read-only report.

1. Resolve the review target

Pick the target from the non-flag arguments:

• No argument → review the local uncommitted changes:
  • Tracked, staged + unstaged: git diff HEAD
  • Untracked new files: git ls-files --others --exclude-standard (then Read each)
  • Scope: git status --short
  • If the working tree is clean, fall back to this branch vs base: git merge-base --fork-point origin/HEAD HEAD (or main/master), then git diff <base>...HEAD. Tell the user that's what you reviewed.
  • The deep hand-offs (section 4) run directly against this working diff.
• A PR number (123) or PR URL → review that GitHub PR:
  • Title/body/branch: gh pr view <n> --json title,body,headRefName,baseRefName,files
  • Diff: gh pr diff <n> (--patch for full hunks)
  • To run the deep hand-offs against a PR, materialize it as a working diff in a throwaway worktree so the user's workspace is untouched:

    WT=$(mktemp -d)
    git worktree add --detach "$WT"
    BASE=$(gh pr view <n> --json baseRefName -q .baseRefName)
    git -C "$WT" fetch origin "$BASE"          # ensure origin/$BASE exists for the reset below
    git -C "$WT" fetch origin "pull/<n>/head"  # fetch PR head last → FETCH_HEAD points to it
    git -C "$WT" checkout FETCH_HEAD
    git -C "$WT" reset --soft "origin/$BASE"   # PR commits now appear as a working diff
    

    Run /code-review and /security-review from inside $WT, then git worktree remove --force "$WT". If the worktree cannot be created (no PR fetch), do the checklist on gh pr diff and note that the deep passes were skipped (no working tree).

State at the top of your report exactly what was reviewed (e.g. "uncommitted working-tree changes: 9 files" or "PR #123: feat/foo → main").

Ignore mechanical churn when summarizing and hunting for issues: lockfiles (package-lock.json, yarn.lock, poetry.lock, Cargo.lock), build output (dist/, build/, *.min.*), vendored/node_modules dirs, and generated files (snapshots, codegen, *.generated.*). Note their presence in one line, don't review their contents.

If the target is empty (no diff, no PR), say so and stop.

2. Detect repo conventions and stated rules

Before reviewing, learn how this repo lints, formats, documents, and what rules it states — never assume a stack.

• Lint/format tooling (in order of authority): package.json scripts (lint, format, typecheck), .eslintrc*/eslint.config.*, .prettierrc*, pyproject.toml/setup.cfg (ruff, black, flake8, isort), .rubocop.yml, go.mod (gofmt/golangci-lint), Cargo.toml (rustfmt/clippy), .editorconfig, Makefile targets.
• Stated rules (unified guidance check) — treat all of these as one body of rules, and a violation of any as a finding: CLAUDE.md/AGENTS.md/CONTRIBUTING* and docs/ style notes, and inline code comments that state guidance in or adjacent to the changed code (e.g. "// do not call this in a loop", "# keep in sync with X"). Note: CLAUDE.md is guidance written for an agent writing code, so not every instruction applies to review — judge relevance.
• Docs surface: README*, docs/, AGENTS.md, CHANGELOG*, in-repo *.md, and doc comments adjacent to changed code.

3. Gather historical and prior-review context

These lenses feed points (2) and (6) below — surface what they reveal as findings:

• Git history / blame of the modified code: git log -n 20 --oneline -- <files> and git blame on changed hunks. Flag changes that contradict why the code was last touched, or that reintroduce a previously-fixed bug.
• Prior-PR-comment continuity: for the changed files, look at recent merged PRs that touched them (gh pr list --state merged --search "<path>", then gh pr view <n> --comments) and flag any earlier review comment that still applies to this change.

4. Run the checklist

Cover all seven points. Be specific: cite file:line and quote the relevant code. Attach a 0–100 confidence score to every finding (see section 5). Prefer high-signal findings over volume; if a point has nothing to report, say "No issues" rather than padding.

Finding IDs: maintain a single counter starting at 1 for the entire report. Every finding — across all seven checklist points, including those folded in from /code-review, /security-review, and /ponytail-review — gets the next ID as [CR-NNN] (zero-padded to 3 digits). IDs are assigned in the order findings appear in the report (section 1 → section 7). They reset each run; the output file (section 7) is the stable reference.

(1) Summary of the changes. A short, faithful description grouped by theme/area — not a file-by-file restatement. Lead with the primary intent.

(2) Outlying changes. Anything a reader would not predict from (1): unrelated edits, drive-by refactors, changed behavior in untouched-seeming areas, dependency bumps, config/flag flips, formatting churn mixed with logic, generated files, deletions. List each with file:line and one line on why it's surprising. Incorporate anything the history/blame lens (section 3) surfaced.

(3) Architecture / security / structure risks. Call out anything forcing — or smuggling in — a substantial change to architecture, security posture, or file structure: new cross-layer dependencies/layering violations, new external surface (endpoints, public APIs), auth/permission/RBAC changes, secrets/credentials, injection/SSRF/path-traversal/deserialization risks, new dependencies (supply chain), data-model/migration changes, broad moves/renames, new top-level dirs, or anything contradicting the stated rules from section 2. For each: severity (high/med/low) and a suggested direction. Deep security hand-off: always run /security-review against the working diff (the throwaway worktree for PR targets), scoped to these changes, and fold its findings into this point. Forward --comment/effort; security findings are generally not auto-fixable. Deep over-engineering hand-off: always run /ponytail-review against the same working diff to hunt reinvented stdlib, unneeded dependencies, speculative abstractions, and dead flexibility; fold its findings into this point. Forward --fix and --comment. Normalize ponytail's output into the same format as other findings — [SEV] file:line — description (confidence N) [CR-NNN] — before folding in; assign IDs from the shared counter (section 4). Since ponytail-review lists fixes but does not apply them, --fix means change-review applies mechanically-fixable ponytail findings (confidence ≥ 80) itself, same as for correctness findings.

(4) Lint/style on new files. For every newly added file, verify it matches the repo's linting/formatting and local conventions. Prefer running the real tools, scoped to the new files: JS/TS npx eslint <files> / npx prettier --check <files>; Python ruff check <files> / black --check <files>; Go gofmt -l <files>; Rust rustfmt --check; Ruby rubocop <files>. Run from the directory that owns the tool config. Filter to signal — do not report pre-existing repo noise: a warning that also fires on unrelated existing files is config noise, not a finding; state when you've discounted warnings as such. (Existing files: only flag style issues you're confident regress the repo norm.)

(5) Documentation freshness. Confirm docs that should reflect these changes do — README, docs/, AGENTS.md, CLAUDE.md, CHANGELOG, command cheatsheets, env-var/config tables, public API references, example snippets. Flag missing or stale docs with the specific file and what to add. If doc-neutral, say so.

(6) Correctness — deep hand-off. Always invoke built-in /code-review against the working diff (the throwaway worktree for PR targets) for the deep correctness + reuse/simplification/ efficiency pass; forward --effort and (with --fix) --fix. Fold its findings into this point. Add your own high-confidence obvious-defect findings the deep pass missed and anything the history/prior-PR lenses (section 3) surfaced.

(7) Tests. Whether changed/added behavior is covered. Did new logic get tests? Were existing tests updated, or are any now stale/broken? Name the specific missing or affected test files. If test-neutral (docs, config, pure formatting), say so.

Coverage: run make coverage (TypeScript) or make coverage-python (Python) if the target exists. Report the overall line % and flag any changed module below 80% as a blocker. For e2e: note any new user-visible flows in the diff that lack a Playwright test.

5. Confidence scoring

Score every finding 0–100 for how confident you are it's a real, relevant issue:

• 0–49 — uncertain / possible false positive / pre-existing. Surface as a suggestion only.
• 50–79 — likely real but a nit or low-impact. Suggestion; never auto-fixed.
• 80–100 — verified, high-impact, or a direct stated-rule violation. Eligible for --fix.

Do not drop findings by score — report them all. With --comment, every comment shows its score, e.g. (confidence 85). With --fix, only findings ≥ 80 that are mechanically fixable are applied; everything else is reported as a suggestion.

6. Apply / comment (only when flagged)

• --fix: apply the high-confidence fixes — run linters/formatters with --fix/--write on new files, edit stale docs, and run /code-review --fix for correctness. Locally, stage them so the user can review and commit. With --ci the agent is read-only: apply fixes to the working tree only and do NOT commit, push, or comment — a separate privileged job stages the edits into an [autofix] commit and pushes it. Report what was fixed vs left as a suggestion.
• --comment: post the report's findings as PR comments (use github-pr-review plumbing or gh pr comment), each with its confidence score; un-fixable and lower-confidence items go here. With --ci the agent instead writes all findings to change-review-findings.json (schema below); the separate privileged job turns each line-anchored finding into an inline review comment and posts the rest in the review body.

CI findings JSON (change-review-findings.json)

When --ci is passed (read-only mode), write findings as JSON, not prose — regardless of --fix/--comment. Decide anchoring yourself — you hold the diff:

• report_markdown (top-level, always present): the entire prose report you would print in CLI mode — every section plus the verdict. This preserves narrative and any content not tied to a specific line; the CI job renders it at the top of the review comment.
• A finding goes in findings[] only if its path + line fall inside the PR diff (an inline comment on a line outside the diff is rejected). Everything else (missing test, absent doc, whole-PR concern) goes in unanchored[].
• A finding's text is three fields: summary (one-line headline), detail (the full explanation), and suggestion (how to fix it). The job renders them as a bold headline, then the detail, then a suggestion line. summary alone is fine when there's nothing more to add; detail/suggestion are optional.
• status: "fixed" iff you applied the fix to the working tree under --fix (confidence ≥ 80, mechanically fixable). All other findings are "unfixed". Do not add the marker to any finding — the job appends it to fixed items.
• side: "RIGHT" for head-side/added/context lines, "LEFT" for a removed line (optional, defaults to "RIGHT"). Set start_line only for a multi-line range, else omit it.
• unanchored[] items take a category (e.g. tests, efficiency, simplification) plus the same summary/detail/suggestion fields.
• Always write the file, even with no findings ({"findings":[],"unanchored":[]}).

{
  "meta": { "target": "PR #123: feat/foo → main", "files_changed": 12 },
  "report_markdown": "## Change review — PR #123\n\n### 1. Summary\n…\n\n### Verdict\n…",
  "findings": [
    { "id": "CR-001", "path": "src/foo.ts", "line": 42, "side": "RIGHT", "severity": "med", "confidence": 85,
      "status": "fixed",
      "summary": "Unused import left after refactor.",
      "detail": "`os` is no longer referenced once readFile moved to fs/promises.",
      "suggestion": "Remove the `import os` line." }
  ],
  "unanchored": [
    { "id": "CR-002", "category": "tests", "confidence": 70,
      "summary": "No test covers the new error path.",
      "detail": "throwOnMissingConfig has no test asserting it throws.",
      "suggestion": "Add a case in tests/foo.test.ts." }
  ]
}

7. Output format

## Change review — <what was reviewed>

### 1. Summary
<grouped prose>

### 2. Outlying changes
- [CR-NNN] <file:line> — <why surprising> (confidence N)   (or: None)

### 3. Architecture / security / structure
- [CR-NNN] [HIGH/MED/LOW] <file:line> — <risk> → <direction> (confidence N)   (or: None)
  <deep security via /security-review folded in, same format>
  <deep over-engineering via /ponytail-review folded in, same format>

### 4. Lint & style (new files)
- [CR-NNN] <file> — <tool result / deviation + fix> (confidence N)   (or: All new files conform)

### 5. Docs
- [CR-NNN] <doc path> — <what needs updating> (confidence N)   (or: Up to date)

### 6. Correctness
- [CR-NNN] <file:line> — <bug + failure scenario> (confidence N)   (or: No obvious defects)
  <deep correctness via /code-review folded in, same format>

### 7. Tests
Coverage: <overall line %> (or: not available)
- [CR-NNN] <test file / area> — <missing or stale coverage> (confidence N)   (or: Adequate / N/A)

### Verdict
**Blockers (fix before merge):**
- <ordered by severity, include CR-NNN>   (or: None)

**Nits (optional):**
- <minor items, include CR-NNN>   (or: None)

<if --fix: what was auto-fixed vs left as a suggestion>
<one line: safe to merge as-is, or address blockers first>

Keep it tight. End with the verdict, blockers first.

Persist the report: after printing (or writing --ci JSON), save the prose report to <repo-root>/.superpowers/03-review/<YYYY-MM-DD>-<slug>.md where slug is uncommitted, pr-<N>, or derived from the branch name. Create the directory if absent. Derive the repo root with git -C "$(git rev-parse --git-common-dir)/.." rev-parse --show-toplevel — this resolves to the main repo even when called from inside a throwaway worktree. Skip this step under --ci when the JSON file is the deliverable.

8. Interactive triage (non---ci only)

After printing the report, collect all unfixed findings from sections 2–7. Use their CR-NNN IDs (assigned during the report — section 4). Skip this step entirely under --ci.

≤ 4 unfixed findings: use AskUserQuestion — one question per finding, all in a single call.

• header: "CR-NNN" (+ section label if helpful, e.g. "CR-007 Tests", max 12 chars)
• question: the finding's impact (what breaks or degrades if left as-is) and the trade-off of each choice, so the user can decide without re-reading the report.
• options — always exactly these 4, in this order (the tool rejects <2 — never omit any):
  1. label "Fix it", description "Apply the change now"
  2. label "Add to queue", description "Defer to /queue for a later session"
  3. label "Log as issue", description "Create a tracker issue and link it"
  4. label "Ignore", description "Drop it"

> 4 unfixed findings: render a markdown table instead, then prompt for dispositions as text:

ID	Section	Finding	Confidence
CR-001	Correctness	foo.ts:42 unused import	85
…	…	…	…

Then ask: "For each finding reply: <CR-NNN> fix|queue|issue|ignore [note]. E.g. CR-001 fix CR-003 queue CR-005 ignore."

The user may attach a free-text note to any choice; read it from annotations[].notes (≤ 4 findings path) or inline in the text reply (> 4 findings path) and carry it into the action (append to the issue body, prefix the queue item, or record alongside an ignored finding).

Act on each selection:

• Fix it: apply the edit now.
• Add to queue: call /queue <finding summary> to defer to this session's backlog.
• Log as issue: gh issue create (or Jira via acli) capturing the finding; link it back.
• Ignore: record it as acknowledged.

Report a one-line summary: what was fixed, queued, logged, and ignored.

Notes

• For large changesets, fan out with the Agent tool (one agent per area or checklist point) and synthesize — the final report must still follow the structure above.
• Respect the repo's own stated rules above generic priors.
• Relationship to other review skills (see review-skills-map.md): this skill is the broad dispatcher. /code-review (built-in) is the deep correctness + code-quality pass it calls; /security-review (built-in) is the deep vuln audit it calls; /ponytail-review (ponytail plugin) is the deep over-engineering pass it calls. Note the name collision: built-in /code-review is not the third-party code-review plugin (PR-by-number, 5 agents) — this skill now absorbs that plugin's git-history, prior-PR, and inline-comment angles, so it is no longer needed in the review flow. After a review, github-pr-review handles replying to and resolving the resulting comment threads.
• Implementation caveat: built-in /code-review reviews the working diff; the worktree soft-reset above materializes a PR as one. If /code-review is confirmed to accept a base...HEAD target directly, the worktree step can be simplified.