feat(apps): Integrate AppRenderer from @mcp-ui/client SDK#7013
feat(apps): Integrate AppRenderer from @mcp-ui/client SDK#7013
Conversation
- Update @mcp-ui/client from ^5.17.3 to ^6.0.0 - Replace custom useSandboxBridge hook with AppRenderer component - Split monolithic handleMcpRequest into separate handler functions: - handleOpenLink, handleMessage, handleCallTool, handleReadResource - Add convertCspToMcpUi helper for CSP format conversion - Delete unused useSandboxBridge.ts file This aligns with the official MCP Apps specification and may help with secure context requirements for Web Payments SDK integration.
…tocol Change 'ui/notifications/sandbox-ready' to 'ui/notifications/sandbox-proxy-ready' to match the message name expected by @mcp-ui/client's AppRenderer.
- Fix race condition where sandboxUrl was fetched twice (once without CSP, once with CSP), causing iframe recreation and breaking PostMessage connection - Add sandboxUrlFetched state to ensure sandbox URL is only fetched once - Capture CSP at fetch time (sandboxCsp) to keep sandboxConfig stable - Inline utils.ts into McpAppRenderer.tsx and delete utils.ts - Add proper McpUiHostContext typing to hostContext - Remove debug logging - Clean up comments
- Add GooseDisplayMode type extending SDK's McpUiDisplayMode with 'standalone' - Replace boolean fullscreen prop with displayMode: GooseDisplayMode - Map 'standalone' to 'fullscreen' for SDK communication - Update StandaloneAppView to use displayMode='standalone'
- Add McpRequestParams and McpRequestResult types in types.ts - Add commented-out handleRequest handler showing sampling/createMessage pattern - Types ready for when SDK adds onRequest prop to AppRenderer
…ABLE_DISPLAY_MODES type
…nt, platform, deviceCapabilities
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- ui/desktop/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
ui/desktop/src/components/ToolCallWithResponse.tsx:129
toolResultpassed toMcpAppRendereris currently the rawCallToolResponse(snake_casestructured_content), soMcpAppRenderernever forwardsstructuredContentto the SDK; convert the result here to theToolResultshape expected byMcpAppRenderer(including mappingstructured_content->structuredContent).
const toolResult = useMemo(() => {
if (!toolResponse) {
return undefined;
}
const resultWithMeta = toolResponse.toolResult as ToolResultWithMeta;
if (resultWithMeta?.status === 'success' && resultWithMeta.value) {
return resultWithMeta.value;
}
return undefined;
…ibe non-positive value behavior
| @@ -119,7 +119,9 @@ function McpAppWrapper({ | |||
| const toolInput = useMemo(() => ({ arguments: toolArguments || {} }), [toolArguments]); | |||
There was a problem hiding this comment.
not part of this change, but what ?
There was a problem hiding this comment.
Good catch. We don't need to memoize this. The toolRequest should be stable.
Replace 6 independent useState calls (resource, error, resourceFetched, sandboxUrlFetched, sandboxUrl, sandboxCsp) with a single useReducer using a discriminated union: idle → loading_resource → loading_sandbox → ready | error. Impossible states are now unrepresentable and the lifecycle is explicit. Extract hook dependencies into variables to satisfy eslint exhaustive-deps.
|
|
||
| fetchResource(); | ||
| fetchResourceData(); | ||
| }, [resourceUri, extensionName, sessionId, cachedHtml]); |
There was a problem hiding this comment.
The cachedHtml dependency causes unnecessary refetches. When cachedHtml changes (e.g., from undefined to a value), the effect runs again even though resourceUri, extensionName, and sessionId haven't changed. This can trigger duplicate network requests. Remove cachedHtml from the dependency array since it's only used as a fallback value within the effect, not as a trigger.
| }, [resourceUri, extensionName, sessionId, cachedHtml]); | |
| }, [resourceUri, extensionName, sessionId]); |
There was a problem hiding this comment.
Disagree — cachedHtml is used as a fallback inside the effect (if the fetch fails or returns no content). If it arrives after the initial render, we want the effect to re-run so the fallback is available. In practice it's a stable prop set once, so this doesn't cause extra fetches.
There was a problem hiding this comment.
Disagreed — it's a fallback used inside the effect, and it's a stable prop in practice
| // rmcp serializes Content with a `type` discriminator via #[serde(tag = "type")]. | ||
| // Our generated TS types don't reflect this, but the wire format matches CallToolResult.content. | ||
| return { | ||
| content: (response.data?.content || []) as unknown as CallToolResult['content'], |
There was a problem hiding this comment.
Unsafe type assertion. The cast as unknown as CallToolResult['content'] bypasses TypeScript's type safety. If the wire format doesn't match CallToolResult.content, this will cause runtime errors in the SDK. Consider creating a proper type guard or validation function to verify the structure before passing to AppRenderer.
There was a problem hiding this comment.
Acknowledged — the cast is intentional and documented in the comment above it. The wire format from rmcp uses #[serde(tag = "type")] which produces the same shape as CallToolResult.content, but our generated TS types don't reflect the discriminator. A runtime validation function would be nice but is out of scope for this PR.
There was a problem hiding this comment.
Acknowledged — cast is intentional, wire format matches, documented in code
| return { | ||
| content: toolResult.content as unknown as CallToolResult['content'], | ||
| structuredContent: toolResult.structuredContent as { [key: string]: unknown } | undefined, | ||
| }; | ||
| }, [toolResult]); | ||
|
|
There was a problem hiding this comment.
Unsafe type assertion. The cast as unknown as CallToolResult['content'] bypasses TypeScript's type safety. If the wire format from toolResult.content doesn't match the SDK's expected structure, this will cause runtime errors. Consider validating the structure or creating a conversion function.
| return { | |
| content: toolResult.content as unknown as CallToolResult['content'], | |
| structuredContent: toolResult.structuredContent as { [key: string]: unknown } | undefined, | |
| }; | |
| }, [toolResult]); | |
| const normalizedContent = normalizeCallToolResultContent(toolResult.content); | |
| if (!normalizedContent) { | |
| // If the content shape is unexpected, avoid passing malformed data to the AppRenderer. | |
| return undefined; | |
| } | |
| return { | |
| content: normalizedContent, | |
| structuredContent: toolResult.structuredContent as { [key: string]: unknown } | undefined, | |
| }; | |
| }, [toolResult]); | |
| function normalizeCallToolResultContent( | |
| content: unknown | |
| ): CallToolResult['content'] | undefined { | |
| if ( | |
| Array.isArray(content) && | |
| content.every( | |
| (item) => | |
| item !== null && | |
| typeof item === 'object' && | |
| 'type' in item && | |
| typeof (item as { type?: unknown }).type === 'string' | |
| ) | |
| ) { | |
| // At this point we've established a minimal structural match with the expected content format. | |
| return content as CallToolResult['content']; | |
| } | |
| console.warn( | |
| 'Received toolResult.content with unexpected shape; dropping content for MCP app rendering.' | |
| ); | |
| return undefined; | |
| } |
There was a problem hiding this comment.
Same as above — the cast is intentional. The rmcp wire format matches CallToolResult.content structurally but our generated types don't capture the serde discriminator. A proper validation layer is a good idea for a follow-up.
There was a problem hiding this comment.
Same — intentional cast, follow-up for validation layer
| return ( | ||
| <AppRenderer | ||
| sandbox={sandboxConfig!} |
There was a problem hiding this comment.
Non-null assertion on potentially null value. If isReady is true but sandboxConfig is still null (which shouldn't happen but isn't prevented by the type system), this will cause a runtime error. Add a safety check: if (!sandboxConfig) return null; before rendering AppRenderer.
| return ( | |
| <AppRenderer | |
| sandbox={sandboxConfig!} | |
| if (!sandboxConfig) { | |
| return null; | |
| } | |
| return ( | |
| <AppRenderer | |
| sandbox={sandboxConfig} |
There was a problem hiding this comment.
Good call — replaced the ! assertion with an explicit if (!sandboxConfig) return null; guard. The state machine guarantees it's non-null when isReady is true, but the guard is cheap insurance and cleaner.
There was a problem hiding this comment.
Agreed — replaced with if (!sandboxConfig) return null; guard
toolInput and toolResult derive from stable props (toolRequest/toolResponse) that are set once per message and never mutate, making memoization redundant.
| const pendingCsp = state.status === 'loading_sandbox' ? state.meta.csp : null; | ||
| useEffect(() => { | ||
| if (state.status !== 'loading_sandbox') return; | ||
|
|
||
| fetchMcpAppProxyUrl(pendingCsp).then((url) => { | ||
| if (url) { | ||
| dispatch({ type: 'SANDBOX_READY', sandboxUrl: url, sandboxCsp: pendingCsp }); | ||
| } else { | ||
| dispatch({ type: 'SANDBOX_FAILED', message: 'Failed to initialize sandbox proxy' }); | ||
| } | ||
| }); | ||
| }, [state.status, pendingCsp]); |
There was a problem hiding this comment.
The pendingCsp dependency can cause unnecessary effect re-runs. Since state.meta.csp is an object that may be recreated with the same values during state updates, pendingCsp will fail referential equality checks even when CSP hasn't meaningfully changed. This defeats the purpose of "Fetched only once" (line 243). Consider using only state.status in the dependency array—when status is 'loading_sandbox', the effect naturally has access to the current state.meta.csp via closure, and adding it as a dependency doesn't provide additional correctness.
| const pendingCsp = state.status === 'loading_sandbox' ? state.meta.csp : null; | |
| useEffect(() => { | |
| if (state.status !== 'loading_sandbox') return; | |
| fetchMcpAppProxyUrl(pendingCsp).then((url) => { | |
| if (url) { | |
| dispatch({ type: 'SANDBOX_READY', sandboxUrl: url, sandboxCsp: pendingCsp }); | |
| } else { | |
| dispatch({ type: 'SANDBOX_FAILED', message: 'Failed to initialize sandbox proxy' }); | |
| } | |
| }); | |
| }, [state.status, pendingCsp]); | |
| useEffect(() => { | |
| if (state.status !== 'loading_sandbox') return; | |
| const currentCsp = state.meta.csp; | |
| fetchMcpAppProxyUrl(currentCsp).then((url) => { | |
| if (url) { | |
| dispatch({ type: 'SANDBOX_READY', sandboxUrl: url, sandboxCsp: currentCsp }); | |
| } else { | |
| dispatch({ type: 'SANDBOX_FAILED', message: 'Failed to initialize sandbox proxy' }); | |
| } | |
| }); | |
| }, [state.status]); |
There was a problem hiding this comment.
The effect already guards with if (state.status !== 'loading_sandbox') return, so it only fires once when entering that status. pendingCsp cannot change without state.status also changing, making re-runs a non-issue in practice.
The suggested fix moves CSP access inside the effect via closure, which would trigger the same ESLint react-hooks/exhaustive-deps warning we extracted pendingCsp to fix in the first place. Keeping it in the dep array is harmless and keeps the linter happy.
| "dependencies": { | ||
| "@mcp-ui/client": "^5.17.3", | ||
| "@mcp-ui/client": "^6.0.0", | ||
| "@modelcontextprotocol/ext-apps": "^1.0.1", |
There was a problem hiding this comment.
Potential version conflict: @mcp-ui/client@6.0.0 depends on @modelcontextprotocol/ext-apps@^0.3.1, but the project directly installs @modelcontextprotocol/ext-apps@^1.0.1. This creates two separate copies of ext-apps in node_modules (see package-lock.json lines 3049-3091 and 3170-3211). If there are type incompatibilities between 0.3.1 and 1.0.1, runtime errors could occur when passing types between SDK components and your code. Verify that the SDK's 6.0.0 release is compatible with ext-apps 1.0.1, or align your direct dependency to match the SDK's requirement.
| "@modelcontextprotocol/ext-apps": "^1.0.1", | |
| "@modelcontextprotocol/ext-apps": "^0.3.1", |
There was a problem hiding this comment.
Tracking that feedback here: MCP-UI-Org/mcp-ui#180 (comment)
|
@DOsinga I slimmed down comments and set up a state machine for lifecycle |
|
qwen3 is a bit flaky, I don't think is related. wonder if we should bump that to qwen3-coder-next as more modern |
|
|
||
| export type ToolInputPartial = McpUiToolInputPartialNotification['params']; | ||
|
|
||
| export type ToolCancelled = McpUiToolCancelledNotification['params']; |
There was a problem hiding this comment.
we should probably rename these sine they are just for MCP-UI/APPS, no? can do in a follow up of course
| * @see SEP-1865 https://github.com/modelcontextprotocol/ext-apps/blob/main/specification/draft/apps.mdx | ||
| * Display modes: | ||
| * - "inline" | "fullscreen" | "pip" — standard MCP display modes | ||
| * - "standalone" — Goose-specific mode for dedicated Electron windows |
There was a problem hiding this comment.
nice! I saw somebody on github claim that fullscreen and standalone is actually the same thing? #7142 if so would be nice to address
There was a problem hiding this comment.
I'd like to address all displayMode options holistically, and that can be something that we prioritize sooner rather than later.
And yeah, I'm not certain if goose adds standalone on top of the core display modes, or if since goose is a desktop application, we should treat fullscreen as standalone. There might be some UX implications of how chat functionality would persist in fullscreen, whereas chat functionality might go away and standalone. We need to workshop this a little bit.
…provenance * origin/main: (68 commits) Upgraded npm packages for latest security updates (#7183) docs: reasoning effort levels for Codex provider (#6798) Fix speech local (#7181) chore: add .gooseignore to .gitignore (#6826) Improve error message logging from electron (#7130) chore(deps): bump jsonwebtoken from 9.3.1 to 10.3.0 (#6924) docs: standalone mcp apps and apps extension (#6791) workflow: auto-update cli-commands on release (#6755) feat(apps): Integrate AppRenderer from @mcp-ui/client SDK (#7013) fix(MCP): decode resource content (#7155) feat: reasoning_content in API for reasoning models (#6322) Fix/configure add provider custom headers (#7157) fix: handle keyring fallback as success (#7177) Update process-wrap to 9.0.3 (9.0.2 is yanked) (#7176) feat: support extra field in chatcompletion tool_calls for gemini openai compat (#6184) fix: replace panic with proper error handling in get_tokenizer (#7175) Lifei/smoke test for developer (#7174) fix text editor view broken (#7167) docs: White label guide (#6857) Add PATH detection back to developer extension (#7161) ... # Conflicts: # .github/workflows/nightly.yml
* origin/main: (21 commits) nit: show dir in title, and less... jank (#7138) feat(gemini-cli): use stream-json output and re-use session (#7118) chore(deps): bump qs from 6.14.1 to 6.14.2 in /documentation (#7191) Switch jsonwebtoken to use aws-lc-rs (already used by rustls) (#7189) chore(deps): bump qs from 6.14.1 to 6.14.2 in /evals/open-model-gym/mcp-harness (#7184) Add SLSA build provenance attestations to release workflows (#7097) fix save and run recipe not working (#7186) Upgraded npm packages for latest security updates (#7183) docs: reasoning effort levels for Codex provider (#6798) Fix speech local (#7181) chore: add .gooseignore to .gitignore (#6826) Improve error message logging from electron (#7130) chore(deps): bump jsonwebtoken from 9.3.1 to 10.3.0 (#6924) docs: standalone mcp apps and apps extension (#6791) workflow: auto-update cli-commands on release (#6755) feat(apps): Integrate AppRenderer from @mcp-ui/client SDK (#7013) fix(MCP): decode resource content (#7155) feat: reasoning_content in API for reasoning models (#6322) Fix/configure add provider custom headers (#7157) fix: handle keyring fallback as success (#7177) ...

Summary
Refactors
McpAppRendererto use the@mcp-ui/clientSDK'sAppRenderercomponent, replacing the custom iframe/sandbox bridge implementation. This significantly simplifies the codebase while adding proper support for MCP Apps spec features.Changes
Major Refactor:
useSandboxBridgehook with@mcp-ui/client'sAppRenderercomponentuseSandboxBridge.tsandutils.ts(no longer needed)@mcp-ui/clientprotocolNew Features:
displayModeprop (inline,fullscreen,pip,standalone) replacing booleanfullscreenstructuredContentin tool results per MCP Apps spechostContextproperties:locale,timeZone,userAgent,platform,deviceCapabilitiesType Improvements:
ToolResulttype for tool execution resultsGooseDisplayModetype for Goose-specific display modesSandboxPermissionstype for iframe sandbox attributesToolInput,ToolInputPartial,ToolCancelled) directly from@modelcontextprotocol/ext-appsCode Quality:
renderContent()helperFollow-up TODOs
onFallbackRequesthandler when SDK supports it (mcp-ui#176) - needed forsampling/createMessagetoolInfotohostContextstyles(CSS variables) tohostContextcontainerDimensionstohostContext(depends on displayMode)hostInfoandhostCapabilitiesprops when available in client SDK (mcp-ui#175)Testing