web: disable Mantine transitions in tests to kill post-teardown timer flake (#1760) (#1761)

* web: disable Mantine transitions in tests to kill post-teardown timer flake (#1760)

A Mantine `Transition`/`Modal` open/close `setTimeout` can fire after happy-dom
tears down `window` at the end of a run, throwing an uncaught
`ReferenceError: window is not defined` that fails the whole `coverage` job even
when every assertion passed (seen on PR #1759's CI, originating in
ServerRemoveConfirmModal.test.tsx).

Set `env="test"` on the shared `renderWithMantine` MantineProvider so Mantine
renders transitions synchronously (no timer). This removes the leak class for
every test that renders through the shared wrapper (Modals, Menus, Popovers, …).
Also apply it to ViewHeader's local forced-dark provider.

The handful of tests that assert mid-flight transition state (ViewHeader's
`data-anim="out"` crossfade checks, #1450) genuinely need real transitions, so
add an opt-in `renderWithMantineTransitions` wrapper (env="default") and route
those three tests through it — each already drives the transition to completion
(waitFor/findBy) so it doesn't reintroduce the leak. Tests that assert only the
entered state stay on the default (leak-free) wrapper.

Full `npm run ci` green; unit run reports zero unhandled errors.

Closes #1760

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: note the renderWithMantine env=test invariant in AGENTS.md (#1760)

Per review feedback: document that tests must render through
`renderWithMantine` (env="test", transition-free) rather than a hand-rolled
`MantineProvider`, and reserve `renderWithMantineTransitions` for asserting
mid-flight animation state (driving the transition to completion). Keeps future
tests from quietly reintroducing the post-teardown timer leak class.

Refs #1760

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

AGENTS.md

@@ -200,7 +200,7 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i
   - **TUI** (`clients/tui`): the gate covers the **non-React logic** only — `logger.ts`, `components/tabsConfig.ts`, and `utils/*` (server resolution lives in `core/` and is measured by the web suite). The Ink components, `App.tsx`, and `hooks/` are an **interim exclusion** in `clients/tui/vitest.config.ts` pending the renderer-based follow-up (#1501). When adding new **non-React** logic under `clients/tui/src`, it falls under the gate automatically — add tests for it.
 - Run `npm run test:integration` (also from `clients/web/`) for the InspectorClient + transport + auth integration suite. It runs under a separate `integration` vitest project in node env (no happy-dom) with 30s timeouts. The script builds `test-servers/` first via `tsc -p ../../test-servers --noCheck` so the stdio MCP test server can be spawned as a real subprocess. CI does not run `test:integration` as its own step — the integration project is covered by the CI `coverage` gate, whose web `test:coverage` runs `--project=unit --project=integration --coverage`.
 - Test files live alongside the source as `<Name>.test.tsx` (or `.test.ts` for non-React modules). Integration tests live under `clients/web/src/test/integration/`, mirroring the `core/` source layout (`mcp/`, `mcp/node/`, `mcp/remote/`, `auth/`, `auth/node/`, `storage/`). Any test file under that folder is automatically picked up by the `integration` vitest project (node env, 30s timeouts) via the folder glob in `vite.config.ts` — placement is the manifest, there is no enumeration to keep in sync. Tests outside the folder run in the `unit` project (happy-dom). When adding a new test for, e.g., `core/mcp/remote/foo.ts`, put it at `src/test/integration/mcp/remote/foo.test.ts`.
-- Use `renderWithMantine` from `src/test/renderWithMantine.tsx` to render components — it wraps in `MantineProvider` with the project theme
+- 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). 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) — and that test MUST drive the transition to completion (`await waitFor`/`findBy`) so its timer resolves before teardown.
 
 ### Responding to Code Reviews
 - When asked to respond to a code review of a PR,

clients/web/src/components/groups/ViewHeader/ViewHeader.test.tsx

@@ -3,6 +3,7 @@ import userEvent from "@testing-library/user-event";
 import {
   act,
   renderWithMantine,
+  renderWithMantineTransitions,
   screen,
   waitFor,
 } from "../../../test/renderWithMantine";
@@ -12,11 +13,13 @@ import { theme } from "../../../theme/theme";
 import { ViewHeader } from "./ViewHeader";
 
 // Render under a forced-dark MantineProvider so `useComputedColorScheme`
-// returns "dark" (the light/dark icon + logo branches).
+// returns "dark" (the light/dark icon + logo branches). `env="test"` disables
+// timer-driven transitions, matching the shared `renderWithMantine` wrapper and
+// avoiding post-teardown `window is not defined` races (#1760).
 function renderDark(ui: React.ReactElement) {
   return render(ui, {
     wrapper: ({ children }) => (
-      <MantineProvider theme={theme} defaultColorScheme="dark">
+      <MantineProvider theme={theme} defaultColorScheme="dark" env="test">
         {children}
       </MantineProvider>
     ),
@@ -240,7 +243,9 @@ describe("ViewHeader", () => {
 
     it("crossfades on disconnect: tab bar exits while the title enters, then the bar unmounts (#1450)", async () => {
       mediaQueryMock.value = true;
-      const { rerender } = renderWithMantine(
+      // Asserts the mid-flight exit ("out") state, so it needs real transitions;
+      // it drives them to completion below (waitFor unmount).
+      const { rerender } = renderWithMantineTransitions(
         <ViewHeader {...connectedProps} />,
       );
       expect(screen.getAllByRole("radio").length).toBeGreaterThan(0);
@@ -309,7 +314,9 @@ describe("ViewHeader", () => {
 
     it("animates the server name and disconnect controls in on connect, out on disconnect (#1450)", async () => {
       mediaQueryMock.value = true;
-      const { rerender } = renderWithMantine(
+      // Asserts the mid-flight exit ("out") state, so it needs real transitions;
+      // it drives them to completion below (waitFor unmount).
+      const { rerender } = renderWithMantineTransitions(
         <ViewHeader {...connectedProps} />,
       );
       // Connected: the server name and the Disconnect control are in their
@@ -368,8 +375,10 @@ describe("ViewHeader", () => {
 
     it("crossfades the title out and the connected header in on connect (#1450)", async () => {
       mediaQueryMock.value = true;
+      // Asserts the mid-flight exit ("out") state, so it needs real transitions;
+      // it drives them to completion below (findByText for the entering header).
       // Start disconnected: the title cell is the entering one.
-      const { rerender } = renderWithMantine(
+      const { rerender } = renderWithMantineTransitions(
         <ViewHeader
           connected={false}
           onToggleTheme={vi.fn()}

clients/web/src/test/renderWithMantine.tsx

@@ -3,9 +3,28 @@ import { render, type RenderOptions } from "@testing-library/react";
 import { MantineProvider } from "@mantine/core";
 import { theme } from "../theme/theme";
 
-function MantineWrapper({ children }: { children: ReactNode }) {
+// `env="test"` makes Mantine render transitions synchronously (no internal
+// `setTimeout`). Without it, a `Transition`/`Modal` open/close timer can fire
+// after happy-dom tears down `window` at the end of the run, throwing an
+// uncaught `ReferenceError: window is not defined` that fails the whole run
+// even when every assertion passed (#1760). This is the right default for the
+// vast majority of tests, which don't assert on mid-transition state.
+function TestEnvWrapper({ children }: { children: ReactNode }) {
   return (
-    <MantineProvider theme={theme} defaultColorScheme="light">
+    <MantineProvider theme={theme} defaultColorScheme="light" env="test">
+      {children}
+    </MantineProvider>
+  );
+}
+
+// Opt-in wrapper that keeps Mantine's timer-driven transitions enabled, for the
+// few tests that assert on transition/animation state that only exists mid-flight
+// (e.g. a `data-anim="out"` cell during an exit crossfade). Such a test MUST
+// drive the transition to completion (`await waitFor`/`findBy`) so its timer
+// resolves before teardown; otherwise it reintroduces the #1760 leak.
+function TransitionsWrapper({ children }: { children: ReactNode }) {
+  return (
+    <MantineProvider theme={theme} defaultColorScheme="light" env="default">
       {children}
     </MantineProvider>
   );
@@ -15,7 +34,14 @@ export function renderWithMantine(
   ui: ReactElement,
   options?: Omit<RenderOptions, "wrapper">,
 ) {
-  return render(ui, { wrapper: MantineWrapper, ...options });
+  return render(ui, { wrapper: TestEnvWrapper, ...options });
+}
+
+export function renderWithMantineTransitions(
+  ui: ReactElement,
+  options?: Omit<RenderOptions, "wrapper">,
+) {
+  return render(ui, { wrapper: TransitionsWrapper, ...options });
 }
 
 export * from "@testing-library/react";