agentleFS
NoaiRoxNoaiRox/specialist-agent/skills/codereview/SKILL.md

codereview

Skill2 starsChanged 9 months agoMITView on GitHub

Use when you need a comprehensive code review combining architecture, security, and test perspectives - especially before merging, releasing, or after major changes.

  • Reads credentials

5 other projects use a version of this.

Adopted by

Most recently taken up by

The file

Show its rules and text

We split this skill into the rules it gives an agent

22 rules. 0 are common, 17 are distinctive, and 5 are very specific to this project.

  • 0

    Common

    Most of the 4 edited versions we read keep these.

  • 17

    Distinctive

    This author's own choices. Worth a look.

  • 5

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  1. Distinctive:

    Run a comprehensive code review that dispatches 3 specialized perspectives concurrently - architecture, security, and testing - then …

    line 11
  2. Distinctive:

    Before merging a pull request

    line 42
  3. Distinctive:

    Before a production release

    line 43
  4. Distinctive:

    After major refactors that touch multiple modules

    line 44
  5. Distinctive:

    After new contributor PRs (unfamiliar with codebase conventions)

    line 45
  6. Distinctive:

    When a change touches auth, payments, or data handling

    line 46
  7. Very specific:

    NOT for: quick single-file fixes (use @reviewer directly)

    line 47
  8. Very specific:

    NOT for: security-only audit (use @security directly)

    line 48
  9. Very specific:

    NOT for: test strategy design (use @tester directly)

    line 49
  10. Very specific:

    Read the project's docs/ARCHITECTURE.md if it exists - this is the source of truth for architecture review.

    line 74
  11. Distinctive:

    Deduplicate: - Remove findings flagged by multiple perspectives (keep highest severity)

    line 160
  12. Distinctive:

    Assign severity: - Every finding gets a severity level (see table below)

    line 162
  13. Distinctive:

    All 3 perspectives were executed (architecture, security, tests)

    line 197
  14. Distinctive:

    Every finding has a severity level assigned

    line 198
  15. Distinctive:

    Every CRITICAL or HIGH finding includes a specific file:line reference

    line 199
  16. Distinctive:

    Every CRITICAL or HIGH finding includes a concrete remediation suggestion

    line 200
  17. Distinctive:

    Read-only: - Never modify files during a review

    line 221
  18. Distinctive:

    Evidence required: - Every finding needs a file:line reference and explanation

    line 222
  19. Distinctive:

    Verdict is mechanical: - Follow the Verdict Rules table, no subjective overrides

    line 224
  20. Distinctive:

    No sycophancy: - Never say "LGTM", "looks good", or "great job" without evidence

    line 225
  21. Distinctive:

    Severity must be justified: - Every rating needs a specific technical reason

    line 227
  22. Very specific:

    Run automated tools: - Never skip npm audit, linting, or type checking if available

    line 228

“Common” is measured: kept by at least 6 in 10 of the 4 edited versions of this file we could read. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

/codereview - Multi-Reviewer Parallel Code Review

Run a comprehensive code review that dispatches 3 specialized perspectives concurrently - architecture, security, and testing - then consolidates into a single actionable verdict.

Target: $ARGUMENTS

Why 3 Perspectives, Not 6

Most multi-reviewer systems use 5-7 overlapping perspectives that waste tokens and produce redundant findings. /codereview uses exactly 3 orthogonal perspectives - each covers a distinct failure mode with zero overlap:

┌──────────────────────────────────────────────────────────────┐
│              PARALLEL REVIEW DISPATCH                        │
│                                                              │
│  ┌─────────────┐  ┌─────────────┐  ┌─────────────┐          │
│  │ @reviewer   │  │ @security   │  │ @tester     │          │
│  │ Architecture│  │ OWASP/Auth  │  │ Coverage    │          │
│  │ + Quality   │  │ + Data      │  │ + Edge Cases│          │
│  └──────┬──────┘  └──────┬──────┘  └──────┬──────┘          │
│         │                │                │                  │
│         └────────────────┼────────────────┘                  │
│                          ▼                                   │
│                  ┌──────────────┐                             │
│                  │ CONSOLIDATE  │                             │
│                  │ Deduplicate  │                             │
│                  │ Assign Sev.  │                             │
│                  │ Verdict      │                             │
│                  └──────────────┘                             │
└──────────────────────────────────────────────────────────────┘

