diff --git a/README.md b/README.md index 1df5ea509c1869a758bf062343d45cfb34485cc4..eb79699c886683d05b0762bb3b308633f4755373 100644 --- a/README.md +++ b/README.md @@ -68,9 +68,9 @@ token count, plus overall metadata usage. I've used and tested them most with filling the main context window. - [resuming-work-through-lunatask](skills/resuming-work-through-lunatask/SKILL.md): Resumes deferred work from [Lunatask] handoff notes via [lune]. -- [reviewing-code](skills/reviewing-code/SKILL.md): Reviews code for - correctness, security, reliability, performance, and quality. Covers PRs, - diffs, and general change review. +- [reviewing-code](skills/reviewing-code/SKILL.md): Reviews changes, commits, + PRs, and patches using Kodus, CodeRabbit, or Amp, preferring whichever is + available in that order. - [scripting-with-go](skills/scripting-with-go/SKILL.md): Creates executable Go scripts using a shell trick (not a true shebang). For automation and tooling outside of Go projects. @@ -330,7 +330,7 @@ Token breakdown: Token breakdown: Name: 25 tokens Description: 93 tokens - Body: 2212 tokens (72 lines) + Body: 2884 tokens (89 lines) References: ht.md 1063 tokens httpie.md 1067 tokens @@ -338,7 +338,7 @@ Token breakdown: python.md 1468 tokens wget.md 1216 tokens ─────────────────────────────────────────────── - Total: 8641 tokens + Total: 9313 tokens === rebasing-with-git === @@ -375,10 +375,10 @@ Token breakdown: Token breakdown: Name: 18 tokens - Description: 125 tokens - Body: 1880 tokens (121 lines) + Description: 132 tokens + Body: 989 tokens (46 lines) ─────────────────────────────────────────────── - Total: 2023 tokens + Total: 1139 tokens === scripting-with-go === @@ -438,9 +438,9 @@ SUMMARY ============================================================ Skills: 21 -Metadata: 3060 tokens -Combined bodies: 57887 tokens -Overall: 183063 tokens +Metadata: 3067 tokens +Combined bodies: 57668 tokens +Overall: 182851 tokens Validation errors: 0 Largest skills (by total tokens): @@ -448,7 +448,7 @@ Largest skills (by total tokens): 2. ast-grep 14533 tokens 3. frontend-accessibility 12922 tokens 4. authoring-skills 10050 tokens - 5. notifying-through-ntfy 8641 tokens + 5. notifying-through-ntfy 9313 tokens ``` --- diff --git a/skills/reviewing-code/SKILL.md b/skills/reviewing-code/SKILL.md index 7de45069288b423f547b5e860563f701ea3632bf..a0bad0aaf536d5671360dae5eb3c2ae06d95c673 100644 --- a/skills/reviewing-code/SKILL.md +++ b/skills/reviewing-code/SKILL.md @@ -1,129 +1,54 @@ --- 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". +description: Rigorously reviews changes, commits, PRs, and patches. Use when asked to review code, a PR, a diff, changes, or when the user says "review", "code review", or "look over this" or similar. license: AGPL-3.0-or-later metadata: author: Amolith --- -Review code thoroughly, providing constructive and actionable feedback. +Run a code review using the preferred tool. -## Workflow +## Selection -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 +If a native `code review` tool is already available, use it directly. Otherwise, check `command -v` for CLI tools and use the first found: -## Review Areas +1. `kodus` +2. `coderabbit` — fall back to the next option if rate-limited +3. `amp` -### Correctness / Behavior (highest priority) +If none are available, tell the user and stop. -- 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? +## CLI usage -### Security +```bash +kodus review --prompt-only # uncommitted changes +kodus review --prompt-only --staged +kodus review --prompt-only --branch main +kodus review --prompt-only --commit abc123 +kodus review --prompt-only src/auth.ts -Flag only with concrete evidence (source → sink flow or missing boundary check). +coderabbit review --prompt-only # all changes +coderabbit review --prompt-only --type committed +coderabbit review --prompt-only --type uncommitted +coderabbit review --prompt-only --base main -- 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 +amp review # uncommitted changes +amp review "HEAD~1" +amp review "main...HEAD" +amp review -f src/auth.ts +amp review -i "explicit, detailed instructions to focus on something(s) in particular" +``` -[Style-only, keep brief or omit] +## Output -## ✅ Strengths +Display issues as a numbered list: -[What's done well, keep brief] +``` +1. category (severity) - [file-basename](file-path#range): verbatim finding from the code review tool ``` -Omit empty sections. For small clean changes, "Looks good" suffices. - -## Anti-Noise Rules +Then ask: +- If other code review tools are available: "Would you like me to fix any of these issues, rerun that, run it differently, or check with [Kodus, CodeRabbit, or Amp] as well?", only listing the available options +- If only the one is: "Would you like me to fix any of these issues, rerun that, or run it differently?" -- **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 +If told to fix anything, do so, then go through the same review flow again and stop to present the follow-up review.