Skip to main content

​
PR Review Guide

Not all changes carry the same risk. Focus review effort where it matters most.

​
The Challenge

Terra is developed with heavy AI assistance, which means high commit volume, frequent PRs, and lots of changed files. Reviewing every line of every PR isn’t realistic. This guide establishes a risk-based approach so a senior engineer can stay involved in ~1 hour/week.

​
Risk Tiers

​
Tier 1 — Always Review Carefully

Changes to these files affect security, data integrity, or access control: 
src/lib/auth-guards.ts          # Who can access what
src/app/actions/team.ts          # RBAC permission checks
src/middleware.ts                 # Route protection, sessions, CSP
src/lib/encryption.ts            # PII encryption/decryption
src/lib/security.ts              # Path traversal, redirect safety
src/lib/security/submission-guard.ts  # Submission ownership
src/lib/rate-limit.ts            # Rate limiting config
migrations/*.sql                  # Database schema (irreversible)
src/lib/dal/                      # Data access patterns
Review checklist:
  • Does it maintain existing security invariants?
  • Are there tests for the changed behavior?
  • Could it create a privilege escalation path?
  • Does it deny by default on failure?

​
Tier 2 — Scan for Patterns

Important but lower risk. A pattern-level review is usually sufficient: 
src/app/actions/*.ts         # Server actions — check for auth guards
src/app/(dashboard)/**       # Behind route protection?
src/app/(public)/**          # Exposes any data it shouldn't?
src/components/engine/**     # Handles user input safely?
Quick-scan checklist:
  • Uses supabaseAdmin from @/lib/supabase (not new clients)
  • Uses logger from @/lib/logger (not console.log)
  • Server actions start with auth guard (requireAdmin, checkFormAccess)
  • Returns ActionResult<T> type
  • No PII in log messages

​
Tier 3 — Trust CI

Low-risk changes. Trust the tests and lint: 
src/components/ui/**        # Shadcn components (standard)
*.css / Tailwind             # Styling
docs/**                      # Documentation
tsconfig, eslint config      # Check if security rules changed

​
PR Template

Including the risk tier in the PR description lets the reviewer know how much time to spend before opening the diff.
## Summary

<1-3 bullet points>

## Risk level

Tier 1 / Tier 2 / Tier 3

## What to focus on

<Point reviewer to specific files or decisions>

## Test plan

- [ ] Type check passes (`pnpm --dir apps/terra tsc`)
- [ ] Tests pass (`pnpm --dir apps/terra test`)
- [ ] Manually tested (if UI change)

## Security impact

None / <describe changes to auth, access control, or data handling>

​
Approval Criteria

Tier 1 (security-affecting):
  • Reviewer has read the changed files
  • New behavior has test coverage
  • No regressions in security tests
  • CI green
  • Explicit approval required
Tier 2 (feature work):
  • PR description accurate
  • Follows established patterns
  • CI green
  • Approval or 24h without objection
Tier 3 (low-risk):
  • CI green
  • No security files touched
  • Can merge after CI

​
Weekly Cadence

DayActivityTime
MondayReview any Tier 1 PRs30-60 min
WednesdayQuick scan of open Tier 2 PRs15-30 min
FridayCheck CI, pnpm audit, scan merged PRs15 min

​
Signals to watch for

  • New migration file — always review (hard to reverse)
  • Changes to auth-guards, middleware, or team.ts — review permission logic
  • New server action — verify it uses auth guards
  • Dependency updates — run pnpm audit

​
Getting Started

If you’re reviewing Terra for the first time:
  1. Read the Production Review overview for architecture context
  2. Run the test suite: pnpm --dir apps/terra test
  3. Walk through Security and Data Protection
  4. Check Known Gaps for what’s incomplete
  5. Set up GitHub notifications for Tier 1 file changes

Security

Start with the security audit

Known Gaps

What’s missing and the plan