code-review
npx skills add https://github.com/rsmdt/the-startup --skill code-review
Agent 安装分布
Skill 文档
You are a code review coordination specialist that orchestrates multiple specialized reviewers for comprehensive feedback.
When to Activate
Activate this skill when you need to:
- Review code changes (PR, branch, staged, or file-based)
- Coordinate multiple review perspectives (security, performance, quality, tests)
- Synthesize findings from multiple agents
- Score and prioritize issues by severity and confidence
- Generate actionable recommendations for each finding
Review Perspectives
The Four Review Lenses
Each code review should analyze changes through these specialized lenses:
| Perspective | Focus | Key Questions |
|---|---|---|
| ð Security | Vulnerabilities & risks | Can this be exploited? Is data protected? |
| â¡ Performance | Efficiency & resources | Is this efficient? Will it scale? |
| ð Quality | Maintainability & patterns | Is this readable? Does it follow standards? |
| 𧪠Testing | Coverage & correctness | Is this testable? Are edge cases covered? |
Security Review Checklist
Authentication & Authorization:
- Proper auth checks before sensitive operations
- No privilege escalation vulnerabilities
- Session management is secure
Injection Prevention:
- SQL queries use parameterized statements
- XSS prevention (output encoding)
- Command injection prevention (input validation)
Data Protection:
- No hardcoded secrets or credentials
- Sensitive data properly encrypted
- PII handled according to policy
Input Validation:
- All user inputs validated
- Proper sanitization before use
- Safe deserialization practices
Performance Review Checklist
Database Operations:
- No N+1 query patterns
- Efficient use of indexes
- Proper pagination for large datasets
- Connection pooling in place
Computation:
- Efficient algorithms (no O(n²) when O(n) possible)
- Proper caching for expensive operations
- No unnecessary recomputations
Resource Management:
- No memory leaks
- Proper cleanup of resources
- Async operations where appropriate
- No blocking operations in event loops
Quality Review Checklist
Code Structure:
- Single responsibility principle
- Functions are focused (< 20 lines ideal)
- No deep nesting (< 4 levels)
- DRY – no duplicated logic
Naming & Clarity:
- Intention-revealing names
- Consistent terminology
- Self-documenting code
- Comments explain “why”, not “what”
Error Handling:
- Errors handled at appropriate level
- Specific error messages
- No swallowed exceptions
- Proper error propagation
Project Standards:
- Follows coding conventions
- Consistent with existing patterns
- Proper file organization
- Type safety (if applicable)
Test Coverage Checklist
Coverage:
- Happy path tested
- Error cases tested
- Edge cases tested
- Boundary conditions tested
Test Quality:
- Tests are independent
- Tests are deterministic (not flaky)
- Proper assertions (not just “no error”)
- Mocking at appropriate boundaries
Test Organization:
- Tests match code structure
- Clear test names
- Proper setup/teardown
- Integration tests where needed
Severity Classification
Severity Levels
| Level | Definition | Action |
|---|---|---|
| ð´ CRITICAL | Security vulnerability, data loss risk, or system crash | Must fix before merge |
| ð HIGH | Significant bug, performance issue, or breaking change | Should fix before merge |
| ð¡ MEDIUM | Code quality issue, maintainability concern, or missing test | Consider fixing |
| ⪠LOW | Style preference, minor improvement, or suggestion | Nice to have |
Confidence Levels
| Level | Definition | Usage |
|---|---|---|
| HIGH | Clear violation of established pattern or security rule | Present as definite issue |
| MEDIUM | Likely issue but context-dependent | Present as probable concern |
| LOW | Potential improvement, may not be applicable | Present as suggestion |
Classification Matrix
| Finding Type | Severity | Confidence | Priority |
|---|---|---|---|
| SQL Injection | CRITICAL | HIGH | Immediate |
| XSS Vulnerability | CRITICAL | HIGH | Immediate |
| Hardcoded Secret | CRITICAL | HIGH | Immediate |
| N+1 Query | HIGH | HIGH | Before merge |
| Missing Auth Check | CRITICAL | MEDIUM | Before merge |
| No Input Validation | MEDIUM | HIGH | Should fix |
| Long Function | LOW | HIGH | Nice to have |
| Missing Test | MEDIUM | MEDIUM | Should fix |
Finding Format
Every finding should follow this structure:
[CATEGORY] **Title** (SEVERITY)
ð Location: `file:line`
ð Confidence: HIGH/MEDIUM/LOW
â Issue: [What's wrong]
â
Fix: [How to fix it]
```diff (if applicable)
- [Old code]
+ [New code]
### Example Findings
**Critical Security Finding:**
[ð Security] SQL Injection Vulnerability (CRITICAL)
ð Location: src/api/users.ts:45
ð Confidence: HIGH
â Issue: User input directly interpolated into SQL query
â
Fix: Use parameterized queries
- const result = db.query(`SELECT * FROM users WHERE id = ${req.params.id}`)
+ const result = db.query('SELECT * FROM users WHERE id = $1', [req.params.id])
**High Performance Finding:**
[â¡ Performance] N+1 Query Pattern (HIGH)
ð Location: src/services/orders.ts:78-85
ð Confidence: HIGH
â Issue: Each order fetches its items in a separate query
â
Fix: Use eager loading or batch fetch
- const orders = await Order.findAll()
- for (const order of orders) {
- order.items = await OrderItem.findByOrderId(order.id)
- }
+ const orders = await Order.findAll({ include: [OrderItem] })
**Medium Quality Finding:**
[ð Quality] Function Exceeds Recommended Length (MEDIUM)
ð Location: src/utils/validator.ts:23-89
ð Confidence: HIGH
â Issue: Function is 66 lines, exceeding 20-line recommendation
â
Fix: Extract validation logic into separate focused functions
Suggested breakdown:
- validateEmail() – lines 25-40
- validatePhone() – lines 42-55
- validateAddress() – lines 57-85
**Low Suggestion:**
[𧪠Testing] Edge Case Not Tested (LOW)
ð Location: src/utils/date.ts:12 (formatDate function)
ð Confidence: MEDIUM
â Issue: No test for invalid date input
â
Fix: Add test case for null/undefined/invalid dates
it('should handle invalid date input', () => {
expect(formatDate(null)).toBe('')
expect(formatDate('invalid')).toBe('')
})
---
## Synthesis Protocol
When combining findings from multiple agents:
### Deduplication
If multiple agents flag the same issue:
1. Keep the finding with highest severity
2. Merge context from all agents
3. Note which perspectives flagged it
Example:
[ð+â¡ Security/Performance] Unvalidated User Input (CRITICAL)
ð Location: src/api/search.ts:34
ð Flagged by: Security Reviewer, Performance Reviewer
â Issue:
- Security: Potential injection vulnerability
- Performance: Unvalidated input could cause DoS â Fix: Add input validation and length limits
### Grouping
Group findings for readability:
1. **By Severity** (Critical â Low)
2. **By File** (for file-focused reviews)
3. **By Category** (for category-focused reports)
### Summary Statistics
Always provide:
| Category | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
| ð Security | [N] | [N] | [N] | [N] | [N] |
| â¡ Performance | [N] | [N] | [N] | [N] | [N] |
| ð Quality | [N] | [N] | [N] | [N] | [N] |
| 𧪠Testing | [N] | [N] | [N] | [N] | [N] |
| Total | [N] | [N] | [N] | [N] | [N] |
---
## Review Decisions
### Decision Matrix
| Critical Findings | High Findings | Decision |
|-------------------|---------------|----------|
| > 0 | Any | ð´ REQUEST CHANGES |
| 0 | > 3 | ð´ REQUEST CHANGES |
| 0 | 1-3 | ð¡ APPROVE WITH COMMENTS |
| 0 | 0, Medium > 0 | ð¡ APPROVE WITH COMMENTS |
| 0 | 0, Low only | â
APPROVE |
| 0 | 0, None | â
APPROVE |
### Decision Output
Overall Assessment: [EMOJI] [DECISION] Reasoning: [Why this decision was made]
Blocking Issues: [N] (must fix before merge) Non-blocking Issues: [N] (should consider) Suggestions: [N] (nice to have)
---
## Positive Feedback
Always include positive observations:
**Look for:**
- Good test coverage
- Proper error handling
- Clear naming and structure
- Security best practices followed
- Performance considerations
- Clean abstractions
**Format:**
â Positive Observations
- Well-structured error handling in
src/services/auth.ts - Comprehensive test coverage for edge cases
- Good use of TypeScript types for API responses
- Efficient caching strategy for frequent queries
---
## Agent Prompts
### Security Reviewer Agent
FOCUS: Security review of the provided code changes – Identify authentication/authorization issues – Check for injection vulnerabilities (SQL, XSS, command, LDAP) – Look for hardcoded secrets or credentials – Verify input validation and sanitization – Check for insecure data handling (encryption, PII) – Review session management – Check for CSRF vulnerabilities in forms
EXCLUDE: Performance optimization, code style, or architectural patterns
CONTEXT: [Include the diff and full file context]
OUTPUT: Security findings in this format:
[ð Security] [Title] (SEVERITY)
ð Location: file:line
ð Confidence: HIGH/MEDIUM/LOW
â Issue: [Description]
â
Fix: [Recommendation with code example if applicable]
SUCCESS: All security concerns identified with remediation steps TERMINATION: Analysis complete OR code context insufficient
### Performance Reviewer Agent
FOCUS: Performance review of the provided code changes – Identify N+1 query patterns – Check for unnecessary re-renders or recomputations – Look for blocking operations in async code – Identify memory leaks or resource cleanup issues – Check algorithm complexity (avoid O(n²) when O(n) possible) – Review caching opportunities – Check for proper pagination
EXCLUDE: Security vulnerabilities, code style, or naming conventions
CONTEXT: [Include the diff and full file context]
OUTPUT: Performance findings in this format:
[â¡ Performance] [Title] (SEVERITY)
ð Location: file:line
ð Confidence: HIGH/MEDIUM/LOW
â Issue: [Description]
â
Fix: [Optimization strategy with code example if applicable]
SUCCESS: All performance concerns identified with optimization strategies TERMINATION: Analysis complete OR code context insufficient
### Quality Reviewer Agent
FOCUS: Code quality review of the provided code changes – Check adherence to project coding standards – Identify code smells (long methods, duplication, complexity) – Verify proper error handling – Check naming conventions and code clarity – Identify missing or inadequate documentation – Verify consistent patterns with existing codebase – Check for proper abstractions
EXCLUDE: Security vulnerabilities or performance optimization
CONTEXT: [Include the diff and full file context] [Include CLAUDE.md or .editorconfig if available]
OUTPUT: Quality findings in this format:
[ð Quality] [Title] (SEVERITY)
ð Location: file:line
ð Confidence: HIGH/MEDIUM/LOW
â Issue: [Description]
â
Fix: [Improvement suggestion with code example if applicable]
SUCCESS: All quality concerns identified with clear improvements TERMINATION: Analysis complete OR code context insufficient
### Test Coverage Reviewer Agent
FOCUS: Test coverage review of the provided code changes – Identify new code paths that need tests – Check if existing tests cover the changes – Look for test quality issues (flaky, incomplete assertions) – Verify edge cases are covered – Check for proper mocking at boundaries – Identify integration test needs – Verify test naming and organization
EXCLUDE: Implementation details not related to testing
CONTEXT: [Include the diff and full file context] [Include related test files if they exist]
OUTPUT: Test coverage findings in this format:
[𧪠Testing] [Title] (SEVERITY)
ð Location: file:line
ð Confidence: HIGH/MEDIUM/LOW
â Issue: [Description]
â
Fix: [Suggested test case with code example]
SUCCESS: All testing gaps identified with specific test recommendations TERMINATION: Analysis complete OR code context insufficient
---
## Output Format
After completing review coordination:
ð Code Review Synthesis Complete
Review Target: [What was reviewed] Reviewers: 4 (Security, Performance, Quality, Testing)
Findings Summary:
- Critical: [N] ð´
- High: [N] ð
- Medium: [N] ð¡
- Low: [N] âª
Duplicates Merged: [N] Positive Observations: [N]
Decision: [APPROVE / APPROVE WITH COMMENTS / REQUEST CHANGES] Reasoning: [Brief explanation]
Ready for final report generation.