code-review

name: code-review description: Review generated code for style, security, and architecture issues; suggest refactorings and performance improvements. Use after implementation is complete, before merging changes, or when refactoring existing code. Follows OWASP guidelines, SOLID principles, and best practices for maintainable code. license: MIT metadata: author: AC Framework version: "1.0"

Code Review

Comprehensive code analysis for quality, security, maintainability, and performance optimization.

When to Use This Skill

Use this skill when:

Reviewing code before merging (pre-merge review)
After implementation is complete in openspec-apply-change
Refactoring existing code for improvement
Auditing code for security vulnerabilities
Optimizing performance-critical sections
Ensuring adherence to project standards

Instructions

Step 1: Initial Assessment

Read the code being reviewed
Understand the context:
- What problem does this solve?
- What are the requirements?
- Are there related files/components?
Identify the scope:
- Lines changed
- Files modified
- Dependencies introduced

Step 2: Security Review (OWASP)

Check for common security vulnerabilities:

Input Validation:

All user inputs are validated/sanitized
No SQL injection vulnerabilities
No command injection risks
Path traversal prevented
File uploads restricted

Authentication & Authorization:

Proper authentication checks
Role-based access control (RBAC)
Session management secure
No hardcoded secrets or credentials
API keys not exposed in client code

Data Protection:

Sensitive data encrypted
Proper handling of PII (Personally Identifiable Information)
Secure communication (HTTPS/TLS)
No sensitive data in logs

Output Encoding:

XSS prevention (output encoding)
CSRF protection
Content Security Policy headers

Step 3: Architecture Review

Evaluate against design principles:

SOLID Principles:

Single Responsibility: Each class/function has one reason to change
Open/Closed: Open for extension, closed for modification
Liskov Substitution: Subtypes are substitutable for base types
Interface Segregation: Small, focused interfaces
Dependency Inversion: Depend on abstractions, not concretions

Design Patterns:

Appropriate use of patterns
No over-engineering
Consistent with project conventions

Coupling & Cohesion:

Low coupling between modules
High cohesion within modules
Clear module boundaries

Step 4: Code Quality Review

Readability:

Clear naming (variables, functions, classes)
Consistent formatting
Appropriate comments (why, not what)
No magic numbers/strings
Functions are focused and small

Maintainability:

DRY (Don't Repeat Yourself)
No code duplication
Easy to test
Documentation exists
Error handling comprehensive

Error Handling:

Exceptions are caught appropriately
Error messages are user-friendly
Failures are logged
Graceful degradation

Step 5: Performance Review

Efficiency:

No unnecessary computations
Efficient data structures
Database queries optimized
No N+1 query problems
Caching used appropriately

Resource Usage:

Memory leaks prevented
Large objects disposed properly
No blocking operations on main thread
Async/await used correctly

Step 6: Testing Review

Unit tests exist for new logic
Tests cover edge cases
Integration tests for API contracts
Test names are descriptive
Mocks are appropriate
Test coverage is adequate (>80%)

Step 7: Documentation Review

Code comments explain complex logic
Public APIs are documented (JSDoc, docstrings)
README updated if needed
Architecture Decision Records (ADRs) for significant changes
Changelog entries added

Step 8: Compile Findings

Organize issues by severity:

## Code Review Report

### Critical (Block Merge)
1. [Security vulnerability description]
   - File: path/to/file.ext:line
   - Issue: [detailed explanation]
   - Fix: [recommended solution]

### High (Should Fix)
1. [Performance or architecture issue]
   ...

### Medium (Nice to Have)
1. [Style or readability improvement]
   ...

### Low (Suggestions)
1. [Minor improvements]
   ...

### Positive Feedback
- [What was done well]

Step 9: Provide Actionable Feedback

For each issue:

Explain the problem (why it's an issue)
Show the problematic code
Suggest a fix (with code example if possible)
Reference best practices (OWASP, SOLID, etc.)

Step 10: Follow-up

Track fixes - Ensure all critical/high issues are addressed
Re-review if significant changes are made
Update documentation based on review findings
Share learnings - Common issues can inform team guidelines

Integration with OpenSpec

Run after openspec-apply-change completes
Block merge in openspec-verify-change until critical issues resolved
Link review findings to tasks in tasks.md
Document architectural decisions in design.md

Guardrails

Be constructive - Focus on the code, not the person
Explain why - Don't just say "change this", explain the reason
Balance thoroughness with pragmatism - Not everything needs to be perfect
Respect existing patterns - Don't force changes unless there's a good reason
Consider the context - Startup MVP vs. enterprise system have different standards

Review Checklist Template

## Security
- [ ] Input validation
- [ ] Authentication/authorization
- [ ] No secrets in code
- [ ] XSS/CSRF protection

## Architecture
- [ ] SOLID principles
- [ ] Appropriate patterns
- [ ] Low coupling

## Quality
- [ ] Naming clarity
- [ ] No duplication
- [ ] Error handling
- [ ] Comments where needed

## Performance
- [ ] Efficient algorithms
- [ ] No N+1 queries
- [ ] Proper async usage

## Testing
- [ ] Unit tests
- [ ] Integration tests
- [ ] Edge cases covered

## Documentation
- [ ] Code comments
- [ ] API docs
- [ ] README updated

Requirements

Access to the codebase
Understanding of project conventions
Knowledge of security best practices (OWASP)
Familiarity with design patterns
Understanding of performance optimization