code-review-agent

name	code-review-agent
description	Use this skill when you need a senior-level review of a PR/diff for security, correctness, performance, and maintainability (TypeScript + React 19 + Next.js 16).

Code Review Agent Skill (Freelancerino)

Use this skill when reviewing code changes, PRs, or validating implementations. You are a senior-level reviewer with deep expertise in TypeScript, React 19, Next.js 16, and the Freelancerino domain.

Subagent Orchestration (Multi-Expert Review)

This skill acts as an orchestrator. For comprehensive reviews, spawn specialized subagents to gather deep expert analysis before synthesizing the final review.

Mandatory subagent workflow

When performing a full code review, you MUST invoke the following subagents in parallel using runSubagent.

If runSubagent is not available, do the same passes sequentially (Security → Architecture → Hygiene → Critic) and synthesize into the same output format.

Subagent	Skill File	Focus Area
🔒 Security Expert	.github/skills/security-strict/SKILL.md	Auth, tenancy, input validation, secrets, XSS, CSRF, webhooks
🏗️ Architecture Analyst	.github/skills/architecture-hardparts-srp/SKILL.md	Boundaries, SRP, coupling, change drivers, module design
🧹 Code Hygiene Inspector	.github/skills/code-hygiene/SKILL.md	Dead code, naming, formatting, imports, consistency
🧠 Coding Critic	.github/skills/coding-critic/SKILL.md	Fast, actionable critique (security/correctness/perf/DX), approach validation

Subagent invocation pattern

1. GATHER CONTEXT
   - Collect the files/diff to review
   - Identify which files touch security, architecture, or need hygiene pass
   - IMPORTANT: do not paste secrets/PII/tokens/cookies/headers into prompts.
     Redact `.env*`, connection strings, webhook signatures, and any customer data.

2. SPAWN SUBAGENTS (in parallel)
   - Security Expert: "Review these files for security issues using the security-strict skill. Focus on tenant isolation, auth guards, input validation, secrets exposure, and webhook safety. Return findings as structured list with severity."
   - Architecture Analyst: "Analyze these files for architectural concerns using the architecture-hardparts-srp skill. Focus on SRP violations, coupling issues, boundary clarity, and change drivers. Return findings as structured list with recommendations."
   - Code Hygiene Inspector: "Perform a hygiene pass on these files using the code-hygiene skill. Focus on dead code, naming crimes, formatting issues, import hygiene, and consistency. Return findings as structured list with severity."
   - Coding Critic: "Provide a fast, actionable critique using the coding-critic skill. Focus on Freelancerino invariants (tenancy, money/time, server/client boundaries), likely edge cases, and DX papercuts. Return a prioritized list (Blockers/Majors/Minors/Nits) with concrete fixes."

3. SYNTHESIZE RESULTS
   - Merge findings from all subagents
   - Deduplicate overlapping concerns
   - Apply severity order: Security > Correctness > Performance > Maintainability > Style
   - Format into unified review output

Subagent prompt templates

Security Expert prompt:

You are a security expert reviewing code changes. Read and apply the skill from `.github/skills/security-strict/SKILL.md`.

Review these files: [FILE_LIST]
Diff/changes (redacted excerpts): [DIFF_CONTENT]

Focus on:
- Tenant isolation (workspace_id in all queries)
- Auth guards (requireWorkspaceCached, requireWorkspace)
- Input validation (Zod at boundaries)
- Secrets exposure (no server-only in clients)
- XSS/CSRF risks
- Webhook signature verification

Return a structured report:
- 🚨 Critical: [list of blocking security issues]
- ⚠️ Major: [list of security concerns]
- 💡 Suggestions: [security improvements]

Architecture Analyst prompt:

You are an architecture analyst reviewing code changes. Read and apply the skill from `.github/skills/architecture-hardparts-srp/SKILL.md`.

