# Review Change > Review a change you just implemented — correctness against its callers, test quality, and code-level security — over a git range or the working tree. Run right after finishing a unit of implementation and before committing it. Not for reviewing a spec or a plan (use /review), not for whole-repo architecture, dependencies or docs drift (use /review-health), not for user flow in a browser (use /review-experience). - Skill: `mxrsv/review-change` (Agent Skill, multi-file: 4 files) - Install (CLI): `npx skillmds@latest add mxrsv/review-change` - Raw SKILL.md: https://api.skillmd.com/api/skills/mxrsv/review-change/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Security - Author: mxrsv (https://skillmd.com/u/mxrsv) - Updated: 2026-09-21 - Page: https://skillmd.com/skills/mxrsv/review-change --- # Review a change > **Harness note.** Skill names are written `/name` (Claude Code slash form); in Codex invoke the same skill as `$name`. Sub-agent dispatch is shown as Claude Code's `Agent({ subagent_type: "general-purpose", … })` — in Codex spawn a sub-agent with the same prompt, or apply the contracts inline, in sequence, when sub-agents are unavailable. Three reviewer contracts over one change: `correctness`, `tests`, `security-code`. Contracts live in [reviewers/](reviewers/) and are the prompt bodies you dispatch — read them, do not summarize them. Shared report format, finding schema and freshness rules: `~/.claude/templates/review-report.md` (absolute path on purpose — this directory is also reached through the `~/.agents/skills/` symlink used by Codex and Cursor, where a relative `../../templates/` would resolve elsewhere). **The report is a comment on the Linear issue that owns the change** (`save_comment`, template §1) — never a file in the repo. Ask for the issue id up front if the conversation has not named one. ## 1. Resolve the source ```bash git -C rev-parse HEAD # head_sha git -C status --porcelain # manifest + dirty flag git -C status --porcelain=v1 | shasum -a 1 | awk '{print $1}' # tree_digest ``` | Situation | `source_kind` | Range / manifest | |---|---|---| | User named a range, branch or PR | `git-range` | `..`; comparing branches → `base = git merge-base HEAD`. Manifest: `git diff --name-status ..` | | Working tree is dirty, no range given | `working-tree` | Base is `HEAD`. Manifest: `git status --porcelain` | | Working tree is clean, no range given | `git-range` | Default `HEAD~1..HEAD` | Manifest comes from `git status --porcelain`, **not `git diff`** — `git diff` is blind to untracked files, and an untracked file is a whole new behavior. `??` entries have no diff: the entire file is the change. `tree_digest` and the dirty flag come from porcelain in **both** modes. **A diff does not expire.** Anchored to `base_sha..head_sha` it stays valid however late you review. Reviewing an old change is not a reason to switch profile — it is a reason to write the range into the report header so the reader knows what was looked at. Ask the user for a one-or-two-sentence **objective** (what the change is trying to do) if it is not already obvious from the conversation. Every reviewer needs it: correctness is judged against intent, not against style. ## 2. Quick triage — mandatory, before any dispatch Two independent decisions. Do not let one answer the other. | Variable | Decided by | Floor | |---|---|---| | **Which reviewers apply** | risk — route **conservatively**, when unsure turn it on | **never 0 reviewers** | | **Inline or subagent** | cost — a small change runs inline | **0 subagents is valid** | A 1-line diff may decide the *run mode*. It may never decide the *coverage* — one line is enough for an auth bypass. ### Variable 1 — which reviewers | Reviewer | On when | |---|---| | `correctness` | **always.** This is the floor; there is no change too small to have callers. | | `tests` | behavior changed — **including when the test files are themselves the thing that changed.** An assertion-free test or a happy-path-only test is precisely what this reviewer exists to catch, so "the diff is only tests" is a reason to run it, not to skip it. | | `security-code` | the change touches any of: auth or session · input arriving at a boundary · env vars · secrets or credentials · dependency manifests · shell commands · CI config · permissions or settings · fetching a URL · path handling · serialization/deserialization. Unsure → on. | **`.md` files are not automatically docs.** In an agent-config repo (`~/.claude`, `~/.agents`, or any repo containing `skills/`, `commands/`, `agents/`), `skills/*/SKILL.md`, `commands/*.md` and `agents/*.md` are **executable behavior** — changing them can change tool access, permissions, or what gets run. Never classify such a diff as "docs only" and never let that classification drop a reviewer. Route them like code: `security-code` on if the file grants tool access, edits an allowlist, or runs a shell command. Declaring a skip is an audit trail, not a safety net — it makes an omission *visible*, it does not prove the routing was right. Safety comes from routing conservatively. ### Variable 2 — inline or subagent - Under roughly 150 changed lines across under 5 files, **or** only one reviewer routed on → run the contracts inline, one after another, in this session. - Otherwise → one subagent per routed reviewer, **all Agent calls sent in a single message** so they run in parallel. - Cap 4 subagents per command (only 3 exist here). - Cutting cost means cutting subagents, never cutting reviewers. ## 3. Dispatch `reviewers/*.md` are supporting files, not agent definitions — no harness loads them automatically (Claude Code discovers agents only from `agents/*.md`, Codex from `~/.codex/agents/*.toml`). So read each contract and **inline its full text into the prompt**. ``` Agent({ subagent_type: "general-purpose", name: "rev-correctness", // rev-tests · rev-security-code model: "sonnet", // correctness → opus for a large diff, or concurrency/auth code prompt: }) ``` Envelope — the same shape for all three: ``` OBJECTIVE: REPO ROOT: SOURCE KIND: git-range | working-tree RANGE: .. # working-tree: "HEAD = , uncommitted" MANIFEST: M src/auth/session.ts A src/auth/refresh.ts ?? skills/foo/SKILL.md CONTRACT: .md> ``` **Do not pre-bundle the evidence.** Reading the diff, the callers and the tests yourself and pasting them into three prompts fails three ways at once: this session floods with the context that subagents exist to keep out, the same evidence is duplicated three times, and a reviewer cannot follow a caller or a trust boundary that falls outside the bundle you chose. Pass objective, range and manifest. Each reviewer reads for itself. ## 4. Collect and post one report Subagents return findings as text. **Only this skill posts the comment.** N subagents posting into the issue race each other and drift in format. 1. **Dedupe.** Same root cause reported twice → keep the copy owned by the scope boundaries in each contract's *Out of scope* section, and note that another reviewer concurred. Same file:line with genuinely different causes → keep both. 2. **Rank** by severity, then confidence. 3. **Post** `save_comment { issueId: , body }` on the issue that owns the change, body per `~/.claude/templates/review-report.md` §2–§4 with `profile: change`. - `scope`: `-` for a range, `worktree` for the working tree. - `run_id`: `LC_ALL=C tr -dc 'a-z0-9' < /dev/urandom | head -c 6` - Header, then the coverage table — every reviewer listed as `ran` / `blocked` / `skipped`, with a reason for anything that is not `ran`. Omitting a reviewer from that table is a defect in the report. - No Linear MCP in this harness → print the same body in chat as one fenced block and name the issue. Never a file. 4. **Report to the user**: blockers first, then the comment URL. Do not bury findings under a summary, and do not fix anything unless asked. Finding keys use the 8 domains fixed in the template — one per reviewer. This command produces `code/*` (correctness), `tests/*` and `sec/*`. A finding that wants any other domain belongs to another command; see each contract's *Out of scope*. ## Hard rules - Route conservatively; never return 0 reviewers. - Never pre-bundle evidence into subagent prompts. - One report comment per run, posted by this skill; never a file in the repo. - Report findings; do not apply fixes in review mode.