Skip to content

Commit e7abac4

Browse files
j15zj15z
committed
feat(copilot): verify managed MCP connections
1 parent 8a1e3f9 commit e7abac4

5 files changed

Lines changed: 310 additions & 0 deletions

File tree

‎apps/sim/lib/copilot/tools/handlers/management/manage-mcp-tool.test.ts‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const {
7+
mockCreateMcpServer,
8+
mockDeleteMcpServer,
9+
mockUpdateMcpServer,
10+
mockVerifyServerConnection,
11+
} = vi.hoisted(() => ({
12+
mockCreateMcpServer: vi.fn(),
13+
mockDeleteMcpServer: vi.fn(),
14+
mockUpdateMcpServer: vi.fn(),
15+
mockVerifyServerConnection: vi.fn(),
16+
}))
17+
18+
vi.mock('@sim/db', () => ({ db: {} }))
19+
20+
vi.mock('@/lib/mcp/orchestration', () => ({
21+
performCreateMcpServer: mockCreateMcpServer,
22+
performDeleteMcpServer: mockDeleteMcpServer,
23+
performUpdateMcpServer: mockUpdateMcpServer,
24+
}))
25+
26+
vi.mock('@/lib/mcp/service', () => ({
27+
mcpService: { verifyServerConnection: mockVerifyServerConnection },
28+
}))
29+
30+
import type { ExecutionContext } from '@/lib/copilot/request/types'
31+
import { executeManageMcpTool } from './manage-mcp-tool'
32+
33+
const context = {
34+
workspaceId: 'workspace-1',
35+
userId: 'user-1',
36+
userPermission: 'write',
37+
} as ExecutionContext
38+
39+
describe('executeManageMcpTool verification', () => {
40+
beforeEach(() => {
41+
vi.clearAllMocks()
42+
})
43+
44+
it('verifies a newly added server before returning it to the agent', async () => {
45+
mockCreateMcpServer.mockResolvedValue({
46+
success: true,
47+
serverId: 'mcp-1',
48+
updated: false,
49+
authType: 'headers',
50+
})
51+
mockVerifyServerConnection.mockResolvedValue({
52+
verified: true,
53+
toolCount: 3,
54+
requiresAuthorization: false,
55+
})
56+
57+
const result = await executeManageMcpTool(
58+
{
59+
operation: 'add',
60+
config: { name: 'Memory', url: 'https://memory.example.com/mcp' },
61+
},
62+
context
63+
)
64+
65+
expect(mockVerifyServerConnection).toHaveBeenCalledWith('user-1', 'mcp-1', 'workspace-1')
66+
expect(result).toEqual(
67+
expect.objectContaining({
68+
success: true,
69+
output: expect.objectContaining({
70+
serverId: 'mcp-1',
71+
verification: {
72+
verified: true,
73+
toolCount: 3,
74+
requiresAuthorization: false,
75+
},
76+
}),
77+
})
78+
)
79+
})
80+
81+
it('retains a created server and reports a failed verification', async () => {
82+
mockCreateMcpServer.mockResolvedValue({
83+
success: true,
84+
serverId: 'mcp-1',
85+
updated: false,
86+
authType: 'headers',
87+
})
88+
mockVerifyServerConnection.mockResolvedValue({
89+
verified: false,
90+
toolCount: 0,
91+
requiresAuthorization: false,
92+
error: 'Connection failed',
93+
})
94+
95+
const result = await executeManageMcpTool(
96+
{
97+
operation: 'add',
98+
config: { name: 'Memory', url: 'https://memory.example.com/mcp' },
99+
},
100+
context
101+
)
102+
103+
expect(result.success).toBe(true)
104+
expect(result.output).toEqual(
105+
expect.objectContaining({
106+
verification: expect.objectContaining({
107+
verified: false,
108+
error: 'Connection failed',
109+
}),
110+
})
111+
)
112+
})
113+
114+
it('skips verification when a newly added server is disabled', async () => {
115+
mockCreateMcpServer.mockResolvedValue({
116+
success: true,
117+
serverId: 'mcp-1',
118+
updated: false,
119+
authType: 'headers',
120+
})
121+
122+
const result = await executeManageMcpTool(
123+
{
124+
operation: 'add',
125+
config: {
126+
name: 'Memory',
127+
url: 'https://memory.example.com/mcp',
128+
enabled: false,
129+
},
130+
},
131+
context
132+
)
133+
134+
expect(mockVerifyServerConnection).not.toHaveBeenCalled()
135+
expect(result.output).toEqual(
136+
expect.objectContaining({
137+
verification: {
138+
verified: false,
139+
toolCount: 0,
140+
requiresAuthorization: false,
141+
skipped: true,
142+
reason: 'server_disabled',
143+
},
144+
})
145+
)
146+
})
147+
148+
it('re-verifies a server after editing its connection config', async () => {
149+
mockUpdateMcpServer.mockResolvedValue({
150+
success: true,
151+
server: { id: 'mcp-1', name: 'Memory', enabled: true },
152+
})
153+
mockVerifyServerConnection.mockResolvedValue({
154+
verified: true,
155+
toolCount: 2,
156+
requiresAuthorization: false,
157+
})
158+
159+
const result = await executeManageMcpTool(
160+
{
161+
operation: 'edit',
162+
serverId: 'mcp-1',
163+
config: { headers: { 'X-API-Key': '{{MEMORY_KEY}}' } },
164+
},
165+
context
166+
)
167+
168+
expect(mockVerifyServerConnection).toHaveBeenCalledWith('user-1', 'mcp-1', 'workspace-1')
169+
expect(result.output).toEqual(
170+
expect.objectContaining({
171+
verification: expect.objectContaining({ verified: true, toolCount: 2 }),
172+
})
173+
)
174+
})
175+
176+
it('skips verification after a cosmetic-only edit', async () => {
177+
mockUpdateMcpServer.mockResolvedValue({
178+
success: true,
179+
server: { id: 'mcp-1', name: 'Renamed Memory', enabled: true },
180+
})
181+
182+
const result = await executeManageMcpTool(
183+
{
184+
operation: 'edit',
185+
serverId: 'mcp-1',
186+
config: { name: 'Renamed Memory' },
187+
},
188+
context
189+
)
190+
191+
expect(mockVerifyServerConnection).not.toHaveBeenCalled()
192+
expect(result.output).toEqual(
193+
expect.objectContaining({
194+
verification: expect.objectContaining({
195+
verified: false,
196+
skipped: true,
197+
reason: 'connection_unchanged',
198+
}),
199+
})
200+
)
201+
})
202+
})