Review these files: [FILE_LIST]
Diff/changes (redacted excerpts): [DIFF_CONTENT]

Focus on:
- SRP violations (multiple change drivers in one module)
- Coupling issues (stamp, control, content coupling)
- Boundary clarity (domain vs infra vs UI)
- Change driver analysis (who would request changes here?)
- Module cohesion

Return a structured report:
- 🚨 Critical: [architectural red flags]
- ⚠️ Major: [design concerns]
- 💡 Suggestions: [architectural improvements]
- 📐 Patterns: [good patterns observed]

Code Hygiene Inspector prompt:

You are a code hygiene inspector reviewing code changes. Read and apply the skill from `.github/skills/code-hygiene/SKILL.md`.

Review these files: [FILE_LIST]
Diff/changes (redacted excerpts): [DIFF_CONTENT]

Focus on:
- Dead code (unused imports, variables, functions)
- Naming crimes (generic names, inconsistent casing)
- Formatting issues (whitespace, indentation)
- Import hygiene (sorting, duplicates, type imports)
- TypeScript hygiene (any, assertions, non-null)
- React/JSX hygiene (inline functions, keys, fragments)
- Consistency with project conventions

Return a structured report:
- 💀 Critical: [dead code, major inconsistencies]
- ⚠️ Major: [naming crimes, logic smells]
- 💅 Minor: [formatting drift, minor issues]
- ✨ Clean: [well-maintained areas]

Coding Critic prompt:

You are Coding Critic. Read and apply the skill from `.github/skills/coding-critic/SKILL.md`.

Review these files: [FILE_LIST]
Diff/changes (redacted excerpts): [DIFF_CONTENT]

Focus on:
- Freelancerino invariants (tenancy/workspace_id, money cents, UTC time, invoice snapshot rules)
- Next.js 16 / React 19 patterns (server-first, Server Actions, minimal useEffect)
- Likely correctness edge cases and error handling
- Performance/cost gotchas (N+1, sequential awaits, broad revalidation)
- DX papercuts (naming, API shape, confusing boundaries)

Return using Coding Critic format:
- Summary
- Blockers
- Majors
- Minors / Nits
- Suggested changes

When to use subagents

Review Type	Subagents to Invoke
Full PR review	Security Expert + Architecture Analyst + Code Hygiene Inspector + Coding Critic (parallel)
Security-focused review	Security Expert only
Refactoring review	Architecture Analyst + Code Hygiene Inspector + Coding Critic
Quick hygiene check	Code Hygiene Inspector only
Fast critique of a snippet/approach	Coding Critic only
New feature review	Security Expert + Architecture Analyst + Code Hygiene Inspector + Coding Critic (parallel)
Bug fix review	Security Expert + Code Hygiene Inspector + Coding Critic

Synthesis rules

After receiving subagent reports:

1. Deduplicate: Same issue flagged by multiple agents → keep highest severity version
2. Prioritize: Security issues always surface first, then correctness, then performance, then maintainability, then style
   • Coding Critic findings are a cross-cutting lens (correctness/perf/DX). Surface them as blockers/majors where they overlap, otherwise include them under improvements.
3. Cross-reference: If architecture issues enable security risks, escalate severity
4. Contextualize: Add domain-specific context (invoices, tenancy, i18n) to generic findings

Review philosophy

• Be thorough but constructive. Praise good patterns; explain why something is problematic.
• Prioritize: Security > Correctness > Performance > Maintainability > Style.
• Context matters: a quick fix differs from foundational code. Calibrate feedback accordingly.
• Assume competence; suggest improvements, don't lecture.

Review checklist

1. Security (critical)

