Review Rules

These rules are extracted from the architecture review checklist and expanded with tenant-isolation-specific guidance. Every PR that touches tenant-scoped code should be checked against these rules. Violations in this section are Critical severity -- they represent potential cross-tenant data leakage.

Rule 1: No Bare db in Tenant-Scoped Code

What the rule says: All tenant-scoped queries must use withTenantContext() or ctx.db -- never the bare db object.

Hard enforcement: ESLint no-restricted-imports bans { db } from @repo/db in packages/api/src/routers/ and packages/mcp/src/tools/. The bare db is not exported from @repo/db's main entry point (only from @repo/db/client).

What to check in review:

• Has any code been added that imports db from @repo/db/client in a place that handles tenant-scoped data?
• Has @repo/db/src/index.ts been modified to re-export db?
• Is there an eslint-disable comment suppressing no-restricted-imports?

What a violation looks like:

// VIOLATION: importing bare db in a router
import { db } from "@repo/db/client";

// This query bypasses RLS entirely
const plans = await db.select().from(researchPlan);

Why bypassing is possible: An AI agent could import from @repo/db/client directly to work around the main entry point restriction, or could modify @repo/db/src/index.ts to re-export db.

See Enforcement for the full history of how this rule evolved.

Rule 2: New Tenant-Scoped Tables Have RLS

What the rule says: New tenant-scoped tables must use tenantPolicies() + .enableRLS().

Hard enforcement: None -- this is inherently semantic (requires understanding whether a table is tenant-scoped).

What to check in review:

• Any new table in packages/db/src/schema/. If the table has an organizationId column, it MUST have .enableRLS() and use tenantPolicies().
• The three-part pattern must be complete: (1) organization_id column via tenantColumns, (2) tenantPolicies() in the table's third argument, (3) .enableRLS() chained on the table definition.

What a violation looks like:

// VIOLATION: has organizationId but no RLS
export const myTable = pgTable("my_table", {
  id: text("id").primaryKey(),
  ...tenantColumns,
  data: text("data"),
});
// Missing: .enableRLS() and tenantPolicies() in the third argument

For the correct pattern, see Data & Storage -- Tenant Scoping.

Rule 3: Feature Routers Use authorizedProcedure

What the rule says: All feature endpoints must use authorizedProcedure (auth + RLS + CASL in one chain).

Hard enforcement: Vitest architecture test in packages/api/src/__tests__/arch.test.ts scans all router files and asserts authorizedProcedure usage with an explicit allowlist for exceptions.

What to check in review:

• Has the AUTHORIZED_PROCEDURE_ALLOWLIST been expanded without a legitimate justification?
• Has a new router been created that uses tenantProcedure or protectedProcedure instead of authorizedProcedure?
• Is the justification for an allowlist entry still valid, or has it become stale?

What a violation looks like:

// VIOLATION: feature router skipping CASL
export const featureRouter = router({
  list: tenantProcedure.query(async ({ ctx }) => {
    // Has RLS via tenantProcedure but no CASL checks
    return ctx.db.query.myTable.findMany();
  }),
});

Rule 4: CASL Checks Before Mutations

What the rule says: Always check ctx.ability.can() or ctx.ability.throwUnlessCan() before performing mutations.

Hard enforcement: None -- the correct ability check depends on business logic.

What to check in review:

• Every .mutation() handler in router files should check the user's ability before modifying data.
• The check should be against the correct subject and action (not a generic catch-all).

What a violation looks like:

// VIOLATION: mutation without permission check
delete: authorizedProcedure
  .input(z.object({ id: z.string() }))
  .mutation(async ({ ctx, input }) => {
    // No ability check -- any member can delete
    await ctx.db.delete(myTable).where(eq(myTable.id, input.id));
  }),

For the role matrix and ability definitions, see Authorization.

Rule 5: MCP Tools Use withTenantContext()

What the rule says: MCP tool handlers must use withTenantContext() for all database queries, not bare db.

Hard enforcement: ESLint no-restricted-imports bans { db } from @repo/db in packages/mcp/src/tools/.

What to check in review: Same bypasses as Rule 1 -- importing from @repo/db/client directly, or modifying the ESLint config to remove the restriction for the MCP tools directory.

For MCP-specific context on how tools resolve organization IDs, see Data & Storage -- Procedure Chain.

Rule 6: Enforcement Mechanisms Are Intact

What the rule says: The enforcement mechanisms themselves must not be weakened.

What to check in review:

File	What Should Not Change
packages/db/src/index.ts	Should NOT export db
packages/config-eslint/restrictions.js	No entries removed from SDK_BOUNDARIES
packages/api/src/__tests__/arch.test.ts	No unjustified additions to AUTHORIZED_PROCEDURE_ALLOWLIST
Any file	No eslint-disable for no-restricted-imports, boundaries/*, or no-console

See Enforcement for why these protections exist and the history behind them.

Quick Reference

Use this table when reviewing a PR:

Change	Check
New table with organizationId	Has .enableRLS() + tenantPolicies()?
New router file	Uses authorizedProcedure?
New mutation handler	Has ability.can() / ability.throwUnlessCan()?
New MCP tool	Uses withTenantContext(), not bare db?
Import from @repo/db/client	Is this one of the four legitimate use cases?
eslint-disable comment	Does it suppress an architectural rule?
Allowlist change in arch tests	Is the justification strong enough?