Skip to content

Commit 5e2bdc8

Browse files
fix(agent): ignore empty advanced MCP bindings
1 parent 54bd538 commit 5e2bdc8

6 files changed

Lines changed: 61 additions & 3 deletions

File tree

apps/sim/executor/handlers/agent/agent-handler.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3779,6 +3779,32 @@ describe('AgentBlockHandler', () => {
37793779
])
37803780
})
37813781

3782+
it('does not create tools for a blank advanced MCP server binding', async () => {
3783+
await handler.execute(
3784+
{
3785+
...mockContext,
3786+
workspaceId: 'test-workspace-123',
3787+
workflowId: 'test-workflow-456',
3788+
},
3789+
mockBlock,
3790+
{
3791+
model: 'gpt-4o',
3792+
userPrompt: 'Continue without MCP tools',
3793+
apiKey: 'test-api-key',
3794+
tools: [
3795+
{
3796+
type: 'mcp-server-advanced',
3797+
params: { serverId: '' },
3798+
usageControl: 'auto' as const,
3799+
},
3800+
],
3801+
}
3802+
)
3803+
3804+
expect(mockDiscoverMcpServerToolsAsExecutor).not.toHaveBeenCalled()
3805+
expect(mockExecuteProviderRequest.mock.calls[0][1].tools).toEqual([])
3806+
})
3807+
37823808
describe('customToolId resolution - DB as source of truth', () => {
37833809
const staleInlineSchema = {
37843810
function: {

apps/sim/executor/handlers/agent/agent-handler.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,8 @@ export class AgentBlockHandler implements BlockHandler {
803803
if (entry.tool.type === 'mcp') {
804804
mcpTools.push(entry)
805805
} else if (entry.tool.type === MCP_SERVER_ADVANCED_TOOL_TYPE) {
806+
const serverId = entry.tool.params?.serverId
807+
if (typeof serverId === 'string' && !serverId.trim()) continue
806808
advancedMcpServers.push(entry)
807809
} else {
808810
otherTools.push(entry)

apps/sim/executor/handlers/mothership/mothership-handler.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,25 @@ describe('MothershipBlockHandler', () => {
10301030
])
10311031
})
10321032

1033+
it('does not forward tools for a blank advanced MCP server binding', async () => {
1034+
fetchMock.mockResolvedValue(createJsonResponse({ content: 'done', toolCalls: [] }))
1035+
1036+
await handler.execute(context, block, {
1037+
prompt: 'Continue without MCP tools',
1038+
tools: [
1039+
{
1040+
type: 'mcp-server-advanced',
1041+
params: { serverId: '' },
1042+
usageControl: 'auto',
1043+
},
1044+
],
1045+
})
1046+
1047+
expect(mockDiscoverMcpServerToolsAsExecutor).not.toHaveBeenCalled()
1048+
const [, options] = fetchMock.mock.calls[0] as [string, RequestInit]
1049+
expect(JSON.parse(String(options.body))).not.toHaveProperty('mcpTools')
1050+
})
1051+
10331052
it('does not scan arbitrary Mothership metadata, attachment names, or payloads', async () => {
10341053
const secret = 'boundary-secret'
10351054
const registry = new ResolvedSecretTraceRegistry([

apps/sim/executor/handlers/mothership/mothership-handler.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,10 @@ async function expandMothershipMcpTools(
161161
throw new Error('MCP Server (Advanced) requires params.serverId')
162162
}
163163
const serverId = candidate.params.serverId
164-
if (typeof serverId !== 'string' || !serverId.trim()) {
164+
if (typeof serverId !== 'string') {
165165
throw new Error('MCP Server (Advanced) requires params.serverId')
166166
}
167+
if (!serverId.trim()) return []
167168
const usageControl: 'auto' | 'force' = candidate.usageControl === 'force' ? 'force' : 'auto'
168169
return [{ serverId, usageControl }]
169170
}

apps/sim/lib/mcp/shared.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,18 @@ describe('assertValidMcpServerToolBindings', () => {
4646
).not.toThrow()
4747
})
4848

49+
it('ignores server-wide bindings with blank server IDs', () => {
50+
expect(() =>
51+
assertValidMcpServerToolBindings([
52+
{ type: 'mcp-server-advanced', params: { serverId: '' } },
53+
{ type: 'mcp-server-advanced', params: { serverId: ' ' } },
54+
])
55+
).not.toThrow()
56+
})
57+
4958
it('fails fast on a malformed active server-wide binding', () => {
5059
expect(() =>
51-
assertValidMcpServerToolBindings([{ type: 'mcp-server-advanced', params: { serverId: '' } }])
60+
assertValidMcpServerToolBindings([{ type: 'mcp-server-advanced', params: {} }])
5261
).toThrow('requires params.serverId')
5362
})
5463
})

apps/sim/lib/mcp/shared.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ export function assertValidMcpServerToolBindings(value: unknown): void {
4949
}
5050
if (tool.type !== MCP_SERVER_ADVANCED_TOOL_TYPE) continue
5151
const serverId = tool.params?.serverId
52-
if (typeof serverId !== 'string' || !serverId.trim()) {
52+
if (typeof serverId !== 'string') {
5353
throw new Error('MCP Server (Advanced) requires params.serverId')
5454
}
55+
if (!serverId.trim()) continue
5556
if (advancedServerIds.has(serverId)) {
5657
throw new Error(`Duplicate MCP Server (Advanced) binding for ${serverId}`)
5758
}

0 commit comments

Comments
 (0)