Code Reviewer Skill

Protocols

cat skills/_shared/protocols/ux-protocol.md 2>/dev/null || true

cat skills/_shared/protocols/input-validation.md 2>/dev/null || true

cat skills/_shared/protocols/tool-efficiency.md 2>/dev/null || true

cat skills/_shared/protocols/code-intelligence.md 2>/dev/null || true

cat .production-grade.yaml 2>/dev/null || echo "No config — using defaults"

Engagement Mode

cat .forgewright/settings.md 2>/dev/null || echo "No settings — using Standard"

Config Paths

.production-grade.yaml

paths.services

paths.frontend

paths.tests

paths.architecture_docs

paths.api_contracts

Read-Only Policy

.forgewright/code-reviewer/

Two-Stage Review Protocol

Inspired by Superpowers two-stage review methodology

Stage 1: Spec Compliance Check (MUST pass before Stage 2)

1. Read the BRD/PRD acceptance criteria
2. For each acceptance criterion, verify:
   • Is it implemented? (PASS / FAIL / PARTIAL)
   • Does the implementation match the spec exactly? (not over-built, not under-built)
   • Are there extra features not in the spec? (flag for removal)
3. If spec compliance fails → report issues. Do NOT proceed to code quality review.
4. If spec compliance passes → proceed to Stage 2.

Stage 2: Code Quality Review (Phases 1-5 below)

• Reviewing quality on code that doesn't match spec = wasted effort
• Spec issues are typically cheaper to fix than quality issues
• Spec compliance catches over/under-building early

Security Scope

Context & Position in Pipeline

services/

libs/

frontend/

tests/

• docs/architecture/

  ,

  api/

  — ADRs, API contracts (OpenAPI/AsyncAPI), data models, sequence diagrams, architectural decisions, technology choices

• services/

  ,

  libs/

  — Backend services, handlers, repositories, domain models, middleware, infrastructure code

• frontend/

  — UI components, pages, hooks, state management, API clients, routing

• tests/

  ,

  .forgewright/qa-engineer/test-plan.md

  — Test suites, coverage thresholds, test plan, fixtures
• BRD / PRD — Business requirements, acceptance criteria, NFRs

Output Structure

.forgewright/code-reviewer/

.forgewright/code-reviewer/
├── review-report.md                    # Full review report — executive summary + all findings
├── architecture-conformance.md         # ADR compliance check — decision-by-decision audit
├── findings/
│   ├── critical.md                     # Findings that block deployment (data loss risks, correctness bugs)
│   ├── high.md                         # Findings that must be fixed before production (arch violations, major bugs)
│   ├── medium.md                       # Findings that should be fixed soon (code quality, maintainability)
│   └── low.md                          # Findings that are advisory (style, minor optimizations)
├── metrics/
│   ├── complexity.json                 # Cyclomatic complexity per function/module
│   ├── coverage-gaps.json              # Untested code paths, missing edge case coverage
│   └── dependency-analysis.json        # Dependency graph, coupling metrics, circular dependencies
└── auto-fixes/                         # Suggested code patches organized by service
    └── <service>/
        └── <file>.patch.md             # Markdown with before/after code blocks and explanation

Severity Levels

Phases

Parallel Execution Strategy

Execute sequentially: Review architecture conformance following Phase 1 checklist. Compare implementation against ADRs. Write to code-reviewer/architecture-conformance.md.
Execute sequentially: Review code quality following Phase 2 checklist (SOLID, DRY, complexity). Write findings to code-reviewer/findings/.
Execute sequentially: Review performance following Phase 3 checklist (N+1, caching, bundle size). Write findings to code-reviewer/findings/.
Execute sequentially: Review test quality following Phase 4 checklist. Cross-reference test plan. Write to code-reviewer/metrics/.

1. Phases 1-4: Arch Conformance + Code Quality + Performance + Test Quality (PARALLEL)
2. Phase 5: Review Report (sequential — synthesizes all findings)

Phase 1 — Architecture Conformance

docs/architecture/

• docs/architecture/

  ADRs (every Architecture Decision Record)

• docs/architecture/

  system architecture diagrams, service boundaries, communication patterns

• api/

  API contracts (OpenAPI/AsyncAPI)

• schemas/

  data models and database design

• services/

  ,

  libs/

  full backend source tree

• frontend/

  full frontend source tree

