docs(adr-012): record the sync-factory invariant and the $app/state migration

The previous revision allowed vi.mock for virtual modules on the "consumer
import is static" argument. #553 proved that argument wrong: a statically-
imported module with an async factory body whose dynamic import landed
after teardown still produced the race. The factory body — not the
consumer — is the failure surface.

- Drop the "residual exceptions" table.
- Add the binding invariant: factory bodies under `**/*.svelte.{test,spec}.ts`
  must be synchronous (no `await`, no `import(...)`).
- Document the canonical vi.hoisted + getter pattern, with file references.
- Record the $app/stores → $app/state architectural call (Markus's
  recommendation), removing one of the last two deprecated-import
  outliers.
- Record the preload-data=off hardening (Tobias's recommendation) as a
  pattern note.
- Update the Enforcement section to list all four defence layers (ESLint,
  CI grep, in-suite meta-test, CI birpc assert) and the coverage-flake-
  probe verification workflow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs/adr/012-browser-test-mocking-strategy.md

@@ -1,8 +1,8 @@
 # ADR 012 — Browser-Mode Test Mocking Strategy
 
 **Status:** Accepted  
-**Date:** 2026-05-11  
-**Issue:** [#535 — Unit & Component Tests job exits 1 from vitest-browser teardown race](https://git.raddatz.cloud/marcel/familienarchiv/issues/535)
+**Date:** 2026-05-11 (revised 2026-05-12)  
+**Issues:** [#535 — original incident](https://git.raddatz.cloud/marcel/familienarchiv/issues/535) · [#553 — revision](https://git.raddatz.cloud/marcel/familienarchiv/issues/553)
 
 ---
 
@@ -65,22 +65,38 @@ For components that fetch data or call services, the `*.test-host.svelte` patter
 
 ---
 
-## Residual exceptions
+## Binding invariant: factory bodies must be synchronous (#553)
 
-The following `vi.mock(module, factory)` calls in browser specs are **acceptable** because the mocked modules are loaded statically at spec module-eval time and cannot produce a teardown race:
+The original revision of this ADR allowed `vi.mock(virtualModule, factory)` for SvelteKit/Vite virtual modules on the argument that their consumer imports were resolved at static-import time. **That reasoning is wrong.** What matters is what the **factory body** does, not where the mocked module is consumed.
 
-| Module | Why it stays as vi.mock |
-|--------|------------------------|
-| `$app/navigation` | SvelteKit virtual module — no DI seam |
-| `$app/forms` | SvelteKit virtual module — no DI seam |
-| `$app/state` | SvelteKit virtual module — no DI seam |
-| `$app/stores` | SvelteKit virtual module — no DI seam |
-| `$env/static/public` | Vite env virtual module — no DI seam |
+`EnrichmentBlock.svelte.spec.ts` (issue #553) was statically imported and still produced the race: its `vi.mock('$app/stores', async () => { const mod = await import(...); return mod; })` factory performed a dynamic import in its body, and that body was invoked asynchronously when Chromium fetched the manually-mocked module — sometimes after the worker's birpc channel had already closed.
 
-These modules are resolved at static import time (before any test runs). Their `vi.mock` factories are served by birpc synchronously during module graph resolution, not after worker teardown.
+**Therefore: under `**/*.svelte.{test,spec}.ts`, every `vi.mock` factory body must be synchronous. No `await`, no `import(...)`.**
+
+If a factory needs to share state with the spec (a mutable ref, a `vi.fn`, a writable store), use `vi.hoisted()` to lift the reference above `vi.mock`'s implicit hoist:
+
+```ts
+const { mockNavigating } = vi.hoisted(() => ({
+    mockNavigating: { type: null as string | null }
+}));
+
+vi.mock('$app/state', () => ({
+    get navigating() {
+        return mockNavigating;
+    }
+}));
+```
+
+The getter defers the read until consumption time; `vi.hoisted` guarantees the reference is initialised before the (also hoisted) `vi.mock` factory runs. See `DropZone.svelte.spec.ts:9`, `NotificationBell.svelte.spec.ts:6-10`, and `EnrichmentBlock.svelte.spec.ts` for canonical examples.
+
+### Architectural follow-on: prefer `$app/state` over `$app/stores`
+
+`$app/stores` is the deprecated subscription-based store API; `$app/state` is the modern reactive proxy. New components should import from `$app/state`. As part of #553 we migrated `EnrichmentBlock.svelte` from `$app/stores.navigating` to `$app/state.navigating` with `!!navigating.type` — matching the pattern already established in `routes/aktivitaeten/+page.svelte:117` and `routes/documents/+page.svelte:261`. Migration eliminated the *need* to mock a store at all in that spec.
 
 **Pattern note:** When an overlay or dropdown triggers a navigation action, use `<button type="button">` with an `onclick` handler that calls `goto(path)` — do **not** use `<a href="…">` with `e.preventDefault()`. SvelteKit registers its link interceptor as a capture-phase `document` listener, so it fires before the component's bubble-phase `onclick`. By the time `e.preventDefault()` runs the router has already initiated navigation, which tears down the vitest-browser Playwright orchestrator iframe. A `<button>` carries no `href`, so the capture-phase interceptor never fires. See `NotificationDropdown.svelte` for the canonical example.
 
+**Pattern note (#553):** Browser-mode tests run with `data-sveltekit-preload-data="off"` (set in `src/test-setup.ts` via the client project's `setupFiles`). Hover-prefetch otherwise fires real fetch requests for route loader chunks; those requests go through the same Playwright route handler that serves mocked modules. An in-flight prefetch landing after iframe teardown can hit the handler with a closed birpc channel, raising an unhandled rejection.
+
 ---
 
 ## Consequences
@@ -88,5 +104,10 @@ These modules are resolved at static import time (before any test runs). Their `
 - New browser-mode specs that need to stub an external library **must not** use `vi.mock(externalLib, factory)`. Add a loader/factory parameter to the underlying hook or service instead.
 - The CI `unit-tests` job includes a permanent grep guard that fails the build if `rpc is closed` appears in any coverage run log. This catches regressions before they reach the acceptance criterion.
 - Acceptance criterion for #535: 60 consecutive green `workflow_dispatch` CI runs against `main` after the fix is merged, with zero `rpc is closed` lines in any log.
-- **Enforcement:** An ESLint `no-restricted-syntax` rule in `eslint.config.js` (scoped to `**/*.spec.ts` and `**/*.test.ts`) flags any `vi.mock('pdfjs-dist', ...)` call with a message referencing this ADR. This turns a ~2-minute CI wait into an immediate lint error on save. The CI birpc grep guard remains as belt-and-suspenders for the coverage run. A CI static grep step (added in issue #546) also catches the pattern before the test suite launches.
+- **Enforcement (four layers, defence in depth):**
+  1. **ESLint `no-restricted-syntax`** in `eslint.config.js` (scoped to `**/*.{spec,test}.ts`) flags two patterns: (a) the literal `vi.mock('pdfjs-dist', ...)` — enforces the libLoader pattern — and (b) any `vi.mock(..., async () => { ... await import(...) ... })` — enforces the synchronous-factory invariant. Both messages point at this ADR. Failure surfaces at save time.
+  2. **CI grep guard** in `.gitea/workflows/ci.yml` runs before the test suite launches. Mirrors the ESLint patterns with `grep -Pzn`. ~10s round-trip.
+  3. **In-suite meta-test** at `frontend/src/__meta__/no-async-mock-factories.test.ts` globs `src/**/*.svelte.{test,spec}.ts` and asserts none match the banned pattern. Catches at every vitest invocation — the layer hardest to disable.
+  4. **CI birpc assert** runs after the coverage step and fails the build if `[birpc] rpc is closed` appears in any log line. Catches the symptom even if all the upstream layers were bypassed.
+- **Acceptance verification:** `coverage-flake-probe.yml` is a `workflow_dispatch`-triggered matrix workflow that runs the coverage suite 20× in parallel against a single SHA and asserts zero birpc lines. One fire, parallel cost, deterministic signal — replaces accumulating 20 sequential push events.
 - **When to revisit the LibLoader home:** If three or more components adopt this pattern, consider extracting a shared `$lib/types/lib-loader.ts` or a generic `DynamicImportLoader<T>` type to avoid parallel type definitions across modules.