review

Code Review Panel

Run a comprehensive code review (security, architecture, clarity) against your current changes.

Arguments

• $ARGUMENTS - Optional: branch or commit range to review (defaults to current branch vs main/master)

Context

• Current branch: !git branch --show-current
• Base ref: !git branch -r 2>/dev/null | grep -oE 'origin/(main|master)' | head -1
• Changes vs main: !git diff --stat origin/main...HEAD 2>/dev/null | head -50
• Changes vs master: !git diff --stat origin/master...HEAD 2>/dev/null | head -50

Phase Protocol

This skill participates in a phase chain. Read ~/.claude/skills/_shared/resources/phase-protocol.md for the full protocol.

Before starting: Check .context/phases/ for prior artifacts:

• Read the latest plan-*.md for the original intent and decisions — review the diff against plan expectations, flagging deviations.
• Read the latest build-*.md for what was built and any notes from the implementer.

After Step 4 (report): Write a review-{ts}.md artifact to .context/phases/ (create with mkdir -p .context/phases). The Handoff section should state: PASS (ready to ship) or FAIL (needs changes, list what), plus any warnings /pr should include in the PR description.

Instructions

You run a code review and produce a consolidated report.

Step 1: Determine Review Scope

IF $ARGUMENTS contains a branch name or commit range:
  Use that as the review scope
ELSE:
  Review current branch changes vs the default branch (main/master)

Gather the diff:

git diff {base}...HEAD        # full diff
git diff {base}...HEAD --name-only  # changed files

If there are no changes to review, tell the user and stop.

Step 2: Read Changed Files

Read the full content of each changed file for context. The reviewer needs to see the code around the changes, not just the diff.

Step 3: Launch Reviewer

Use 1 Task tool call with subagent_type="general-purpose", model: "sonnet":

Review the following code changes for security, architecture, and clarity.

DIFF:
{full diff}

CHANGED FILES:
{full file contents}

CHECK EACH -- SECURITY:
- Injection flaws (SQL, command, LDAP, XPath)
- Authentication/authorization issues
- Sensitive data exposure (secrets, PII, credentials in code or logs)
- Input validation and sanitization
- XSS potential
- Insecure deserialization
- Vulnerable dependencies added
- Error handling exposing internals
- Missing rate limiting
- Insecure direct object references

CHECK EACH -- ARCHITECTURE:
- Single Responsibility Principle
- Separation of concerns
- Dependency direction (abstractions over concretions)
- Coupling level
- Cohesion
- Consistency with existing codebase patterns
- Error handling strategy
- Extensibility for likely changes
- Circular dependencies

CHECK EACH -- CLARITY:
- Function/method naming clarity
- Variable naming
- Comments where logic is non-obvious
- Public API documentation
- Cyclomatic complexity
- Dead code or unreachable branches
- Magic numbers/strings
- Consistent code style
- Log message quality

Classify each finding as:
- BLOCKING: Must fix before merging
- WARNING: Should fix, real risk
- INFO: Improvement suggestion

Tag each finding with its category: [security], [architecture], or [clarity].
If no issues in a category: explicitly state so.

Output your findings grouped by severity (BLOCKING first, then WARNING, then INFO).