Code Review Skill
Reviews all new and modified files for correctness, security, performance, and best practices.
Severity Levels
P0 — Critical (Must Fix Before Merge)
- Security vulnerabilities (injection, auth bypass, hardcoded secrets, insecure storage)
- Data corruption or data loss risks
- Crashes or critical runtime errors
- Memory leaks (undisposed controllers, streams)
- Main thread blocking operations
- Breaking changes without migration
P1 — Important (Should Fix)
- Logic errors in edge cases
- Missing error handling for critical paths
- Significant performance issues (unnecessary rebuilds, expensive build())
- Missing input validation
- Pattern violations causing maintenance burden
- Missing keys in dynamic lists
- Sensitive data logged
P2 — Nice-to-have (Consider)
- Code style inconsistencies
- Minor performance improvements (missing const)
- Documentation gaps
- Refactoring opportunities
Review Process
Step 1: Gather Context
1. Identify files to review (new + modified, from implementation summary)
2. Read AGENTS.md patterns
3. Read original requirements if available
Step 2: Review Each File
For each file, run all five checks below.
Check 1: Correctness
□ Business logic implements requirements correctly
□ Calculations and conditions are accurate
□ Null/empty/boundary cases handled
□ State transitions are correct (no stale state)
□ Async operations handled properly (await, error propagation)
□ Race conditions considered
Check 2: Security
P0 — Critical Security
□ No hardcoded credentials, API keys, or secrets
□ No SQL injection (parameterized queries only)
□ No command injection (shell input unescaped)
□ Authentication check present on protected operations
□ Authorization / permission check on all resource access
□ Sensitive data not stored in plain text (use secure storage)
□ No path traversal vulnerabilities (validate file paths)
P1 — Important Security
□ User input validated before use
□ Error messages don't leak internal info (no raw stack traces to UI)
□ Sensitive data (tokens, passwords) not in logs
□ HTTPS enforced for all sensitive API calls
□ Weak cryptography avoided (no MD5/SHA1 for security purposes)
□ CSRF protection on state-changing operations (web)
P2 — Minor Security
□ Verbose error messages minimized
□ Security headers present (web)
□ Input length limits on text fields
Check 3: Performance
P0 — Critical Performance
□ No infinite loops or unbounded recursion
□ All controllers/streams/subscriptions disposed (no memory leaks)
□ No blocking synchronous I/O on main thread (readAsStringSync, etc.)
□ No O(n²) algorithms on large datasets
□ No unbounded list/map growth
P1 — Important Performance
□ No unnecessary widget rebuilds (use const where possible)
□ No expensive operations inside build() (sorting, parsing, filtering)
□ Dynamic lists use keys (ValueKey or ObjectKey)
□ Long lists use lazy builders (ListView.builder, not ListView)
□ Same calculation not repeated multiple times without caching
□ Widget methods (_buildX) replaced with separate StatelessWidget classes
P2 — Minor Performance
□ const constructors used where possible
□ String concatenation in loops uses StringBuffer
□ Repeated network calls consider local caching
Check 4: Pattern Compliance (AGENTS.md)
□ State management follows project pattern (check AGENTS.md)
□ Models use project model pattern
□ Styling uses project constants (no hardcoded colors, sizes, text styles)
□ Widget structure follows project convention
□ File organization follows project structure
□ Naming conventions correct
Check 5: Test Coverage
□ Critical logic has unit tests
□ Edge cases covered in tests
□ Error paths tested
□ UI states tested (loading, error, empty, success)
□ No real API calls in tests (proper mocking)
Output Template
Save to OUTPUT_DIR/review-{feature}.md:
# Code Review: {Feature Name}
## Summary
| Severity | Count | Status |
|----------|-------|--------|
| P0 (Critical) | {n} | {BLOCKING / CLEAR} |
| P1 (Important) | {n} | |
| P2 (Nice-to-have) | {n} | |
**Verdict**: {APPROVE / REQUEST CHANGES}
---
## P0 — Critical Issues
{If none: "No critical issues found."}
### P0-1: {Issue Title}
- **File**: `path/to/file.dart:{line}`
- **Category**: {Security / Performance / Correctness}
- **Issue**: {description}
- **Impact**: {what goes wrong}
- **Fix**: {how to fix}
---
## P1 — Important Issues
### P1-1: {Issue Title}
- **File**: `path:{line}`
- **Category**: {category}
- **Issue**: {description}
- **Fix**: {suggestion}
---
## P2 — Nice-to-have
### P2-1: {Issue Title}
- **File**: `path:{line}`
- **Suggestion**: {improvement}
---
## Pattern Compliance
| Pattern | Status | Notes |
|---------|--------|-------|
| State Management | ✅/❌ | {notes} |
| Model Pattern | ✅/❌ | {notes} |
| Styling | ✅/❌ | {notes} |
| Widget Structure | ✅/❌ | {notes} |
| File Organization | ✅/❌ | {notes} |
---
## Files Reviewed
| File | Issues |
|------|--------|
| `path` | P0: {n}, P1: {n}, P2: {n} |
---
## Verdict
{APPROVED / APPROVED WITH COMMENTS / CHANGES REQUESTED}
{Reasoning. List must-fix items if changes requested.}
Quick Commands
/code-review — Review recent changes (all new/modified files)
/code-review path/to/file — Review specific file
/code-review --security — Security-focused review only
/code-review --staged — Review staged git changes