feat(copilot): verify managed MCP connections

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

@@ -0,0 +1,289 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const {
+  mockCreateMcpServer,
+  mockDeleteMcpServer,
+  mockUpdateMcpServer,
+  mockVerifyServerConnection,
+} = vi.hoisted(() => ({
+  mockCreateMcpServer: vi.fn(),
+  mockDeleteMcpServer: vi.fn(),
+  mockUpdateMcpServer: vi.fn(),
+  mockVerifyServerConnection: vi.fn(),
+}))
+
+vi.mock('@sim/db', () => ({ db: {} }))
+
+vi.mock('@/lib/mcp/orchestration', () => ({
+  performCreateMcpServer: mockCreateMcpServer,
+  performDeleteMcpServer: mockDeleteMcpServer,
+  performUpdateMcpServer: mockUpdateMcpServer,
+}))
+
+vi.mock('@/lib/mcp/service', () => ({
+  mcpService: { verifyServerConnection: mockVerifyServerConnection },
+}))
+
+import type { ExecutionContext } from '@/lib/copilot/request/types'
+import { executeManageMcpTool } from './manage-mcp-tool'
+
+const context = {
+  workspaceId: 'workspace-1',
+  userId: 'user-1',
+  userPermission: 'write',
+} as ExecutionContext
+
+describe('executeManageMcpTool verification', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('verifies a newly added server before returning it to the agent', async () => {
+    mockCreateMcpServer.mockResolvedValue({
+      success: true,
+      serverId: 'mcp-1',
+      updated: false,
+      authType: 'headers',
+    })
+    mockVerifyServerConnection.mockResolvedValue({
+      verified: true,
+      toolCount: 3,
+      requiresAuthorization: false,
+    })
+
+    const result = await executeManageMcpTool(
+      {
+        operation: 'add',
+        config: { name: 'Memory', url: 'https://memory.example.com/mcp' },
+      },
+      context
+    )
+
+    expect(mockVerifyServerConnection).toHaveBeenCalledWith('user-1', 'mcp-1', 'workspace-1')
+    expect(result).toEqual(
+      expect.objectContaining({
+        success: true,
+        output: expect.objectContaining({
+          serverId: 'mcp-1',
+          verification: {
+            verified: true,
+            toolCount: 3,
+            requiresAuthorization: false,
+          },
+        }),
+      })
+    )
+  })
+
+  it('retains a created server and reports a failed verification', async () => {
+    mockCreateMcpServer.mockResolvedValue({
+      success: true,
+      serverId: 'mcp-1',
+      updated: false,
+      authType: 'headers',
+    })
+    mockVerifyServerConnection.mockResolvedValue({
+      verified: false,
+      toolCount: 0,
+      requiresAuthorization: false,
+      error: 'Connection failed',
+    })
+
+    const result = await executeManageMcpTool(
+      {
+        operation: 'add',
+        config: { name: 'Memory', url: 'https://memory.example.com/mcp' },
+      },
+      context
+    )
+
+    expect(result.success).toBe(true)
+    expect(result.output).toEqual(
+      expect.objectContaining({
+        verification: expect.objectContaining({
+          verified: false,
+          error: 'Connection failed',
+        }),
+      })
+    )
+  })
+
+  it('skips verification when a newly added server is disabled', async () => {
+    mockCreateMcpServer.mockResolvedValue({
+      success: true,
+      serverId: 'mcp-1',
+      updated: false,
+      authType: 'headers',
+    })
+
+    const result = await executeManageMcpTool(
+      {
+        operation: 'add',
+        config: {
+          name: 'Memory',
+          url: 'https://memory.example.com/mcp',
+          enabled: false,
+        },
+      },
+      context
+    )
+
+    expect(mockVerifyServerConnection).not.toHaveBeenCalled()
+    expect(result.output).toEqual(
+      expect.objectContaining({
+        verification: {
+          verified: false,
+          toolCount: 0,
+          requiresAuthorization: false,
+          skipped: true,
+          reason: 'server_disabled',
+        },
+      })
+    )
+  })
+
+  it('re-verifies a server after editing its connection config', async () => {
+    mockUpdateMcpServer.mockResolvedValue({
+      success: true,
+      server: { id: 'mcp-1', name: 'Memory', enabled: true },
+      connectionChanged: true,
+    })
+    mockVerifyServerConnection.mockResolvedValue({
+      verified: true,
+      toolCount: 2,
+      requiresAuthorization: false,
+    })
+
+    const result = await executeManageMcpTool(
+      {
+        operation: 'edit',
+        serverId: 'mcp-1',
+        config: { headers: { 'X-API-Key': '{{MEMORY_KEY}}' } },
+      },
+      context
+    )
+
+    expect(mockVerifyServerConnection).toHaveBeenCalledWith('user-1', 'mcp-1', 'workspace-1')
+    expect(result.output).toEqual(
+      expect.objectContaining({
+        verification: expect.objectContaining({ verified: true, toolCount: 2 }),
+      })
+    )
+  })
+
+  it('skips verification after a cosmetic-only edit', async () => {
+    mockUpdateMcpServer.mockResolvedValue({
+      success: true,
+      server: { id: 'mcp-1', name: 'Renamed Memory', enabled: true },
+      connectionChanged: false,
+    })
+
+    const result = await executeManageMcpTool(
+      {
+        operation: 'edit',
+        serverId: 'mcp-1',
+        config: { name: 'Renamed Memory' },
+      },
+      context
+    )
+
+    expect(mockVerifyServerConnection).not.toHaveBeenCalled()
+    expect(result.output).toEqual(
+      expect.objectContaining({
+        verification: expect.objectContaining({
+          verified: false,
+          skipped: true,
+          reason: 'connection_unchanged',
+        }),
+      })
+    )
+  })
+
+  it('skips verification when an edit echoes unchanged connection config', async () => {
+    mockUpdateMcpServer.mockResolvedValue({
+      success: true,
+      server: { id: 'mcp-1', name: 'Renamed Memory', enabled: true },
+      connectionChanged: false,
+    })
+
+    const result = await executeManageMcpTool(
+      {
+        operation: 'edit',
+        serverId: 'mcp-1',
+        config: {
+          name: 'Renamed Memory',
+          url: 'https://memory.example.com/mcp',
+          transport: 'streamable-http',
+          headers: { Authorization: 'Bearer {{MEMORY_KEY}}' },
+          timeout: 30000,
+          enabled: true,
+        },
+      },
+      context
+    )
+
+    expect(mockVerifyServerConnection).not.toHaveBeenCalled()
+    expect(result.output).toEqual(
+      expect.objectContaining({
+        verification: expect.objectContaining({
+          skipped: true,
+          reason: 'connection_unchanged',
+        }),
+      })
+    )
+  })
+
+  it('re-verifies a server when an edit re-enables it', async () => {
+    mockUpdateMcpServer.mockResolvedValue({
+      success: true,
+      server: { id: 'mcp-1', name: 'Memory', enabled: true },
+      connectionChanged: true,
+    })
+    mockVerifyServerConnection.mockResolvedValue({
+      verified: true,
+      toolCount: 2,
+      requiresAuthorization: false,
+    })
+
+    await executeManageMcpTool(
+      {
+        operation: 'edit',
+        serverId: 'mcp-1',
+        config: { enabled: true },
+      },
+      context
+    )
+
+    expect(mockVerifyServerConnection).toHaveBeenCalledWith('user-1', 'mcp-1', 'workspace-1')
+  })
+
+  it('skips verification when the edited server is disabled', async () => {
+    mockUpdateMcpServer.mockResolvedValue({
+      success: true,
+      server: { id: 'mcp-1', name: 'Memory', enabled: false },
+      connectionChanged: true,
+    })
+
+    const result = await executeManageMcpTool(
+      {
+        operation: 'edit',
+        serverId: 'mcp-1',
+        config: { url: 'https://new-memory.example.com/mcp', enabled: false },
+      },
+      context
+    )
+
+    expect(mockVerifyServerConnection).not.toHaveBeenCalled()
+    expect(result.output).toEqual(
+      expect.objectContaining({
+        verification: expect.objectContaining({
+          skipped: true,
+          reason: 'server_disabled',
+        }),
+      })
+    )
+  })
+})

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