1. Service boundaries — Does each service own exactly the domain it was designed to own? Are there cross-boundary data accesses that bypass APIs?
2. Communication patterns — If the ADR specifies async messaging between services, verify no synchronous HTTP calls exist between them. If REST was specified, verify no gRPC or GraphQL was introduced without an ADR.
3. Technology choices — If ADR says PostgreSQL, verify no MongoDB usage. If ADR says Redis for caching, verify no in-memory caches that bypass Redis.
4. Data ownership — Does each service have its own database/schema? Are there shared tables or direct DB-to-DB queries that violate data isolation?
5. API contract adherence — Do implemented endpoints match the OpenAPI spec exactly (paths, methods, request/response schemas, status codes)?
6. Authentication/authorization model — Does the implementation follow the auth architecture (JWT validation, RBAC, API keys) as designed?
7. Error handling strategy — Does the implementation follow the error handling patterns defined in the architecture (error codes, error response format, retry policies)?
8. Configuration management — Are secrets managed as designed (env vars, vault, SSM)? Are there hardcoded values that should be configurable?

.forgewright/code-reviewer/architecture-conformance.md

• A table listing every ADR from

  docs/architecture/

  and its conformance status (Conformant / Partial / Violated)
• For each violation: the ADR reference, what was specified, what was implemented, severity, and recommended fix
• For partial conformance: what is correct and what deviates

Phase 2 — Code Quality Analysis

• services/

  ,

  libs/

  all backend source files

• frontend/

  all frontend source files

• DRY violations — duplicated business logic (not just strings) across multiple places
• Cyclomatic complexity — flag functions > 10, record in

  metrics/complexity.json

• Error handling — flag swallowed exceptions, generic catches (

  catch (e: any)

  ), lost stack traces
• Logging — verify structured (JSON), appropriate levels, sensitive fields redacted

• Flag components > 200 lines mixing data fetching + business logic + presentation
• Flag prop drilling > 3 levels, global state for local concerns
• Flag useEffect with missing dependencies or missing cleanup
• Flag missing ARIA labels, alt text, keyboard navigation

.forgewright/code-reviewer/findings/

.forgewright/code-reviewer/metrics/complexity.json

Phase 3 — Performance Review

• services/

  ,

  libs/

  all backend source files (especially data access, API handlers, middleware)

• frontend/

  all frontend source files (especially data fetching, rendering, bundle composition)

• docs/architecture/

  NFRs (latency targets, throughput requirements)

1. N+1 queries — Flag any loop that executes a database query per iteration. Verify eager loading or batch queries are used for list endpoints.
2. Missing database indexes — Cross-reference query WHERE clauses and JOIN conditions against migration files. Flag unindexed columns used in frequent queries.
3. Unbounded queries — Flag SELECT queries without LIMIT. Flag list endpoints without pagination.
4. Missing caching — Identify read-heavy, rarely-changing data that should be cached. Flag cache invalidation gaps.
5. Synchronous bottlenecks — Flag synchronous calls to external services in the request path. Verify async/queue patterns for non-time-critical operations (email sending, PDF generation, analytics).
6. Connection pool configuration — Verify database and HTTP client connection pools are sized appropriately and have timeouts configured.
7. Memory leaks — Flag event listeners without cleanup, growing maps/arrays without eviction, unclosed resources (file handles, DB connections, streams).
8. Serialization overhead — Flag large object serialization in hot paths. Verify API responses do not include unnecessary fields.

import _ from 'lodash'

import get from 'lodash/get'

.forgewright/code-reviewer/findings/

.forgewright/code-reviewer/metrics/dependency-analysis.json

Phase 4 — Test Quality Review

tests/

• tests/

  all test files

• .forgewright/qa-engineer/test-plan.md

  traceability matrix

• .forgewright/qa-engineer/coverage/thresholds.json

• services/

  ,

  libs/

  source files (to identify untested paths)

1. Coverage gaps — Identify source files with no corresponding test file. Identify public functions with no test. Identify error handling branches with no test.
2. Assertion quality — Flag tests that only assert on status codes without checking response bodies. Flag tests with no assertions (they always pass). Flag tests that assert on

   true

   /

   false

   instead of specific values.
3. Missing edge cases — For each tested function, identify untested boundary conditions: null inputs, empty collections, maximum values, concurrent access, timeout scenarios.
4. Test independence — Flag tests that depend on execution order. Flag tests that share mutable state through module-level variables. Flag tests that depend on the output of other tests.
5. Test naming — Flag test names that describe implementation ("calls processOrder method") instead of behavior ("creates an order with calculated total when items are valid").
6. Mock quality — Flag mocks that are too permissive (accept any input). Flag mocks that are too brittle (assert on call count or argument order for non-critical interactions).
7. Integration test isolation — Flag integration tests that leave data behind. Flag integration tests that fail when run in a different order.
8. E2E test reliability — Flag E2E tests with hardcoded waits. Flag E2E tests that depend on specific data IDs. Flag E2E tests that are not idempotent.
9. Missing test types — Cross-reference the test plan traceability matrix. Flag acceptance criteria with no corresponding test.
10. Performance test realism — Flag k6 scripts with unrealistic load profiles (e.g., 10,000 VUs for an internal tool). Flag scripts with missing thresholds.

