prodcheck

A staff-level review of code that is about to hit production. The value of this skill is not the checklist — it is the refusal to report anything you have not proven.

Non-negotiables

1. Fact first, opinion never. Every finding cites file:line and quotes the real code. If a claim depends on how a framework/ORM/middleware behaves, open the config, the migration, the base class, or node_modules/site-packages and confirm it. "Prisma probably lazy-loads this" is not a finding.
2. No finding without a trigger. You must be able to write a concrete failure scenario: these inputs / this state → this wrong outcome. If you cannot, delete the finding. Silence beats noise.
3. Do not write code in this skill unless the user says to. Review is the deliverable. Fixing is a separate, explicitly authorized step (Step 6).
4. No style commentary. Naming, formatting, and taste are out of scope unless they cause a real defect. Linters exist.
5. Report what you did not cover. If you skipped generated files, vendored code, or a subsystem you could not read, say so.

Step 1 — Scope the review

Determine exactly what is under review, in this order:

• An explicit argument (path, PR number, commit range) wins.
• Otherwise: git status --porcelain + git diff for uncommitted work, and git diff $(git merge-base HEAD origin/main)...HEAD for the branch.
• If both are empty, ask the user what to review. Do not review the whole repo uninvited.

State the scope in one line before you start (Reviewing 14 files, +812/-90, branch feat/billing vs origin/main).

Step 2 — Build ground truth before judging

Read enough of the surrounding system that you are reviewing behavior, not diffs. At minimum, for the code in scope:

• The schema: migrations / schema.prisma / models — real columns, real indexes, real constraints, real nullability.
• The auth path: middleware, guards, decorators, route registration. Know what runs before the handler and what it actually enforces.
• The callers: who invokes the changed functions, with what values, from where (HTTP, cron, queue, webhook, admin script).
• The existing conventions: how the rest of this codebase already does authz, scoping, transactions, and error handling. A change that breaks the house pattern is a finding; a change that follows a pattern you personally dislike is not.

Step 3 — Review passes

Run each pass deliberately. Skip a pass only if the diff cannot possibly touch it, and say which passes you skipped.

Security

• Authn and authz on every new or modified entry point — including webhooks, health/debug routes, admin actions, and queue consumers.
• IDOR: any resource fetched by an id from the request must be re-checked against the caller's permissions, not just found.
• Injection: raw SQL/string-built queries, eval, shell interpolation, unsanitized ORM where fragments, NoSQL operator injection.
• Mass assignment: request bodies spread into create/update without an allowlist.
• Secrets: keys, tokens, connection strings in code, tests, fixtures, or logs.
• Output safety: dangerouslySetInnerHTML, unescaped templating, reflected user input.
• SSRF and path traversal on any URL/file input; file upload type and size handling.
• PII in logs, error payloads, and third-party telemetry.
• Rate limiting on auth, password reset, and expensive endpoints.

Data isolation (multi-tenancy)

• Every query against a tenant-scoped table filters by tenant/org/workspace id — on reads, writes, updates, and deletes.
• The scope comes from the session/token, never from the request body.
• Background jobs, exports, cron, and admin tooling carry the tenant scope too; these are where isolation usually breaks.
• Cache keys, rate-limit keys, and file/storage paths include the tenant id.
• Aggregates and count() calls are scoped (a leaked count is a leak).

Database and queries

• N+1: loops or map/Promise.all that issue one query per item; lazy relations touched inside serializers, templates, or response mappers; missing include/select_related/prefetch_related/joinedload/with(). Trace the loop to the query call to prove it, then say how many queries it costs for a realistic N.
• Indexes: confirm in the migration files that columns used in WHERE, JOIN, ORDER BY, and uniqueness checks are indexed. Flag new foreign keys with no index.
• Unbounded reads: queries with no LIMIT/pagination, SELECT * of wide rows, loading a full table into memory.
• Transactions: multi-write operations that must be atomic but are not; external HTTP calls or long work held inside a transaction; missing rollback.
• Concurrency: check-then-act races (findFirst then create), lost updates without optimistic locking or SELECT ... FOR UPDATE, missing unique constraints backing an application-level uniqueness check.
• Migrations: destructive or table-locking DDL, non-nullable column added without a default or backfill, no rollback path, ordering assumptions between deploy and migrate.

Correctness and failure modes

• Unawaited promises, unhandled rejections, swallowed exceptions (catch {}), errors logged and then continued as if success.
• Network calls with no timeout, no retry, or retry without backoff/jitter; non-idempotent retries.
• Money/quantity in floats; timezone and DST handling; off-by-one on ranges.
• Null/undefined paths the types claim are impossible but the data allows.
• Breaking API/contract changes for existing clients; enum/status values added without handling everywhere they are switched on.
• What happens on partial failure — is the system left in a valid state?

Step 4 — Grill your own findings

Before writing the report, attack each candidate finding as a skeptic whose job is to kill it:

• Re-open the file and confirm the code still says what you claim.
• Look for the guard you might have missed — a middleware, a base repository that auto-scopes, a DB constraint, a @Transactional, a global default.
• Ask whether the trigger is actually reachable from real callers.
• Default to dropping the finding when you are unsure.

Findings that survive are reported as Confirmed. A finding you strongly suspect but could not fully verify may be reported as Unverified — clearly labeled, with the exact check you could not complete and how to complete it. Never blend the two.

Step 5 — Report

Order by severity, worst first. Keep it dense.

### [Blocker] Tenant scope missing on invoice lookup
apps/api/src/invoices/invoices.service.ts:48

  const invoice = await db.invoice.findUnique({ where: { id } })

`id` comes straight from the route param and the query has no orgId filter.
The guard at invoices.controller.ts:22 only checks that a session exists.
Trigger: any authenticated user calls GET /invoices/<other-org-invoice-id>
and reads another tenant's invoice, including amounts and customer email.
Fix: scope the query to session.orgId (matches the pattern in
subscriptions.service.ts:31) and return 404 on miss.

Severity ladder:

• Blocker — data leak, auth bypass, data loss, or guaranteed production breakage. Do not ship.
• High — will hurt under real traffic or real data (N+1 on a hot path, missing index on a growing table, race that corrupts state).
• Medium — real defect, limited blast radius.
• Nit — worth knowing, not worth blocking. Cap this section at five items.

Close with a one-line verdict: Ship / Ship after blockers / Do not ship, and the list of passes you skipped.

Step 6 — Ask before fixing

If — and only if — at least one issue was found, ask once:

Found N issues (X blockers). Want me to fix them, or is this review-only?

Options to offer: fix blockers + high only, fix everything, or report only. Then stop and wait. If the user does not ask for fixes, write no code at all — not even "while I'm here" cleanups.

Step 7 — Fix protocol (only if authorized)

• Fix in severity order, smallest correct change per issue.
• Follow the codebase's existing pattern for that concern; do not introduce a new abstraction to fix one bug.
• One issue per commit-sized change, and re-state which finding it closes.
• Run the project's typecheck/lint/tests after the fixes and report the real output. If something fails, say so — do not claim green.
• Do not fix Unverified findings. Verify first, or leave them.