fix(mcp): make pinned Agent teardown idempotent to avoid double-destroy on cancel/failed connect

Author: waleedlatif1

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

@@ -299,6 +299,22 @@ describe('McpClient notification handler', () => {
     expect(mockPinnedClose).toHaveBeenCalledTimes(1)
   })
 
+  it('does not destroy the pinned transport Agent twice when a failed connect is followed by disconnect', async () => {
+    mockSdkConnect.mockRejectedValueOnce(new Error('connect boom'))
+    const client = new McpClient({
+      config: createConfig(),
+      securityPolicy: { requireConsent: false, auditLevel: 'basic' },
+      resolvedIP: '203.0.113.10',
+    })
+
+    await expect(client.connect()).rejects.toThrow()
+    // The caller (e.g. withConnectTimeout) may still call disconnect() afterward;
+    // teardown must be idempotent so the Agent is destroyed exactly once.
+    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,7 +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()
+        // The Agent is released by the shared catch below, which this throw enters.
         throw new McpConnectionError('Connection attempt cancelled', this.config.name)
       }
 
@@ -231,10 +231,15 @@ export class McpClient {
    * 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.
+   * Idempotent: the handle is cleared before use so repeat calls (a failed connect
+   * followed by the caller's `disconnect()`) never destroy the same Agent twice.
    */
   private async closeTransportAgent(): Promise<void> {
+    const close = this.closePinnedTransport
+    if (!close) return
+    this.closePinnedTransport = undefined
     try {
-      await this.closePinnedTransport?.()
+      await close()
     } catch (error) {
       logger.warn(`Error closing pinned transport for ${this.config.name}:`, error)
     }