From 77af56783f66e6718a2d96dedc4e9c27edc04897 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 13:43:58 -0400 Subject: [PATCH 1/3] web: surface ui/message and app logging in panels below the running app AppRenderer: - New `onMessage` prop backs an `onmessage` bridge handler that surfaces the view's ui/message content and returns the spec-required empty result; with no handler the submission is declined (isError). - New `onLog` prop forwards MCP `notifications/message` log entries via the bridge `loggingmessage` event, honoring the advertised `logging` capability. AppsScreen: - Adds `messages`/`appLogs` state (cleared with the reported height on select/open/close/back via resetAppChannels) and `handleMessage`/`handleLog`. - Renders a "Messages from app" panel (data-testid="apps-messages", reusing ContentViewer) and a default-expanded collapsible "App logs" panel (data-testid="apps-logs", reusing LogLevelBadge) with logger names and a Clear button, both as pinned panels below the running widget. Adds AppRenderer tests (log forward + no-handler no-throw, ui/message route/empty-result/decline) and AppsScreen tests (message log render + empty result + clear-on-close, log panel expand/collapse/clear). Closes #1570 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../elements/AppRenderer/AppRenderer.test.tsx | 73 ++++++ .../elements/AppRenderer/AppRenderer.tsx | 38 +++- .../screens/AppsScreen/AppsScreen.test.tsx | 101 ++++++++- .../screens/AppsScreen/AppsScreen.tsx | 212 +++++++++++++++++- 4 files changed, 417 insertions(+), 7 deletions(-) diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx index 005142464..548d15091 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx @@ -28,6 +28,10 @@ interface MockBridge { onrequestdisplaymode?: (params: { mode: "inline" | "fullscreen" | "pip"; }) => Promise<{ mode: "inline" | "fullscreen" | "pip" }>; + onmessage?: (params: { + role: "user"; + content: unknown[]; + }) => Promise>; /** Test helper: dispatch a bridge event (e.g. "initialized") to listeners. */ emit: (event: string, payload?: unknown) => void; } @@ -321,6 +325,75 @@ describe("AppRenderer", () => { ).resolves.toEqual({ mode: "inline" }); }); + it("forwards MCP log notifications to onLog", async () => { + const bridge = createMockBridge(); + const onLog = vi.fn(); + renderWithMantine( + asBridge(bridge)} + onLog={onLog} + />, + ); + await flushAsync(); + await act(async () => { + bridge.emit("loggingmessage", { level: "warning", data: "disk full" }); + }); + expect(onLog).toHaveBeenCalledWith({ level: "warning", data: "disk full" }); + }); + + it("does not throw on a log notification when no onLog is provided", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => { + bridge.emit("loggingmessage", { level: "info", data: "hi" }); + }); + expect(screen.getByTitle("Cohort App")).toBeInTheDocument(); + }); + + it("routes ui/message to onMessage and returns the spec-required empty result", async () => { + const bridge = createMockBridge(); + const onMessage = vi.fn(); + renderWithMantine( + asBridge(bridge)} + onMessage={onMessage} + />, + ); + await flushAsync(); + const params = { + role: "user" as const, + content: [{ type: "text", text: "hello host" }], + }; + await expect(bridge.onmessage?.(params)).resolves.toEqual({}); + expect(onMessage).toHaveBeenCalledWith(params); + }); + + it("declines ui/message with isError when no onMessage handler is provided", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await expect( + bridge.onmessage?.({ role: "user", content: [] }), + ).resolves.toEqual({ isError: true }); + }); + 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 diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx index 0da563bc2..b88d35f13 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx @@ -11,8 +11,13 @@ import type { AppBridge, AppBridgeEventMap, McpUiDisplayMode, + McpUiMessageRequest, } from "@modelcontextprotocol/ext-apps/app-bridge"; -import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { + CallToolResult, + LoggingMessageNotification, + Tool, +} from "@modelcontextprotocol/sdk/types.js"; import { currentStyles, currentTheme, @@ -62,6 +67,17 @@ export interface AppRendererProps { * by returning its current one. */ onRequestDisplayMode?: (requested: McpUiDisplayMode) => McpUiDisplayMode; + /** + * Called when the running view submits a user-role message via + * `ui/message`. The renderer returns the spec-required empty result on + * the host's behalf, so the callback is fire-and-forget. + */ + onMessage?: (params: McpUiMessageRequest["params"]) => void; + /** + * Called for each MCP log notification (`notifications/message`) the + * running view emits. Backs the advertised `logging` host capability. + */ + onLog?: (params: LoggingMessageNotification["params"]) => void; /** * The host-controlled box the app renders within, used to derive * `hostContext.containerDimensions`. This MUST be an element whose size is @@ -120,6 +136,8 @@ export function AppRenderer({ onSizeChange, displayMode, onRequestDisplayMode, + onMessage, + onLog, containerRef, ref, }: AppRendererProps) { @@ -143,11 +161,15 @@ export function AppRenderer({ const onSizeChangeRef = useRef(onSizeChange); const displayModeRef = useRef(displayMode); const onRequestDisplayModeRef = useRef(onRequestDisplayMode); + const onMessageRef = useRef(onMessage); + const onLogRef = useRef(onLog); useEffect(() => { onErrorRef.current = onError; onSizeChangeRef.current = onSizeChange; displayModeRef.current = displayMode; onRequestDisplayModeRef.current = onRequestDisplayMode; + onMessageRef.current = onMessage; + onLogRef.current = onLog; }); // Flush buffered tool input/result to the view, but only once the bridge @@ -277,6 +299,11 @@ export function AppRenderer({ bridge.addEventListener("sizechange", (size) => { onSizeChangeRef.current?.(size); }); + // Forward the view's MCP log notifications so the host can honor the + // advertised `logging` capability instead of dropping them. + bridge.addEventListener("loggingmessage", (params) => { + onLogRef.current?.(params); + }); // Handle ui/request-display-mode: let the host (AppsScreen) decide what // mode to actually apply and return that. With no handler the request is // declined by returning the current host-side mode. @@ -287,6 +314,15 @@ export function AppRenderer({ : (displayModeRef.current ?? "inline"); return { mode: applied }; }; + // Handle ui/message: surface the submitted content and return the + // spec-required empty result. With no handler the submission is + // declined by returning isError. + bridge.onmessage = async (params) => { + const handler = onMessageRef.current; + if (!handler) return { isError: true }; + handler(params); + return {}; + }; flushPending(); }) .catch((err) => { diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx index 456bffd6e..36a4e6bae 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx @@ -3,7 +3,7 @@ import { act } from "@testing-library/react"; import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import type { McpUiDisplayMode } from "@modelcontextprotocol/ext-apps/app-bridge"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { ContentBlock, Tool } from "@modelcontextprotocol/sdk/types.js"; import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; import { renderWithMantine, @@ -119,6 +119,26 @@ async function requestDisplayMode( return result; } +// Invoke the onmessage handler the screen attached to the latest bridge, +// mimicking a ui/message request from the running view. +async function sendUiMessage( + bridges: AppBridge[], + content: ContentBlock[], +): Promise> { + const onmessage = bridges.at(-1)?.onmessage as unknown as + | ((params: { + role: "user"; + content: ContentBlock[]; + }) => Promise>) + | undefined; + if (!onmessage) throw new Error("no onmessage handler attached"); + let result: Record = {}; + await act(async () => { + result = await onmessage({ role: "user", content }); + }); + return result; +} + function buildProps(overrides: Partial = {}): AppsScreenProps { return { tools: [fieldedApp, noFieldsApp, cohortApp] as Tool[], @@ -397,6 +417,85 @@ describe("AppsScreen", () => { expect(screen.getByText("MCP Apps (3)")).toBeInTheDocument(); }); + it("surfaces ui/message content from the view in the message log", async () => { + const user = userEvent.setup(); + const { factory, bridges } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await vi.waitFor(() => + expect(bridges.at(-1)?.onmessage).toBeTypeOf("function"), + ); + await sendUiMessage(bridges, [ + { type: "text", text: "hello from the app" }, + ]); + expect(screen.getByText(/Messages from app \(1\)/)).toBeInTheDocument(); + expect(screen.getByText(/hello from the app/)).toBeInTheDocument(); + expect(screen.getByTestId("apps-messages")).toBeInTheDocument(); + }); + + it("returns an empty ui/message result (no conversation content leak)", async () => { + const user = userEvent.setup(); + const { factory, bridges } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await vi.waitFor(() => + expect(bridges.at(-1)?.onmessage).toBeTypeOf("function"), + ); + const result = await sendUiMessage(bridges, [ + { type: "text", text: "secret" }, + ]); + expect(result).toEqual({}); + }); + + it("clears the message log when the app is closed", async () => { + const user = userEvent.setup(); + const { factory, bridges } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await vi.waitFor(() => + expect(bridges.at(-1)?.onmessage).toBeTypeOf("function"), + ); + await sendUiMessage(bridges, [ + { type: "text", text: "hello from the app" }, + ]); + expect(screen.getByText(/Messages from app/)).toBeInTheDocument(); + await user.click(screen.getByLabelText("Close")); + expect(screen.queryByText(/Messages from app/)).not.toBeInTheDocument(); + }); + + it("surfaces app log notifications in a default-expanded collapsible panel with a working Clear", async () => { + const user = userEvent.setup(); + const { factory, emit } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(screen.queryByText(/App logs/)).not.toBeInTheDocument(); + await act(async () => { + emit("loggingmessage", { level: "warning", data: "disk almost full" }); + emit("loggingmessage", { + level: "error", + logger: "render", + data: { code: 500 }, + }); + }); + const toggle = screen.getByRole("button", { name: /App logs \(2\)/ }); + expect(toggle).toBeInTheDocument(); + expect(toggle.getAttribute("aria-expanded")).toBe("true"); + // Default-expanded: entries are visible without clicking the toggle. + expect(screen.getByText("disk almost full")).toBeInTheDocument(); + expect(screen.getByText("render")).toBeInTheDocument(); + expect(screen.getByText('{"code":500}')).toBeInTheDocument(); + expect(screen.getByTestId("apps-logs")).toBeInTheDocument(); + // Collapsing still works, and Clear empties the panel. + await user.click(toggle); + expect(toggle.getAttribute("aria-expanded")).toBe("false"); + await user.click(screen.getByRole("button", { name: "Clear" })); + expect(screen.queryByText(/App logs/)).not.toBeInTheDocument(); + }); + it("calls onCloseApp and clears selection on Close", async () => { const user = userEvent.setup(); const onCloseApp = vi.fn(); diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx index 41fa1d744..831afb95b 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx @@ -3,11 +3,16 @@ import { ActionIcon, Button, Card, + Code, + Collapse, Flex, Group, Image, + Paper, + ScrollArea, Stack, Text, + Title, Tooltip, } from "@mantine/core"; import { @@ -16,7 +21,11 @@ import { MdFullscreen, MdFullscreenExit, } from "react-icons/md"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { + ContentBlock, + LoggingMessageNotification, + Tool, +} from "@modelcontextprotocol/sdk/types.js"; import type { McpUiDisplayMode } from "@modelcontextprotocol/ext-apps/app-bridge"; import { AppRenderer, @@ -26,6 +35,8 @@ import { import { HOST_AVAILABLE_DISPLAY_MODES } from "../../elements/AppRenderer/createAppBridgeFactory"; import { AppDetailPanel } from "../../groups/AppDetailPanel/AppDetailPanel"; import { AppControls } from "../../groups/AppControls/AppControls"; +import { ContentViewer } from "../../elements/ContentViewer/ContentViewer"; +import { LogLevelBadge } from "../../elements/LogLevelBadge/LogLevelBadge"; import { hasInputFields, resolveDisplayLabel } from "../../../utils/toolUtils"; import { collectSchemaDefaults } from "../../../utils/jsonUtils"; @@ -146,6 +157,101 @@ const ContentStack = Stack.withProps({ h: "100%", }); +// Pinned panel below the running app (used by both the message log and the +// app-log panel). `0 0 auto` keeps it at its content height (capped by the +// inner scroll's `mah`) so it never squeezes out the iframe above it. +const PinnedPanel = Stack.withProps({ + gap: "xs", + flex: "0 0 auto", + mih: 0, +}); + +const LogScroll = ScrollArea.withProps({ + mah: 200, + type: "auto", + scrollbars: "y", + offsetScrollbars: true, +}); + +const MessageLogStack = Stack.withProps({ + gap: "sm", +}); + +const MessageItem = Paper.withProps({ + p: "md", + radius: "md", + withBorder: true, +}); + +const MessageItemStack = Stack.withProps({ + gap: "xs", +}); + +const MonoCaption = Text.withProps({ + size: "xs", + c: "dimmed", + ff: "monospace", +}); + +const AppLogList = Stack.withProps({ + gap: "xs", +}); + +const AppLogRow = Group.withProps({ + gap: "sm", + wrap: "nowrap", + align: "flex-start", +}); + +const AppLogData = Code.withProps({ + block: true, + fz: "xs", +}); + +const CompactSubtleButton = Button.withProps({ + variant: "subtle", + size: "compact-xs", +}); + +const PanelHeaderRow = Group.withProps({ + justify: "space-between", + wrap: "nowrap", + gap: "sm", +}); + +/** Render a log payload as a string for display. */ +function formatLogData(data: unknown): string { + if (typeof data === "string") return data; + try { + return JSON.stringify(data); + } catch { + /* v8 ignore next -- JSON.stringify only throws on a BigInt or a circular + structure; a log payload delivered over postMessage is already + structured-clone-safe, so this fallback is unreachable in practice. */ + return String(data); + } +} + +// A user-role message submitted by the running view through ui/message. The +// inspector has no conversation to append to, so it just records the content +// blocks for display. Shape matches McpUiMessageRequest["params"]. +interface AppMessage { + role: "user"; + content: ContentBlock[]; +} + +/** + * One MCP `notifications/message` log entry from the running app, with the + * payload stringified once at capture time so the render path can use it + * directly. `id` is a stable React key. + */ +interface AppLogEntry { + id: number; + level: LoggingMessageNotification["params"]["level"]; + logger?: string; + text: string; +} + export function AppsScreen({ tools, listChanged, @@ -164,11 +270,25 @@ export function AppsScreen({ const [running, setRunning] = useState(false); const [maximized, setMaximized] = useState(false); const rendererContainerRef = useRef(null); + const nextLogIdRef = useRef(0); // Height (px) the running view last reported via ui/notifications/size-changed. // Undefined until the view reports (or after it's torn down), in which case // the iframe fills the available card space as before. Local to the screen // like `running`/`maximized`: it's tied to the live iframe, not persisted. const [appHeight, setAppHeight] = useState(undefined); + // Messages the running view has pushed via ui/message. The inspector has no + // chat loop, so they're collected here and shown in a log below the app + // rather than continuing a conversation. Local to the screen like `running`: + // tied to the live bridge, cleared when the open ends or the app changes. + const [messages, setMessages] = useState([]); + // Standard MCP log notifications (notifications/message) the running app + // emits. The host advertises the `logging` capability; without surfacing + // these here they'd be silently dropped by the bridge. Same lifecycle as + // `messages`: tied to the live bridge, cleared on open/close/switch. + const [appLogs, setAppLogs] = useState([]); + // Expanded by default so a widget developer sees the entries without an extra + // click. The user can still collapse it for the rest of the run. + const [appLogsExpanded, setAppLogsExpanded] = useState(true); const selectedTool = selectedAppName ? tools.find((t) => t.name === selectedAppName) @@ -185,6 +305,31 @@ export function AppsScreen({ if (size.height != null) setAppHeight(size.height); } + function handleMessage(params: AppMessage) { + setMessages((prev) => [...prev, params]); + } + + function handleLog(params: LoggingMessageNotification["params"]) { + setAppLogs((prev) => [ + ...prev, + { + id: nextLogIdRef.current++, + level: params.level, + logger: params.logger, + text: formatLogData(params.data), + }, + ]); + } + + // Clear the message + log panels (and the reported height). Called when a run + // ends or the selected app changes so a new run starts clean. + function resetAppChannels() { + setAppHeight(undefined); + setMessages([]); + setAppLogs([]); + setAppLogsExpanded(true); + } + // The app's display mode is derived from the existing maximized toggle. // Passed to AppRenderer so the running view receives it via // host-context-changed; the Maximize/Restore button below keeps toggling @@ -214,7 +359,7 @@ export function AppsScreen({ formValues: collectSchemaDefaults(next.inputSchema), }); setMaximized(false); - setAppHeight(undefined); + resetAppChannels(); onSelectApp(name); // No-input apps auto-launch on selection so the user lands directly in // the running view; apps with fields wait for the explicit Open App click. @@ -228,7 +373,7 @@ export function AppsScreen({ function handleOpen() { if (!selectedTool) return; - setAppHeight(undefined); + resetAppChannels(); setRunning(true); onOpenApp(selectedTool.name, formValues); } @@ -237,14 +382,14 @@ export function AppsScreen({ setRunning(false); onUiChange({ ...ui, selectedAppName: undefined, formValues: {} }); setMaximized(false); - setAppHeight(undefined); + resetAppChannels(); onCloseApp(); } function handleBackToInput() { setRunning(false); setMaximized(false); - setAppHeight(undefined); + resetAppChannels(); } // No sandbox proxy URL means the host can't embed the trusted outer iframe @@ -355,6 +500,8 @@ export function AppsScreen({ onSizeChange={handleSizeChange} displayMode={displayMode} onRequestDisplayMode={handleRequestDisplayMode} + onMessage={handleMessage} + onLog={handleLog} containerRef={rendererContainerRef} ref={rendererRef} /> @@ -378,6 +525,61 @@ export function AppsScreen({ onOpenApp={handleOpen} /> )} + {running && messages.length > 0 && ( + + Messages from app ({messages.length}) + + + {messages.map((message, index) => ( + + + + [{index}] role: {message.role} + + {message.content.map((block, blockIndex) => ( + + ))} + + + ))} + + + + )} + {running && appLogs.length > 0 && ( + + + setAppLogsExpanded((e) => !e)} + aria-expanded={appLogsExpanded} + > + App logs ({appLogs.length}) + + setAppLogs([])}> + Clear + + + + + + {appLogs.map((entry) => ( + + + {entry.logger && ( + {entry.logger} + )} + {entry.text} + + ))} + + + + + )} ) : ( Select an app to view details From 019022001d088dc8667f67b5f5aefa94ac38f0a5 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 19:18:43 -0400 Subject: [PATCH 2/3] review: type-honest formatLogData, cap channels, log-panel a11y (PR #1658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the claude[bot] review: - #1 (applied): formatLogData coalesces JSON.stringify(undefined) → "" so the `: string` return type is honest for a data-less log. - #2 (applied): cap retained messages/logs at MAX_APP_CHANNEL_ENTRIES (500) via appendCapped — a chatty widget no longer grows the panels/DOM without bound between Clear/close; oldest entries drop. - #3 (declined): keep the Messages panel without a Clear/collapse — messages are sparser and already cleared on close/switch; keeping it simple. - #4 (applied): the App-logs toggle now has aria-controls pointing at the Collapse region (id="apps-logs-region") for assistive tech. Adds tests: no-data log renders (coalesce branch), 501 logs cap at 500 (oldest dropped), aria-controls assertion. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../screens/AppsScreen/AppsScreen.test.tsx | 39 +++++++++++++++++++ .../screens/AppsScreen/AppsScreen.tsx | 34 ++++++++++++---- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx index 6d43388f9..ca0ccc431 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx @@ -511,10 +511,49 @@ describe("AppsScreen", () => { // Collapsing still works, and Clear empties the panel. await user.click(toggle); expect(toggle.getAttribute("aria-expanded")).toBe("false"); + // The toggle points at the collapse region for assistive tech. + expect(toggle.getAttribute("aria-controls")).toBe("apps-logs-region"); await user.click(screen.getByRole("button", { name: "Clear" })); expect(screen.queryByText(/App logs/)).not.toBeInTheDocument(); }); + it("renders a log notification that carries no data without crashing", async () => { + const user = userEvent.setup(); + const { factory, emit } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + // No `data` field — formatLogData must coalesce to "" (not "undefined"). + emit("loggingmessage", { level: "info" }); + }); + expect( + screen.getByRole("button", { name: /App logs \(1\)/ }), + ).toBeInTheDocument(); + }); + + it("caps retained app logs at the soft limit, dropping the oldest", async () => { + const user = userEvent.setup(); + const { factory, emit } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + // 501 entries exceeds the 500 cap → oldest dropped, count clamps at 500. + for (let i = 0; i < 501; i++) { + emit("loggingmessage", { level: "info", data: `log-${i}` }); + } + }); + expect( + screen.getByRole("button", { name: /App logs \(500\)/ }), + ).toBeInTheDocument(); + // The very first entry was dropped; the most recent is retained. + expect(screen.queryByText("log-0")).not.toBeInTheDocument(); + expect(screen.getByText("log-500")).toBeInTheDocument(); + }); + it("calls onCloseApp and clears selection on Close", async () => { const user = userEvent.setup(); const onCloseApp = vi.fn(); diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx index 5a27b12a5..b2501a26a 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx @@ -235,7 +235,10 @@ const PanelHeaderRow = Group.withProps({ function formatLogData(data: unknown): string { if (typeof data === "string") return data; try { - return JSON.stringify(data); + // JSON.stringify(undefined) returns the value `undefined`, not a string, so + // coalesce to "" to keep the `: string` return type honest for a data-less + // log (spec-required, so this is only defensive against a malformed view). + return JSON.stringify(data) ?? ""; } catch { /* v8 ignore next -- JSON.stringify only throws on a BigInt or a circular structure; a log payload delivered over postMessage is already @@ -244,6 +247,21 @@ function formatLogData(data: unknown): string { } } +/** + * Soft cap on retained message / log entries per run. Chatty widgets can emit + * logs in a loop; keep only the most recent so the panels (and their DOM rows) + * don't grow without bound between Clear/close. Oldest entries are dropped. + */ +const MAX_APP_CHANNEL_ENTRIES = 500; + +/** Append to a capped list, dropping the oldest entries past the cap. */ +function appendCapped(prev: T[], next: T): T[] { + const grown = [...prev, next]; + return grown.length > MAX_APP_CHANNEL_ENTRIES + ? grown.slice(grown.length - MAX_APP_CHANNEL_ENTRIES) + : grown; +} + // A user-role message submitted by the running view through ui/message. The // inspector has no conversation to append to, so it just records the content // blocks for display. Shape matches McpUiMessageRequest["params"]. @@ -321,19 +339,18 @@ export function AppsScreen({ } function handleMessage(params: AppMessage) { - setMessages((prev) => [...prev, params]); + setMessages((prev) => appendCapped(prev, params)); } function handleLog(params: LoggingMessageNotification["params"]) { - setAppLogs((prev) => [ - ...prev, - { + setAppLogs((prev) => + appendCapped(prev, { id: nextLogIdRef.current++, level: params.level, logger: params.logger, text: formatLogData(params.data), - }, - ]); + }), + ); } // Clear the message + log panels (and the reported height). Called when a run @@ -576,6 +593,7 @@ export function AppsScreen({ setAppLogsExpanded((e) => !e)} aria-expanded={appLogsExpanded} + aria-controls="apps-logs-region" > App logs ({appLogs.length}) @@ -583,7 +601,7 @@ export function AppsScreen({ Clear - + {appLogs.map((entry) => ( From e8e0a5cd9850825443b89cf697da61610b449c44 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 11 Jul 2026 19:26:08 -0400 Subject: [PATCH 3/3] review: stable id key for Messages panel (PR #1658 round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the round-2 nit: give AppMessage a stable `id` (assigned via nextMessageIdRef in handleMessage) and key the Messages panel on it instead of the array index — so the appendCapped front-drop can't renumber keys once a run exceeds the cap, matching AppLogEntry's treatment. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- .../components/screens/AppsScreen/AppsScreen.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx index b2501a26a..677885116 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx @@ -264,8 +264,11 @@ function appendCapped(prev: T[], next: T): T[] { // A user-role message submitted by the running view through ui/message. The // inspector has no conversation to append to, so it just records the content -// blocks for display. Shape matches McpUiMessageRequest["params"]. +// blocks for display. `role`/`content` mirror McpUiMessageRequest["params"]; +// `id` is a stable React key (like AppLogEntry) so the appendCapped front-drop +// can't renumber keys the way an array index would. interface AppMessage { + id: number; role: "user"; content: ContentBlock[]; } @@ -301,6 +304,7 @@ export function AppsScreen({ const [maximized, setMaximized] = useState(false); const rendererContainerRef = useRef(null); const nextLogIdRef = useRef(0); + const nextMessageIdRef = useRef(0); // Height (px) the running view last reported via ui/notifications/size-changed. // Undefined until the view reports (or after it's torn down), in which case // the iframe fills the available card space as before. Local to the screen @@ -338,8 +342,10 @@ export function AppsScreen({ if (size.height != null && size.height > 0) setAppHeight(size.height); } - function handleMessage(params: AppMessage) { - setMessages((prev) => appendCapped(prev, params)); + function handleMessage(params: Omit) { + setMessages((prev) => + appendCapped(prev, { id: nextMessageIdRef.current++, ...params }), + ); } function handleLog(params: LoggingMessageNotification["params"]) { @@ -568,7 +574,7 @@ export function AppsScreen({ {messages.map((message, index) => ( - + [{index}] role: {message.role}