When to Use

  • Before merging a pull request
  • Before a production release
  • After major refactors that touch multiple modules
  • After new contributor PRs (unfamiliar with codebase conventions)
  • When a change touches auth, payments, or data handling
  • NOT for: quick single-file fixes (use @reviewer directly)
  • NOT for: security-only audit (use @security directly)
  • NOT for: test strategy design (use @tester directly)

Workflow

Step 1: Scope Detection

Determine the review target:

IF $ARGUMENTS is a file/directory path → review that path
IF $ARGUMENTS is a module name → find and review the module
IF $ARGUMENTS is a PR number/URL → extract changed files from PR diff
IF $ARGUMENTS is empty → review staged/uncommitted changes

Gather scope metrics:

# Count files and lines in scope
find $TARGET -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" -o -name "*.vue" -o -name "*.svelte" \) | head -200 | xargs wc -l 2>/dev/null || true

# If PR, get the diff
gh pr diff $PR_NUMBER 2>/dev/null || git diff main...HEAD 2>/dev/null || true

Read the project's docs/ARCHITECTURE.md if it exists - this is the source of truth for architecture review.

Step 2: Parallel Review Dispatch

Dispatch 3 concurrent review perspectives. Each reviewer gets the full file context but applies a different lens.


Perspective 1: Architecture Review (@reviewer focus)

Review the code for structural integrity and quality:

CheckWhat to Look For
Layer separationServices don't import from components, no cross-module imports
Naming conventionsFiles, functions, variables follow project conventions
Design patternsDRY, SOLID, composition over inheritance
API contractsDTOs/schemas at boundaries, typed inputs and outputs
Dependency directionLayers depend inward, not outward
Code smellsLong methods (>40 lines), deep nesting (>3 levels), god objects
Type safetyNo any abuse, strict mode, proper generics
Error handlingTry/catch at boundaries, custom error types, no swallowed errors
Import hygieneNo circular dependencies, barrel file discipline
# Architecture checks
grep -rn ": any\|as any" $TARGET --include="*.ts" --include="*.tsx" 2>/dev/null | head -20
grep -rn "console\.\|debugger" $TARGET --include="*.ts" --include="*.tsx" --include="*.vue" 2>/dev/null | head -20

Perspective 2: Security Review (@security focus)

Review the code for vulnerabilities and data safety:

CheckWhat to Look For
InjectionSQL/NoSQL injection, command injection, template injection
XSSUnsanitized output, dangerouslySetInnerHTML, v-html, raw HTML
Auth/AuthZMissing auth checks, broken access control, IDOR vulnerabilities
Input validationMissing or insufficient validation, type coercion issues
SecretsHardcoded API keys, tokens, passwords, connection strings
Data exposureSensitive data in logs, error messages, API responses, URLs
CSRFMissing CSRF tokens on state-changing operations
CryptographyWeak algorithms, predictable tokens, missing encryption
HeadersMissing security headers (CSP, HSTS, X-Frame-Options)
DependenciesKnown CVEs in packages
# Security checks
npm audit --json 2>/dev/null | head -50 || true
grep -rn "password\|secret\|token\|api_key\|apikey" $TARGET --include="*.ts" --include="*.tsx" --include="*.env*" 2>/dev/null | head -20
grep -rn "v-html\|dangerouslySetInnerHTML\|innerHTML" $TARGET --include="*.ts" --include="*.tsx" --include="*.vue" 2>/dev/null | head -20
grep -rn "eval(\|new Function(" $TARGET --include="*.ts" --include="*.tsx" --include="*.js" 2>/dev/null | head -20

Perspective 3: Test Review (@tester focus)

Review the code for test quality and coverage:

CheckWhat to Look For
Coverage gapsBusiness logic without corresponding tests
Missing edge casesNull, empty, boundary values, error paths not tested
Test qualityTests that test implementation vs behavior
Mocking correctnessOver-mocking, mocking what you own, leaking mocks
Assertion strengthWeak assertions (toBeTruthy vs toEqual), missing assertions
Test isolationShared mutable state, order-dependent tests
Naming clarityDescriptive it('should...') vs vague test names
Test typesRight balance of unit/integration/e2e for the change
Regression riskChanged code without updated tests
Flakiness signalsTiming dependencies, network calls in unit tests
# Test checks
find $TARGET -name "*.spec.ts" -o -name "*.test.ts" -o -name "*.spec.tsx" -o -name "*.test.tsx" 2>/dev/null | wc -l
npx vitest run --passWithNoTests --reporter=json 2>/dev/null || npx jest --json --passWithNoTests 2>/dev/null || true

