Agent Skills: Fresh Eyes Review

This skill should be used when reviewing code before commit, conducting quality gates, or when "review", "fresh eyes", "pre-commit review", or "quality gate" are mentioned.

UncategorizedID: outfitter-dev/agents/code-review

Install this agent skill to your local

pnpm dlx add-skill https://github.com/outfitter-dev/agents/tree/HEAD/plugins/outfitter/skills/code-review

Skill Files

Browse the full folder contents for code-review.

Download Skill

Loading file tree…

plugins/outfitter/skills/code-review/SKILL.md

Skill Metadata

Name
code-review
Description
This skill should be used when reviewing code before commit, conducting quality gates, or when "review", "fresh eyes", "pre-commit review", or "quality gate" are mentioned.

Fresh Eyes Review

Systematic pre-commit quality gate → checklist-based review → findings → summary.

<when_to_use>

  • Pre-commit code review and quality gates
  • Pre-merge pull request reviews
  • Systematic code audits before deployment
  • Quality verification for critical changes
  • Second-opinion review requests

NOT for: quick sanity checks, trivial typo fixes, formatting-only changes

</when_to_use>

<announcement_protocol>

Starting Review

Review Scope: { files/areas under review } Focus Areas: { specific concerns or general quality gate } Checklist: { full or targeted categories }

During Review

Emit findings as discovered:

  • {SEVERITY} {FILE_PATH}:{LINE} — { issue description }
  • Impact: { consequences if shipped }
  • Fix: { concrete remediation }

Completing Review

Review Complete

Findings Summary:

  • ◆◆ Severe: {COUNT} — blocking issues
  • ◆ Moderate: {COUNT} — should fix before merge
  • ◇ Minor: {COUNT} — consider addressing

Recommendation: { ship / fix blockers / needs rework }

{ detailed findings below if any found }

</announcement_protocol>

<checklist>

Type Safety

  • ✓ No any types without justification comment
  • ✓ Null/undefined handled explicitly (optional chaining, nullish coalescing)
  • ✓ Type guards used for union types
  • ✓ Discriminated unions for state machines
  • ✓ Generic constraints specified where needed
  • ✓ Return types explicit on public functions
  • ✓ No type assertions without safety comment

Error Handling

  • ✓ All error paths handled (no silent failures)
  • ✓ Meaningful error messages with context
  • ✓ Errors propagated or logged appropriately
  • ✓ Result types used for expected failures
  • ✓ Try/catch blocks have specific error handling
  • ✓ Promise rejections handled
  • ✓ Resource cleanup in finally blocks

Security

  • ✓ User input validated before use
  • ✓ No hardcoded secrets or credentials
  • ✓ Authentication/authorization checks present
  • ✓ Parameterized queries (no SQL injection)
  • ✓ XSS prevention (sanitized output)
  • ✓ CSRF protection where applicable
  • ✓ Sensitive data encrypted/hashed
  • ✓ Rate limiting on public endpoints

Testing

  • ✓ Tests exist for new functionality
  • ✓ Edge cases covered
  • ✓ Error scenarios tested
  • ✓ Actual assertions (not just execution)
  • ✓ No test pollution (proper setup/teardown)
  • ✓ Mocks used appropriately (not overused)
  • ✓ Test names describe behavior
  • ✓ Integration tests for critical paths

Code Quality

  • ✓ Names reveal intent (functions, variables, types)
  • ✓ Functions <50 lines (single responsibility)
  • ✓ Files <500 lines (consider splitting)
  • ✓ No magic numbers (use named constants)
  • ✓ DRY violations eliminated
  • ✓ Nested conditionals <3 deep
  • ✓ Cyclomatic complexity reasonable
  • ✓ Dead code removed

Documentation

  • ✓ Public APIs have JSDoc/TSDoc
  • ✓ Complex algorithms explained
  • ✓ Non-obvious decisions documented
  • ✓ Breaking changes noted
  • ✓ TODOs have context and owner
  • ✓ README updated if behavior changes
  • ✓ Examples provided for complex usage

Performance

  • ✓ No obvious N+1 queries
  • ✓ Appropriate data structures used
  • ✓ Unnecessary allocations avoided
  • ✓ Heavy operations async/batched
  • ✓ Caching where beneficial
  • ✓ Database indexes considered

