name: reviewing-code description: Reviews code for correctness, security, reliability, performance, and quality. Use when asked to review code, a PR, a diff, changes, or when the user says "review", "code review", or "look over this". license: AGPL-3.0-or-later metadata: author: Amolith amolith@secluded.site
Review code thoroughly, providing constructive and actionable feedback.
Workflow
- Gather context: Understand intent (PR description, commit message, surrounding code, PR comments or email thread)
- Identify risk areas: Auth, payments, migrations, concurrency, external integrations
- Analyze systematically: Prioritize correctness and reliability over style
- Report findings: Use output format; require evidence for each issue
Review Areas
Correctness / Behavior (highest priority)
- Does the change match stated intent? Any unintended behavior changes?
- Edge cases: null/empty, off-by-one, boundary conditions, unicode/encoding
- State transitions: can objects reach impossible states?
- Concurrency: races, ordering assumptions, atomicity, async cancellation
- Backwards compatibility: API contracts, wire formats, schema changes
- Error semantics: fail open vs closed? Errors propagated correctly?
Security
Flag only with concrete evidence (source → sink flow or missing boundary check).
- Input validation and sanitization
- Authentication and authorization at boundaries
- Injection: SQL, XSS, command, SSRF, path traversal
- Insecure deserialization, open redirects
- Crypto misuse: weak randomness, homegrown crypto, token handling
- Secrets/credentials: hardcoded or logged
- Multi-tenant isolation, authorization gaps
- New dependencies: overly broad permissions, supply chain risk
Reliability / Operability
- Timeouts, retries, backoff, circuit breaking
- Idempotency (jobs, webhooks, payments)
- Resource cleanup: files, sockets, connections, cancellation
- Logging/metrics: enough to debug? No secrets/PII logged?
- Configuration: safe defaults, env var handling
Data / Persistence
- Transaction boundaries, isolation, partial writes
- Schema/migrations: locking, backfills, rollback safety
- Query efficiency: N+1, missing indexes, incorrect joins
Performance
Flag only with clear hotspot evidence (loops over large input, repeated I/O, known N+1).
- Algorithm complexity and bottlenecks
- Memory allocation patterns
- Unnecessary computations
Code Quality
Style issues are nits—only mention if they violate local conventions or materially harm readability.
- Naming and style consistency with codebase
- Function/class size and single responsibility
- Error handling completeness
- Dead code, unused imports
Testing
- Coverage for new/changed code paths
- Edge cases and error paths tested
- Test clarity
Output Format
## 🔴 Must Fix
### [Issue title]
**Location:** `path/to/file.ts:42`
**Impact**
[What breaks or could break]
**Evidence**
[Concrete code path or condition that triggers this]
**Fix**
[Suggested solution]
## 🟠 Should Fix
[Medium confidence or meaningful impact]
## 🟡 Consider
[Trade-offs, optional improvements]
## 💬 Questions
[When intent is unclear: ask, don't assert]
## 📝 Nits
[Style-only, keep brief or omit]
## ✅ Strengths
[What's done well, keep brief]
Omit empty sections. For small clean changes, "Looks good" suffices.
Anti-Noise Rules
- Correctness over style: Don't request refactors unless they reduce bug risk
- Evidence required: Each must-fix needs a concrete trigger condition
- Ask vs assert: When uncertain, ask a question and describe the risk
- No hypotheticals: Avoid speculative performance/security concerns without evidence
- Respect local conventions: Check surrounding code before flagging style