diff --git a/README.md b/README.md index f462f322ed3b47659bb96c37e271dc4d6f1c88a3..625db83d1b3bd11d0095597b6b24484dadf61e9a 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,9 @@ Refer to [§ Token stats](#token-stats) for a detailed breakdown of each skill's programming. Covers both full applications and one-off scripts. - [resolving-secrets](skills/resolving-secrets/SKILL.md): Wraps shell commands to resolve secret references in environment variables. +- [reviewing-code](skills/reviewing-code/SKILL.md): Reviews code for + correctness, security, reliability, performance, and quality. Use when + asked to review code, a PR, a diff, or changes. ## Installation @@ -259,6 +262,15 @@ Token breakdown: ─────────────────────────────────────────────── Total: 2088 tokens +=== reviewing-code === + +Token breakdown: + Name: 18 tokens + Description: 125 tokens + Body: 1880 tokens (121 lines) + ─────────────────────────────────────────────── + Total: 2023 tokens + === scripting-with-go === Token breakdown: @@ -285,10 +297,10 @@ Token breakdown: SUMMARY ============================================================ -Skills: 11 -Metadata: 1543 tokens -Combined bodies: 16672 tokens -Overall: 121864 tokens +Skills: 12 +Metadata: 1686 tokens +Combined bodies: 18552 tokens +Overall: 123887 tokens Validation errors: 0 Largest skills (by total tokens): diff --git a/skills/reviewing-code/SKILL.md b/skills/reviewing-code/SKILL.md new file mode 100644 index 0000000000000000000000000000000000000000..7de45069288b423f547b5e860563f701ea3632bf --- /dev/null +++ b/skills/reviewing-code/SKILL.md @@ -0,0 +1,129 @@ +--- +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 +--- + +Review code thoroughly, providing constructive and actionable feedback. + +## Workflow + +1. **Gather context**: Understand intent (PR description, commit message, surrounding code, PR comments or email thread) +2. **Identify risk areas**: Auth, payments, migrations, concurrency, external integrations +3. **Analyze systematically**: Prioritize correctness and reliability over style +4. **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 + +```markdown +## 🔴 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