fix(mcp): close pinned h2 Agent on disconnect and revoke OAuth tokens on auth-type change

Author: waleedlatif1

apps/sim/lib/core/security/input-validation.server.ts

@@ -444,6 +444,19 @@ export function createPinnedFetch(
   resolvedIP: string,
   options?: { allowH2?: boolean }
 ): typeof fetch {
+  return createPinnedFetchWithDispatcher(resolvedIP, options).fetch
+}
+
+/**
+ * Same as {@link createPinnedFetch} but also returns the underlying `Agent` so a
+ * caller with a defined connection lifetime (e.g. a long-lived MCP transport) can
+ * tear the Agent down on close instead of waiting for its idle timeout. Closing
+ * the Agent is what releases any pooled keep-alive / HTTP/2 sockets it holds.
+ */
+export function createPinnedFetchWithDispatcher(
+  resolvedIP: string,
+  options?: { allowH2?: boolean }
+): { fetch: typeof fetch; dispatcher: Agent } {
   const dispatcher = new Agent({
     allowH2: options?.allowH2 ?? false,
     connect: { lookup: createPinnedLookup(resolvedIP) },
@@ -459,7 +472,7 @@ export function createPinnedFetch(
     return response as unknown as Response
   }
 
-  return pinned
+  return { fetch: pinned, dispatcher }
 }
 
 /**

apps/sim/lib/mcp/client.ts

@@ -73,6 +73,7 @@ export class McpClient {
   private onToolsChanged?: McpToolsChangedCallback
   private authProvider?: McpClientOptions['authProvider']
   private isConnected = false
+  private closePinnedTransport?: () => Promise<void>
 
   constructor(options: McpClientOptions) {
     this.config = options.config
@@ -95,10 +96,12 @@ export class McpClient {
       throw new McpError('OAuth MCP server requires an authProvider')
     }
     const useOauth = this.config.authType === 'oauth'
+    const pinned = resolvedIP ? createPinnedMcpFetch(resolvedIP) : undefined
+    this.closePinnedTransport = pinned?.close
     this.transport = new StreamableHTTPClientTransport(new URL(this.config.url), {
       authProvider: useOauth ? this.authProvider : undefined,
       requestInit: { headers: this.config.headers },
-      ...(resolvedIP ? { fetch: createPinnedMcpFetch(resolvedIP) } : {}),
+      ...(pinned ? { fetch: pinned.fetch } : {}),
     })
 
     this.client = new Client(
@@ -214,6 +217,12 @@ export class McpClient {
       logger.warn(`Error during disconnect from ${this.config.name}:`, error)
     }
 
+    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}`)

apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts

@@ -14,9 +14,10 @@ import {
 } from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockClearCache, mockOauthCredsChanged } = vi.hoisted(() => ({
+const { mockClearCache, mockOauthCredsChanged, mockRevokeOauthTokens } = vi.hoisted(() => ({
   mockClearCache: vi.fn(),
   mockOauthCredsChanged: vi.fn(),
+  mockRevokeOauthTokens: vi.fn(),
 }))
 
 vi.mock('@sim/audit', () => auditMock)
@@ -41,7 +42,7 @@ vi.mock('@/lib/mcp/domain-check', () => ({
 vi.mock('@/lib/mcp/oauth', () => ({
   detectMcpAuthType: vi.fn(),
   oauthCredsChanged: mockOauthCredsChanged,
-  revokeMcpOauthTokens: vi.fn(),
+  revokeMcpOauthTokens: mockRevokeOauthTokens,
 }))
 vi.mock('@/lib/mcp/service', () => ({
   mcpService: { clearCache: mockClearCache },
@@ -136,5 +137,7 @@ describe('MCP server lifecycle orchestration', () => {
         lastError: null,
       })
     )
+    // ...and revoke the now-orphaned OAuth tokens rather than leaving them stored and valid.
+    expect(mockRevokeOauthTokens).toHaveBeenCalledWith('server-1')
   })
 })

apps/sim/lib/mcp/orchestration/server-lifecycle.ts

@@ -349,9 +349,11 @@ export async function performUpdateMcpServer(
       currentClientId: currentServer.oauthClientId,
       currentEncryptedClientSecret: currentServer.oauthClientSecret,
     })
-    const shouldClearOauth = urlChanged || credsChanged
     const resolvedAuthType = (updateData.authType ?? currentServer.authType) as McpAuthType
     const authTypeChanged = resolvedAuthType !== currentServer.authType
+    // Turning OAuth off must revoke and delete its now-orphaned tokens, not just reset the connection.
+    const oauthDisabled = currentServer.authType === 'oauth' && resolvedAuthType !== 'oauth'
+    const shouldClearOauth = urlChanged || credsChanged || oauthDisabled
     // An auth-type flip (either direction) or OAuth creds/URL change invalidates the connection: reset and clear stale state.
     if (authTypeChanged || (shouldClearOauth && resolvedAuthType === 'oauth')) {
       updateData.connectionStatus = 'disconnected'

apps/sim/lib/mcp/pinned-fetch.ts

@@ -1,18 +1,34 @@
 import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'
-import { createPinnedFetch } from '@/lib/core/security/input-validation.server'
+import {
+  createPinnedFetch,
+  createPinnedFetchWithDispatcher,
+} from '@/lib/core/security/input-validation.server'
 import { validateMcpServerSsrf } from '@/lib/mcp/domain-check'
 
+/** Pinned fetch for the live MCP transport, plus a handle to release its sockets. */
+export interface PinnedMcpFetch {
+  /** Pinned fetch to hand to the MCP transport's `fetch` option. */
+  fetch: typeof fetch
+  /** Tears down the underlying HTTP/2 Agent; call when the MCP client disconnects. */
+  close: () => Promise<void>
+}
+
 /**
  * Pinned fetch for the long-lived MCP transport, which reuses one Agent across
  * a connection's requests. MCP servers are commonly behind HTTP/2 fronts (CDNs,
  * cloud LBs), and undici's Agent is h1.1-only unless opted into h2 via ALPN, so
  * the transport enables it. h2 is *not* used for one-shot flows (OAuth discovery,
  * auth-type probe), where a per-request Agent would leave idle h2 sessions with
  * no reuse benefit. Pinning is unaffected: the pinned lookup forces the socket to
- * `resolvedIP` regardless of negotiated protocol.
+ * `resolvedIP` regardless of negotiated protocol. The returned `close` binds the
+ * Agent's teardown to the transport lifecycle so h2 sessions don't linger past
+ * disconnect.
  */
-export function createPinnedMcpFetch(resolvedIP: string): typeof fetch {
-  return createPinnedFetch(resolvedIP, { allowH2: true })
+export function createPinnedMcpFetch(resolvedIP: string): PinnedMcpFetch {
+  const { fetch: pinnedFetch, dispatcher } = createPinnedFetchWithDispatcher(resolvedIP, {
+    allowH2: true,
+  })
+  return { fetch: pinnedFetch, close: () => dispatcher.destroy() }
 }
 
 /**