# Review checklist

Review or self-review a change against the repository's rules, with the command or test that checks each item.

Use this list before opening a pull request and when reviewing one. Skip the sections the change does not touch. Each item names what checks it: `[type]`, `[lint]`, `[test]`, `[build]`, `[deploy]` or `[convention]`. A `[convention]` item is checked only by a reviewer, so read it carefully.

## Always [#always]

- [ ] `pnpm check-types`, `pnpm lint`, `FORMAT_BASE_REF=origin/main pnpm format:check` and `pnpm test` pass. `[type]` `[lint]` `[test]`
- [ ] Database behavior is tested against the disposable cluster: `node scripts/test-local-postgres.mjs exec pnpm test:integration`. `TEST_DATABASE_URL` never points at a shared or hosted database. `[test]`
- [ ] A user-visible change has a changelog entry, and a changed flow updates its guide in the same change. `[convention]`
- [ ] Commits are logical Conventional Commits, and file moves are not mixed with behavior changes. `[convention]`

## Backend feature shape [#backend-feature-shape]

- [ ] The feature lives in `packages/api/src/features/<feature>/` with role names (`contract`, `procedures`, `service`, `errors`, `jobs`, `http`) as described in [ADR-0011](/docs/adr/0011-backend-feature-shape). `[test]` boundary test
- [ ] The contract file starts with the `LEAF MODULE` header and imports only leaves. `[test]` `scripts/leaf-modules.test.mjs`
- [ ] Nothing new imports another feature's internals, a `*.procedures` file or a `*.jobs` file, and `platform/` imports no feature. `[lint]` `.oxlintrc.json` · `[test]` `scripts/feature-boundaries.test.mjs`
- [ ] No import cycle was introduced. `[test]` `scripts/import-cycles.test.mjs`
- [ ] A migrated feature's `*.service.ts`, `*.rules.ts`, `*.policy.ts` and `*.procedures.ts` are listed in the D2 and D8 overrides of `.oxlintrc.json`. `[lint]`

## Tenant isolation [#tenant-isolation]

- [ ] Procedures use `context.db`, which runs as `app_tenant`. The `organizationId` filter is still present. `[test]` `rls.isolation.test.ts`
- [ ] A new table uses `pgTable.withRLS` with `tenantPolicy(...)`, gets its GRANT in `packages/db/scripts/rls-baseline.mjs` and ships with a generated, reviewed migration. `drizzle-kit push` is never used. `[test]` `rls-catalog.test.ts`
- [ ] A new cross-tenant path uses `asSystem(db, reason, fn)` with a new `SystemReason`, and `node scripts/check-system-call-sites.mjs --write` ran in the same change. `[lint]`
- [ ] `withTenant` holds no network I/O. `[convention]`
- [ ] An operation limited to some Org roles chains `requireOrgRole(...)` after `orgProcedure`; no service compares role strings. `[test]` `require-org-role.test.ts` · `[convention]`
- [ ] A tenancy change ran `pnpm --filter @template-cloudflare/db test:rls-matrix`, and `node scripts/rls-preview-proof.mjs <previewUrl>` passed against the preview before a production deploy. `[test]` `[deploy]`

## Client/server boundary [#clientserver-boundary]

- [ ] A client imports only leaves, contracts or `import type`. No barrel was added to a server-capable module ([ADR-0002](/docs/adr/0002-no-barrels-on-server-capable-modules)). `[lint]`
- [ ] A new cross-package import added a deliberate `exports` entry, and the package page names it. No `./routers/*` subpath comes back; web and mobile import `<feature>/contract`. `[test]` `packages/api/src/exports.test.ts`, docs package-exports test
- [ ] `pnpm --filter web build` passes, including `assert-client-bundle-clean.mjs`. `[build]`
- [ ] `apps/mobile` changed only if the change is meant for the native app. `[convention]`

## Errors and Effect [#errors-and-effect]

- [ ] A domain failure is a `Schema.TaggedError` in `<feature>.errors.ts`, and the feature mapper handles it. A service imports no `@orpc/*`. `[type]` `[lint]`
- [ ] A new client-visible code is in `APP_ERRORS`, `ERROR_MESSAGE_KEYS` and every locale (en, de, fr). `[type]` `[test]` locale tests
- [ ] A procedure body calls `runProcedure` once. No `Layer`, `ManagedRuntime` or I/O runtime exists at module scope ([ADR-0010](/docs/adr/0010-procedures-run-one-effect-program), [ADR-0005](/docs/adr/0005-effect-boundary)). `[convention]`
- [ ] Only idempotent reads are retried. A webhook POST or an email send is never retried in-round. `[convention]`

## Configuration and logging [#configuration-and-logging]

- [ ] A new environment variable is declared in `packages/env` and documented in the environments guide. It is read through the schema, not `process.env`. `[lint]` `node/no-process-env`
- [ ] Expected failures (4xx) are not logged as errors. Unexpected failures carry the request ID. Logs contain no credentials, prompts or private payloads. `[convention]`

## Runtime, bundle and decisions [#runtime-bundle-and-decisions]

- [ ] A runtime, deadline or heavy-dependency change has a dated report in `docs/measurements/` with exact commands and limits. `[convention]`
- [ ] A change to an established boundary reads the relevant ADR first. A new boundary decision gets an ADR with a status line, and the ADR index is updated. `[convention]`
- [ ] Dependency changes follow the two-day release quarantine and exact pins. `[test]` tooling tests

Read [Enforcement](/docs/development/enforcement) for the full list of rules and how each is checked.