From f31dacac70e5755f2aeb75104ccec7e64e4f404e Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Thu, 25 Jun 2026 13:57:39 -0700 Subject: [PATCH] fix(mcp): harden app bridge lifecycle and gate ui:// rendering to HTML Guard the app bridge against stale-resource races. A resourceId switch or unmount while the iframe is still loading or bridge.connect() is pending now cancels the in-flight handleLoad, removes the load listener, and closes the orphaned bridge, so a single iframe can no longer end up with two bridges issuing duplicate sandbox and tool requests. Build the sandbox URL from apiBaseUrl() and let new URL() resolve it against the window origin. apiBaseUrl() already reflects an absolute or non-root base href, so the extra origin prefix produced a malformed URL that broke every app iframe. Emit a renderable ui:// marker only for HTML resources, since MCP Apps defines text/html;profile=mcp-app as the sole renderable type; json and remote-dom payloads fall through to plain resource text instead of a marker the host cannot render. Hide the iframe when an app sends request-teardown so a torn-down app no longer leaves a blank frame mounted. --- .../Chat/Messages/Content/ToolCall.tsx | 15 ++++- .../Messages/Content/UIResourceCarousel.tsx | 6 ++ .../MCPUIResource/MCPUIResource.tsx | 6 ++ client/src/hooks/MCP/useAppBridge.ts | 15 +++++ client/src/utils/mcpApps.ts | 2 +- .../api/src/mcp/__tests__/parsers.test.ts | 61 +++++++++++++------ packages/api/src/mcp/parsers.ts | 7 ++- 7 files changed, 89 insertions(+), 23 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/ToolCall.tsx b/client/src/components/Chat/Messages/Content/ToolCall.tsx index feb000deb0..d048e7552a 100644 --- a/client/src/components/Chat/Messages/Content/ToolCall.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCall.tsx @@ -34,6 +34,7 @@ const MCPAppView = React.memo(function MCPAppView({ const [height, setHeight] = useState(undefined); const [loaded, setLoaded] = useState(false); const [timedOut, setTimedOut] = useState(false); + const [tornDown, setTornDown] = useState(false); const sandboxUrl = useMemo(() => getMCPSandboxUrl(), []); useEffect(() => { @@ -59,7 +60,19 @@ const MCPAppView = React.memo(function MCPAppView({ } }, []); - useAppBridge(iframeRef, app, toolArgs, toolResult, handleSizeChanged, () => setLoaded(true)); + useAppBridge( + iframeRef, + app, + toolArgs, + toolResult, + handleSizeChanged, + () => setLoaded(true), + () => setTornDown(true), + ); + + if (tornDown) { + return null; + } const isAppBacked = isMcpAppResource(app); if (!isAppBacked && app.text) { diff --git a/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx b/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx index 22be1c3c6c..5bff6813af 100644 --- a/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx +++ b/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx @@ -18,6 +18,7 @@ function MCPAppCard({ const iframeRef = React.useRef(null); const localize = useLocalize(); const [loaded, setLoaded] = useState(false); + const [tornDown, setTornDown] = useState(false); const sandboxUrl = React.useMemo(() => getMCPSandboxUrl(), []); const toolResult = React.useMemo(() => buildAppToolResult(resource), [resource]); @@ -39,8 +40,13 @@ function MCPAppCard({ toolResult, handleSizeChanged, () => setLoaded(true), + () => setTornDown(true), ); + if (tornDown) { + return null; + } + if (isMcpAppResource(resource)) { return ( <> diff --git a/client/src/components/MCPUIResource/MCPUIResource.tsx b/client/src/components/MCPUIResource/MCPUIResource.tsx index beb9b37942..23c73071d3 100644 --- a/client/src/components/MCPUIResource/MCPUIResource.tsx +++ b/client/src/components/MCPUIResource/MCPUIResource.tsx @@ -25,6 +25,7 @@ export function MCPUIResource(props: MCPUIResourceProps) { const iframeRef = useRef(null); const [loaded, setLoaded] = useState(false); + const [tornDown, setTornDown] = useState(false); const [height, setHeight] = useState(undefined); const sandboxUrl = useMemo(() => getMCPSandboxUrl(), []); @@ -47,8 +48,13 @@ export function MCPUIResource(props: MCPUIResourceProps) { toolResult, handleSizeChanged, () => setLoaded(true), + () => setTornDown(true), ); + if (tornDown) { + return null; + } + if (!uiResource) { return ( diff --git a/client/src/hooks/MCP/useAppBridge.ts b/client/src/hooks/MCP/useAppBridge.ts index f085f16537..0fd461847c 100644 --- a/client/src/hooks/MCP/useAppBridge.ts +++ b/client/src/hooks/MCP/useAppBridge.ts @@ -30,6 +30,7 @@ export function useAppBridge( toolResult: AppToolResult | undefined, onSizeChanged: (params: SizeParams) => void, onLoaded?: () => void, + onTeardown?: () => void, ) { const user = useRecoilValue(store.user); const { ask } = useOptionalMessagesOperations(); @@ -41,11 +42,13 @@ export function useAppBridge( const askRef = useRef(ask); const onSizeChangedRef = useRef(onSizeChanged); const onLoadedRef = useRef(onLoaded); + const onTeardownRef = useRef(onTeardown); const toolArgsRef = useRef(toolArgs); const toolResultRef = useRef(toolResult); askRef.current = ask; onSizeChangedRef.current = onSizeChanged; onLoadedRef.current = onLoaded; + onTeardownRef.current = onTeardown; toolArgsRef.current = toolArgs; toolResultRef.current = toolResult; @@ -54,6 +57,10 @@ export function useAppBridge( if (!iframe || !resource.serverName) return; let bridge: AppBridge | null = null; + // A resourceId switch or unmount can run cleanup while the iframe is still loading or + // bridge.connect() is pending; this flag stops the stale handleLoad from attaching a second + // bridge to the same iframe. + let cancelled = false; // The sandbox proxy re-emits `sandbox-proxy-ready` every 500ms until it receives the resource, // so fetch and send it only once to avoid overlapping reads and repeated inner-frame creation. let sandboxReadyHandled = false; @@ -186,6 +193,7 @@ export function useAppBridge( bridge.addEventListener('requestteardown', async () => { await bridge!.teardownResource({}).catch(() => {}); + onTeardownRef.current?.(); }); bridge.addEventListener('loggingmessage', (event) => { @@ -196,6 +204,10 @@ export function useAppBridge( await bridge .connect(transport) .catch((err: unknown) => logger.error('[MCP App] bridge.connect failed', err)); + if (cancelled) { + bridge.close(); + return; + } bridgeRef.current = bridge; }; @@ -207,9 +219,12 @@ export function useAppBridge( iframe.src = iframe.getAttribute('data-sandbox-url') ?? ''; return () => { + cancelled = true; + iframe.removeEventListener('load', handleLoad); bridgeRef.current?.teardownResource({}).catch(() => {}); bridgeRef.current?.close(); bridgeRef.current = null; + bridge?.close(); bridge = null; }; // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/client/src/utils/mcpApps.ts b/client/src/utils/mcpApps.ts index 7d952a335a..7c7f32b026 100644 --- a/client/src/utils/mcpApps.ts +++ b/client/src/utils/mcpApps.ts @@ -51,7 +51,7 @@ export function buildAppToolResult(resource: UIResource): AppToolResult | undefi export function getMCPSandboxUrl(): string { const configured = (import.meta.env as Record).VITE_MCP_SANDBOX_URL; - const base = configured ?? `${window.location.origin}${apiBaseUrl()}/api/mcp/sandbox`; + const base = configured ?? `${apiBaseUrl()}/api/mcp/sandbox`; try { const url = new URL(base, window.location.origin); url.searchParams.set('parentOrigin', window.location.origin); diff --git a/packages/api/src/mcp/__tests__/parsers.test.ts b/packages/api/src/mcp/__tests__/parsers.test.ts index 7ca4fccce1..dec533d4a2 100644 --- a/packages/api/src/mcp/__tests__/parsers.test.ts +++ b/packages/api/src/mcp/__tests__/parsers.test.ts @@ -256,8 +256,8 @@ describe('formatToolContent', () => { type: 'resource', resource: { uri: 'ui://carousel', - mimeType: 'application/json', - text: '{"items": []}', + mimeType: 'text/html;profile=mcp-app', + text: '
carousel
', }, }, ], @@ -268,18 +268,39 @@ describe('formatToolContent', () => { expect(content).toContain('UI Resource ID:'); expect(content).toContain('UI Resource Marker: \\ui{'); expect(content).toContain('Resource URI: ui://carousel'); - expect(content).toContain('Resource MIME Type: application/json'); + expect(content).toContain('Resource MIME Type: text/html;profile=mcp-app'); const uiResourceArtifact = artifacts?.ui_resources?.data?.[0]; expect(uiResourceArtifact).toBeTruthy(); expect(uiResourceArtifact).toMatchObject({ uri: 'ui://carousel', - mimeType: 'application/json', - text: '{"items": []}', + mimeType: 'text/html;profile=mcp-app', + text: '
carousel
', }); expect(uiResourceArtifact?.resourceId).toEqual(expect.any(String)); }); + it('treats non-HTML ui:// resources as plain text rather than renderable markers', () => { + const result: t.MCPToolCallResponse = { + content: [ + { + type: 'resource', + resource: { + uri: 'ui://legacy', + mimeType: 'application/json', + text: '{"items": []}', + }, + }, + ], + }; + + const [content, artifacts] = formatToolContent(result, 'openai'); + expect(content).toContain('Resource Text: {"items": []}'); + expect(content).toContain('Resource URI: ui://legacy'); + expect(content).not.toContain('UI Resource Marker:'); + expect(artifacts).toBeUndefined(); + }); + it('attaches the tool result to embedded ui:// resources for the app bridge', () => { const result: t.MCPToolCallResponse = { content: [ @@ -375,8 +396,8 @@ describe('formatToolContent', () => { type: 'resource', resource: { uri: 'ui://button', - mimeType: 'application/json', - text: '{"label": "Click me"}', + mimeType: 'text/html;profile=mcp-app', + text: '', }, }, { @@ -394,14 +415,14 @@ describe('formatToolContent', () => { expect(content).toContain('Some text'); expect(content).toContain('UI Resource Marker: \\ui{'); expect(content).toContain('Resource URI: ui://button'); - expect(content).toContain('Resource MIME Type: application/json'); + expect(content).toContain('Resource MIME Type: text/html;profile=mcp-app'); expect(content).toContain('Resource URI: file://data.csv'); const uiResource = artifacts?.ui_resources?.data?.[0]; expect(uiResource).toMatchObject({ uri: 'ui://button', - mimeType: 'application/json', - text: '{"label": "Click me"}', + mimeType: 'text/html;profile=mcp-app', + text: '', }); expect(uiResource?.resourceId).toEqual(expect.any(String)); }); @@ -415,8 +436,8 @@ describe('formatToolContent', () => { type: 'resource', resource: { uri: 'ui://graph', - mimeType: 'application/json', - text: '{"type": "line"}', + mimeType: 'text/html;profile=mcp-app', + text: 'graph', }, }, ], @@ -427,7 +448,7 @@ describe('formatToolContent', () => { expect(content).toContain('Content with multimedia'); expect(content).toContain('UI Resource Marker: \\ui{'); expect(content).toContain('Resource URI: ui://graph'); - expect(content).toContain('Resource MIME Type: application/json'); + expect(content).toContain('Resource MIME Type: text/html;profile=mcp-app'); expect(artifacts).toEqual({ content: [ { @@ -439,8 +460,8 @@ describe('formatToolContent', () => { data: [ { uri: 'ui://graph', - mimeType: 'application/json', - text: '{"type": "line"}', + mimeType: 'text/html;profile=mcp-app', + text: 'graph', resourceId: expect.any(String), content: expect.any(Array), }, @@ -478,8 +499,8 @@ describe('formatToolContent', () => { type: 'resource', resource: { uri: 'ui://chart', - mimeType: 'application/json', - text: '{"type": "bar"}', + mimeType: 'text/html;profile=mcp-app', + text: 'chart', }, }, { @@ -501,7 +522,7 @@ describe('formatToolContent', () => { expect(content).toContain('UI Resource ID:'); expect(content).toContain('UI Resource Marker: \\ui{'); expect(content).toContain('Resource URI: ui://chart'); - expect(content).toContain('Resource MIME Type: application/json'); + expect(content).toContain('Resource MIME Type: text/html;profile=mcp-app'); expect(content).toContain('Resource URI: https://api.example.com/data'); expect(content).toContain('Conclusion'); expect(content).toContain('UI Resource Markers Available:'); @@ -520,8 +541,8 @@ describe('formatToolContent', () => { data: [ { uri: 'ui://chart', - mimeType: 'application/json', - text: '{"type": "bar"}', + mimeType: 'text/html;profile=mcp-app', + text: 'chart', resourceId: expect.any(String), }, ], diff --git a/packages/api/src/mcp/parsers.ts b/packages/api/src/mcp/parsers.ts index d33edbf393..99722eab4f 100644 --- a/packages/api/src/mcp/parsers.ts +++ b/packages/api/src/mcp/parsers.ts @@ -208,7 +208,12 @@ export function formatToolContent( }, resource: (item) => { - const isUiResource = item.resource.uri.startsWith('ui://'); + // MCP Apps defines a single renderable resource type, text/html;profile=mcp-app, and the + // host renders HTML only. ui:// resources with other mime types (json, remote-dom) have no + // renderer, so they fall through to plain resource text instead of an unrenderable marker. + const mimeType = + typeof item.resource.mimeType === 'string' ? item.resource.mimeType : 'text/html'; + const isUiResource = item.resource.uri.startsWith('ui://') && mimeType.includes('html'); const resourceText: string[] = []; if (isUiResource) {