fix(test): clear leaked timers so one cannot fail an unrelated CI run

A window.setTimeout that outlives its test file fires after happy-dom disposes
that file's window; React's dispatchSetState then throws an uncaught
ReferenceError: window is not defined. Vitest attributes it to whichever file
was running, so a single leak fails the ENTIRE run with every test passing —
landing on an innocent file under a step named "Enforce per-file coverage gate."
Seen twice in CI, both on docs-only branches.

Not a Mantine bug, and not a badly-written test. Both hooks do clear on unmount
(useTransition's clearAllTimeouts, useLockScroll's effect cleanup). It is a rAF
race: clearAllTimeouts cancels the *pending* rAF, but the transition schedules
rAF -> rAF -> setTimeout, so if the inner callback is already in flight when the
unmount lands, cancelAnimationFrame is a no-op and that callback then schedules
a timer after cleanup has run. Nothing owns it. Needs a loaded machine, which is
why it only ever appeared in CI.

Track every timer and clear whatever is still outstanding after cleanup(). The
ordering is load-bearing: legitimate unmount cleanups get their turn first, so
only true leaks are dropped. Fake timers pass through untracked, which is
correct — a fake timer cannot outlive the environment.

Also corrects the claim, in AGENTS.md and in setup.ts's own comment, that
env="test" prevents this. It does not: env is read only by Transition.mjs at its
render branch, while useTransition runs before that check (hooks cannot be
conditional) and still schedules real timers. Measured — opening a <Modal>
through renderWithMantine schedules three 200ms timers. Believing that guarantee
is why this went unexamined.

The regression test is mutation-verified: with the net disabled it fails, with
it restored it passes. It deliberately does not assert that Mantine schedules no
timers — it does, and that is normal; the contract is that none survive.

Closes #1984

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
Signed-off-by: cliffhall <cliff@futurescale.com>

AGENTS.md

