# Code Review

> Code review a pull request, commit, file, or local git changes. Use when the user says "review", "code review", "/code-review", "review local changes", "review PR", "review commit", or asks for a review of changes.

- **Type:** Skill
- **Install:** `agentstack add skill-napnap11-claude-skills-code-review`
- **Verified:** Yes — security-reviewed for prompt injection and unsafe behavior
- **Seller:** [napnap11](https://agentstack.voostack.com/s/napnap11)
- **Installs:** 0
- **Category:** [Agent Skills](https://agentstack.voostack.com/c/agent-skills)
- **Latest version:** 0.1.0
- **License:** MIT
- **Upstream author:** [napnap11](https://github.com/napnap11)
- **Source:** https://github.com/napnap11/claude-skills/tree/main/code-review

## Install

```sh
agentstack add skill-napnap11-claude-skills-code-review
```

Requires the [AgentStack CLI](https://agentstack.voostack.com/docs/cli). Works with Claude Code, Cursor, and any MCP-compatible agent.

## About

# Code Review Skill

Run a careful, line-level review of a change set and record the outcome in `REVIEWED.md` at the project root (or wherever the user prefers to keep review notes — confirm if the destination is ambiguous).

## Gather context first

This skill ships standalone, so it can't assume your project's conventions, house style, or preferences the way it could for its author. Before doing the main work:

1. **Auto-detect what you safely can** from the repo — language/stack, base branch, build/test commands, existing config and docs. Never ask for something you can read for yourself.
2. **Ask, don't assume, for the rest.** Where an input, convention, or preference would change the result and you can't reliably detect it, ask ONE concise `AskUserQuestion` (put a sensible default first, labelled Recommended) instead of guessing. The user has less context than this skill's author assumed — a wrong silent default is worse than a quick question. Don't ask about things you can detect, and don't ask more than you need.

For this skill, confirm up front (only the items you can't already detect):
- **Output destination** — write `REVIEWED.md` at the project root (the default), append to an existing one, report inline, or use a path the user keeps review notes in.
- **Base branch to diff against** — for any branch-level or structural pass; auto-detect the repo's default branch (`git symbolic-ref refs/remotes/origin/HEAD`) and confirm only when it's ambiguous (multiple long-lived branches, a fork, a non-`main` trunk).
- **How strict the severity bar is** — the `MUST FIX` / `SHOULD FIX` / `FOLLOW-UP` ladder and verdict mapping, including whether unfixed sibling-branch code paths should escalate to `MUST FIX` / `REQUEST_CHANGES`.
- **Review depth** — line-level only, or also the structural (pre-merge-review-style) specialist pass (and, per *Subagents & parallelism*, inline vs parallel).

## Subagents & parallelism (opt-in)

By default this skill runs **inline in a single context** — no subagents, no Workflow fan-out — to keep it cheap to run.

Parallel subagents can be faster on large jobs (big diffs, many files, the multi-lens structural pass). **Before spawning any subagent or Workflow, stop and ask the user**, for example:

> This change is large ([N files / big diff], plus the structural specialist panel). I can review it inline here (cheaper, slower) or fan out [M] parallel subagents — one per area or specialist lens (faster, more tokens). Which do you want?

Spawn subagents/workflows only after an explicit yes. If the user declines or doesn't answer, do the whole job inline.

## Determining What to Review

Inspect `$ARGUMENTS`:
- Empty → review uncommitted local work: `git diff HEAD`
- A PR number (e.g. `123`) → that GitHub PR: `gh pr diff 123`
- A commit ref (e.g. `abc1234`) → that commit: `git show `
- A file path → that file's changes: `git diff HEAD -- `

If there are no arguments and the working tree is clean, say so and stop.

## Task Validation

When the PR or branch is tied to a task (ClickUp, Linear, a GitHub issue, or something the user names), confirm the change actually resolves the reported problem:

1. **Pull the task** — use the right tool (`clickup_get_task` for ClickUp IDs like `86d2cdpcr`, `gh issue view` for GitHub issues) to read the bug description, expected behavior, and repro steps.
2. **Trace the bug** — follow the flow end to end, from the user action in the report through handlers → services → adapters → external APIs → response, and pin down the root cause of the reported behavior.
3. **Diff it against the fix** — decide whether this PR addresses that root cause, or whether it patches something else / the wrong layer.
4. **Verdict** — add a `## Bug Validation` section to the output containing:
   - The reported bug (1–2 sentences)
   - The traced root cause (with file paths)
   - Whether the PR fixes it: **Yes** / **Partially** / **No**
   - If No/Partially: what the real fix should target (files and approach)

If the PR does **not** fix the linked task, add a `BLOCKER` row to the summary table at impact 10/10. This overrides every other verdict — a PR that doesn't solve its stated problem cannot be approved.

### Sibling-branch coverage check (MUST FIX trigger)

Once you've confirmed the fix works on the demonstrated repro path, enumerate the **sibling branches that share the same root pattern** — the other arms of the same `if/else`, other values of the same enum, other config flags (`required_card`, `is_admin`, locale variants, and so on), other call sites of the same helper. For each one ask: *under realistic data, would the originally reported symptom still show up here?*

If the answer is yes, that sibling is a **MUST FIX**, not a SHOULD FIX. A bug ticket is, by definition, a complaint about broken behavior; a partial fix that still ships the same user-visible symptom for a meaningful slice of traffic belongs at MUST FIX (impact 7–8/10 depending on the traffic share), and the verdict becomes `REQUEST_CHANGES`.

The reasoning: bug-fix PRs exist to *eliminate* the reported behavior. Filing a partial fix as SHOULD FIX understates the gap — the user re-opens the same ticket, and the next fix re-derives context the reviewer already had in hand. Close it now.

**Skip this section** when no task/ticket is linked or mentioned.

## Review Process

1. Run the right git command to obtain the diff.
2. For every changed file, read the surrounding context — not only the diff lines — so you understand intent.
3. Evaluate the change across:
   - **Bugs** — logic errors, nil/null dereferences, off-by-one, race conditions
   - **Security** — injection, auth bypass, sensitive-data exposure, missing input validation
   - **Correctness** — does it do what it claims? which edge cases are missed?
   - **Data integrity** — could this lose or corrupt data?
   - **Performance** — N+1 queries, DB calls inside loops that should be batched, unbounded loops, missing indexes, needless allocations
   - **Maintainability** — dead code, misleading names, missing error handling at boundaries
4. Assign each issue a severity:
   - `MUST FIX` — bug, data loss, security, broken behavior. **Includes any sibling code path where the originally reported bug still reproduces** (see the sibling-branch check above).
   - `SHOULD FIX` — correctness concern, UX regression, missing guard
   - `FOLLOW-UP` — dead code, style, leftover `console.log`
5. Rate impact 1–10:
   - 9–10: data loss, security, crashes
   - 7–8: incorrect behavior visible to users
   - 5–6: degraded UX, stale data, performance
   - 3–4: maintainability, dead code, naming
   - 1–2: style, whitespace, minor inconsistency

## Structural Escalation (pre-merge-review pass)

After the line-level pass, decide whether the change warrants a deeper **structural** review — the same specialist-lens analysis the `/pre-merge-review` skill performs. Run that structural pass when ANY of these hold:

- The diff touches **database queries, API integrations, or auth/security code**
- The diff adds **new enum values, status strings, or type constants**
- The line-level review surfaced **any MUST FIX tied to race conditions, data safety, or trust boundaries**
- The diff changes **>5 files with logic** (not just config/assets)
- The diff introduces **new user-facing data flows** (form submissions, API calls that write data)

**Skip the structural pass for:**
- Asset-only changes (images, fonts, config files)
- Pure style/CSS changes with no logic
- Documentation-only changes
- Diffs with fewer than 3 files and no data/security concerns

**By default, run the structural pass inline in this context:** pick the specialist lenses the diff calls for and walk each one's checklist sequentially yourself, then fold the findings into REVIEWED.md. This keeps everything in a single context and cheap. Run the structural pass here directly — don't invoke the `/pre-merge-review` skill as a separate step from inside this review.

If the user opted into parallelism (see *Subagents & parallelism*), you may instead fan the specialist panel out as a dynamic Workflow — one `agent()` per lens, in parallel:

1. **Select specialists** by what the diff touches — security, performance, testing, maintainability, data-migration, api-contract, design (skip design if the diff is  () =>
  agent(
    `You are a specialist code reviewer for ${s.domain} on a ${args.stack} project.\n\n` +
    `Run \`git diff origin/${args.base}\` to get the full diff. Review against the checklist below.\n\n` +
    `For each finding return: severity (CRITICAL|INFORMATIONAL), confidence (1-10), path, line (optional), ` +
    `category, summary, fix, fingerprint (path:line:category), specialist ("${s.domain}"); optionally evidence ` +
    `and a test_stub (minimal ${args.testFw} skeleton if a test would catch it). Before flagging a commented ` +
    `line, walk up to the nearest block boundary; if the enclosing function is itself commented, it is dead — ` +
    `do not file it. If no findings, return an empty array. Findings only, no preamble.\n\n` +
    `CHECKLIST:\n${s.checklist}`,
    { label: `specialist:${s.key}`, phase: 'Specialist', model: 'opus', schema: FINDING_SCHEMA }
  )
))
return reviews.filter(Boolean)
```

3. **Merge + fix-first:** dedupe by fingerprint, then classify each finding as AUTO-FIX (mechanical — dead code, N+1, magic numbers, missing output validation, unused imports → fix directly) or ASK (security, race conditions, design decisions, >20-line fixes, behavior changes → one AskUserQuestion). Any finding carrying a `test_stub` reclassifies to ASK. The full classification table lives in the `/pre-merge-review` skill, Step 4.
4. **Write** the findings into REVIEWED.md under a `## Staff Review Findings` section.

Whether you run the structural pass inline or as a Workflow, the findings land in the same `## Staff Review Findings` section. If a Workflow fails or times out, fall back to walking the specialist checklists inline (or note that the pass was skipped).

## Figma Comparison

When the diff changes UI, compare the **committed code against the Figma design spec** for the components this PR touches. This is a thin, PR-scoped, **static** spec diff — read-only, no rendering, no 0–10 / A–F scores, no fixes. It answers exactly one question: *does the built code match the Figma spec for the components this PR changed?*

**Run automatically when BOTH are true:**
- The diff touches **UI** — `.tsx`/`.jsx`/`.css`/Tailwind/component/page files with visual changes (not pure logic, config, tests, or assets).
- A **Figma reference is discoverable** (see *Finding the Figma ref* below).

**Do NOT run for:**
- Backend-only / API / config / logic-only diffs with no rendered surface.
- Docs / tests / assets only.
- UI diffs where no Figma ref turns up after the discovery steps — **skip silently, never block** (emit one line: `**Figma ref:** none found (skipped)`).

### Finding the Figma ref

The Figma MCP has **no search-by-name** — you must supply a concrete `fileKey` + `nodeId`. Discover them in order; stop at the first hit:
1. **PR body / comments** — grep `gh pr view ` and `gh pr view  --comments` for a `figma.com/...` URL or a fileKey/nodeId.
2. **Linked task** — resolve the task from the PR/branch, then `clickup_get_task` (description) **and `clickup_get_task_comments`**. The Figma URL often lives in the task **comments**, not the description.
3. **Project memory** — fall back to `reference_*figma*.md` for known fileKey/node IDs when the PR is on a matching feature branch.
4. **Ask the user** — one `AskUserQuestion` for the Figma URL or node ID (plain labels, no preview), only when 1–3 turn up nothing.

Parse `figma.com/design/:fileKey/:name?node-id=14100-712660` into `fileKey` and `nodeId = "14100:712660"` (the `-` and `:` forms are interchangeable). For a branch URL, use the `branchKey` as the fileKey.

### Pulling the spec (read-only Figma MCP)

Load the tools via ToolSearch (`select:mcp__claude_ai_Figma__get_design_context,mcp__claude_ai_Figma__get_screenshot,mcp__claude_ai_Figma__get_variable_defs,mcp__claude_ai_Figma__get_metadata,mcp__claude_ai_Figma__get_code_connect_map`), then for each touched component's node:
1. `get_screenshot(fileKey, nodeId)` — the visual reference (the returned PNG URL is short-lived; fetch it immediately).
2. `get_design_context(fileKey, nodeId, clientFrameworks:"react")` — the structural/styling intent to diff JSX/Tailwind against. If it degrades on an oversized frame, run `get_metadata` first and drill into child node IDs.
3. `get_variable_defs(fileKey, nodeId)` — resolved design tokens (colors, spacing, type) to diff against the project's Tailwind/theme constants.
4. `get_code_connect_map(fileKey, nodeId)` *(optional)* — resolves each Figma node to its repo component file so you compare against the right source. Empty when Code Connect isn't set up.

**Read-only only** — never call `use_figma`, `create_*`, `upload_*`, or any write/sync tool from a review.

### What to diff (objective build-vs-spec only)

For the components in the diff, flag concrete deviations:
- **Design tokens** — color / spacing / radius / font-size / weight / line-height in the code vs `get_variable_defs` values (and the project's Tailwind tokens).
- **Type ramp** — heading/body sizes, weights, hierarchy order.
- **Structure & content** — element order, labels/microcopy, iconography vs the Figma frame.
- **States present in Figma but absent from the build** — empty / error / hover / disabled / loading variants the design frames show.

Report each as a standard code-review finding (`file:line` + the Figma node it deviates from), severity-mapped:
- A user-visible structural / content / missing-state mismatch → **SHOULD FIX** (UX regression, impact 5–6).
- A pixel/token nit (spacing, color shade, radius) → **FOLLOW-UP** (impact 1–2).
- **MUST FIX** only when a mismatch breaks behavior — e.g. a required CTA or field present in the Figma spec is absent from the build.

Findings go in `REVIEWED.md` under `## Figma Comparison` and as rows in the shared Summary Table.

### Hand off depth — don't reimplement the design skills

This step does a static spec diff only. Defer anything deeper:
- **→ `/design-review`** when the gap is a *missing design decision* (unspecified states, IA/hierarchy questions, responsive/a11y not in the Figma, or no `DESIGN.md`). It rates dimensions 0–10 and edits the plan; code-review only names the gap and points there.
- **→ `/ui-polish`** when the deviation needs *eyes on the running app* or *source fixes* (rendered pixel/spacing polish, cross-page consistency, motion, performance-as-design, AI-slop grading, "fix it in source"). It screenshots the rendered app and commits fixes; code-review does neither.

Don't re-inline their AI-Slop blacklist or Design Hard Rules, and don't claim the bare "design review" trigger phrase.

## Output Format

**Always** record the review using this exact format — by default create or append to `REVIEWED.md` in the project root, or use the destination confirmed up front (an existing file, an inline report, or a custom path):

```md
# Review —  (``)

**Date:** YYYY-MM-DD
**Author:** 
**Files touched:** 
**Verdict:** APPROVE | APPROVE WITH FOLLOW-UPS | REQUEST_CHANGES | NEEDS_REVISION
**Linked task:** 

---

## Bug Validation

_(Include this section only when a task/ticket is linked — see Task Validation above)_

**Reported bug:** 
**Root cause:** 
**Does this PR fix it:** Yes / Partially / No
**If No/Partially:** 

---

## Issues

### [SEVERITY] N. 

**File:** `path/to/file.ts:LINE_START–LINE_END`
**Impact score:** X/10
**Reason:** 

**Current code:**
```lang
// the problematic code
```

**Fix:**
```lang
// the corrected code
```

---

## Staff Review Findings

_(Include this section only when the structural pass was run)_

**Structural pass status:** Ran inline / Ran as Workflow / Skipped (reason)
**Auto-fixed:** N items
**Needs input:** N ite

…

## Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

- **Author:** [napnap11](https://github.com/napnap11)
- **Source:** [napnap11/claude-skills](https://github.com/napnap11/claude-skills)
- **License:** MIT

Install and usage instructions live in the source repository linked above.

## Pricing

- **Free** — Free

## Security capabilities

Automated source analysis of v0.1.0 — what this tool can access:

- **Network access:** no
- **Filesystem access:** no
- **Shell / process execution:** no
- **Environment & secrets:** no
- **Dynamic code execution:** no

*"Yes" means the capability is present in the source — more access means more to trust, not that it is unsafe.*


## Versions

- **0.1.0** — security scan: passed — Imported from the upstream source.

## Links

- Listing page: https://agentstack.voostack.com/l/skill-napnap11-claude-skills-code-review
- Seller: https://agentstack.voostack.com/s/napnap11
- Browse the marketplace: https://agentstack.voostack.com/browse

---
Listed on AgentStack — the marketplace for AI agent skills and MCP servers. Every listing is security-reviewed. Creators keep 70%.