From 679a3e2715554aa69574211bb42885e3178327cb Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 13:10:57 -0400 Subject: [PATCH 1/2] web: full hostContext delivery + live host-context-changed updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - createAppBridgeFactory now seeds the full hostContext via snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES): theme, styles (Mantine design tokens), displayMode ("inline"), availableDisplayModes, and containerDimensions — replacing the theme-only seed. The local currentTheme helper is dropped in favor of hostContext.ts. - AppRenderer gains `displayMode` and `containerRef` props and pushes live host-context changes via the SDK's sendHostContextChange (partial host-context-changed notifications), never a hand-maintained snapshot: - theme + styles via a MutationObserver on , - containerDimensions via a ResizeObserver on the host container (or the iframe fallback), gated on `initialized`, skipping 0x0 and value-equal repeats, plus a one-shot push on initialize once layout settles, - displayMode whenever the prop changes (Maximize/Restore), gated on init. Adds AppRenderer host-context tests (theme flip incl. resolved styles, displayMode push + init gate, container-dimensions on init and resize, observer teardown) and updates the factory + story mocks for the new hostContext shape and sendHostContextChange method. Closes #1567 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../AppRenderer/AppRenderer.stories.tsx | 3 + .../elements/AppRenderer/AppRenderer.test.tsx | 327 +++++++++++++++++- .../elements/AppRenderer/AppRenderer.tsx | 121 ++++++- .../createAppBridgeFactory.test.ts | 11 +- .../AppRenderer/createAppBridgeFactory.ts | 46 +-- .../screens/AppsScreen/AppsScreen.stories.tsx | 2 + 6 files changed, 474 insertions(+), 36 deletions(-) diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx index 2ee718b3e..2ff8c0eb9 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx @@ -20,6 +20,9 @@ function createMockBridge(): AppBridge { sendToolInput: async () => {}, sendToolResult: async () => {}, sendToolCancelled: async () => {}, + sendHostContextChange: async () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, teardownResource: async () => ({}), close: async () => {}, } as unknown as AppBridge; diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx index 8a10a82dc..214cc634a 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx @@ -1,6 +1,6 @@ import { createRef, StrictMode } from "react"; import { act } from "@testing-library/react"; -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js"; import { renderWithMantine, screen } from "../../../test/renderWithMantine"; @@ -20,6 +20,7 @@ interface MockBridge { sendToolInput: ReturnType; sendToolResult: ReturnType; sendToolCancelled: ReturnType; + sendHostContextChange: ReturnType; teardownResource: ReturnType; close: ReturnType; addEventListener: ReturnType; @@ -34,6 +35,7 @@ function createMockBridge(): MockBridge { sendToolInput: vi.fn().mockResolvedValue(undefined), sendToolResult: vi.fn().mockResolvedValue(undefined), sendToolCancelled: vi.fn().mockResolvedValue(undefined), + sendHostContextChange: vi.fn().mockResolvedValue(undefined), teardownResource: vi.fn().mockResolvedValue({}), close: vi.fn().mockResolvedValue(undefined), addEventListener: vi.fn((event: string, handler: (p: unknown) => void) => { @@ -230,6 +232,329 @@ describe("AppRenderer", () => { }); }); + it("pushes a displayMode change to the running view via host-context-changed", async () => { + const bridge = createMockBridge(); + // Stable factory identity so the rerender reuses the live bridge instead of + // rebuilding (which would reset `initialized` and gate the push). + const factory: BridgeFactory = () => asBridge(bridge); + const { rerender } = renderWithMantine( + , + ); + await flushAsync(); + await act(async () => bridge.emit("initialized")); + bridge.sendHostContextChange.mockClear(); + rerender( + , + ); + await flushAsync(); + expect(bridge.sendHostContextChange).toHaveBeenCalledWith({ + displayMode: "fullscreen", + }); + }); + + it("does not push a displayMode change before the view is initialized", async () => { + const bridge = createMockBridge(); + const factory: BridgeFactory = () => asBridge(bridge); + const { rerender } = renderWithMantine( + , + ); + await flushAsync(); + bridge.sendHostContextChange.mockClear(); + // No `initialized` emitted yet — the push must be gated. + rerender( + , + ); + await flushAsync(); + expect(bridge.sendHostContextChange).not.toHaveBeenCalledWith({ + displayMode: "fullscreen", + }); + }); + + it("pushes a live theme flip (with resolved styles) to the running bridge via host-context-changed", async () => { + const bridge = createMockBridge(); + // Stub the computed design tokens so currentStyles() resolves a non-empty + // McpUiHostStyles — exercises the theme observer's styles-included path. + const realGetComputedStyle = window.getComputedStyle; + const getComputedStyleSpy = vi + .spyOn(window, "getComputedStyle") + .mockImplementation((el: Element, pseudo?: string | null) => { + const decl = realGetComputedStyle.call(window, el, pseudo ?? undefined); + return { + ...decl, + getPropertyValue: (prop: string) => + prop === "--mantine-color-body" + ? "#101113" + : decl.getPropertyValue(prop), + } as CSSStyleDeclaration; + }); + try { + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + // Ignore any seeding from Mantine's own mount-time write — assert only the + // 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(); + }); + expect(bridge.sendHostContextChange).toHaveBeenCalledWith( + expect.objectContaining({ + theme: "dark", + styles: expect.objectContaining({ + variables: expect.objectContaining({ + "--color-background-primary": "#101113", + }), + }), + }), + ); + } finally { + getComputedStyleSpy.mockRestore(); + document.documentElement.removeAttribute("data-mantine-color-scheme"); + } + }); + + it("stops observing theme changes after the renderer unmounts", async () => { + const bridge = createMockBridge(); + const { unmount } = renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => { + unmount(); + await Promise.resolve(); + await Promise.resolve(); + }); + bridge.sendHostContextChange.mockClear(); + + await act(async () => { + document.documentElement.setAttribute( + "data-mantine-color-scheme", + "dark", + ); + await Promise.resolve(); + }); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + document.documentElement.removeAttribute("data-mantine-color-scheme"); + }); + + describe("containerDimensions", () => { + // Stub ResizeObserver so tests can drive the callback directly: capture the + // callback + the observed element so its getBoundingClientRect can be + // patched before each fire. + let resizeCallback: (() => void) | undefined; + let observedEl: HTMLElement | undefined; + let originalResizeObserver: typeof ResizeObserver | undefined; + + beforeEach(() => { + resizeCallback = undefined; + observedEl = undefined; + originalResizeObserver = globalThis.ResizeObserver; + globalThis.ResizeObserver = class { + constructor(cb: () => void) { + resizeCallback = cb; + } + observe(el: Element) { + observedEl = el as HTMLElement; + } + unobserve() {} + disconnect() { + resizeCallback = undefined; + } + } as unknown as typeof ResizeObserver; + }); + afterEach(() => { + if (originalResizeObserver) { + globalThis.ResizeObserver = originalResizeObserver; + } else { + delete (globalThis as { ResizeObserver?: unknown }).ResizeObserver; + } + }); + + function stubSize(el: HTMLElement, width: number, height: number) { + vi.spyOn(el, "getBoundingClientRect").mockReturnValue({ + width, + height, + top: 0, + left: 0, + right: width, + bottom: height, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect); + } + + it("pushes containerDimensions on initialize when the container has a layout box", async () => { + const bridge = createMockBridge(); + const container = document.createElement("div"); + stubSize(container, 320, 200); + renderWithMantine( + asBridge(bridge)} + containerRef={{ current: container }} + />, + ); + await flushAsync(); + await act(async () => bridge.emit("initialized")); + expect(bridge.sendHostContextChange).toHaveBeenCalledWith({ + containerDimensions: { width: 320, height: 200 }, + }); + }); + + it("does not push containerDimensions on initialize when the container has no layout box", async () => { + const bridge = createMockBridge(); + const container = document.createElement("div"); + stubSize(container, 0, 0); + renderWithMantine( + asBridge(bridge)} + containerRef={{ current: container }} + />, + ); + await flushAsync(); + // A mount-time theme write may have already pushed a {theme} change; + // clear so we assert only about the initialize-time containerDimensions. + bridge.sendHostContextChange.mockClear(); + await act(async () => bridge.emit("initialized")); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + }); + + it("does not push containerDimensions on resize before the view is initialized", async () => { + const bridge = createMockBridge(); + const container = document.createElement("div"); + renderWithMantine( + asBridge(bridge)} + containerRef={{ current: container }} + />, + ); + await flushAsync(); + bridge.sendHostContextChange.mockClear(); + stubSize(container, 640, 480); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + }); + + it("pushes containerDimensions on resize once initialized; skips a 0×0 box and a value-equal repeat", async () => { + const bridge = createMockBridge(); + const container = document.createElement("div"); + renderWithMantine( + asBridge(bridge)} + containerRef={{ current: container }} + />, + ); + await flushAsync(); + await act(async () => bridge.emit("initialized")); + bridge.sendHostContextChange.mockClear(); + + stubSize(container, 640, 480); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).toHaveBeenCalledWith({ + containerDimensions: { width: 640, height: 480 }, + }); + + bridge.sendHostContextChange.mockClear(); + stubSize(container, 640, 480); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + + stubSize(container, 0, 0); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + }); + + it("observes the host-supplied containerRef element instead of the iframe when provided", async () => { + const bridge = createMockBridge(); + const container = document.createElement("div"); + renderWithMantine( + asBridge(bridge)} + containerRef={{ current: container }} + />, + ); + await flushAsync(); + expect(observedEl).toBe(container); + }); + + it("falls back to observing the iframe when no containerRef is provided", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + expect(observedEl).toBeInstanceOf(HTMLIFrameElement); + }); + + it("disconnects the ResizeObserver on unmount", async () => { + const bridge = createMockBridge(); + const { unmount } = renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => bridge.emit("initialized")); + await act(async () => { + unmount(); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(resizeCallback).toBeUndefined(); + }); + }); + it("builds a single bridge and does not dispose it under StrictMode double-invoke", async () => { // React StrictMode runs effects setup→cleanup→setup in dev. The bridge // (a stateful handshake) must survive that as ONE instance — rebuilding it diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx index 54d9f684a..6b427adf9 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx @@ -5,9 +5,18 @@ import { useImperativeHandle, useRef, type Ref, + type RefObject, } from "react"; -import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { + AppBridge, + McpUiDisplayMode, +} from "@modelcontextprotocol/ext-apps/app-bridge"; import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js"; +import { + currentStyles, + currentTheme, + measureContainerDimensions, +} from "./hostContext"; /** * Constructs the `AppBridge` for a freshly mounted sandbox iframe. Wrap with @@ -32,6 +41,20 @@ export interface AppRendererProps { tool: Tool; bridgeFactory: BridgeFactory; onError?: (err: Error) => void; + /** + * Current host display mode for the app frame. Pushed to the running view + * via `host-context-changed` whenever it changes (e.g. Maximize/Restore), so + * an app can adapt its layout to inline vs fullscreen. + */ + displayMode?: McpUiDisplayMode; + /** + * The host-controlled box the app renders within, used to derive + * `hostContext.containerDimensions`. This MUST be an element whose size is + * driven by the host's layout (window resize, sidebar toggle, maximize) and + * NOT by the view's own `size-changed` reports — otherwise the two signals + * couple into a feedback loop. Falls back to the iframe element when omitted. + */ + containerRef?: RefObject; ref?: Ref; } @@ -79,6 +102,8 @@ export function AppRenderer({ tool, bridgeFactory, onError, + displayMode, + containerRef, ref, }: AppRendererProps) { const iframeRef = useRef(null); @@ -210,6 +235,18 @@ export function AppRenderer({ // drives), so the view's `initialized` signal is never missed. bridge.addEventListener("initialized", () => { initializedRef.current = true; + // The factory already seeded theme/styles/displayMode into the + // handshake hostContext; the observers below cover any subsequent + // changes. Only containerDimensions can plausibly differ between + // bridge construction and initialization (layout settles), so push + // that one field now via the SDK's partial-change notification. + const container = containerRef?.current ?? iframeRef.current; + const containerDimensions = container + ? measureContainerDimensions(container) + : undefined; + if (containerDimensions) { + void bridge.sendHostContextChange({ containerDimensions }); + } flushPending(); }); flushPending(); @@ -219,7 +256,87 @@ export function AppRenderer({ }); return scheduleDispose; - }, [bridgeFactory, sandboxPath, tool, flushPending, scheduleDispose]); + }, [ + bridgeFactory, + sandboxPath, + tool, + containerRef, + flushPending, + scheduleDispose, + ]); + + // Push live host-context changes to the running view as discrete partial + // updates via AppBridge.sendHostContextChange (the SDK's + // ui/notifications/host-context-changed sender). Each effect observes one + // host signal and sends only the field(s) it owns, so the view receives the + // spec's "only changed fields" partials without any host-side snapshot + // bookkeeping. Reading `bridgeRef.current` at callback time (not capturing a + // bridge) means the observers always target the live bridge, even though it + // resolves asynchronously after these effects run. + + // Theme + styles: Mantine writes the resolved scheme to + // ``; observe that attribute and forward + // changes through the live bridge. + useEffect(() => { + /* v8 ignore next 5 -- SSR/non-DOM guard: MutationObserver and document are + always defined under happy-dom, so this early return is unreachable in + the test environment. */ + if ( + typeof MutationObserver === "undefined" || + typeof document === "undefined" + ) { + return; + } + const observer = new MutationObserver(() => { + const styles = currentStyles(); + void bridgeRef.current?.sendHostContextChange({ + theme: currentTheme(), + ...(styles ? { styles } : {}), + }); + }); + observer.observe(document.documentElement, { + attributes: true, + attributeFilter: ["data-mantine-color-scheme"], + }); + return () => observer.disconnect(); + }, []); + + // Container size: observes the host-controlled container (or the iframe as a + // fallback) — NOT an element whose height is driven by the view's own + // size-changed reports, which would couple the two signals into a feedback + // loop. Gated on the view's `initialized` signal so the notification only + // fires once the handshake is complete; a 0×0 (not-yet-laid-out) measurement + // and a value-equal repeat are both skipped. + useEffect(() => { + const target = containerRef?.current ?? iframeRef.current; + /* v8 ignore next -- SSR/non-DOM guard: ResizeObserver is stubbed/defined + and the iframe (or containerRef) target is always present after mount in + tests, so neither disjunct is reachable here. */ + if (typeof ResizeObserver === "undefined" || !target) return; + let last: { width: number; height: number } | undefined; + const observer = new ResizeObserver(() => { + if (!initializedRef.current) return; + const next = measureContainerDimensions(target); + if (!next) return; + if (last && last.width === next.width && last.height === next.height) { + return; + } + last = next; + void bridgeRef.current?.sendHostContextChange({ + containerDimensions: next, + }); + }); + observer.observe(target); + return () => observer.disconnect(); + }, [containerRef]); + + // Display mode: pushes whenever the prop changes (Maximize/Restore). Gated on + // `initialized` for the same reason as the other host-context pushes. + useEffect(() => { + if (displayMode === undefined) return; + if (!initializedRef.current) return; + void bridgeRef.current?.sendHostContextChange({ displayMode }); + }, [displayMode]); useImperativeHandle( ref, diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 84e6bc63f..99cd1ade0 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -134,7 +134,16 @@ describe("createAppBridgeFactory", () => { expect(bridge.ctorArgs[0]).toBe(fakeClient); expect(bridge.ctorArgs[1]).toMatchObject({ name: "MCP Inspector" }); expect(bridge.ctorArgs[2]).toMatchObject({ serverTools: {} }); - expect(bridge.ctorArgs[3]).toEqual({ hostContext: { theme: "dark" } }); + // hostContext is the full snapshot: theme (from the DOM attribute), + // the inline display mode, and the host's available display modes. + // styles/containerDimensions are omitted for the bare test iframe. + expect(bridge.ctorArgs[3]).toMatchObject({ + hostContext: { + theme: "dark", + displayMode: "inline", + availableDisplayModes: ["inline", "fullscreen"], + }, + }); expect(bridge.connect).toHaveBeenCalledTimes(1); } finally { document.documentElement.removeAttribute("data-mantine-color-scheme"); diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index 1bf90a041..fb785edb9 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -4,6 +4,7 @@ import { getToolUiResourceUri, } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { + McpUiDisplayMode, McpUiHostCapabilities, McpUiResourceMeta, } from "@modelcontextprotocol/ext-apps/app-bridge"; @@ -17,6 +18,7 @@ import { buildSandboxCspPolicy, wrapSandboxedHtml, } from "../../../lib/sandbox-csp"; +import { snapshotHostContext } from "./hostContext"; import type { BridgeFactory } from "./AppRenderer"; /** @@ -42,6 +44,17 @@ export const HOST_CAPABILITIES: McpUiHostCapabilities = { logging: {}, }; +/** + * Display modes the inspector host supports, advertised in the handshake + * hostContext (`availableDisplayModes`). AppsScreen renders an app either + * inline within its layout card or maximized to fill the screen, so only those + * two are offered. + */ +export const HOST_AVAILABLE_DISPLAY_MODES: readonly McpUiDisplayMode[] = [ + "inline", + "fullscreen", +]; + export interface AppBridgeFactoryDeps { /** The connected SDK client to back the bridge, or null when disconnected. */ getClient: () => Client | null; @@ -55,37 +68,6 @@ export interface AppBridgeFactoryDeps { onResourceError?: (err: Error) => void; } -/** - * Resolve the host theme from the DOM at bridge-build time. Mantine writes the - * resolved color scheme to ``. Reading it here - * (rather than capturing React state in the factory deps) keeps the factory's - * identity stable across theme toggles — the AppRenderer treats a new factory - * identity as "rebuild the bridge", which would reload a running app's iframe on - * every theme flip. The theme is read once per bridge build (the value at open - * time); pushing live theme updates to an already-open app would need an - * AppBridge.setHostContext follow-up. - * - * The attribute is only ever `"light"` or `"dark"` — Mantine resolves - * `defaultColorScheme="auto"` to the system value before paint and never writes - * `"auto"` here, so no `auto` branch is needed. The matchMedia fallback only - * covers the attribute being absent (e.g. a hydration race). - */ -function currentTheme(): "light" | "dark" { - if (typeof document !== "undefined") { - const attr = document.documentElement.getAttribute( - "data-mantine-color-scheme", - ); - if (attr === "dark" || attr === "light") return attr; - } - if ( - typeof window !== "undefined" && - window.matchMedia?.("(prefers-color-scheme: dark)").matches - ) { - return "dark"; - } - return "light"; -} - /** First text content block of a UI resource, plus its `_meta` (sandbox hints). */ function extractHtmlAndMeta(result: ReadResourceResult): { html: string; @@ -140,7 +122,7 @@ export function createAppBridgeFactory( // declare its own csp/permissions. const hostCapabilities: McpUiHostCapabilities = { ...HOST_CAPABILITIES }; const bridge = new AppBridge(client, HOST_INFO, hostCapabilities, { - hostContext: { theme: currentTheme() }, + hostContext: snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES), }); // The double-iframe proxy posts `sandboxready` once it can receive content. diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.stories.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.stories.tsx index a517e8feb..319dc7f2a 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.stories.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.stories.tsx @@ -34,6 +34,7 @@ function createMockBridge(): AppBridge { sendToolInput: async () => {}, sendToolResult: async () => {}, sendToolCancelled: async () => {}, + sendHostContextChange: async () => {}, teardownResource: async () => ({}), close: async () => {}, addEventListener: () => {}, @@ -220,6 +221,7 @@ export const EchoRunning: Story = { setEcho(first?.type === "text" ? first.text : ""); }, sendToolCancelled: async () => {}, + sendHostContextChange: async () => {}, teardownResource: async () => ({}), close: async () => {}, }; From 0426f1ba1962f019ea88f776f570f7b1a75b10b2 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 18:26:20 -0400 Subject: [PATCH 2/2] review: gate theme observer on initialized + document seed/dep assumptions (PR #1654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the claude[bot] review: - Finding #1 (applied): gate the theme+styles MutationObserver on the view's `initialized` signal, matching the container/displayMode pushes — a theme flip in the construction→handshake window no longer races ui/initialize. The factory seeds the construction-time theme, and the first post-init flip carries the current value. Added tests: pre-init flip is dropped; the live flip test now completes the handshake first. - Finding #2 (applied, comment): note in snapshotHostContext that the `displayMode: "inline"` seed assumes inline-at-open (AppsScreen always opens inline; #1568's displayMode push carries transitions). - Finding #4 (applied, comment): explain why `containerRef` is a build-effect dep yet doesn't force a rebuild (sameInputs ignores it; read lazily). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../elements/AppRenderer/AppRenderer.test.tsx | 27 +++++++++++++++++++ .../elements/AppRenderer/AppRenderer.tsx | 13 ++++++++- .../elements/AppRenderer/hostContext.ts | 5 ++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx index 214cc634a..f6bf43f0f 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx @@ -316,6 +316,9 @@ describe("AppRenderer", () => { />, ); await flushAsync(); + // The theme observer is gated on the view's `initialized` signal, so + // complete the handshake before flipping. + await act(async () => bridge.emit("initialized")); // Ignore any seeding from Mantine's own mount-time write — assert only the // flip we trigger below. bridge.sendHostContextChange.mockClear(); @@ -344,6 +347,30 @@ describe("AppRenderer", () => { } }); + it("does not push a theme flip before the view is initialized", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + // 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(); + }); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + document.documentElement.removeAttribute("data-mantine-color-scheme"); + }); + it("stops observing theme changes after the renderer unmounts", async () => { const bridge = createMockBridge(); const { unmount } = renderWithMantine( diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx index 6b427adf9..7421d3620 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx @@ -256,6 +256,11 @@ export function AppRenderer({ }); return scheduleDispose; + // `containerRef` is listed for exhaustive-deps completeness, but a change to + // its identity does NOT force a rebuild: the `sameInputs` check above + // ignores it, so a new ref object hits the StrictMode reuse path (the + // `initialized` handler reads `containerRef?.current` lazily, so the live + // ref is always used regardless). The other deps are the real rebuild keys. }, [ bridgeFactory, sandboxPath, @@ -276,7 +281,12 @@ export function AppRenderer({ // Theme + styles: Mantine writes the resolved scheme to // ``; observe that attribute and forward - // changes through the live bridge. + // changes through the live bridge. Gated on the view's `initialized` signal + // — like the container and displayMode pushes below — so a theme flip in the + // window between bridge construction and the handshake doesn't race + // `ui/initialize`. Nothing is lost by waiting: the factory seeds the + // construction-time theme/styles into the handshake hostContext, and the + // first post-init flip carries the current value. useEffect(() => { /* v8 ignore next 5 -- SSR/non-DOM guard: MutationObserver and document are always defined under happy-dom, so this early return is unreachable in @@ -288,6 +298,7 @@ export function AppRenderer({ return; } const observer = new MutationObserver(() => { + if (!initializedRef.current) return; const styles = currentStyles(); void bridgeRef.current?.sendHostContextChange({ theme: currentTheme(), diff --git a/clients/web/src/components/elements/AppRenderer/hostContext.ts b/clients/web/src/components/elements/AppRenderer/hostContext.ts index 9ea6f8f08..eca568827 100644 --- a/clients/web/src/components/elements/AppRenderer/hostContext.ts +++ b/clients/web/src/components/elements/AppRenderer/hostContext.ts @@ -148,6 +148,11 @@ export function snapshotHostContext( : undefined; return { theme: currentTheme(), + // Seed assumes the app opens inline. AppsScreen always mounts the renderer + // inline (maximize is a later user action), so this holds today; the live + // displayMode push (AppRenderer's displayMode effect, wired by #1568) + // carries any subsequent inline↔fullscreen transition. If a caller ever + // mounts already-maximized, thread the actual mode in here instead. displayMode: "inline", availableDisplayModes: [...availableDisplayModes], ...(styles ? { styles } : {}),