‎apps/sim/lib/copilot/tools/handlers/management/manage-mcp-tool.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import {
99
performDeleteMcpServer,
1010
performUpdateMcpServer,
1111
} from '@/lib/mcp/orchestration'
12+
import { mcpService } from '@/lib/mcp/service'
13+
import type { McpServerVerificationResult } from '@/lib/mcp/types'
1214

1315
const logger = createLogger('CopilotToolExecutor')
1416

@@ -29,6 +31,18 @@ interface ManageMcpToolParams {
2931
config?: ManageMcpToolConfig
3032
}
3133

34+
function skippedVerification(
35+
reason: 'server_disabled' | 'connection_unchanged'
36+
): McpServerVerificationResult {
37+
return {
38+
verified: false,
39+
toolCount: 0,
40+
requiresAuthorization: false,
41+
skipped: true,
42+
reason,
43+
}
44+
}
45+
3246
export async function executeManageMcpTool(
3347
rawParams: Record<string, unknown>,
3448
context: ExecutionContext
@@ -109,6 +123,11 @@ export async function executeManageMcpTool(
109123
}
110124
}
111125

126+
const verification =
127+
config.enabled === false
128+
? skippedVerification('server_disabled')
129+
: await mcpService.verifyServerConnection(context.userId, result.serverId, workspaceId)
130+
112131
return {
113132
success: true,
114133
output: {
@@ -119,6 +138,7 @@ export async function executeManageMcpTool(
119138
message: result.updated
120139
? `Updated existing MCP server "${config.name}"`
121140
: `Added MCP server "${config.name}"`,
141+
verification,
122142
},
123143
}
124144
}
@@ -147,6 +167,15 @@ export async function executeManageMcpTool(
147167
return { success: false, error: `MCP server not found: ${params.serverId}` }
148168
}
149169

170+
const changesConnection = ['url', 'transport', 'headers', 'timeout', 'enabled'].some(
171+
(key) => config[key as keyof ManageMcpToolConfig] !== undefined
172+
)
173+
const verification = !result.server.enabled
174+
? skippedVerification('server_disabled')
175+
: changesConnection
176+
? await mcpService.verifyServerConnection(context.userId, params.serverId, workspaceId)
177+
: skippedVerification('connection_unchanged')
178+
150179
return {
151180
success: true,
152181
output: {
@@ -155,6 +184,7 @@ export async function executeManageMcpTool(
155184
serverId: params.serverId,
156185
name: result.server.name,
157186
message: `Updated MCP server "${result.server.name}"`,
187+
verification,
158188
},
159189
}
160190
}

