Code review
Review agent-generated code for spec compliance, test quality, architectural alignment, and agent anti-patterns. Use when a feature branch is ready for review, before creating a PR.From its SKILL.md
npx -y skills add atalariq/skills --skill code-reviewAssembled from the repository path, not quoted from the project. Check it against their README if it does not work.
One thing to look at
- 1 stars1 stars. Stars are a popularity signal and not a quality one, but at this level it is likely that nobody has read this closely except its author, and you would be relying on your own review.
SKILL.md
2.7 KB, as published. Nobody here has run it
Code Review — Agent Output Gate
Review agent-generated code before merging. Focus on four areas specific to agentic output.
Process
1. Read context
- The issue or PRD this branch addresses
CONTEXT.md— domain vocabulary and conventions- Recent ADRs in
docs/adr/ - The diff (
git diff main...HEADorgit log main..HEAD --oneline)
2. Review against checklist
Spec compliance
- Does the code solve the stated problem? (Not a different problem, not a subset.)
- Are there edge cases from the spec that are unhandled?
- Are there features NOT in the spec that were added? (Speculative feature = flag.)
Test quality
- Do tests verify behavior through public interfaces? (Not implementation details.)
- Would tests survive a refactor that doesn't change behavior?
- Are there tests for critical paths, or only happy paths?
- Do the tests actually fail when the behavior is broken? (Try the deletion test on the implementation.)
Architectural alignment
- Are new types/functions named consistently with
CONTEXT.md? - Do architectural patterns match existing code? (Or is there a novel pattern without a justifying ADR?)
- Are there new dependencies that should be flagged?
Agent anti-patterns
- Dead code: stubs, unused exports, commented-out code, variables declared+mutated but never read
- Over-engineering: abstractions for hypothetical future needs
- Hallucinated APIs: functions/types that don't exist in the project
- Inconsistent style: formatting, naming, import conventions that drift from the rest of the codebase
- XSS via escapeHtml fail:
escapeHtmldefined withdiv.innerHTML = s(decodes, not escapes) instead ofdiv.textContent = s. Or escapeHtml defined but never called on untrusted data injected into innerHTML. Flag BOTH: wrong implementation + missing call sites. The fix uses textContent for true escaping, and separates trusted HTML (authored by us) from untrusted data.
3. Report
Classify each finding:
| Severity | Meaning | Action |
|---|---|---|
| 🔴 Critical | Breaks spec, test doesn't verify behavior, or hallucinated API | Must fix before merge |
| 🟡 Warning | Over-engineering, naming drift, missing edge case | Should fix, discuss if unsure |
| ⚪ Nit | Style preference, minor cleanup | Optional, up to author |
4. Outcome
- No criticals → "Approved. Address warnings at your discretion."
- Criticals found → "Blocked. Fix [N] critical issues before merge."
- Mixed → "Fix criticals, discuss warnings."