@@ -9,6 +9,8 @@ import {
   performDeleteMcpServer,
   performUpdateMcpServer,
 } from '@/lib/mcp/orchestration'
+import { mcpService } from '@/lib/mcp/service'
+import type { McpServerVerificationResult } from '@/lib/mcp/types'
 
 const logger = createLogger('CopilotToolExecutor')
 
@@ -29,6 +31,18 @@ interface ManageMcpToolParams {
   config?: ManageMcpToolConfig
 }
 
+function skippedVerification(
+  reason: 'server_disabled' | 'connection_unchanged'
+): McpServerVerificationResult {
+  return {
+    verified: false,
+    toolCount: 0,
+    requiresAuthorization: false,
+    skipped: true,
+    reason,
+  }
+}
+
 export async function executeManageMcpTool(
   rawParams: Record<string, unknown>,
   context: ExecutionContext
@@ -109,6 +123,11 @@ export async function executeManageMcpTool(
         }
       }
 
+      const verification =
+        config.enabled === false
+          ? skippedVerification('server_disabled')
+          : await mcpService.verifyServerConnection(context.userId, result.serverId, workspaceId)
+
       return {
         success: true,
         output: {
@@ -119,6 +138,7 @@ export async function executeManageMcpTool(
           message: result.updated
             ? `Updated existing MCP server "${config.name}"`
             : `Added MCP server "${config.name}"`,
+          verification,
         },
       }
     }
@@ -147,6 +167,12 @@ export async function executeManageMcpTool(
         return { success: false, error: `MCP server not found: ${params.serverId}` }
       }
 
+      const verification = !result.server.enabled
+        ? skippedVerification('server_disabled')
+        : result.connectionChanged
+          ? await mcpService.verifyServerConnection(context.userId, params.serverId, workspaceId)
+          : skippedVerification('connection_unchanged')
+
       return {
         success: true,
         output: {
@@ -155,6 +181,7 @@ export async function executeManageMcpTool(
           serverId: params.serverId,
           name: result.server.name,
           message: `Updated MCP server "${result.server.name}"`,
+          verification,
         },
       }
     }