Skip to content

Commit e0b1573

Browse files
committed
fix(mcp): harden result delivery and app lifecycle
1 parent 29fcfd9 commit e0b1573

26 files changed

Lines changed: 640 additions & 219 deletions

File tree

‎.github/workflows/checks.yml‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -215,29 +215,34 @@ jobs:
215215
retention-days: 7
216216

217217
mcp-app:
218-
name: mcp-app
219-
runs-on: ubuntu-latest
218+
name: mcp-app (${{ matrix.browser }})
219+
runs-on: *runner-4vcpu
220220
timeout-minutes: 10
221+
strategy:
222+
fail-fast: false
223+
matrix:
224+
browser: [chromium, webkit]
221225
steps:
222226
- *checkout-mirror
223227
- *checkout-plain
224228
- name: Setup workspace
225229
uses: ./.github/actions/setup
226230
with:
227231
provider: ${{ vars.CI_PROVIDER }}
228-
- name: Install Chromium
232+
- name: Install browser
229233
working-directory: apps/sim
230-
run: bunx playwright install --with-deps chromium
234+
run: bunx playwright install --with-deps ${{ matrix.browser }}
231235
- name: Exercise MCP App sandbox
232236
working-directory: apps/sim
233237
env:
238+
MCP_APP_E2E_BROWSER: ${{ matrix.browser }}
234239
MCP_APP_E2E_REPORT_PATH: ${{ runner.temp }}/mcp-app.json
235240
run: bun run test:mcp-app:e2e
236241
- name: Upload MCP App report
237242
if: failure()
238243
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
239244
with:
240-
name: mcp-app-report
245+
name: mcp-app-report-${{ matrix.browser }}
241246
path: ${{ runner.temp }}/mcp-app.json*
242247
if-no-files-found: warn
243248
retention-days: 7