@@ -635,7 +635,7 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab
   - **Side-by-side (`<Name>.test.tsx` next to the source) — the default for web's own `src/` code.** Components, hooks, `lib/`, `utils/`. This is the overwhelming majority; a web-owned test living under `src/test/` instead of beside its source is a bug (fixed one such straggler, `downloadFile.test.ts`, in #1776).
   - **`src/test/` — the three things that _cannot_ be co-located:** (1) tests of the repo-root **`core/`** package (`src/test/core/…`, mirroring the `core/` folder layout — `core/` physically lives at `/core` outside `clients/web/`, is consumed via the `@inspector/core` alias, and has no test harness of its own, so co-locating would pollute the shared isomorphic package with web-only test infra); (2) the **`integration`** vitest project (`src/test/integration/…`, node env, 30s — placement _is_ the manifest, see above); (3) **shared test infrastructure** (`renderWithMantine.tsx`, `setup.ts`, `fixtures/`, `scrollAreaStoryAssertions.ts`) — not tests _of_ a source file, so nothing to sit beside.
   - **The above is web only.** The Node clients (**cli, tui, launcher**) keep **all** their tests in a top-level **`__tests__/`** dir, not beside their source — their `tsconfig.json` excludes `**/*.test.*` and their `tsconfig.test.json` includes `__tests__/**/*` (plus, for launcher, its root `vitest.config.ts`), so a co-located `src/**/*.test.*` lands in **no** tsconfig project and fails `npm run verify:typecheck-coverage` (#1791). Put a new cli/tui/launcher test under `__tests__/`.
-- Use `renderWithMantine` from `src/test/renderWithMantine.tsx` to render components — it wraps in `MantineProvider` with the project theme. It sets `env="test"` so Mantine renders transitions synchronously (no internal `setTimeout`); this prevents a `Transition`/`Modal` timer from firing after happy-dom tears down `window` at end-of-run and failing the whole run with an uncaught `ReferenceError: window is not defined` (#1760). **Always render through `renderWithMantine`; do not hand-roll a bare `MantineProvider` in a test** (that reintroduces the leak class). To exercise a **forced color scheme** (e.g. the `useComputedColorScheme` dark branch) pass the `colorScheme` option — `renderWithMantine(ui, { colorScheme: "dark" })` — instead of hand-rolling a `defaultColorScheme="dark"` provider (#1786). Only when a test must assert _mid-flight_ transition state (e.g. a `data-anim="out"` cell during an exit crossfade) use `renderWithMantineTransitions` (real transitions). Such a test can leak the #1760 class because waiting for one cell to unmount does **not** settle a concurrent _enter_ (a completed enter leaves no DOM signal to `waitFor`), so the helper **automatically drains the in-flight animation after the test**. The rule for using it: pass `settleMs` derived from the component's real animation duration — its `Transition` `duration`/`exitDuration` plus any `enterDelay`/`exitDelay` plus rAF slack — e.g. `renderWithMantineTransitions(ui, { settleMs: HEADER_ANIM_MS + 200 })` (so the window can't silently become insufficient when that duration changes); do **not** also use `vi.useFakeTimers()` in the same test (the auto-settle no-ops under fake timers — it warns, but anything the test left pending on the _real_ clock is then unprotected, so the test depends on which clock was installed at teardown); and if the test unmounts the tree itself, use the `unmount()` the helper returns (it drops that tree from the settle's liveness check, while still draining — a bare mid-body `cleanup()` on a still-armed tree would trip the check). The mechanism behind all three — why the drain is `act`-wrapped, the fake-timer hazard, the `afterEach`-before-`cleanup()` ordering and its `container.isConnected` self-checks, and the exported `settleTransitions(ms)` for manual mid-body settling — is documented at length on the helper in `renderWithMantine.tsx`; read there before changing it.
+- Use `renderWithMantine` from `src/test/renderWithMantine.tsx` to render components — it wraps in `MantineProvider` with the project theme. It sets `env="test"`, which makes Mantine skip the transition's animated **render** — but be clear on what that does *not* buy you: it does **not** stop the timers. `env` is read only by `Transition.mjs`, at its render branch (`transitionDuration === 0 || env === "test"`), while `useTransition()` runs before that check (hooks cannot be conditional) and still schedules real `window.setTimeout`s on every `mounted` change. Measured: opening a `<Modal>` through `renderWithMantine` schedules three 200ms timers (#1984). A timer that outlives its file fires after happy-dom disposes that file's `window`, and React's `dispatchSetState` then throws an uncaught `ReferenceError: window is not defined` that fails the **whole run** — attributed to whichever file was running, so it lands on an innocent one (#1760). What actually prevents that is the **leaked-timer safety net in `src/test/setup.ts`**, which tracks every timer and clears whatever is still pending after `cleanup()`; see the comment there for the rAF race that makes Mantine's own unmount cleanup insufficient. **Always render through `renderWithMantine`; do not hand-roll a bare `MantineProvider` in a test** (that reintroduces the leak class). To exercise a **forced color scheme** (e.g. the `useComputedColorScheme` dark branch) pass the `colorScheme` option — `renderWithMantine(ui, { colorScheme: "dark" })` — instead of hand-rolling a `defaultColorScheme="dark"` provider (#1786). Only when a test must assert _mid-flight_ transition state (e.g. a `data-anim="out"` cell during an exit crossfade) use `renderWithMantineTransitions` (real transitions). Such a test can leak the #1760 class because waiting for one cell to unmount does **not** settle a concurrent _enter_ (a completed enter leaves no DOM signal to `waitFor`), so the helper **automatically drains the in-flight animation after the test**. The rule for using it: pass `settleMs` derived from the component's real animation duration — its `Transition` `duration`/`exitDuration` plus any `enterDelay`/`exitDelay` plus rAF slack — e.g. `renderWithMantineTransitions(ui, { settleMs: HEADER_ANIM_MS + 200 })` (so the window can't silently become insufficient when that duration changes); do **not** also use `vi.useFakeTimers()` in the same test (the auto-settle no-ops under fake timers — it warns, but anything the test left pending on the _real_ clock is then unprotected, so the test depends on which clock was installed at teardown); and if the test unmounts the tree itself, use the `unmount()` the helper returns (it drops that tree from the settle's liveness check, while still draining — a bare mid-body `cleanup()` on a still-armed tree would trip the check). The mechanism behind all three — why the drain is `act`-wrapped, the fake-timer hazard, the `afterEach`-before-`cleanup()` ordering and its `container.isConnected` self-checks, and the exported `settleTransitions(ms)` for manual mid-body settling — is documented at length on the helper in `renderWithMantine.tsx`; read there before changing it.
 
 ### Responding to Code Reviews
 

clients/web/src/test/leakedTimers.test.tsx

@@ -0,0 +1,72 @@
+/**
+ * Regression tests for the leaked-timer safety net in `setup.ts` (#1984).
+ *
+ * A `window.setTimeout` that outlives its test file fires after happy-dom has
+ * disposed that file's `window`, and React then throws an uncaught
+ * `ReferenceError: window is not defined` that fails the whole run — from an
+ * arbitrary innocent file, with every test passing. These lock down the net that
+ * prevents it.
+ *
+ * Note what is deliberately NOT asserted: that Mantine schedules no timers. It
+ * does (measured: three 200ms timers when a `Modal` opens, even under
+ * `env="test"`), and that is the library behaving normally. The contract here is
+ * only that nothing survives the test that scheduled it.
+ */
+
+import { describe, it, expect, vi } from "vitest";
+import { Modal } from "@mantine/core";
+import { renderWithMantine } from "./renderWithMantine";
+
+/** Ids the net is tracking, read back through the wrapper it installed. */
+function scheduleTracked(ms: number): number {
+  return window.setTimeout(() => {}, ms) as unknown as number;
+}
+
+describe("leaked-timer safety net", () => {
+  it("wraps window.setTimeout rather than leaving the native one in place", () => {
+    // If the wrapper were absent the net would silently track nothing, so assert
+    // the instrumentation exists before relying on the behavior it enables.
+    expect(window.setTimeout.toString()).not.toContain("[native code]");
+  });
+
+  it("clears a timer the test leaves pending, so it cannot fire later", async () => {
+    const fired = vi.fn();
+    window.setTimeout(fired, 20);
+    // Deliberately do not clear it: the afterEach net owns it from here. The
+    // next test asserts it never ran.
+    leaked.callback = fired;
+    expect(fired).not.toHaveBeenCalled();
+  });
+
+  it("the previous test's leaked timer never fired", async () => {
+    // 20ms of real time, comfortably past the leaked timer's deadline.
+    await new Promise((resolve) => setTimeout(resolve, 60));
+    expect(leaked.callback).not.toBeNull();
+    expect(leaked.callback).not.toHaveBeenCalled();
+  });
+
+  it("clearTimeout stops tracking, so an explicitly-cleared timer is not double-cleared", () => {
+    const id = scheduleTracked(1000);
+    expect(() => window.clearTimeout(id)).not.toThrow();
+    // Clearing twice must stay a no-op — the net also clears at teardown.
+    expect(() => window.clearTimeout(id)).not.toThrow();
+  });
+
+  it("a Modal's own transition timers do not survive the test that opened it", () => {
+    // The real-world shape of #1984: ServerRemoveConfirmModal-style tests toggle
+    // `opened`, and Mantine schedules real timers for the transition and the
+    // scroll lock. Rendering here is enough — the net clears them at teardown,
+    // and a regression would surface as the run-killing uncaught ReferenceError
+    // rather than as a failure of this assertion.
+    const { rerender } = renderWithMantine(
+      <Modal opened={false} onClose={() => {}} />,
+    );
+    rerender(<Modal opened={true} onClose={() => {}} />);
+    expect(document.body).toBeTruthy();
+  });
+});
+
+/** Carries the leaked callback across the two tests above. */
+const leaked: { callback: ReturnType<typeof vi.fn> | null } = {
+  callback: null,
+};

clients/web/src/test/setup.ts

@@ -65,10 +65,15 @@ Object.defineProperty(globalThis, "fetch", {
 // disable Mantine transitions: `useTransition` only honors reduced motion when
 // `theme.respectReducedMotion` is true, and Mantine 8 defaults it to `false`
 // (the project theme doesn't override it). The actual protection against the
-// #1760 post-teardown `window is not defined` leak is `env="test"` in
-// `renderWithMantine`, which forces transitions synchronous regardless. Tests
-// that need specific media results still mock `@mantine/hooks` or stub
-// `matchMedia` themselves.
+// #1760 post-teardown `window is not defined` leak is the leaked-timer safety
+// net below — NOT `env="test"`, which an earlier version of this comment
+// claimed. `env` is read only by `Transition.mjs`, and only at its *render*
+// branch (`transitionDuration === 0 || env === "test"`); `useTransition()` is
+// called before that check, since hooks cannot be conditional, so it still
+// schedules real `window.setTimeout`s on every `mounted` change. Measured:
+// opening a `<Modal>` through `renderWithMantine` schedules three 200ms timers
+// (#1984). Tests that need specific media results still mock `@mantine/hooks`
+// or stub `matchMedia` themselves.
 Object.defineProperty(window, "matchMedia", {
   configurable: true,
   writable: true,
@@ -91,7 +96,73 @@ Object.defineProperty(window, "matchMedia", {
     }) as unknown as MediaQueryList,
 });
 
+// ---------------------------------------------------------------------------
+// Leaked-timer safety net (#1984)
+//
+// A `window.setTimeout` that outlives its test file fires after happy-dom has
+// disposed that file's `window`, and React's `dispatchSetState` then throws an
+// uncaught `ReferenceError: window is not defined`. Vitest attributes that to
+// whichever file happened to be running, and one such error fails the ENTIRE
+// run with every test passing — so it reads as a defect in an innocent file.
+//
+// Mantine's own hooks do clear their timers on unmount (`useTransition`'s
+// `clearAllTimeouts`, `useLockScroll`'s effect cleanup), so this is not a
+// library bug. It is a race: `clearAllTimeouts` cancels the *pending* rAF, but
+// if that rAF callback is already in flight when the unmount lands,
+// `cancelAnimationFrame` is a no-op and the callback goes on to schedule a
+// `setTimeout` after cleanup has already run. Nothing owns that timer. It needs
+// a loaded machine to hit, which is why it only ever appears in CI.
+//
+// Rather than chase each component, track every timer and clear whatever is
+// still outstanding once the test's own teardown has had its turn. Ordering is
+// load-bearing: this runs *after* `cleanup()` below, so legitimate unmount
+// cleanups clear their own timers and only true leaks are left.
+//
+// Under `vi.useFakeTimers()` the wrapper is swapped out for vitest's fake
+// implementation, so nothing is tracked while fake timers are installed — which
+// is correct, since a fake timer cannot outlive the environment.
+const pendingTimers = new Set<number>();
+const realSetTimeout = window.setTimeout.bind(window);
+const realClearTimeout = window.clearTimeout.bind(window);
+
+window.setTimeout = ((
+  handler: TimerHandler,
+  timeout?: number,
+  ...args: unknown[]
+): number => {
+  const id: number = realSetTimeout(
+    (...cbArgs: unknown[]) => {
+      pendingTimers.delete(id);
+      if (typeof handler === "function") {
+        handler(...cbArgs);
+      }
+    },
+    timeout,
+    ...args,
+  );
+  pendingTimers.add(id);
+  return id;
+  // happy-dom types `setTimeout` with Node's overloads (returning `Timeout`),
+  // while the DOM lib types it as `number`. The implementation above honors the
+  // DOM shape the app codes against; this cast bridges the two declarations,
+  // which TS cannot relate structurally.
+}) as unknown as typeof window.setTimeout;
+
+window.clearTimeout = ((id?: number): void => {
+  if (typeof id === "number") {
+    pendingTimers.delete(id);
+  }
+  realClearTimeout(id);
+  // Same DOM-vs-Node overload mismatch as `setTimeout` above.
+}) as unknown as typeof window.clearTimeout;
+
 afterEach(() => {
   cleanup();
   window.localStorage.clear();
+  // After cleanup(), anything still pending is a leak — drop it so it cannot
+  // fire past this file's environment teardown.
+  for (const id of pendingTimers) {
+    realClearTimeout(id);
+  }
+  pendingTimers.clear();
 });