fix(mcp): release pinned Agent on failed connect and point DCR error at client-id/secret setup

Author: waleedlatif1

apps/sim/app/api/mcp/oauth/start/route.test.ts

@@ -176,7 +176,7 @@ describe('MCP OAuth start route', () => {
 
     expect(response.status).toBe(422)
     expect(body.error).toBe(
-      "This server doesn't support OAuth client registration. Configure a token instead."
+      "This server doesn't support automatic OAuth client registration. Add a pre-registered OAuth client ID and secret, or configure a token instead."
     )
   })
 })

apps/sim/app/api/mcp/oauth/start/route.ts

@@ -26,7 +26,7 @@ const logger = createLogger('McpOauthStartAPI')
 const OAUTH_START_TTL_MS = 10 * 60 * 1000
 const MAX_SURFACED_ERROR_LENGTH = 250
 const DCR_UNSUPPORTED_MESSAGE =
-  "This server doesn't support OAuth client registration. Configure a token instead."
+  "This server doesn't support automatic OAuth client registration. Add a pre-registered OAuth client ID and secret, or configure a token instead."
 
 function isDynamicClientRegistrationUnsupported(error: unknown): boolean {
   return getErrorMessage(error, '')

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

@@ -4,7 +4,7 @@
 import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockLogger, mockSdkConnect, mockSdkListTools } = vi.hoisted(() => ({
+const { mockLogger, mockSdkConnect, mockSdkListTools, mockPinnedClose } = vi.hoisted(() => ({
   mockLogger: {
     debug: vi.fn(),
     error: vi.fn(),
@@ -13,12 +13,17 @@ const { mockLogger, mockSdkConnect, mockSdkListTools } = vi.hoisted(() => ({
   },
   mockSdkConnect: vi.fn().mockResolvedValue(undefined),
   mockSdkListTools: vi.fn().mockResolvedValue({ tools: [] }),
+  mockPinnedClose: vi.fn().mockResolvedValue(undefined),
 }))
 
 vi.mock('@sim/logger', () => ({
   createLogger: () => mockLogger,
 }))
 
+vi.mock('@/lib/mcp/pinned-fetch', () => ({
+  createPinnedMcpFetch: vi.fn(() => ({ fetch: vi.fn(), close: mockPinnedClose })),
+}))
+
 /**
  * Capture the notification handler registered via `client.setNotificationHandler()`.
  * This lets us simulate the MCP SDK delivering a `tools/list_changed` notification.
@@ -266,6 +271,34 @@ describe('McpClient notification handler', () => {
     expect(JSON.stringify(mockLogger.error.mock.calls)).not.toContain(secret)
   })
 
+  it('closes the pinned transport Agent when connect fails', async () => {
+    mockSdkConnect.mockRejectedValueOnce(new Error('connect boom'))
+    const client = new McpClient({
+      config: createConfig(),
+      securityPolicy: { requireConsent: false, auditLevel: 'basic' },
+      resolvedIP: '203.0.113.10',
+    })
+
+    // A failed connect discards the client without a disconnect(), so the Agent
+    // must be released on the failure path or its h2 sockets leak.
+    await expect(client.connect()).rejects.toThrow()
+
+    expect(mockPinnedClose).toHaveBeenCalledTimes(1)
+  })
+
+  it('closes the pinned transport Agent on disconnect', async () => {
+    const client = new McpClient({
+      config: createConfig(),
+      securityPolicy: { requireConsent: false, auditLevel: 'basic' },
+      resolvedIP: '203.0.113.10',
+    })
+
+    await client.connect()
+    await client.disconnect()
+
+    expect(mockPinnedClose).toHaveBeenCalledTimes(1)
+  })
+
   it('does not misclassify rejected static headers as an OAuth authorization flow', async () => {
     mockSdkConnect.mockRejectedValueOnce(new UnauthorizedError('Static token rejected'))
     const client = new McpClient({

apps/sim/lib/mcp/client.ts

@@ -162,6 +162,7 @@ export class McpClient {
         await this.client.close().catch((error) => {
           logger.warn(`Error closing cancelled connection to ${this.config.name}:`, error)
         })
+        await this.closeTransportAgent()
         throw new McpConnectionError('Connection attempt cancelled', this.config.name)
       }
 
@@ -187,6 +188,8 @@ export class McpClient {
       })
     } catch (error) {
       this.isConnected = false
+      // A failed connect discards this client without a disconnect(), so release the Agent here.
+      await this.closeTransportAgent()
       const errorMessage = getErrorMessage(error, 'Unknown error')
       const outcome = classifyConnectionOutcome(error, this.config.authType)
       logger.error(`Failed to connect to MCP server ${this.config.name}`, {
@@ -217,15 +220,24 @@ export class McpClient {
       logger.warn(`Error during disconnect from ${this.config.name}:`, error)
     }
 
+    await this.closeTransportAgent()
+
+    this.isConnected = false
+    this.connectionStatus.connected = false
+    logger.info(`Disconnected from MCP server: ${this.config.name}`)
+  }
+
+  /**
+   * Tears down the pinned transport's HTTP/2 Agent, releasing its sockets. Must run
+   * on every terminal path — successful disconnect, and failed or cancelled connect —
+   * since a failed `connect()` discards this client without a `disconnect()` call.
+   */
+  private async closeTransportAgent(): Promise<void> {
     try {
       await this.closePinnedTransport?.()
     } catch (error) {
       logger.warn(`Error closing pinned transport for ${this.config.name}:`, error)
     }
-
-    this.isConnected = false
-    this.connectionStatus.connected = false
-    logger.info(`Disconnected from MCP server: ${this.config.name}`)
   }
 
   getStatus(): McpConnectionStatus {