modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

test(web): await the theme observer instead of a wall-clock budget

Browse source 
The AppRenderer "live theme flip" test had its inner `waitFor` raised twice
(bare assertion -> 1000ms -> 5000ms) and failed a third time at 5032ms under
`local:gate` load. The budget was never the problem.

Root cause: happy-dom 20.11.1's `MutationObserverListener` stores its dispatch
callback as `new WeakRef((record) => this.report(record))` — an arrow function
nothing else holds strongly. A GC between `observe()` and the mutation collects
it, `callback.deref()` returns undefined in `Node[reportMutation]`, and the
mutation is dropped silently and permanently. That is why the failure tracks
machine load rather than any wall-clock value, and why no raise ever fixed it.

Reproduced directly against 20.11.1: an observer on `documentElement`, three
forced `global.gc()` passes, then `setAttribute` — callback count 0. Without
the forced GC, 1. Upstream holds the arrow strongly in a private field from
20.11.2 onward; the same probe against the bumped copy reports 1.

So:

- bump `happy-dom`'s floor to `^20.11.2` (lockfile resolves 20.14.5), which is
  the actual fix;
- replace the 5000ms `waitFor` with `flipThemeAttributeAndAwaitObservers`, an
  awaited condition per AGENTS.md: a probe MutationObserver registered after
  the component's is notified after it, so resolving on the probe's callback
  proves the component's already ran. No polling, no timeout, nothing
  load-sensitive.
- use the same helper in the two negative theme tests, which is what makes
  their "not called" a real gate rather than a vacuous pass on a flip that was
  never delivered.

The file's 52 tests now run in ~120ms.

Closes #2384

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-16 08:58:21 -04:00
commit eae68f2fc6
3 changed files with 52 additions and 60 deletions
Download patch file Download diff file Expand all files Collapse all files

8
clients/web/package-lock.json generated
View file

