docs(review): clarify what the no-mocking rule is actually for

The literal reading of "never mock anything" trips up AI reviewers
(and humans new to the repo) — they flag any `vi.mock` / `vi.fn` /
`vi.spyOn` they see, even when the usage isn't actually faking
behavior. Three patterns are fine and should NOT be flagged:

1. Module-load workarounds — vi.mock("~/db.server") at the top of a
   unit test to stop prisma.$connect() firing at import. Cuts the
   import graph, doesn't fake DB behavior.
2. Hand-written DI doubles where the real implementation has its own
   dedicated infra-backed tests (CapturingMollifierBuffer, MockPayloadProcessor,
   etc.). Unit test covers wiring, integration test covers the seam target.
3. vi.fn as a DI-seam probe — convenience for "was the seam called."
   Equivalent to a closure-counter; not load-bearing on what's proven.

Still 🔴: spying on the code path under test then asserting the spy was
called (tautology), or replacing real infra with mocks in tests meant
to cover real behavior (e.g. mocking Redis in a Redis-queue test).

Author: d-cs

.claude/REVIEW.md

@@ -23,6 +23,14 @@ Reserve 🔴 for things that would page someone or block a rollback. In this cod
 ## Always check
 
 - **Tests use testcontainers, not mocks.** Vitest with `redisTest` / `postgresTest` / `containerTest` from `@internal/testcontainers`. Any new `vi.mock(...)` on Redis, Postgres, BullMQ, or other infra is wrong here — 🔴 if added in production-path tests, 🟡 if isolated unit test.
+
+  **What the rule is actually for.** The intent is "don't fool yourself by mocking real flow into oblivion" — not "literally zero use of `vi.mock` / `vi.fn` / `vi.spyOn` anywhere ever." Three things are fine and should NOT be flagged:
+
+  1. **Module-load workarounds.** `vi.mock("~/db.server", ...)` at the top of a unit test purely to stop Prisma `$connect()` firing at import time. The test isn't faking DB behavior — it's cutting the import graph so the module under test loads without bringing in `prisma.$connect()`. The actual code under test still runs.
+  2. **Hand-written DI doubles where the real implementation has its own dedicated tests.** Pattern: a test file constructs the service-under-test with a stub injected through a DI seam (e.g. `CapturingMollifierBuffer`, `MockPayloadProcessor`, `MockTriggerTaskValidator` already in the repo). Acceptable when the stubbed dependency has its OWN dedicated suite hitting real infra (e.g. `mollifierTripEvaluator.test.ts` exercises the real `MollifierBuffer` via `redisTest`). The unit test verifies the wiring at the seam; the integration test verifies the seam's target.
+  3. **`vi.fn` used as a probe at a DI seam.** Sometimes the test only needs "was the seam invoked, with what args" — `vi.fn` is a convenience for this. The same shape can be written as a plain closure incrementing a captured counter, but the `vi.fn` form is not load-bearing on whether the test proves anything. Prefer the closure form for new code; don't 🔴 an existing `vi.fn` probe that follows the host file's prior art.
+
+  Still 🔴: mocking out the actual code path under test (e.g. `vi.spyOn(realService, "doTheThing").mockResolvedValue(...)` then asserting "doTheThing was called" — that's a tautology, not a test). Still 🔴: replacing real infra with mocks in tests that are meant to cover the real behavior (e.g. mocking Redis in a test of a Redis-backed queue).
 - **Public-package changes have a changeset.** `pnpm run changeset:add` produces `.changeset/*.md`. Required for any edit under `packages/*`. Missing → 🟡; missing on a breaking change → 🔴.
 - **Server-only changes have `.server-changes/*.md`.** Required for `apps/webapp/`, `apps/supervisor/` edits with no public-package change. Body should be 1-2 sentences (it has to fit as one bullet in a future changelog). Missing → 🟡.
 - **Lua script naming.** Coexisting scripts use behavior-descriptive suffixes (`Tracked`), never `V2`. Old name must keep working until the next deploy clears it.