code-reviewer

name	code-reviewer
description	Reviews all changes in a feature branch. Does not modify code.

Code Reviewer Skill

Performs a comprehensive code review of all changes in the current feature branch. Reports findings but does NOT make code changes.

Workflow

1. Determine Base Branch

# Auto-detect main or master:
git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@'
# Fallback: check if 'main' exists, then 'master'

2. Gather Changes

# Get the merge base (common ancestor)
MERGE_BASE=$(git merge-base <base-branch> HEAD)

# List all changed files
git diff --name-only $MERGE_BASE HEAD

# Get the full diff for analysis
git diff $MERGE_BASE HEAD

3. Analyze Each Changed File

For each modified file:

1. Read the current file content
2. Understand the context of changes
3. Evaluate against review criteria (see below)

4. Decision Point

If code is great (no issues found):

• Output APPROVED and exit
• No report file is created

If issues are found:

• Save report to docs/local/review-X.md (where X is incremented for each review iteration)
• Output NEEDS FIXES with a summary

---

Review Criteria

Evaluate changes against these categories:

Critical Issues

Issues that must be fixed before merging:

• Unsafe code: unnecessary unsafe blocks, unsound abstractions, undefined behavior
• Memory safety: use-after-free patterns, data races, incorrect Send/Sync implementations
• Error handling: silently swallowed errors, unwrap()/expect() in non-test code without justification
• Security: command injection, path traversal, hardcoded secrets
• Breaking changes: unintended public API contract violations

Bugs & Logic Errors

• Off-by-one errors, incorrect pattern matching, missing match arms
• Incorrect ownership/borrowing that compiles but produces wrong results
• Missing error propagation (should use ? instead of ignoring Result)
• Race conditions in async code, incorrect tokio usage
• Edge cases that will cause panics or incorrect results

Performance Concerns

• Unnecessary cloning or allocation where borrowing suffices
• O(n^2) or worse algorithms when linear alternatives exist
• Unbounded buffers or data structures
• Blocking calls inside async contexts

Code Quality

• Overly complex functions that should be broken down
• Duplicated logic that should be abstracted
• Poor naming that obscures intent
• Non-idiomatic Rust (e.g., &String instead of &str, manual loops instead of iterators)

Refactoring Opportunities

Actively look for code that should be refactored:

• Functions exceeding ~30 lines
• Deep nesting (3+ levels)
• Duplicated logic (3+ occurrences)
• Boolean function parameters
• Long parameter lists (4+)
• Comments explaining "what" code does
• Mixed abstraction levels

Style & Conventions

• Deviations from project patterns and conventions
• Clippy warnings that should be addressed
• Formatting inconsistencies (cargo fmt)

---

Report Format

Save to: docs/local/review-X.md

Where X starts at 1 and increments for each review iteration.

# Code Review: <branch-name>

**Date**: <timestamp>
**Base Branch**: <base-branch>
**Files Changed**: X files (+Y/-Z lines)

## Summary

[2-3 sentence overview of the changes and issues found]

## Critical Issues

[List items or "None"]

## Bugs & Logic Errors

[List items or "None"]

## Performance Concerns

[List items or "None"]

## Code Quality

[List items or "None"]

## Refactoring Opportunities

[List items or "None"]

## Style & Conventions

[List items or "None"]

---

**Status**: NEEDS FIXES

Issue Format

Each issue should include:

• File and line: src/engine.rs:45-52
• Description: Clear explanation of the problem

Example:

### `src/backend.rs:23-28`

**Unnecessary clone in hot path**

`prompt.clone()` allocates a new String on every backend invocation. Since `invoke()` takes `&str`, pass a reference instead.

---

Output

When APPROVED:

APPROVED

No issues found. Code is ready for PR.

When issues found:

NEEDS FIXES

Found X issues. Report saved to: docs/local/review-X.md