From d65c228ceab32dca387d1511dc64742f84469bbe Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Tue, 23 Jun 2026 18:18:51 -0700 Subject: [PATCH] fix(mcp): address second round of Codex review findings Fixes 13 correctness issues flagged in the second Codex review pass on the feat/mcp-apps-support branch. Core server-side changes: resource URI and model-only-tool caches are now scoped per user/server key so OAuth and user-sourced servers with differing tool lists cannot cross-contaminate each other. The model-only visibility check in appToolCall now blocks iframe-initiated calls to tools declared as visibility: ['model']. appToolCall also runs processMCPEnv to resolve runtime env/user vars and set request headers before forwarding to tools/call, and throws for servers that require per-call OBO token minting (unsupported in this path). parsers.ts now includes structuredContent in the synthetic resourceId hash to guarantee uniqueness across repeated same-app calls with different results, skips the early-return guard when a synthetic app resource is present, appends the ui{} marker to the synthetic text block, and forwards the raw content array alongside structuredContent so text/image-only app results are not silently dropped. Client-side changes: fetchMCPResourceHtml now returns the full _meta.ui from the resources/read content item so CSP and permissions come from the canonical location in the spec rather than the tool descriptor. useAppBridge falls back to the resource-level values when the read result carries no overrides. The sandbox retry interval clears when sandbox-resource-ready arrives, fixing the race where the ready notification arrived before the transport was connected. The size-change handler in MCPUIResource and UIResourceCarousel now applies the reported height to the wrapper element, and MCPUIResource's iframe style uses height: 100% so inline apps are not clipped. The carousel loading placeholder now uses the localized key. Dockerfile.multi copies the sandbox from client/dist (the Vite output) rather than the source tree, which is the only path present in the multi-stage runtime image. baseUriDomains from the CSP config are now honoured in buildCspPolicy instead of always emitting base-uri 'self'. serverResources was removed from the AppBridge capabilities advertisement because no resource handlers are registered on the bridge. --- Dockerfile.multi | 1 + client/public/mcp-sandbox.html | 15 ++- .../Chat/Messages/Content/ToolCall.tsx | 11 +- .../Messages/Content/UIResourceCarousel.tsx | 15 ++- .../MCPUIResource/MCPUIResource.tsx | 20 +++- client/src/hooks/MCP/useAppBridge.ts | 8 +- client/src/utils/mcpApps.ts | 31 +++++- packages/api/package.json | 1 + packages/api/src/mcp/MCPManager.ts | 103 ++++++++++++++---- packages/api/src/mcp/parsers.ts | 4 +- packages/data-provider/src/schemas.ts | 1 + 11 files changed, 167 insertions(+), 43 deletions(-) diff --git a/Dockerfile.multi b/Dockerfile.multi index ecff4370e5..71253ef86e 100644 --- a/Dockerfile.multi +++ b/Dockerfile.multi @@ -114,6 +114,7 @@ COPY --from=data-provider-build /app/packages/data-provider/dist ./packages/data COPY --from=data-schemas-build /app/packages/data-schemas/dist ./packages/data-schemas/dist COPY --from=api-package-build /app/packages/api/dist ./packages/api/dist COPY --from=client-build /app/client/dist ./client/dist +COPY --from=client-build /app/client/dist/mcp-sandbox.html ./client/public/mcp-sandbox.html # Propagate build metadata into runtime env so /api/config can expose it. # Declared here (after the heavy install/copy steps) so that commit/date # changing on every CI run does not bust the cache for those layers. diff --git a/client/public/mcp-sandbox.html b/client/public/mcp-sandbox.html index 27ca95f9b0..3652cfcdbb 100644 --- a/client/public/mcp-sandbox.html +++ b/client/public/mcp-sandbox.html @@ -16,6 +16,7 @@ let innerFrame = null; let innerFrameBlobUrl = null; let trustedOrigin = null; + let readyInterval = null; const SANDBOX_PREFIX = 'ui/notifications/sandbox-'; function notifyReady() { @@ -23,6 +24,16 @@ { jsonrpc: '2.0', method: 'ui/notifications/sandbox-proxy-ready', params: {} }, '*' ); + if (!readyInterval) { + readyInterval = setInterval(() => { + if (!innerFrame) { + window.parent.postMessage( + { jsonrpc: '2.0', method: 'ui/notifications/sandbox-proxy-ready', params: {} }, + '*' + ); + } + }, 500); + } } window.addEventListener('message', (event) => { @@ -42,6 +53,8 @@ } if (msg.method === 'ui/notifications/sandbox-resource-ready') { + clearInterval(readyInterval); + readyInterval = null; createInnerFrame(msg.params); return; } @@ -140,7 +153,7 @@ ("font-src " + (resourceDomains || "'none'")).trim(), "frame-src " + frameDomains, "object-src 'none'", - "base-uri 'self'" + "base-uri " + (toDomainList(csp.baseUriDomains) || "'self'") ].join('; '); } diff --git a/client/src/components/Chat/Messages/Content/ToolCall.tsx b/client/src/components/Chat/Messages/Content/ToolCall.tsx index dc95a98722..ed9cdad36b 100644 --- a/client/src/components/Chat/Messages/Content/ToolCall.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCall.tsx @@ -51,9 +51,14 @@ const MCPAppView = React.memo(function MCPAppView({ const toolResult = useMemo(() => { const sc = app.structuredContent as Record | undefined | null; - if (!sc || typeof sc !== 'object' || Array.isArray(sc)) return undefined; - return { content: [] as [], structuredContent: sc }; - }, [app.structuredContent]); + const content = (app.content as [] | undefined) ?? []; + if ((!sc || typeof sc !== 'object' || Array.isArray(sc)) && content.length === 0) + return undefined; + return { + content, + ...(sc && typeof sc === 'object' && !Array.isArray(sc) ? { structuredContent: sc } : {}), + }; + }, [app.structuredContent, app.content]); const handleSizeChanged = useCallback((params: { height?: number; width?: number }) => { if (params.height && params.height > 0) { diff --git a/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx b/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx index 2c168888c3..2236a65540 100644 --- a/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx +++ b/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx @@ -2,6 +2,7 @@ import React, { useState } from 'react'; import type { UIResource } from 'librechat-data-provider'; import { getMCPSandboxUrl } from '~/utils/mcpApps'; import { useAppBridge } from '~/hooks/MCP'; +import { useLocalize } from '~/hooks'; interface UIResourceCarouselProps { uiResources: UIResource[]; @@ -9,14 +10,20 @@ interface UIResourceCarouselProps { function MCPAppCard({ resource }: { resource: UIResource }) { const iframeRef = React.useRef(null); + const localize = useLocalize(); const [loaded, setLoaded] = useState(false); const sandboxUrl = React.useMemo(() => getMCPSandboxUrl(), []); const toolResult = React.useMemo(() => { const sc = resource.structuredContent as Record | undefined | null; - if (!sc || typeof sc !== 'object' || Array.isArray(sc)) return undefined; - return { content: [] as [], structuredContent: sc }; - }, [resource.structuredContent]); + const content = (resource.content as [] | undefined) ?? []; + if ((!sc || typeof sc !== 'object' || Array.isArray(sc)) && content.length === 0) + return undefined; + return { + content, + ...(sc && typeof sc === 'object' && !Array.isArray(sc) ? { structuredContent: sc } : {}), + }; + }, [resource.structuredContent, resource.content]); const handleSizeChanged = React.useCallback((params: { height?: number; width?: number }) => { if (params.height && params.height > 0) { @@ -31,7 +38,7 @@ function MCPAppCard({ resource }: { resource: UIResource }) { <> {!loaded && (
- Loading interactive view... + {localize('com_ui_loading_interactive_view')}
)}