Code Review Skill
Goal
Produce a structured, actionable code review covering: security vulnerabilities, performance issues, architectural violations, and code style — in that priority order.
Steps
Gather diff
- Run
git diff main...HEAD (or the target branch) to see all changes
- Note the files changed and their layers (domain, application, infrastructure, etc.)
Security review (highest priority)
- Scan for: SQL injection, XSS, CSRF, path traversal, command injection
- Check: secrets or API keys hardcoded in code
- Check: authentication/authorization logic bypasses
- Check: input validation at system boundaries (HTTP, file upload, CLI)
- Check:
npm audit / composer audit output for new vulnerable deps
- Flag severity: CRITICAL | HIGH | MEDIUM | LOW
Architecture review
- Verify layer imports: does any inner layer import from an outer layer?
- Check for circular dependencies (run
madge --circular src or deptrac)
- Check: are new classes/functions in the correct layer?
- Check: does any new code violate AGENTS.md architecture rules?
Performance review
- Identify N+1 query patterns in any database access code
- Check: are expensive operations cached where appropriate?
- Check: are there unnecessary synchronous operations that could be async?
- Check: unbounded loops over large datasets without pagination
Code quality review
- Check: Cyclomatic complexity (functions > 10 → flag for refactor)
- Check: function/class length (functions > 30 lines, files > 300 lines → flag)
- Check: code duplication (same logic > 3 occurrences → suggest abstraction)
- Check: naming (variables, functions, classes follow project conventions)
- Check: dead code (unused variables, unreachable branches, unused exports)
Test coverage review
- Check: are new public functions covered by tests?
- Check: are new edge cases covered (null inputs, error paths, boundaries)?
- Check: no test bypasses (
.skip, .only left in production branch)
Produce structured report
## Code Review Report
### Security Findings
- [CRITICAL/HIGH/MEDIUM/LOW] file.ts:42 — Description + fix recommendation
### Architecture Violations
- file.ts:15 imports from infrastructure layer — violates Clean Architecture
### Performance Concerns
- N+1 query pattern in UserService.getAll() — add eager loading
### Code Quality
- getUserData() has complexity 14 — extract validation into separate function
### Tests
- Missing test for null email edge case in UserValidator
### Approved Changes
- [list of changes that look good]
### Summary
MERGE: [YES/NO/WITH_CHANGES]
Blocking issues: [count]
Constraints
- Security findings are always blocking (must be fixed before merge)
- Architecture violations are always blocking
- Performance and quality findings may be non-blocking based on severity
- Never approve a PR with hardcoded secrets or critical CVEs
Output Format
Structured markdown report with severity-tagged findings and specific file:line references.
1---2name: code-review-323description: Systematic code review covering security, performance, architecture, and style4---5
6# Code Review Skill
7
8## Goal
9Produce a structured, actionable code review covering: security vulnerabilities, performance issues, architectural violations, and code style — in that priority order.
10
11## Steps
12
131. **Gather diff**
14 - Run `git diff main...HEAD` (or the target branch) to see all changes
15 - Note the files changed and their layers (domain, application, infrastructure, etc.)
16
172. **Security review** (highest priority)
18 - Scan for: SQL injection, XSS, CSRF, path traversal, command injection
19 - Check: secrets or API keys hardcoded in code
20 - Check: authentication/authorization logic bypasses
21 - Check: input validation at system boundaries (HTTP, file upload, CLI)
22 - Check: `npm audit` / `composer audit` output for new vulnerable deps
23 - Flag severity: CRITICAL | HIGH | MEDIUM | LOW
24
253. **Architecture review**
26 - Verify layer imports: does any inner layer import from an outer layer?
27 - Check for circular dependencies (run `madge --circular src` or `deptrac`)
28 - Check: are new classes/functions in the correct layer?
29 - Check: does any new code violate AGENTS.md architecture rules?
30
314. **Performance review**
32 - Identify N+1 query patterns in any database access code
33 - Check: are expensive operations cached where appropriate?
34 - Check: are there unnecessary synchronous operations that could be async?
35 - Check: unbounded loops over large datasets without pagination
36
375. **Code quality review**
38 - Check: Cyclomatic complexity (functions > 10 → flag for refactor)
39 - Check: function/class length (functions > 30 lines, files > 300 lines → flag)
40 - Check: code duplication (same logic > 3 occurrences → suggest abstraction)
41 - Check: naming (variables, functions, classes follow project conventions)
42 - Check: dead code (unused variables, unreachable branches, unused exports)
43
446. **Test coverage review**
45 - Check: are new public functions covered by tests?
46 - Check: are new edge cases covered (null inputs, error paths, boundaries)?
47 - Check: no test bypasses (`.skip`, `.only` left in production branch)
48
497. **Produce structured report**
50 ```markdown
51 ## Code Review Report
52
53 ### Security Findings
54 - [CRITICAL/HIGH/MEDIUM/LOW] file.ts:42 — Description + fix recommendation
55
56 ### Architecture Violations
57 - file.ts:15 imports from infrastructure layer — violates Clean Architecture
58
59 ### Performance Concerns
60 - N+1 query pattern in UserService.getAll() — add eager loading
61
62 ### Code Quality
63 - getUserData() has complexity 14 — extract validation into separate function
64
65 ### Tests
66 - Missing test for null email edge case in UserValidator
67
68 ### Approved Changes
69 - [list of changes that look good]
70
71 ### Summary
72 MERGE: [YES/NO/WITH_CHANGES]
73 Blocking issues: [count]
74 ```
75
76## Constraints
77- Security findings are always blocking (must be fixed before merge)
78- Architecture violations are always blocking
79- Performance and quality findings may be non-blocking based on severity
80- Never approve a PR with hardcoded secrets or critical CVEs
81
82## Output Format
83Structured markdown report with severity-tagged findings and specific file:line references.
majiayu000