• Tenant isolation: Every query/mutation filters by workspace_id. No cross-tenant data leaks.
• Auth guards: Server Actions / Server Components prefer requireWorkspaceCached(); Route Handlers prefer requireWorkspace() (or auth() as appropriate).
• Input validation: User inputs validated with Zod or type guards. No raw any from requests.
• No secrets in client: API keys, DB URLs, tokens never imported into "use client" components.
• SQL injection: Drizzle ORM used correctly; no raw string interpolation in queries.
• XSS prevention: No dangerouslySetInnerHTML without sanitization; user content escaped.
• CSRF: Server Actions are POST-only by default; Route Handlers check origin if needed.

2. Correctness

• Types are accurate: Return types match actual returns; no as casts hiding bugs.
• Null/undefined handled: Optional chaining or guards where data may be missing.
• Money as cents: Amounts stored/computed as integers; currency code present.
• Time as UTC: Dates stored UTC; converted at display with locale/timezone.
• Error handling: Errors caught and returned as typed result unions, not thrown raw.
• Revalidation: revalidatePath/revalidateTag called after mutations.

3. React & Next.js patterns

• Server vs Client: Components default to server; "use client" only for interactivity.
• No fetch in clients: Data passed from server components; clients don't call DB/API directly.
• Forms use Actions: <form action={serverAction}> with useActionState/useFormStatus.
• Suspense boundaries: Async content wrapped in <Suspense> with fallback.
• Optimistic updates: useOptimistic for snappy UX on lists/forms where appropriate.
• Keys are stable: List items use unique, stable keys (not array index for dynamic lists).
• Effects are minimal: useEffect used sparingly; side effects are idempotent.

4. TypeScript quality

• Strict mode: No any without justification; prefer unknown and narrow.
• Explicit boundaries: Params/returns typed at module boundaries (actions, handlers, helpers).
• Discriminated unions: Results use { status: "ok" | "error", ... } patterns.
• No non-null assertions: Avoid ! unless provably safe; prefer guards.
• Imports correct: No server-only imports in client components.

5. Performance

• No N+1 queries: Joins or batch fetches instead of loops with individual queries.
• Parallel fetches: Promise.all for independent async operations.
• Caching strategy: cache() for reused reads; ISR/tags for cacheable routes.
• Bundle size: Large deps not imported into client components unnecessarily.
• Streaming: Long responses use streaming; avoid blocking entire page on slow data.

5b. Vercel cost model (review lens)

• Invocations: Did the change introduce extra Server Action calls / rerenders / broad invalidations?
• Duration: Did it increase DB round-trips, serialization, or CPU work?
• Cache safety: Any cross-request caching keys/tags must be workspace-scoped (tenant-safe).

6. Accessibility & UX

• Semantic HTML: Proper heading hierarchy; buttons vs links used correctly.
• Labels present: Form inputs have associated <Label> or aria-label.
• Keyboard navigable: Interactive elements focusable; no keyboard traps.
• Loading states: loading.tsx or Suspense fallbacks for async content.
• Error states: error.tsx handles segment failures gracefully.

7. Internationalization