‎apps/sim/lib/mcp/service.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,31 @@ describe('McpService.discoverTools per-server caching', () => {
366366
expect(after.map((t) => t.name)).toEqual(['a1'])
367367
expect(mockListTools).toHaveBeenCalledTimes(1)
368368
})
369+
370+
it('verifies a server with a force-refreshed live tools handshake', async () => {
371+
mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
372+
mockListTools.mockResolvedValueOnce([tool('a1', 'mcp-a'), tool('a2', 'mcp-a')])
373+
374+
await expect(
375+
mcpService.verifyServerConnection(USER_ID, 'mcp-a', WORKSPACE_ID)
376+
).resolves.toEqual({
377+
verified: true,
378+
toolCount: 2,
379+
requiresAuthorization: false,
380+
})
381+
})
382+
383+
it('returns a structured verification failure without throwing', async () => {
384+
mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
385+
mockListTools.mockRejectedValueOnce(new Error('Request timed out\ninternal detail'))
386+
387+
await expect(
388+
mcpService.verifyServerConnection(USER_ID, 'mcp-a', WORKSPACE_ID)
389+
).resolves.toEqual({
390+
verified: false,
391+
toolCount: 0,
392+
requiresAuthorization: false,
393+
error: 'Request timed out',
394+
})
395+
})
369396
})

‎apps/sim/lib/mcp/service.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { mcpServers } from '@sim/db/schema'
55
import { createLogger } from '@sim/logger'
66
import { getErrorMessage } from '@sim/utils/errors'
77
import { sleep } from '@sim/utils/helpers'
8+
import { truncate } from '@sim/utils/string'
89
import { and, eq, isNull } from 'drizzle-orm'
910
import { isTest } from '@/lib/core/config/env-flags'
1011
import { generateRequestId } from '@/lib/core/utils/request'
@@ -33,6 +34,7 @@ import {
3334
type McpServerConfig,
3435
type McpServerStatusConfig,
3536
type McpServerSummary,
37+
type McpServerVerificationResult,
3638
type McpTool,
3739
type McpToolCall,
3840
type McpToolResult,
@@ -41,6 +43,7 @@ import {
4143
import { MCP_CLIENT_CONSTANTS, MCP_CONSTANTS } from '@/lib/mcp/utils'
4244

4345
const logger = createLogger('McpService')
46+
const MAX_VERIFICATION_ERROR_LENGTH = 200
4447

4548
function serverCacheKey(workspaceId: string, serverId: string): string {
4649
return `workspace:${workspaceId}:server:${serverId}`
@@ -420,6 +423,30 @@ class McpService {
420423
}
421424
}
422425

426+
async verifyServerConnection(
427+
userId: string,
428+
serverId: string,
429+
workspaceId: string
430+
): Promise<McpServerVerificationResult> {
431+
try {
432+
const tools = await this.discoverServerTools(userId, serverId, workspaceId, true)
433+
return {
434+
verified: true,
435+
toolCount: tools.length,
436+
requiresAuthorization: false,
437+
}
438+
} catch (error) {
439+
const message = getErrorMessage(error, 'Connection failed').split('\n')[0]
440+
return {
441+
verified: false,
442+
toolCount: 0,
443+
requiresAuthorization:
444+
error instanceof McpOauthAuthorizationRequiredError || error instanceof UnauthorizedError,
445+
error: truncate(message, MAX_VERIFICATION_ERROR_LENGTH, ''),
446+
}
447+
}
448+
}
449+
423450
async discoverTools(
424451
userId: string,
425452
workspaceId: string,

‎apps/sim/lib/mcp/types.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,30 @@ export interface McpServerStatusConfig {
1010
lastSuccessfulDiscovery: string | null
1111
}
1212

13+
export type McpServerVerificationResult =
14+
| {
15+
verified: true
16+
toolCount: number
17+
requiresAuthorization: false
18+
error?: never
19+
skipped?: false
20+
}
21+
| {
22+
verified: false
23+
toolCount: 0
24+
requiresAuthorization: boolean
25+
error: string
26+
skipped?: false
27+
}
28+
| {
29+
verified: false
30+
toolCount: 0
31+
requiresAuthorization: false
32+
error?: never
33+
skipped: true
34+
reason: 'server_disabled' | 'connection_unchanged'
35+
}
36+
1337
export interface McpServerConfig {
1438
id: string
1539
name: string

0 commit comments

Comments
 (0)