Step 3: Consolidation

Merge findings from all 3 perspectives:

  1. Deduplicate - Remove findings flagged by multiple perspectives (keep highest severity)
  2. Cross-reference - A security finding that also lacks tests is escalated one severity level
  3. Assign severity - Every finding gets a severity level (see table below)
  4. Sort - CRITICAL first, then HIGH, MEDIUM, LOW
  5. Group - By file, then by severity within each file

Step 4: Verdict

Apply verdict rules based on the consolidated findings. The verdict is final and non-negotiable - it follows the rules mechanically.

Severity Levels

LevelDescriptionAction
CRITICALSecurity vulnerability, data loss risk, auth bypassBlock merge. Fix immediately.
HIGHArchitecture violation, missing validation, untested critical pathRequest changes. Fix before merge.
MEDIUMCode quality issue, weak test, minor vulnerabilityApprove with notes. Fix in next sprint.
LOWStyle issue, minor improvement, optional optimizationApprove. Fix when convenient.

Verdict Rules

ConditionVerdictMeaning
Any CRITICAL findingBLOCKDo not merge under any circumstances
Any HIGH finding (no CRITICAL)REQUEST_CHANGESFix HIGH findings, then re-review
MEDIUM findings onlyAPPROVE WITH NOTESSafe to merge, but address MEDIUM items soon
LOW findings only (or none)APPROVEClean to merge

Escalation rules:

  • A security finding without a corresponding test → severity +1 level
  • A finding that recurs from a previous review → severity +1 level
  • A finding in auth, payments, or PII handling → minimum severity HIGH

Verification Protocol

Before claiming the review is complete:

  1. All 3 perspectives were executed (architecture, security, tests)
  2. Every finding has a severity level assigned
  3. Every CRITICAL or HIGH finding includes a specific file:line reference
  4. Every CRITICAL or HIGH finding includes a concrete remediation suggestion
  5. Automated checks were run where available (npm audit, tsc, linting, test suite)
  6. The verdict follows the Verdict Rules mechanically - no exceptions
  7. Cross-referencing was performed (security findings checked for test coverage)

Anti-Rationalization

ExcuseReality
"No security issues found"Did you check all OWASP categories? Absence of evidence is not evidence of absence.
"Tests look fine"Did you check edge cases, error paths, and assertion strength? Or just that tests exist?
"Architecture is clean"Did you trace actual imports and dependency direction? Or just scan filenames?
"It's just a small change"Small changes cause production outages. Review scope does not reduce rigor.
"The CI passed"CI checks syntax, not logic. A green build is not a security audit.
"Previous review approved this pattern"Patterns can be wrong. Evaluate independently every time.
"Too many files to review thoroughly"Scope down to critical paths (auth, data, business logic). Never skip security.
"The author is senior, they know what they're doing"Seniority does not prevent bugs. Review the code, not the author.

Rules

  1. All 3 perspectives are mandatory - Skipping one defeats the purpose of multi-reviewer review
  2. Read-only - Never modify files during a review
  3. Evidence required - Every finding needs a file:line reference and explanation
  4. Remediation required - Findings without fix suggestions are useless
  5. Verdict is mechanical - Follow the Verdict Rules table, no subjective overrides
  6. No sycophancy - Never say "LGTM", "looks good", or "great job" without evidence
  7. Cross-reference findings - A security issue without tests is worse than either alone
  8. Severity must be justified - Every rating needs a specific technical reason
  9. Run automated tools - Never skip npm audit, linting, or type checking if available
  10. Fresh context per perspective - Each reviewer perspective starts from the code, not from another reviewer's output

Output

──── /codereview ────
Target: [path or PR]
Scope: [X files, Y lines]

Architecture: [N findings] - @reviewer
Security:     [N findings] - @security
Tests:        [N findings] - @tester

Severity:
  CRITICAL: N
  HIGH: N
  MEDIUM: N
  LOW: N

Top Findings:
1. [CRITICAL] [description] - [file:line]
2. [HIGH] [description] - [file:line]
3. [HIGH] [description] - [file:line]

Verdict: [APPROVE | APPROVE WITH NOTES | REQUEST_CHANGES | BLOCK]
Reason: [1-line justification]

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.