Rust-Specific (when applicable)

  • rustfmt and clippy passing
  • Result preferred over panic
  • ✓ No unwrap/expect outside tests/startup
  • ✓ Ownership/borrowing idiomatic
  • Send/Sync bounds respected
  • ✓ Unsafe code justified with comments
  • ✓ Proper error types (thiserror/anyhow)
</checklist> <stages>

1. Announce (activeForm: Announcing review)

Emit starting protocol:

  • Scope of review
  • Focus areas
  • Checklist approach (full or targeted)

2. Checklist (activeForm: Running checklist review)

Systematically verify each category:

  • Type Safety → Error Handling → Security → Testing → Quality → Docs → Performance
  • Flag violations immediately with severity
  • Note clean areas briefly

3. Deep Dive (activeForm: Investigating findings)

For each finding:

  • Verify it's actually a problem (not false positive)
  • Assess severity and impact
  • Determine concrete fix
  • Check for pattern across codebase

4. Summarize (activeForm: Compiling review summary)

Emit completion protocol:

  • Findings count by severity
  • Recommendation (ship / fix blockers / rework)
  • Detailed findings list
  • Optional: patterns noticed, suggestions for future

Load the maintain-tasks skill for tracking review stages.

</stages>

<finding_format>

{SEVERITY} {FILE_PATH}:{LINE_RANGE}

Issue: { clear description of problem }

Impact: { consequences if shipped — security risk, runtime error, maintenance burden, etc }

Fix: { concrete steps to remediate }

Pattern: { if issue appears multiple times, note scope }


Example:

◆◆ src/auth/login.ts:45-52

Issue: Password compared using == instead of constant-time comparison

Impact: Timing attack vulnerability — attacker can infer password length and content through response timing

Fix: Use crypto.timingSafeEqual() or bcrypt's built-in comparison

Pattern: Single occurrence


</finding_format>

<severity_guidance>

◆◆ Severe (blocking):

  • Security vulnerabilities
  • Data loss risks
  • Runtime crashes in common paths
  • Breaking changes without migration
  • Test failures or missing critical tests

◆ Moderate (should fix):

  • Type safety violations
  • Unhandled error cases
  • Poor error messages
  • Missing tests for edge cases
  • Significant code quality issues
  • Missing documentation for public APIs

◇ Minor (consider addressing):

  • Code style inconsistencies
  • Overly complex but functional code
  • Minor performance optimizations
  • Documentation improvements
  • TODOs without context
  • Naming improvements

</severity_guidance>

<workflow>

Loop: Scan → Verify → Document → Next category

  1. Announce review — scope, focus, approach
  2. Run checklist — systematically verify each category
  3. Document findings — severity, location, issue, impact, fix
  4. Investigate patterns — does finding repeat? Broader issue?
  5. Deep dive blockers — verify severity assessment, ensure fix is clear
  6. Compile summary — counts by severity, recommendation
  7. Deliver findings — completion protocol with detailed list

At each finding:

  • Verify it's actually a problem
  • Assess impact if shipped
  • Determine concrete fix
  • Note if pattern across files
</workflow> <validation>

Before completing review:

Check coverage:

  • ✓ All checklist categories verified?
  • ✓ Both happy path and error paths reviewed?
  • ✓ Tests examined for actual assertions?
  • ✓ Security-sensitive areas given extra scrutiny?

Check findings quality:

  • ✓ Severity accurately assessed?
  • ✓ Impact clearly explained?
  • ✓ Fix actionable and concrete?
  • ✓ False positives eliminated?

Check recommendation:

  • ✓ Aligned with findings severity?
  • ✓ Blockers clearly marked?
  • ✓ Path forward unambiguous?
</validation> <rules>

ALWAYS:

  • Announce review start with scope and focus
  • Run systematic checklist, don't skip categories
  • Emit findings as discovered, don't batch at end
  • Assess severity honestly (err toward caution)
  • Provide concrete fixes, not just complaints
  • Complete with summary and recommendation
  • Mark false positives if checklist item doesn't apply
  • Consider patterns (single issue or systemic?)

NEVER:

  • Skip checklist review for "quick check"
  • Assume code is safe without verification
  • Flag style preferences as blockers
  • Provide vague findings without fix guidance
  • Approve severe findings "for later fix"
  • Complete review without announcement protocol
  • Miss security checks on user input paths
  • Ignore test quality (execution != validation)
</rules> <references>

Core methodology:

  • checklist.md — extended checklist details, examples, severity guidance

Related skills:

  • codebase-recon — evidence-based investigation (foundation for review)
  • debugging — structured bug investigation
</references>