.forgewright/code-reviewer/findings/

.forgewright/code-reviewer/metrics/coverage-gaps.json

Phase 5 — Review Report

• All findings from Phases 1-4
• All metrics from Phases 2-3

1. Write

   .forgewright/code-reviewer/review-report.md

   with the following sections:
   • Executive Summary — Total finding count by severity. Overall assessment (Pass / Pass with Conditions / Fail). Top 3 most critical issues.
   • Findings by Category — Architecture, Code Quality, Performance, Test Quality. Each finding includes: ID, severity, category, location (file + line), description, impact, and recommended fix.
   • Metrics Summary — Cyclomatic complexity distribution, coverage gap summary, dependency health.
   • Recommendations — Prioritized list of actions. What to fix now, what to fix next sprint, what to add to tech debt backlog.
   • Sign-off Criteria — Conditions that must be met before this review is considered passed: all Critical findings resolved, all High findings resolved or accepted with justification.
2. Write individual findings files to

   .forgewright/code-reviewer/findings/

   :

   • critical.md

     — Findings that block deployment

   • high.md

     — Findings that must be fixed before production

   • medium.md

     — Findings that should be fixed soon

   • low.md

     — Advisory findings
   Each finding:

   ### [FINDING-ID] Short description

   with Severity, Category, Location (

   file:line

   ), Description, Impact, Evidence (code block), and Recommendation.
3. Generate auto-fix suggestions for mechanical, unambiguous fixes (missing null checks, auth middleware, input validation, unused imports, missing indexes). Write to

   .forgewright/code-reviewer/auto-fixes/<service>/<file>.patch.md

   with before/after code blocks.
4. Compile metrics:

   • .forgewright/code-reviewer/metrics/complexity.json

     — Cyclomatic complexity per function, flagged functions with complexity > 10

   • .forgewright/code-reviewer/metrics/coverage-gaps.json

     — List of untested files, untested functions, untested branches

   • .forgewright/code-reviewer/metrics/dependency-analysis.json

     — Service dependency graph, coupling score per service, circular dependency detection

.forgewright/code-reviewer/

Key Constraints

• Never report linter-level issues — focus on structural/architectural issues linters miss
• Always cross-reference ADRs before flagging architectural concerns
• Every finding needs: specific file location, concrete description, impact, and recommended fix
• Group related symptoms under one root-cause finding
• Skip generated code (migrations, protobuf stubs) or apply relaxed rules
• Never modify source files — write all output to

  .forgewright/code-reviewer/

• Defer security analysis to security-engineer

Phase 6 — Git Workflow Review

1. Branching strategy — Is there a clear strategy (Trunk-based, GitFlow, GitHub Flow)? Flag ad-hoc branch naming, long-lived feature branches (> 1 week), and missing branch protection rules.
2. Commit hygiene — Are commits atomic (one logical change per commit)? Flag commits mixing unrelated changes, commits with messages like "fix", "wip", "update". Check for conventional commit format (

   feat:

   ,

   fix:

   ,

   chore:

   ,

   docs:

   ).
3. PR quality — Do PRs have descriptions? Are they appropriately sized (< 400 lines changed)? Flag PRs > 1000 lines. Check for PR templates.
4. Code review process — Is there a minimum reviewer count? Are reviews resolved before merge? Flag force-push-to-main or direct commits to protected branches.
5. Merge strategy — Is squash-merge, rebase-merge, or merge-commit used consistently? Flag mixed strategies. Check for clean git history (no merge commit spaghetti).
6. CI integration — Do CI checks run on PRs? Are they required to pass before merge? Flag missing status checks.

review-report.md

Execution Checklist

• architecture-conformance.md

  audits every ADR in

  docs/architecture/

  with a conformance status
• Every finding has: ID, severity, category, file location, description, impact, and recommendation
• Performance review checks for N+1 queries, missing indexes, unbounded queries, and caching gaps
• Test quality review cross-references the

  .forgewright/qa-engineer/test-plan.md

  traceability matrix for coverage gaps

• review-report.md

  has an executive summary with total finding counts and overall assessment
• Findings are correctly distributed across

  critical.md

  ,

  high.md

  ,

  medium.md

  , and

  low.md

• metrics/complexity.json

  has per-function cyclomatic complexity scores

• metrics/coverage-gaps.json

  identifies untested files, functions, and branches

• metrics/dependency-analysis.json

  maps service dependencies and flags circular dependencies
• Auto-fixes exist for all mechanical issues (missing null checks, missing auth, etc.)
• No files were created or modified outside of .forgewright/code-reviewer/
• The report is actionable — a developer can read a finding and know exactly what to fix and where
• No OWASP or security review was performed — security analysis is deferred to security-engineer