@ -46,7 +46,7 @@
"eslint-plugin-react-hooks": "^7.1.1",
"eslint-plugin-react-refresh": "^0.5.2",
"eslint-plugin-storybook": "^10.5.5",
"happy-dom": "^20.9.0",
"happy-dom": "^20.11.2",
"playwright": "^1.58.2",
"storybook": "^10.5.5",
"tsup": "^8.5.1",
@ -5264,9 +5264,9 @@
"optional": true
},
"node_modules/happy-dom": {
"version": "20.11.1",
"resolved": "https://registry.npmjs.org/happy-dom/-/happy-dom-20.11.1.tgz",
"integrity": "sha512-XSt8tMzbW9ymE7687xztkO1ckR7qJNQ3LywY9vlYGhGi3zXrGBHuUo2Cl1ztZaICW+1eAGdkLbj6iwVqDT33kg==",
"version": "20.14.5",
"resolved": "https://registry.npmjs.org/happy-dom/-/happy-dom-20.14.5.tgz",
"integrity": "sha512-x/RzkpWO40bTjIoT30iQtt64FLLmH/iRcUCN2X//bLx7H3ifkdfPXyqsro/OYtqzIAhiLMMA7mmiOR9C3NOKjQ==",
"dev": true,
"license": "MIT",
"dependencies": {

2
clients/web/package.json
View file

@ -70,7 +70,7 @@
"eslint-plugin-react-hooks": "^7.1.1",
"eslint-plugin-react-refresh": "^0.5.2",
"eslint-plugin-storybook": "^10.5.5",
"happy-dom": "^20.9.0",
"happy-dom": "^20.11.2",
"playwright": "^1.58.2",
"storybook": "^10.5.5",
"tsup": "^8.5.1",

86
clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx
View file

@ -1,5 +1,5 @@
import { createRef, StrictMode } from "react";
import { act, waitFor } from "@testing-library/react";
import { act } from "@testing-library/react";
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge";
import type { CallToolResult, Tool } from "@modelcontextprotocol/client";
@ -79,6 +79,34 @@ async function flushAsync(): Promise<void> {
});
}
// Runs `mutate` and resolves once every MutationObserver that was already
// watching `data-mantine-color-scheme` on `documentElement` has had its
// callback delivered — which is what the AppRenderer theme push rides on.
//
// This is an awaited condition rather than a wall-clock budget, per #2384.
// A probe observer registered *after* the component's is notified after it:
// observers are queued in registration order and each delivery is an ordinary
// microtask, so by the time the probe's callback runs the component's has
// already run and the assertion below is settled. No polling and no timeout is
// involved, so nothing here is load-sensitive.
async function flipThemeAttributeAndAwaitObservers(
value: string,
): Promise<void> {
await act(async () => {
await new Promise<void>((resolve) => {
const probe = new MutationObserver(() => {
probe.disconnect();
resolve();
});
probe.observe(document.documentElement, {
attributes: true,
attributeFilter: ["data-mantine-color-scheme"],
});
document.documentElement.setAttribute("data-mantine-color-scheme", value);
});
});
}
describe("AppRenderer", () => {
it("renders an iframe with the sandbox path and tool title", () => {
const bridge = createMockBridge();
@ -538,36 +566,8 @@ describe("AppRenderer", () => {
// flip we trigger below.
bridge.sendHostContextChange.mockClear();
await act(async () => {
document.documentElement.setAttribute(
"data-mantine-color-scheme",
"dark",
);
// MutationObserver callbacks are delivered on a microtask.
await Promise.resolve();
});
// `waitFor`, not a bare assertion after the single microtask above: how
// many ticks happy-dom takes to deliver the MutationObserver callback is
// load-dependent, so under a full parallel run the observer can still be
// pending here. Observed failing exactly once that way, with 0 calls.
//
// The default 1000ms then turned out not to be enough either — it failed
// twice in a row in `npm run local:gate` (at ~1021ms, still 0 calls) while
// passing every time this file runs alone, which is the signature of a
// budget that is too tight rather than of a broken observer. Raised
// rather than retried: a `waitFor` that is generous costs nothing on the
// passing path, since it returns as soon as the assertion holds.
//
// #2323 briefly deleted this argument as one of its "a budget inside an
// equal budget can never be observed" cases, which it was: 5000 inside a
// 5000ms test could never win. That is fixed from the other end now — the
// enclosing budget is 15000 — so the raise is observable again and has to
// stay, because the value it beats is a measured failure rather than a
// default. It does NOT ride on `asyncUtilTimeout`, which is pinned at
// 1000 precisely because raising it globally was measured worse
// (Copilot).
await waitFor(
() =>
await flipThemeAttributeAndAwaitObservers("dark");
expect(bridge.sendHostContextChange).toHaveBeenCalledWith(
expect.objectContaining({
theme: "dark",
@ -577,8 +577,6 @@ describe("AppRenderer", () => {
}),
}),
}),
),
{ timeout: 5000 },
);
} finally {
getComputedStyleSpy.mockRestore();
@ -599,13 +597,10 @@ describe("AppRenderer", () => {
// No `initialized` emitted — the theme observer is gated, like the
// container and displayMode pushes, so a pre-handshake flip is dropped.
bridge.sendHostContextChange.mockClear();
await act(async () => {
document.documentElement.setAttribute(
"data-mantine-color-scheme",
"dark",
);
await Promise.resolve();
});
// Same awaited-delivery helper as the positive case: it is what makes this
// "not called" a real gate rather than a vacuous pass on a flip that was
// never delivered at all.
await flipThemeAttributeAndAwaitObservers("dark");
expect(bridge.sendHostContextChange).not.toHaveBeenCalled();
document.documentElement.removeAttribute("data-mantine-color-scheme");
});
@ -627,13 +622,10 @@ describe("AppRenderer", () => {
});
bridge.sendHostContextChange.mockClear();
await act(async () => {
document.documentElement.setAttribute(
"data-mantine-color-scheme",
"dark",
);
await Promise.resolve();
});
// Same awaited-delivery helper as the positive case: it is what makes this
// "not called" a real gate rather than a vacuous pass on a flip that was
// never delivered at all.
await flipThemeAttributeAndAwaitObservers("dark");
expect(bridge.sendHostContextChange).not.toHaveBeenCalled();
document.documentElement.removeAttribute("data-mantine-color-scheme");
});