‎apps/sim/app/api/mothership/chats/[chatId]/mcp-results/[id]/assets/[index]/route.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ export const GET = defineInternalBinaryRoute({
1212
contract: getMcpPresentationAssetContract,
1313
auth: internalSessionAuth,
1414
operation: readMcpResultAsset.operation,
15-
rateLimit: internalRateLimits.none({
16-
reason:
17-
'Bounded private MCP result delivery requires current chat ownership and connection authorization for live resources.',
18-
}),
15+
rateLimit: internalRateLimits.user({ bucketName: 'mcp-apps' }),
1916
errorPolicy: internalOrchestrationErrorPolicy,
2017
mapInput: ({ params }) => params,
2118
useCase: readMcpResultAsset,

‎apps/sim/app/api/mothership/chats/[chatId]/mcp-results/[id]/frame/route.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ export const GET = defineInternalBinaryRoute({
1212
contract: getMcpAppFrameContract,
1313
auth: internalSessionAuth,
1414
operation: readMcpAppFrame.operation,
15-
rateLimit: internalRateLimits.none({
16-
reason:
17-
'Bounded private MCP result delivery requires current chat ownership and connection authorization for live resources.',
18-
}),
15+
rateLimit: internalRateLimits.user({ bucketName: 'mcp-apps' }),
1916
errorPolicy: internalOrchestrationErrorPolicy,
2017
mapInput: ({ params }) => params,
2118
useCase: readMcpAppFrame,
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { getMcpPresentationMetadataContract } from '@/lib/api/contracts/mcp-presentations'
2+
import {
3+
defineInternalJsonRoute,
4+
internalRateLimits,
5+
internalSessionAuth,
6+
} from '@/lib/api/server/routes'
7+
import { internalOrchestrationErrorPolicy } from '@/lib/api/server/routes/internal-json-route'
8+
import { readMcpResultMetadata } from '@/lib/mothership/chat/application/mcp-results'
9+
10+
export const dynamic = 'force-dynamic'
11+
export const GET = defineInternalJsonRoute({
12+
contract: getMcpPresentationMetadataContract,
13+
auth: internalSessionAuth,
14+
operation: readMcpResultMetadata.operation,
15+
rateLimit: internalRateLimits.user({ bucketName: 'mcp-apps' }),
16+
errorPolicy: internalOrchestrationErrorPolicy,
17+
mapInput: ({ params }) => params,
18+
useCase: readMcpResultMetadata,
19+
staticResponseHeaders: { 'Cache-Control': 'private, no-store' },
20+
})

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/mcp-result/mcp-app.tsx‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,19 @@ function McpAppFrame({ chatId, id, data, onClose }: McpAppFrameProps) {
2929
const { resolvedTheme } = useTheme()
3030
const theme = resolvedTheme === 'dark' ? 'dark' : 'light'
3131
const themeRef = useRef<'dark' | 'light'>(theme)
32-
themeRef.current = theme
3332
const [height, setHeight] = useState(400)
3433
const [error, setError] = useState<string>()
3534
const [closing, setClosing] = useState(false)
3635

36+
useEffect(() => {
37+
themeRef.current = theme
38+
bridgeRef.current?.setHostContext({
39+
theme,
40+
displayMode: 'inline',
41+
availableDisplayModes: ['inline'],
42+
})
43+
}, [theme])
44+
3745
useEffect(() => {
3846
const frame = frameRef.current
3947
if (!frame?.contentWindow) return
@@ -50,7 +58,6 @@ function McpAppFrame({ chatId, id, data, onClose }: McpAppFrameProps) {
5058
},
5159
}
5260
)
53-
bridgeRef.current = bridge
5461
const reportError = (cause: unknown) => {
5562
if (!lifecycle.signal.aborted) setError(getErrorMessage(cause))
5663
}
@@ -84,6 +91,12 @@ function McpAppFrame({ chatId, id, data, onClose }: McpAppFrameProps) {
8491
if (initialized || lifecycle.signal.aborted) return
8592
window.clearTimeout(initializationTimer)
8693
initialized = true
94+
bridgeRef.current = bridge
95+
bridge.setHostContext({
96+
theme: themeRef.current,
97+
displayMode: 'inline',
98+
availableDisplayModes: ['inline'],
99+
})
87100
void (async () => {
88101
await bridge.sendToolInput({ arguments: data.arguments })
89102
if (!lifecycle.signal.aborted) await bridge.sendToolResult(data.result)
@@ -123,14 +136,6 @@ function McpAppFrame({ chatId, id, data, onClose }: McpAppFrameProps) {
123136
}
124137
}, [chatId, id, data, callTool, readResource])
125138

126-
useEffect(() => {
127-
bridgeRef.current?.setHostContext({
128-
theme,
129-
displayMode: 'inline',
130-
availableDisplayModes: ['inline'],
131-
})
132-
}, [theme])
133-
134139
const close = async () => {
135140
if (closing) return
136141
setClosing(true)

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/mcp-result/mcp-result.tsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useState } from 'react'
3+
import { useRef, useState } from 'react'
44
import { Chip, ChipLink } from '@sim/emcn'
55
import { type McpPresentationReceipt, mcpPresentationAssetUrl } from '@/lib/mcp/presentation'
66
import { useChatSurface } from '@/app/workspace/[workspaceId]/home/components/chat-surface-context'
@@ -15,12 +15,14 @@ export function McpResult({ receipt }: McpResultProps) {
1515
const { chatId } = useChatSurface()
1616
const resources = useOptionalMothershipResources()
1717
const [appOpen, setAppOpen] = useState(false)
18+
const openButtonRef = useRef<HTMLButtonElement>(null)
1819
if (!chatId) return null
1920
return (
2021
<div className='flex min-w-0 flex-col gap-3 py-2'>
21-
{receipt.hasApp && !appOpen && (
22+
{receipt.hasApp && (
2223
<div className='flex items-center gap-2'>
2324
<Chip
25+
ref={openButtonRef}
2426
variant='border'
2527
active={appOpen}
2628
onClick={() => setAppOpen(true)}
@@ -35,7 +37,10 @@ export function McpResult({ receipt }: McpResultProps) {
3537
key={`${chatId}:${receipt.id}`}
3638
chatId={chatId}
3739
id={receipt.id}
38-
onClose={() => setAppOpen(false)}
40+
onClose={() => {
41+
setAppOpen(false)
42+
openButtonRef.current?.focus()
43+
}}
3944
/>
4045
)}
4146
{receipt.items.map((item) => {

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/mcp-resource-content.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import { getErrorMessage } from '@sim/utils/errors'
55
import { mcpPresentationAssetUrl } from '@/lib/mcp/presentation'
66
import type { MothershipResource } from '@/lib/mothership/resources/types'
77
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
8+
import { resolveFileCategory } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-category'
89
import { FileViewer } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer'
9-
import { useMcpPresentation } from '@/hooks/queries/mcp-presentations'
10+
import { useMcpPresentationMetadata } from '@/hooks/queries/mcp-presentations'
1011
import type { FileContentSource } from '@/hooks/use-file-content-source'
1112

1213
interface McpResourceContentProps {
@@ -23,7 +24,7 @@ interface McpArtifactPreviewProps {
2324
const SNAPSHOT_DATE = new Date(0)
2425

2526
function McpArtifactPreview({ chatId, presentationId, index }: McpArtifactPreviewProps) {
26-
const { data, error, isPending } = useMcpPresentation(chatId, presentationId)
27+
const { data, error, isPending } = useMcpPresentationMetadata(chatId, presentationId)
2728
if (isPending) return <p className='p-4 text-[var(--text-muted)] text-small'>Opening result…</p>
2829
if (error)
2930
return (
@@ -41,7 +42,7 @@ function McpArtifactPreview({ chatId, presentationId, index }: McpArtifactPrevie
4142
const previewable =
4243
plainText ||
4344
item.kind === 'image' ||
44-
item.kind === 'audio' ||
45+
(item.kind === 'audio' && resolveFileCategory(item.mimeType, '') === 'audio-previewable') ||
4546
item.mimeType === 'application/pdf'
4647
if (!previewable)
4748
return (

‎apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-tool-event.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,10 @@ function runToolResultSideEffects(ctx: StreamLoopContext, node: ToolNode, replay
6868
? extractResourcesFromToolResult(name, params, output)
6969
: []
7070
const mcpResources = extractedResources.filter((resource) => resource.type === 'mcp')
71-
if (replay && mcpResources.length) {
71+
if (mcpResources.length) {
7272
const chatId = deps.chatIdRef.current
7373
if (chatId)
7474
void deps.queryClient.invalidateQueries({ queryKey: mothershipChatKeys.detail(chatId) })
75-
} else {
76-
for (const resource of mcpResources) deps.addResource(resource)
7775
}
7876
if (!deps.workspaceId) return
7977
const calledBy = agentIdForSpan(ctx, node.spanId)

‎apps/sim/hooks/queries/mcp-presentations.ts‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ import { requestJson } from '@/lib/api/client/request'
33
import {
44
callMcpAppToolContract,
55
getMcpPresentationContract,
6+
getMcpPresentationMetadataContract,
67
type McpAppResourceBody,
8+
type McpAppResourceResponse,
79
type McpAppToolBody,
10+
type McpAppToolResponse,
11+
type McpPresentationMetadataResponse,
812
readMcpAppResourceContract,
913
} from '@/lib/api/contracts/mcp-presentations'
1014
import { mcpKeys } from '@/hooks/queries/utils/mcp-keys'
@@ -23,9 +27,27 @@ export function useMcpPresentation(chatId: string, id: string) {
2327
})
2428
}
2529

30+
export function useMcpPresentationMetadata(chatId: string, id: string) {
31+
return useQuery({
32+
queryKey: mcpKeys.presentationMetadata(chatId, id),
33+
queryFn: ({ signal }): Promise<McpPresentationMetadataResponse> =>
34+
requestJson(getMcpPresentationMetadataContract, { params: { chatId, id }, signal }),
35+
staleTime: MCP_PRESENTATION_STALE_TIME,
36+
gcTime: 0,
37+
refetchOnWindowFocus: false,
38+
retry: false,
39+
})
40+
}
41+
2642
export function useMcpAppTool(chatId: string, id: string) {
2743
return useMutation({
28-
mutationFn: ({ body, signal }: { body: McpAppToolBody; signal: AbortSignal }) =>
44+
mutationFn: ({
45+
body,
46+
signal,
47+
}: {
48+
body: McpAppToolBody
49+
signal: AbortSignal
50+
}): Promise<McpAppToolResponse> =>
2951
requestJson(callMcpAppToolContract, { params: { chatId, id }, body, signal }),
3052
gcTime: 0,
3153
retry: false,
@@ -34,7 +56,13 @@ export function useMcpAppTool(chatId: string, id: string) {
3456

3557
export function useMcpAppResource(chatId: string, id: string) {
3658
return useMutation({
37-
mutationFn: ({ body, signal }: { body: McpAppResourceBody; signal: AbortSignal }) =>
59+
mutationFn: ({
60+
body,
61+
signal,
62+
}: {
63+
body: McpAppResourceBody
64+
signal: AbortSignal
65+
}): Promise<McpAppResourceResponse> =>
3866
requestJson(readMcpAppResourceContract, { params: { chatId, id }, body, signal }),
3967
gcTime: 0,
4068
retry: false,

‎apps/sim/hooks/queries/utils/mcp-keys.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ export const mcpKeys = {
22
all: ['mcp'] as const,
33
presentations: () => [...mcpKeys.all, 'presentation'] as const,
44
presentation: (chatId: string, id: string) => [...mcpKeys.presentations(), chatId, id] as const,
5+
presentationMetadata: (chatId: string, id: string) =>
6+
[...mcpKeys.presentation(chatId, id), 'metadata'] as const,
57
servers: () => [...mcpKeys.all, 'servers'] as const,
68
serversList: (workspaceId?: string) => [...mcpKeys.servers(), workspaceId ?? ''] as const,
79
managedCatalog: () => [...mcpKeys.all, 'managedCatalog'] as const,

0 commit comments

Comments
 (0)