Skip to content

Commit 027adbc

Browse files
waleedlatif1waleedlatif1
committed
fix(mcp): retain partial artifacts and complete native previews
1 parent 1fb8cb3 commit 027adbc

17 files changed

Lines changed: 404 additions & 109 deletions

File tree

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,18 @@ function McpArtifactPreview({ chatId, presentationId, index }: McpArtifactPrevie
3939
const plainText =
4040
item.mimeType.startsWith('text/') ||
4141
['application/json', 'application/xml'].includes(item.mimeType)
42+
const category = resolveFileCategory(item.mimeType, '')
4243
const previewable =
4344
plainText ||
44-
(item.kind === 'image' && resolveFileCategory(item.mimeType, '') === 'image-previewable') ||
45-
(item.kind === 'audio' && resolveFileCategory(item.mimeType, '') === 'audio-previewable') ||
46-
item.mimeType === 'application/pdf'
45+
(item.kind === 'image' && category === 'image-previewable') ||
46+
(item.kind === 'audio' && category === 'audio-previewable') ||
47+
[
48+
'iframe-previewable',
49+
'video-previewable',
50+
'docx-previewable',
51+
'pptx-previewable',
52+
'xlsx-previewable',
53+
].includes(category)
4754
if (!previewable)
4855
return (
4956
<div className='p-4'>

‎apps/sim/lib/credential-groups/application/authorization.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export async function requireCredentialGroupCredentialAccess(
188188
if (binding && !isManagedCredentialGroupBindingLive(binding)) {
189189
throw new OrchestrationError('forbidden', 'Credential Group credential access denied')
190190
}
191-
if (actorAccess && !managedMcp && !binding) {
191+
if (actorAccess && !binding && (!managedMcp || !context.organizationId)) {
192192
throw new OrchestrationError('forbidden', 'Credential Group credential access denied')
193193
}
194194
if (context.organizationId) {

‎apps/sim/lib/internal/mcp/execute-tool.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,12 @@ export const executeMcpTool: InternalToolOperationHandler = async (request) => {
216216
callChain: request.context.callChain,
217217
timeoutMs,
218218
signal: request.signal,
219-
includePresentation:
220-
!!request.context.copilotToolExecution &&
221-
!!request.context.chatId &&
222-
!request.context.mcpBlockId,
219+
presentation:
220+
request.context.copilotToolExecution &&
221+
request.context.chatId &&
222+
!request.context.mcpBlockId
223+
? ('snapshot' as const)
224+
: undefined,
223225
}
224226
let result: ExecuteMcpToolResult
225227
if (target.kind === 'shared_server') {

‎apps/sim/lib/internal/mcp/presentation.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { CallToolResultSchema, ReadResourceResultSchema } from '@modelcontextprotocol/sdk/types.js'
1+
import {
2+
CallToolResultSchema,
3+
ReadResourceResultSchema,
4+
ToolSchema,
5+
} from '@modelcontextprotocol/sdk/types.js'
26
import { createLogger } from '@sim/logger'
37
import { isPlainRecord } from '@sim/utils/object'
48
import {
@@ -53,7 +57,11 @@ export async function presentMcpToolResult(
5357
result: projectMcpEncodedContents(result, registry),
5458
resources: projectMcpEncodedContents({ contents: presentation.resources ?? [] }, registry)
5559
.contents,
56-
title: tool.title || tool.name,
60+
tool: {
61+
name: tool.name,
62+
title: tool.title || tool.name,
63+
_meta: { ui: { resourceUri: getMcpAppResourceUri(tool) } },
64+
},
5765
}
5866
const projection = projectResolvedSecretModelJsonContent(
5967
value,
@@ -63,15 +71,16 @@ export async function presentMcpToolResult(
6371
if (
6472
projection.safe &&
6573
isPlainRecord(projection.value) &&
66-
isPlainRecord(projection.value.arguments) &&
67-
typeof projection.value.title === 'string'
74+
isPlainRecord(projection.value.arguments)
6875
) {
6976
const input = {
7077
chatId: context.chatId,
7178
workspaceId: context.workspaceId,
7279
connectionId,
7380
toolCallId: context.toolCallId,
74-
tool: { ...tool, title: projection.value.title },
81+
tool: ToolSchema.pick({ name: true, title: true, _meta: true }).parse(
82+
projection.value.tool
83+
),
7584
arguments: projection.value.arguments,
7685
result: CallToolResultSchema.parse(projection.value.result),
7786
resources: ReadResourceResultSchema.shape.contents.parse(projection.value.resources),
@@ -122,7 +131,7 @@ export async function presentMcpToolResult(
122131
? { type: 'text' as const, text: item.resource.text }
123132
: {
124133
type: 'text' as const,
125-
text: receipt
134+
text: receipt?.items.some((asset) => asset.index === index)
126135
? `Attached result: ${receipt.items.find((asset) => asset.index === index)?.title || 'file'}`
127136
: 'MCP file output could not be displayed.',
128137
}

‎apps/sim/lib/mcp/application/execute-managed-tool.ts‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import {
1717
transformToolResult,
1818
validateToolArguments,
1919
} from '@/lib/mcp/application/execute-tool'
20-
import { loadManagedMcpAuthProvider } from '@/lib/mcp/application/managed-auth-provider'
20+
import {
21+
createManagedMcpAuthProvider,
22+
loadManagedMcpAuthProvider,
23+
} from '@/lib/mcp/application/managed-auth-provider'
2124
import {
2225
loadMcpOperationAccess,
2326
requireMcpOperationAccess,
@@ -42,7 +45,7 @@ export interface ExecuteManagedMcpToolInput {
4245
callChain?: string[]
4346
timeoutMs?: number
4447
signal?: AbortSignal
45-
includePresentation?: boolean
48+
presentation?: 'snapshot' | 'app'
4649
appOrigin?: { toolName: string; resourceUri: string }
4750
onResolvedSecretTraceProvenance?: (provenance: ResolvedSecretTraceProvenanceV1) => void
4851
}
@@ -182,7 +185,8 @@ export const executeManagedMcpToolUseCase = defineAuthorizedWorkspaceUseCase({
182185
})
183186
input.signal?.throwIfAborted()
184187
const result = transformToolResult(providerResult)
185-
if (input.includePresentation && userId)
188+
if (input.presentation) result.presentation = { tool, result: providerResult, arguments: args }
189+
if (input.presentation === 'snapshot' && userId)
186190
result.presentation = await createMcpToolPresentation(
187191
{ tool, result: providerResult, arguments: args },
188192
(uri, signal) =>
@@ -196,8 +200,29 @@ export const executeManagedMcpToolUseCase = defineAuthorizedWorkspaceUseCase({
196200
managed: {
197201
connectionId: context.credentialId,
198202
scope: current.scope,
199-
loadAuthProvider: () =>
200-
loadManagedMcpAuthProvider(context.credentialId, context.workspaceId),
203+
loadAuthProvider: async () => {
204+
const latest = await loadManagedMcpCredentialApplicationContext(
205+
context.credentialId,
206+
context.workspaceId
207+
)
208+
if (!latest || latest.mcpServerId !== current.mcpServerId)
209+
throw new OrchestrationError('forbidden', 'Managed MCP connection changed')
210+
await requireCredentialGroupCredentialAccess(
211+
principal,
212+
latest,
213+
credentialOperations.useManagedMcp.resourcePolicy
214+
)
215+
const grant = await loadManagedMcpRuntimeCredential(
216+
context.credentialId,
217+
context.workspaceId
218+
)
219+
if (
220+
grant.oauthConfigVersion !== current.oauthConfigVersion ||
221+
grant.grantedAt.getTime() !== current.grantedAt.getTime()
222+
)
223+
throw new OrchestrationError('forbidden', 'Managed MCP grant changed')
224+
return createManagedMcpAuthProvider(grant)
225+
},
201226
},
202227
}),
203228
input.signal

‎apps/sim/lib/mcp/application/execute-tool.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export interface ExecuteMcpToolInput {
3939
callChain?: string[]
4040
timeoutMs?: number
4141
signal?: AbortSignal
42-
includePresentation?: boolean
42+
presentation?: 'snapshot' | 'app'
4343
appOrigin?: { toolName: string; resourceUri: string }
4444
onResolvedSecretTraceProvenance?: (provenance: ResolvedSecretTraceProvenanceV1) => void
4545
}
@@ -200,7 +200,8 @@ export const executeMcpToolUseCase = defineAuthorizedWorkspaceUseCase({
200200
)
201201
input.signal?.throwIfAborted()
202202
const result = transformToolResult(providerResult)
203-
if (input.includePresentation)
203+
if (input.presentation) result.presentation = { tool, result: providerResult, arguments: args }
204+
if (input.presentation === 'snapshot')
204205
result.presentation = await createMcpToolPresentation(
205206
{ tool, result: providerResult, arguments: args },
206207
(uri, signal) =>

‎apps/sim/lib/mcp/application/presentation.integration.ts‎

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
mcpPresentationCleanupKeys,
2727
planForkMcpPresentations,
2828
} from '@/lib/mcp/presentation-lifecycle'
29+
import { loadMcpPresentation } from '@/lib/mcp/presentation-storage'
2930
import { mcpService } from '@/lib/mcp/service'
3031
import { changeChatResources } from '@/lib/mothership/chat/application/change-resources'
3132
import {
@@ -70,6 +71,8 @@ let listingOnlyPolicy = false
7071
let providerTitle = 'Quarterly report'
7172
let linkedReportText = 'Remote resource bytes'
7273
let linkedResourceMissing = false
74+
let resourceReads = 0
75+
let echoAppUri = false
7376
let connectDomain = 'https://allowed.test'
7477
let rejectResourceStatus = 0
7578
let rejectResourcesPersistently = false
@@ -111,7 +114,14 @@ const provider = createServer(async (request, response) => {
111114
name: 'show_report',
112115
title: reflectCredential ? String(request.headers['x-fixture-token']) : providerTitle,
113116
inputSchema: { type: 'object' as const },
114-
_meta: appAvailable ? { ui: { resourceUri: appUri } } : {},
117+
_meta: appAvailable
118+
? {
119+
ui: {
120+
resourceUri: echoAppUri ? `${appUri}/${credentialCanary}` : appUri,
121+
visibility: ['model'],
122+
},
123+
}
124+
: {},
115125
},
116126
{
117127
name: 'change_report',
@@ -128,6 +138,12 @@ const provider = createServer(async (request, response) => {
128138
protocol.setRequestHandler(CallToolRequestSchema, async ({ params }) => {
129139
if (params.name === 'change_report') {
130140
appCalls++
141+
if (params.arguments?.linked)
142+
return {
143+
content: [
144+
{ type: 'resource_link', name: 'Report', uri: sourceUri, mimeType: 'text/plain' },
145+
],
146+
}
131147
if (encodedCredential)
132148
return {
133149
content: [
@@ -178,6 +194,12 @@ const provider = createServer(async (request, response) => {
178194
),
179195
},
180196
},
197+
{
198+
type: 'resource_link' as const,
199+
uri: 'file:///later.txt',
200+
name: 'Later report',
201+
mimeType: 'text/plain',
202+
},
181203
]
182204
: []),
183205
],
@@ -245,7 +267,9 @@ const provider = createServer(async (request, response) => {
245267
],
246268
}))
247269
protocol.setRequestHandler(ReadResourceRequestSchema, async ({ params }) => {
248-
if (linkedResourceMissing) throw new Error('Synthetic missing linked resource')
270+
resourceReads++
271+
if (linkedResourceMissing && params.uri === sourceUri)
272+
throw new Error('Synthetic missing linked resource')
249273
return {
250274
contents: [
251275
{
@@ -408,6 +432,7 @@ beforeAll(async () => {
408432
afterEach(() => {
409433
linkedReportText = 'Remote resource bytes'
410434
linkedResourceMissing = false
435+
echoAppUri = false
411436
})
412437

413438
afterAll(async () => {
@@ -602,17 +627,54 @@ describe('native MCP results over real transport, storage and Postgres', () => {
602627
}
603628
)
604629

605-
it('keeps private metadata and encoded files out of model output when a linked snapshot fails', async () => {
630+
it('preserves valid attachments and the App when a linked snapshot fails', async () => {
606631
linkedResourceMissing = true
607-
const { result } = await invokeReport({ linked: true })
608-
expect(compactMcpPresentation(result.output)).toBeUndefined()
632+
const { result, receipt } = await executeReport({ linked: true })
633+
expect(receipt.hasApp).toBe(true)
634+
expect(receipt.items.map((item) => item.index)).toEqual([1, 2])
635+
const asset = await readMcpResultAsset.execute({
636+
principal: session,
637+
input: { chatId, id: receipt.id, index: 1 },
638+
})
639+
expect(asset.buffer.toString()).toContain('Encoded report: ')
640+
expect(asset.buffer.toString()).not.toContain(credentialCanary)
641+
const later = await readMcpResultAsset.execute({
642+
principal: session,
643+
input: { chatId, id: receipt.id, index: 2 },
644+
})
645+
expect(later.buffer.toString()).toBe('Remote resource bytes')
646+
await expect(
647+
readMcpResultAsset.execute({
648+
principal: session,
649+
input: { chatId, id: receipt.id, index: 0 },
650+
})
651+
).rejects.toThrow('MCP file not found')
609652
expect(JSON.stringify(result.output)).not.toContain('Only the app should receive this')
610653
expect(JSON.stringify(result.output)).not.toContain(
611654
Buffer.from(`Encoded report: ${credentialCanary}:end`).toString('base64')
612655
)
613656
expect(JSON.stringify(result.output)).toContain('could not be displayed')
614657
})
615658

659+
it('does not download linked resources returned by a live App call', async () => {
660+
const { receipt } = await executeReport()
661+
const before = resourceReads
662+
const result = await callMcpAppTool.execute({
663+
principal: session,
664+
input: { chatId, id: receipt.id, name: 'change_report', arguments: { linked: true } },
665+
})
666+
expect(result.content[0]).toMatchObject({ type: 'resource_link', uri: sourceUri })
667+
expect(resourceReads).toBe(before)
668+
})
669+
670+
it('redacts credentials reflected in the discovered App address before storage', async () => {
671+
echoAppUri = true
672+
const { receipt } = await executeReport()
673+
const manifest = await loadMcpPresentation(chatId, receipt.id)
674+
expect(manifest.appUri).toContain('ui://fixture/view.html/')
675+
expect(JSON.stringify(manifest)).not.toContain(credentialCanary)
676+
})
677+
616678
it('redacts encoded credentials in linked snapshot bytes before reopening', async () => {
617679
encodedCredential = true
618680
try {
@@ -632,18 +694,19 @@ describe('native MCP results over real transport, storage and Postgres', () => {
632694
}
633695
})
634696

635-
it.each(['audio/aiff', 'image/tiff'])(
636-
'downloads unsupported %s bytes without attempting playback',
637-
async (mimeType) => {
638-
const { receipt } = await executeReport({ unsupportedMedia: mimeType })
639-
const asset = await readMcpResultAsset.execute({
640-
principal: session,
641-
input: { chatId, id: receipt.id, index: 0 },
642-
})
643-
expect(asset.disposition).toBe('attachment')
644-
expect(asset.buffer.toString()).toBe('Unsupported fixture bytes')
645-
}
646-
)
697+
it.each([
698+
['audio/aiff', 'attachment'],
699+
['image/tiff', 'attachment'],
700+
['audio/ogg; codecs=opus', 'inline'],
701+
] as const)('serves %s with %s disposition', async (mimeType, disposition) => {
702+
const { receipt } = await executeReport({ unsupportedMedia: mimeType })
703+
const asset = await readMcpResultAsset.execute({
704+
principal: session,
705+
input: { chatId, id: receipt.id, index: 0 },
706+
})
707+
expect(asset.disposition).toBe(disposition)
708+
expect(asset.buffer.toString()).toBe('Unsupported fixture bytes')
709+
})
647710

648711
it('reports credentials once for both a cold and a pooled resource read', async () => {
649712
await evictMcpServerConnections(serverId, 'cold resource fixture')

‎apps/sim/lib/mcp/application/presentation.ts‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,28 @@ export async function createMcpToolPresentation(
2424
for (const item of presentation.result.content) {
2525
if (item.type !== 'resource_link' || resources.some((resource) => resource.uri === item.uri))
2626
continue
27-
if (!item.uri || item.uri.length > 2048)
28-
throw new OrchestrationError('validation', 'Invalid MCP resource URI')
29-
deadline.throwIfAborted()
30-
const result = await readResource(item.uri, deadline)
31-
if (!isJsonWithinByteLimit(result, MCP_PRESENTATION_MAX_BYTES))
32-
throw new OrchestrationError('payload_too_large', 'MCP resource exceeds 12 MiB')
33-
const resource = result.contents.find((resource) => resource.uri === item.uri)
34-
if (!resource) throw new OrchestrationError('not_found', 'MCP linked resource not found')
35-
resources.push(resource)
36-
if (!isJsonWithinByteLimit({ ...presentation, resources }, MCP_PRESENTATION_MAX_BYTES))
37-
throw new OrchestrationError('payload_too_large', 'MCP presentation exceeds 12 MiB')
27+
try {
28+
if (!item.uri || item.uri.length > 2048)
29+
throw new OrchestrationError('validation', 'Invalid MCP resource URI')
30+
deadline.throwIfAborted()
31+
const result = await readResource(item.uri, deadline)
32+
if (!isJsonWithinByteLimit(result, MCP_PRESENTATION_MAX_BYTES))
33+
throw new OrchestrationError('payload_too_large', 'MCP resource exceeds 12 MiB')
34+
const resource = result.contents.find((resource) => resource.uri === item.uri)
35+
if (!resource) throw new OrchestrationError('not_found', 'MCP linked resource not found')
36+
if (
37+
!isJsonWithinByteLimit(
38+
{ ...presentation, resources: [...resources, resource] },
39+
MCP_PRESENTATION_MAX_BYTES
40+
)
41+
)
42+
throw new OrchestrationError('payload_too_large', 'MCP presentation exceeds 12 MiB')
43+
resources.push(resource)
44+
} catch {
45+
signal?.throwIfAborted()
46+
logger.warn('An MCP linked resource could not be captured')
47+
if (deadline.aborted) break
48+
}
3849
}
3950
return { ...presentation, resources }
4051
} catch {

0 commit comments

Comments
 (0)