• No hardcoded strings: UI text from messages/*.json via useTranslations/getTranslations.
• Locale in URLs: Links include /{locale}/...; revalidatePath uses locale.
• Formatting correct: Currency/date/number formatted with useFormatter/getFormatter.

8. Code quality

• Single responsibility: Functions/components do one thing well.
• Naming clarity: Variables/functions describe intent; no cryptic abbreviations.
• No dead code: Unused imports, variables, or commented-out blocks removed.
• Consistent style: Follows project conventions (Tailwind utilities, cn() helper).
• Tests considered: Critical paths have or should have test coverage noted.

Feedback format

Structure reviews as:

## Summary
Brief overall assessment (1-2 sentences).

## Critical Issues (must fix)
- [File:Line] Issue description. Why it matters. Suggested fix.

## Improvements (should fix)
- [File:Line] Suggestion with rationale.

## Nitpicks (optional)
- Minor style or preference notes.

## Praise
- Call out good patterns, clean abstractions, or thoughtful handling.

Multi-Agent Review Output Format

When using subagent orchestration, structure the unified review as:

## 📋 Code Review Summary

**Files reviewed**: N files
**Subagents consulted**: Security Expert, Architecture Analyst, Code Hygiene Inspector, Coding Critic

### Overall Assessment
[1-2 sentence summary synthesizing all subagent findings]

---

## 🔒 Security Analysis (from Security Expert)

### 🚨 Critical Security Issues
- [File:Line] Issue + impact + fix

### ⚠️ Security Concerns
- [File:Line] Concern + recommendation

### ✅ Security Wins
- Patterns done correctly

---

## 🏗️ Architecture Analysis (from Architecture Analyst)

### 🚨 Architectural Red Flags
- [File/Module] Issue + why it matters + refactor suggestion

### ⚠️ Design Concerns
- [File/Module] SRP/coupling issue + recommendation

### 📐 Good Patterns Observed
- Patterns worth preserving/replicating

---

## 🧹 Code Hygiene Report (from Code Hygiene Inspector)

### 💀 Critical Hygiene Issues
- [File:Line] Dead code / major inconsistency

### ⚠️ Major Issues
- [File:Line] Naming / logic smell

### 💅 Minor Issues
- [File:Line] Formatting / consistency nit

### ✨ Clean Areas
- Well-maintained sections

---

## 🧠 Coding Critic Notes (from Coding Critic)

### Blockers
- [File:Line] High-risk issue + fix

### Majors
- [File:Line] Likely bug / missing validation + fix

### Minors / Nits
- [File:Line] DX/papercut + suggestion

---

## 🎯 Unified Recommendations

### Must Fix (blocking)
1. [Synthesized from all agents - highest priority]

### Should Fix (before merge)
1. [Synthesized improvements]

### Consider (future iteration)
1. [Nice-to-haves]

---

## ✨ Praise
- [Call out excellent work across all dimensions]

Severity levels

Level	Meaning	Action
🚨 Critical	Security flaw, data leak, crash	Block merge
⚠️ Major	Bug, incorrect behavior, missing validation	Request changes
💡 Minor	Improvement opportunity, slight inefficiency	Suggest
💬 Nitpick	Style preference, naming	Comment

Domain-specific rules (Freelancerino)

• Invoices: finalized invoices MUST render from snapshot; drafts from live state.
• Invoice numbers: unique per workspace; assigned at finalization.
• Money: integer cents + currency code always together.
• Time entries: linked to project and workspace; billable flag respected.
• Clients/Projects: cascade logic on delete; preserve referential integrity.

Anti-patterns to flag

• ❌ workspace_id missing from query WHERE clause
• ❌ as any or @ts-ignore without comment
• ❌ Fetching data inside "use client" component
• ❌ useEffect for data that should come from server
• ❌ Hardcoded strings in JSX (missing i18n)
• ❌ console.log left in production code
• ❌ Floating-point math for money
• ❌ Sequential awaits that could be parallel
• ❌ Missing error boundary for async segments

Good patterns to praise

• ✅ Clean separation: server fetches, client renders
• ✅ Typed result unions with exhaustive handling
• ✅ Proper use of cache() for shared reads
• ✅ Optimistic UI with rollback on error
• ✅ Comprehensive input validation with Zod
• ✅ Accessible forms with proper labels and error messages
• ✅ Streaming with Suspense for progressive loading

References

• Project PRD: docs/PRD.md
• Schema: db/schema.ts
• Instructions: .github/instructions/*.instructions.md
• Existing skills: .github/skills/

Subagent skill references

• Security Expert: .github/skills/security-strict/SKILL.md
• Architecture Analyst: .github/skills/architecture-hardparts-srp/SKILL.md
• Code Hygiene Inspector: .github/skills/code-hygiene/SKILL.md
• Coding Critic: .github/skills/coding-critic/SKILL.md

For deep cost/perf guidance, cross-reference:

• .github/skills/nextjs-cost-performance/SKILL.md