From e55fff5d7a73db3a731fec96eadd60d04180c241 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 14:44:20 -0700
Subject: [PATCH 01/13] feat(mcp): reuse warm connections for tool execution
 and discovery

---
 apps/sim/lib/mcp/connection-pool.test.ts | 237 +++++++++++++++++++++
 apps/sim/lib/mcp/connection-pool.ts      | 258 +++++++++++++++++++++++
 apps/sim/lib/mcp/service-pool.test.ts    | 160 ++++++++++++++
 apps/sim/lib/mcp/service.ts              | 135 ++++++++----
 4 files changed, 747 insertions(+), 43 deletions(-)
 create mode 100644 apps/sim/lib/mcp/connection-pool.test.ts
 create mode 100644 apps/sim/lib/mcp/connection-pool.ts
 create mode 100644 apps/sim/lib/mcp/service-pool.test.ts

diff --git a/apps/sim/lib/mcp/connection-pool.test.ts b/apps/sim/lib/mcp/connection-pool.test.ts
new file mode 100644
index 00000000000..62311f11285
--- /dev/null
+++ b/apps/sim/lib/mcp/connection-pool.test.ts
@@ -0,0 +1,237 @@
+/**
+ * @vitest-environment node
+ */
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { McpClient } from '@/lib/mcp/client'
+import { McpConnectionPool } from '@/lib/mcp/connection-pool'
+
+vi.mock('@sim/logger', () => ({
+  createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }),
+}))
+
+interface FakeClient extends McpClient {
+  __fireClose(): void
+  __setConnected(connected: boolean): void
+}
+
+function makeFakeClient(init: { connected?: boolean; pingRejects?: boolean } = {}): FakeClient {
+  let connected = init.connected ?? true
+  const closeCallbacks: Array<() => void> = []
+  const client = {
+    getStatus: vi.fn(() => ({ connected })),
+    ping: vi.fn(async () => {
+      if (init.pingRejects) throw new Error('ping failed')
+      return {}
+    }),
+    disconnect: vi.fn(async () => {
+      connected = false
+    }),
+    onClose: vi.fn((cb: () => void) => {
+      closeCallbacks.push(cb)
+    }),
+    __fireClose: () => {
+      for (const cb of closeCallbacks) cb()
+    },
+    __setConnected: (value: boolean) => {
+      connected = value
+    },
+  }
+  // double-cast-allowed: test double only implements the McpClient surface the pool touches
+  return client as unknown as FakeClient
+}
+
+describe('McpConnectionPool', () => {
+  let pool: McpConnectionPool
+
+  beforeEach(() => {
+    vi.useFakeTimers()
+    pool = new McpConnectionPool()
+  })
+
+  afterEach(() => {
+    pool.dispose()
+    vi.useRealTimers()
+  })
+
+  it('reuses a warm connection across acquires (connects once)', async () => {
+    const client = makeFakeClient()
+    const create = vi.fn(async () => client)
+
+    const first = await pool.acquire({ key: 's1', create })
+    const second = await pool.acquire({ key: 's1', create })
+
+    expect(create).toHaveBeenCalledTimes(1)
+    expect(first).toBe(client)
+    expect(second).toBe(client)
+  })
+
+  it('dedups concurrent creates into a single connect (single-flight)', async () => {
+    let resolveCreate: ((client: McpClient) => void) | undefined
+    const created = new Promise<McpClient>((resolve) => {
+      resolveCreate = resolve
+    })
+    const create = vi.fn(() => created)
+
+    const p1 = pool.acquire({ key: 's1', create })
+    const p2 = pool.acquire({ key: 's1', create })
+
+    const client = makeFakeClient()
+    resolveCreate?.(client)
+    const [c1, c2] = await Promise.all([p1, p2])
+
+    expect(create).toHaveBeenCalledTimes(1)
+    expect(c1).toBe(client)
+    expect(c2).toBe(client)
+  })
+
+  it('rebuilds and disconnects the old connection when the config changes', async () => {
+    const oldClient = makeFakeClient()
+    const newClient = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockResolvedValueOnce(oldClient)
+      .mockResolvedValueOnce(newClient)
+
+    await pool.acquire({ key: 's1', configUpdatedAt: '2026-01-01T00:00:00.000Z', create })
+    const second = await pool.acquire({
+      key: 's1',
+      configUpdatedAt: '2026-01-02T00:00:00.000Z',
+      create,
+    })
+
+    expect(create).toHaveBeenCalledTimes(2)
+    expect(oldClient.disconnect).toHaveBeenCalledTimes(1)
+    expect(second).toBe(newClient)
+  })
+
+  it('does not rebuild when the config timestamp is unchanged or older', async () => {
+    const client = makeFakeClient()
+    const create = vi.fn(async () => client)
+
+    await pool.acquire({ key: 's1', configUpdatedAt: '2026-01-02T00:00:00.000Z', create })
+    await pool.acquire({ key: 's1', configUpdatedAt: '2026-01-01T00:00:00.000Z', create })
+
+    expect(create).toHaveBeenCalledTimes(1)
+    expect(client.disconnect).not.toHaveBeenCalled()
+  })
+
+  it('rebuilds after the max connection age (SSRF pin re-resolves)', async () => {
+    const oldClient = makeFakeClient()
+    const newClient = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockResolvedValueOnce(oldClient)
+      .mockResolvedValueOnce(newClient)
+
+    await pool.acquire({ key: 's1', create })
+    // Jump the clock past the 10-minute max age without firing the idle sweep.
+    vi.setSystemTime(Date.now() + 11 * 60 * 1000)
+    const second = await pool.acquire({ key: 's1', create })
+
+    expect(create).toHaveBeenCalledTimes(2)
+    expect(oldClient.disconnect).toHaveBeenCalledTimes(1)
+    expect(second).toBe(newClient)
+  })
+
+  it('caches liveness for 60s, then pings and rebuilds on ping failure', async () => {
+    const client = makeFakeClient()
+    const create = vi.fn(async () => client)
+
+    await pool.acquire({ key: 's1', create })
+    // Within the liveness TTL: no ping.
+    vi.setSystemTime(Date.now() + 30 * 1000)
+    await pool.acquire({ key: 's1', create })
+    expect(client.ping).not.toHaveBeenCalled()
+
+    // Past the TTL: pings once and reuses (ping resolves).
+    vi.setSystemTime(Date.now() + 61 * 1000)
+    await pool.acquire({ key: 's1', create })
+    expect(client.ping).toHaveBeenCalledTimes(1)
+    expect(create).toHaveBeenCalledTimes(1)
+  })
+
+  it('evicts and rebuilds when the liveness ping fails', async () => {
+    const dead = makeFakeClient({ pingRejects: true })
+    const fresh = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockResolvedValueOnce(dead)
+      .mockResolvedValueOnce(fresh)
+
+    await pool.acquire({ key: 's1', create })
+    vi.setSystemTime(Date.now() + 61 * 1000)
+    const second = await pool.acquire({ key: 's1', create })
+
+    expect(dead.disconnect).toHaveBeenCalledTimes(1)
+    expect(second).toBe(fresh)
+    expect(create).toHaveBeenCalledTimes(2)
+  })
+
+  it('rebuilds a connection that reports it is no longer connected', async () => {
+    const client = makeFakeClient()
+    const fresh = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockResolvedValueOnce(client)
+      .mockResolvedValueOnce(fresh)
+
+    await pool.acquire({ key: 's1', create })
+    client.__setConnected(false)
+    const second = await pool.acquire({ key: 's1', create })
+
+    expect(client.disconnect).toHaveBeenCalledTimes(1)
+    expect(second).toBe(fresh)
+  })
+
+  it('drops a connection from the pool when its transport closes', async () => {
+    const client = makeFakeClient()
+    const fresh = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockResolvedValueOnce(client)
+      .mockResolvedValueOnce(fresh)
+
+    await pool.acquire({ key: 's1', create })
+    client.__fireClose()
+    const second = await pool.acquire({ key: 's1', create })
+
+    expect(create).toHaveBeenCalledTimes(2)
+    expect(second).toBe(fresh)
+  })
+
+  it('evict disconnects the pooled connection', async () => {
+    const client = makeFakeClient()
+    const create = vi.fn(async () => client)
+
+    await pool.acquire({ key: 's1', create })
+    await pool.evict('s1', 'test')
+
+    expect(client.disconnect).toHaveBeenCalledTimes(1)
+    // A follow-up acquire rebuilds.
+    await pool.acquire({ key: 's1', create: vi.fn(async () => makeFakeClient()) })
+    expect(create).toHaveBeenCalledTimes(1)
+  })
+
+  it('evicts idle connections on the background sweep', async () => {
+    const client = makeFakeClient()
+    await pool.acquire({ key: 's1', create: async () => client })
+
+    // Advance past the 5-minute idle timeout so the 60s sweep evicts it.
+    await vi.advanceTimersByTimeAsync(6 * 60 * 1000)
+
+    expect(client.disconnect).toHaveBeenCalledTimes(1)
+  })
+
+  it('bypasses the pool once disposed (connects without caching)', async () => {
+    const client = makeFakeClient()
+    const create = vi.fn(async () => client)
+
+    pool.dispose()
+    const acquired = await pool.acquire({ key: 's1', create })
+
+    expect(acquired).toBe(client)
+    // Not cached: a second acquire connects again.
+    await pool.acquire({ key: 's1', create })
+    expect(create).toHaveBeenCalledTimes(2)
+  })
+})
diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
new file mode 100644
index 00000000000..3f17a031127
--- /dev/null
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -0,0 +1,258 @@
+/**
+ * MCP Connection Pool
+ *
+ * Serves warm, reused MCP connections to the tool-execution hot paths
+ * (`executeTool` / discovery), replacing the connect-per-operation pattern that
+ * pays a full connect + `initialize` + teardown on every call.
+ *
+ * Design (verified against LibreChat, Cline, VS Code, and the MCP TS SDK):
+ * - **One warm connection per server**, keyed by server id. Concurrent requests
+ *   multiplex safely over a single transport (SDK tracks each request by id), so
+ *   no per-server sub-pool is needed.
+ * - **Demand-driven reconnect, not a background loop**: on a dead/stale
+ *   connection the caller's existing retry re-acquires a fresh one. This mirrors
+ *   VS Code's model and keeps the pool free of reconnect-storm machinery.
+ * - **Single-flight creation** so a burst of concurrent acquires for the same
+ *   server performs exactly one connect (the LibreChat app-repo race this fixes).
+ * - **Liveness is TTL-cached** — at most one `ping()` per {@link LIVENESS_TTL_MS}
+ *   window, never per request.
+ * - **Eviction** on: config change (`updatedAt` moved), max age (so the pinned
+ *   SSRF `resolvedIP` re-resolves), idle timeout, transport close, and explicit
+ *   session-error eviction by the caller.
+ *
+ * Connections are per-process (no cross-replica sharing on ECS), consistent with
+ * every reference client. Callers must NOT disconnect an acquired client — the
+ * pool owns its lifecycle.
+ *
+ * A server that also supports `listChanged` keeps a separate connection in the
+ * connection manager (for notifications); the two lifecycles are intentionally
+ * distinct — this pool is not the notification path and does not consolidate it.
+ */
+
+import { createLogger } from '@sim/logger'
+import { isTest } from '@/lib/core/config/env-flags'
+import type { McpClient } from '@/lib/mcp/client'
+
+const logger = createLogger('McpConnectionPool')
+
+const MAX_POOL_SIZE = 100
+/** Max connection lifetime; on expiry the pinned SSRF `resolvedIP` re-resolves. */
+const MAX_CONNECTION_AGE_MS = 10 * 60 * 1000
+/** Liveness (`ping`) is checked at most once per this window per connection. */
+const LIVENESS_TTL_MS = 60 * 1000
+const IDLE_TIMEOUT_MS = 5 * 60 * 1000
+const IDLE_CHECK_INTERVAL_MS = 60 * 1000
+
+/** Parameters for acquiring (or lazily creating) a pooled connection. */
+export interface AcquireParams {
+  /** Pool key — the MCP server id. */
+  key: string
+  /**
+   * The server config's `updatedAt` (ISO string) at acquire time. When it moves
+   * past the pooled connection's creation time the connection is rebuilt, so a
+   * config edit never rides a stale warm connection.
+   */
+  configUpdatedAt?: string
+  /**
+   * Factory that constructs and connects a fresh {@link McpClient}. Invoked only
+   * on a genuine miss; concurrent misses for the same key share one invocation.
+   */
+  create: () => Promise<McpClient>
+}
+
+interface PoolEntry {
+  client: McpClient
+  createdAt: number
+  /** `configUpdatedAt` (ms epoch) the connection was built against; 0 if unknown. */
+  configUpdatedAtMs: number
+  lastActivityAt: number
+  lastLivenessCheckAt: number
+}
+
+/**
+ * Parses an ISO timestamp to epoch ms, or 0 when absent/invalid. A `0` sentinel
+ * means "unknown" and never triggers staleness on its own (0 is always ≤ an
+ * entry's recorded value), so a missing `updatedAt` degrades to no-op, not churn.
+ */
+function toEpochMs(iso: string | undefined): number {
+  if (!iso) return 0
+  const ms = Date.parse(iso)
+  return Number.isNaN(ms) ? 0 : ms
+}
+
+export class McpConnectionPool {
+  private entries = new Map<string, PoolEntry>()
+  private pending = new Map<string, Promise<McpClient>>()
+  private idleCheckTimer: ReturnType<typeof setInterval> | null = null
+  private disposed = false
+
+  /**
+   * Return a warm, live connection for `key`, creating one on a miss. The caller
+   * must not disconnect the returned client; call {@link evict} to drop it (e.g.
+   * on a session error) so the next acquire rebuilds it.
+   */
+  async acquire(params: AcquireParams): Promise<McpClient> {
+    if (this.disposed) return params.create()
+
+    const inFlight = this.pending.get(params.key)
+    if (inFlight) return inFlight
+
+    const reusable = await this.tryReuse(params)
+    if (reusable) return reusable
+
+    return this.create(params)
+  }
+
+  /**
+   * Returns the pooled client for `key` if it passes every reuse check
+   * (config-fresh, within max age, connected, and live), refreshing its liveness
+   * stamp as a side effect. Evicts and returns `null` on any failed check.
+   */
+  private async tryReuse(params: AcquireParams): Promise<McpClient | null> {
+    const entry = this.entries.get(params.key)
+    if (!entry) return null
+
+    const now = Date.now()
+    const configUpdatedAtMs = toEpochMs(params.configUpdatedAt)
+
+    if (configUpdatedAtMs > entry.configUpdatedAtMs) {
+      await this.evict(params.key, 'config changed')
+      return null
+    }
+    if (now - entry.createdAt > MAX_CONNECTION_AGE_MS) {
+      await this.evict(params.key, 'max age reached')
+      return null
+    }
+    if (!entry.client.getStatus().connected) {
+      await this.evict(params.key, 'not connected')
+      return null
+    }
+    if (now - entry.lastLivenessCheckAt > LIVENESS_TTL_MS) {
+      const alive = await entry.client
+        .ping()
+        .then(() => true)
+        .catch(() => false)
+      if (!alive) {
+        await this.evict(params.key, 'ping failed')
+        return null
+      }
+      entry.lastLivenessCheckAt = now
+    }
+
+    entry.lastActivityAt = now
+    return entry.client
+  }
+
+  /** Single-flight creation: concurrent misses for one key share this invocation. */
+  private create(params: AcquireParams): Promise<McpClient> {
+    // Re-check under the synchronous part of this call: two acquires can both
+    // clear the top-of-`acquire` pending check before either reaches here (the
+    // `await tryReuse` between them yields), so the first to arrive registers the
+    // pending promise and any follower joins it instead of connecting again.
+    const inFlight = this.pending.get(params.key)
+    if (inFlight) return inFlight
+
+    const creation = (async () => {
+      const client = await params.create()
+      if (this.disposed) {
+        await client.disconnect().catch(() => {})
+        return client
+      }
+      this.evictLruIfFull()
+      const now = Date.now()
+      this.entries.set(params.key, {
+        client,
+        createdAt: now,
+        configUpdatedAtMs: toEpochMs(params.configUpdatedAt),
+        lastActivityAt: now,
+        lastLivenessCheckAt: now,
+      })
+      client.onClose(this.makeTransportCloseHandler(params.key))
+      this.ensureIdleCheck()
+      return client
+    })()
+
+    this.pending.set(params.key, creation)
+    return creation.finally(() => {
+      // Identity guard: only clear our own pending entry, never a newer attempt's.
+      if (this.pending.get(params.key) === creation) this.pending.delete(params.key)
+    })
+  }
+
+  /**
+   * Builds the transport-close handler in its own scope so it captures only the
+   * `key` string — never the `create` params, which transitively retain the
+   * resolved config (and its auth secrets), `resolvedIP`, and the service. Those
+   * must not stay alive for the connection's lifetime.
+   */
+  private makeTransportCloseHandler(key: string): () => void {
+    return () => void this.evict(key, 'transport closed')
+  }
+
+  /** Drop and disconnect the pooled connection for `key`, if any. */
+  async evict(key: string, reason: string): Promise<void> {
+    const entry = this.entries.get(key)
+    if (!entry) return
+    this.entries.delete(key)
+    logger.info(`Evicting pooled MCP connection ${key}: ${reason}`)
+    await entry.client.disconnect().catch((error) => {
+      logger.warn(`Error disconnecting evicted MCP connection ${key}:`, error)
+    })
+  }
+
+  /** Evict the least-recently-used connection when the pool is at capacity. */
+  private evictLruIfFull(): void {
+    if (this.entries.size < MAX_POOL_SIZE) return
+    let lruKey: string | undefined
+    let oldest = Number.POSITIVE_INFINITY
+    for (const [key, entry] of this.entries) {
+      if (entry.lastActivityAt < oldest) {
+        oldest = entry.lastActivityAt
+        lruKey = key
+      }
+    }
+    if (lruKey) void this.evict(lruKey, 'pool at capacity (LRU)')
+  }
+
+  private ensureIdleCheck(): void {
+    if (this.idleCheckTimer) return
+    this.idleCheckTimer = setInterval(() => {
+      const now = Date.now()
+      for (const [key, entry] of this.entries) {
+        if (now - entry.lastActivityAt > IDLE_TIMEOUT_MS) {
+          void this.evict(key, 'idle timeout')
+        }
+      }
+      if (this.entries.size === 0 && this.idleCheckTimer) {
+        clearInterval(this.idleCheckTimer)
+        this.idleCheckTimer = null
+      }
+    }, IDLE_CHECK_INTERVAL_MS)
+    // Never keep the process (or a test runner) alive for the sweep.
+    this.idleCheckTimer.unref?.()
+  }
+
+  /** Tear down every connection and the idle sweep. */
+  dispose(): void {
+    this.disposed = true
+    if (this.idleCheckTimer) {
+      clearInterval(this.idleCheckTimer)
+      this.idleCheckTimer = null
+    }
+    const clients = [...this.entries.values()].map((e) => e.client)
+    this.entries.clear()
+    this.pending.clear()
+    void Promise.allSettled(clients.map((client) => client.disconnect()))
+  }
+}
+
+type McpPoolGlobal = typeof globalThis & {
+  _mcpConnectionPool?: McpConnectionPool | null
+}
+
+const _g = globalThis as McpPoolGlobal
+if (!('_mcpConnectionPool' in _g)) {
+  _g._mcpConnectionPool = isTest ? null : new McpConnectionPool()
+}
+
+export const mcpConnectionPool: McpConnectionPool | null = _g._mcpConnectionPool ?? null
diff --git a/apps/sim/lib/mcp/service-pool.test.ts b/apps/sim/lib/mcp/service-pool.test.ts
new file mode 100644
index 00000000000..04947060bde
--- /dev/null
+++ b/apps/sim/lib/mcp/service-pool.test.ts
@@ -0,0 +1,160 @@
+/**
+ * @vitest-environment node
+ *
+ * Integration coverage for the connection-reuse wiring in `McpService`
+ * (`withServerClient`): the pooled path reuses without disconnecting, per-request
+ * headers bypass the pool, and a failed operation evicts the pooled connection.
+ * The pool's own behavior is unit-tested in `connection-pool.test.ts`.
+ */
+import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
+import { loggerMock } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const {
+  MockMcpClient,
+  mockCallTool,
+  mockConnect,
+  mockDisconnect,
+  mockAcquire,
+  mockEvict,
+  mockResolveEnvVars,
+  mockCacheAdapter,
+  poolClient,
+} = vi.hoisted(() => {
+  const mockCallTool = vi.fn()
+  const mockConnect = vi.fn()
+  const mockDisconnect = vi.fn()
+  const poolClient = { callTool: mockCallTool, disconnect: vi.fn() }
+  const cacheStore = new Map<string, unknown>()
+  return {
+    mockCallTool,
+    mockConnect,
+    mockDisconnect,
+    poolClient,
+    mockAcquire: vi.fn(async () => poolClient),
+    mockEvict: vi.fn(async () => {}),
+    mockResolveEnvVars: vi.fn(),
+    mockCacheAdapter: {
+      get: vi.fn(async () => null),
+      set: vi.fn(async () => {}),
+      delete: vi.fn(async () => {}),
+      clear: vi.fn(async () => {}),
+      dispose: () => {},
+    },
+    MockMcpClient: vi.fn().mockImplementation(
+      class {
+        constructor() {
+          Object.assign(this, {
+            connect: mockConnect,
+            disconnect: mockDisconnect,
+            callTool: mockCallTool,
+            listTools: vi.fn(async () => []),
+            hasListChangedCapability: vi.fn(() => false),
+            onClose: vi.fn(),
+          })
+        }
+      }
+    ),
+    __cacheStore: cacheStore,
+  }
+})
+
+vi.mock('@sim/logger', () => loggerMock)
+vi.mock('@/lib/core/config/env-flags', () => ({ isTest: true }))
+vi.mock('@/lib/mcp/connection-pool', () => ({
+  mcpConnectionPool: { acquire: mockAcquire, evict: mockEvict },
+}))
+vi.mock('@/lib/mcp/connection-manager', () => ({ mcpConnectionManager: null }))
+vi.mock('@/lib/mcp/client', () => ({ McpClient: MockMcpClient }))
+
+const WORKSPACE_ID = 'ws-1'
+const USER_ID = 'user-1'
+
+vi.mock('@sim/db', () => {
+  const where = () => {
+    const rows = Promise.resolve([
+      {
+        id: 'server-1',
+        name: 'Server 1',
+        description: null,
+        transport: 'streamable-http',
+        url: 'https://server-1.example.com/mcp',
+        authType: 'headers',
+        workspaceId: WORKSPACE_ID,
+        headers: {},
+        timeout: 30000,
+        retries: 3,
+        enabled: true,
+        deletedAt: null,
+        createdAt: new Date('2026-01-01T00:00:00Z'),
+        updatedAt: new Date('2026-01-01T00:00:00Z'),
+      },
+    ])
+    return Object.assign(rows, { limit: (n: number) => rows.then((r) => r.slice(0, n)) })
+  }
+  return {
+    db: { select: vi.fn().mockReturnValue({ from: vi.fn().mockReturnValue({ where }) }) },
+  }
+})
+vi.mock('@/lib/mcp/domain-check', () => ({
+  isMcpDomainAllowed: () => true,
+  validateMcpDomain: () => {},
+  validateMcpServerSsrf: async () => '203.0.113.10',
+}))
+vi.mock('@/lib/mcp/oauth', () => ({
+  getOrCreateOauthRow: vi.fn(),
+  loadPreregisteredClient: vi.fn(),
+  SimMcpOauthProvider: vi.fn(),
+  withMcpOauthRefreshLock: vi.fn(),
+}))
+vi.mock('@/lib/mcp/resolve-config', () => ({
+  resolveMcpConfigEnvVars: (...args: unknown[]) => mockResolveEnvVars(...args),
+}))
+vi.mock('@/lib/mcp/storage', () => ({
+  createMcpCacheAdapter: () => mockCacheAdapter,
+  getMcpCacheType: () => 'memory',
+}))
+
+import { mcpService } from '@/lib/mcp/service'
+
+describe('McpService connection reuse wiring', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockResolveEnvVars.mockImplementation(async (config: unknown) => ({ config }))
+    mockAcquire.mockResolvedValue(poolClient)
+    mockCallTool.mockResolvedValue({ content: [] })
+  })
+
+  it('executes a tool through the pool without disconnecting the reused connection', async () => {
+    await mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID)
+
+    expect(mockAcquire).toHaveBeenCalledTimes(1)
+    expect(mockAcquire).toHaveBeenCalledWith(expect.objectContaining({ key: 'server-1' }))
+    expect(mockCallTool).toHaveBeenCalledTimes(1)
+    // Pooled connection is returned to the pool, never disconnected here.
+    expect(poolClient.disconnect).not.toHaveBeenCalled()
+    expect(mockDisconnect).not.toHaveBeenCalled()
+  })
+
+  it('bypasses the pool for calls carrying per-request headers', async () => {
+    await mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID, {
+      Authorization: 'Bearer per-call',
+    })
+
+    // extraHeaders → ephemeral connect-per-op, not the shared pool.
+    expect(mockAcquire).not.toHaveBeenCalled()
+    expect(mockConnect).toHaveBeenCalledTimes(1)
+    expect(mockDisconnect).toHaveBeenCalledTimes(1)
+    expect(mockCallTool).toHaveBeenCalledTimes(1)
+  })
+
+  it('evicts the pooled connection when the tool call fails', async () => {
+    mockCallTool.mockRejectedValue(new StreamableHTTPError(404, 'session gone'))
+
+    await expect(
+      mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID)
+    ).rejects.toThrow()
+
+    expect(mockEvict).toHaveBeenCalledWith('server-1', expect.any(String))
+  })
+})
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index 960007bf868..abfc43e1443 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -12,6 +12,7 @@ import { isTest } from '@/lib/core/config/env-flags'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { McpClient } from '@/lib/mcp/client'
 import { mcpConnectionManager } from '@/lib/mcp/connection-manager'
+import { mcpConnectionPool } from '@/lib/mcp/connection-pool'
 import {
   isMcpDomainAllowed,
   validateMcpDomain,
@@ -302,6 +303,46 @@ class McpService {
     })
   }
 
+  /**
+   * Run `fn` against a connected client for `config`. When allowed, the client is
+   * drawn from the warm pool and returned to it afterward (never disconnected); a
+   * throw from `fn` — which for the MCP transport signals a protocol/session/
+   * transport failure, not a tool-level error (those return `isError`) — evicts
+   * the pooled connection so the caller's retry rebuilds it. Otherwise a one-shot
+   * connection is created and always disconnected. Pooling is skipped for calls
+   * carrying per-request headers, which must not ride a connection shared under
+   * different headers.
+   */
+  private async withServerClient<T>(
+    config: McpServerConfig,
+    resolvedIP: string | null,
+    userId: string | undefined,
+    opts: { allowPool: boolean },
+    fn: (client: McpClient) => Promise<T>
+  ): Promise<T> {
+    const pool = mcpConnectionPool
+    if (opts.allowPool && pool) {
+      const client = await pool.acquire({
+        key: config.id,
+        configUpdatedAt: config.updatedAt,
+        create: () => this.createClient(config, resolvedIP, userId),
+      })
+      try {
+        return await fn(client)
+      } catch (error) {
+        await pool.evict(config.id, 'operation failed')
+        throw error
+      }
+    }
+
+    const client = await this.createClient(config, resolvedIP, userId)
+    try {
+      return await fn(client)
+    } finally {
+      await client.disconnect()
+    }
+  }
+
   /**
    * Execute a tool on a specific server with retry logic for session errors.
    * Retries once on session-related errors (400, 404, session ID issues).
@@ -332,18 +373,20 @@ class McpService {
           userId,
           workspaceId
         )
-        if (extraHeaders && Object.keys(extraHeaders).length > 0) {
+        const hasExtraHeaders = Boolean(extraHeaders && Object.keys(extraHeaders).length > 0)
+        if (hasExtraHeaders) {
           resolvedConfig.headers = { ...resolvedConfig.headers, ...extraHeaders }
         }
-        const client = await this.createClient(resolvedConfig, resolvedIP, userId)
 
-        try {
-          const result = await client.callTool(toolCall)
-          logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
-          return result
-        } finally {
-          await client.disconnect()
-        }
+        const result = await this.withServerClient(
+          resolvedConfig,
+          resolvedIP,
+          userId,
+          { allowPool: !hasExtraHeaders },
+          (client) => client.callTool(toolCall)
+        )
+        logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
+        return result
       } catch (error) {
         if (this.isSessionError(error) && attempt < maxRetries - 1) {
           logger.warn(
@@ -576,16 +619,17 @@ class McpService {
               userId,
               workspaceId
             )
-            const client = await this.createClient(resolvedConfig, resolvedIP, userId)
-            try {
-              const tools = await client.listTools()
-              logger.debug(
-                `[${requestId}] Discovered ${tools.length} tools from server ${config.name}`
-              )
-              return { kind: 'fetched', tools, resolvedConfig, resolvedIP }
-            } finally {
-              await client.disconnect()
-            }
+            const tools = await this.withServerClient(
+              resolvedConfig,
+              resolvedIP,
+              userId,
+              { allowPool: true },
+              (client) => client.listTools()
+            )
+            logger.debug(
+              `[${requestId}] Discovered ${tools.length} tools from server ${config.name}`
+            )
+            return { kind: 'fetched', tools, resolvedConfig, resolvedIP }
           } catch (error) {
             if (isOauthAuthorizationError(error, config.authType)) {
               return { kind: 'oauth-pending' }
@@ -788,27 +832,27 @@ class McpService {
           userId,
           workspaceId
         )
-        const client = await this.createClient(resolvedConfig, resolvedIP, userId)
-
-        try {
-          const tools = await client.listTools()
-          logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
-          await Promise.allSettled([
-            this.cacheAdapter
-              .set(serverCacheKey(workspaceId, serverId), tools, this.cacheTimeout)
-              .catch((err) =>
-                logger.warn(`[${requestId}] Cache write failed for ${config.name}:`, err)
-              ),
-            this.clearServerFailure(workspaceId, serverId),
-            this.updateServerStatus(serverId, workspaceId, {
-              outcome: 'connected',
-              toolCount: tools.length,
-            }),
-          ])
-          return tools
-        } finally {
-          await client.disconnect()
-        }
+        const tools = await this.withServerClient(
+          resolvedConfig,
+          resolvedIP,
+          userId,
+          { allowPool: true },
+          (client) => client.listTools()
+        )
+        logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
+        await Promise.allSettled([
+          this.cacheAdapter
+            .set(serverCacheKey(workspaceId, serverId), tools, this.cacheTimeout)
+            .catch((err) =>
+              logger.warn(`[${requestId}] Cache write failed for ${config.name}:`, err)
+            ),
+          this.clearServerFailure(workspaceId, serverId),
+          this.updateServerStatus(serverId, workspaceId, {
+            outcome: 'connected',
+            toolCount: tools.length,
+          }),
+        ])
+        return tools
       } catch (error) {
         if (isRetryableDiscoveryError(error) && attempt < maxRetries - 1) {
           logger.warn(
@@ -859,9 +903,13 @@ class McpService {
             userId,
             workspaceId
           )
-          const client = await this.createClient(resolvedConfig, resolvedIP, userId)
-          const tools = await client.listTools()
-          await client.disconnect()
+          const tools = await this.withServerClient(
+            resolvedConfig,
+            resolvedIP,
+            userId,
+            { allowPool: true },
+            (client) => client.listTools()
+          )
 
           summaries.push({
             id: config.id,
@@ -921,6 +969,7 @@ class McpService {
           rows.flatMap((r) => [
             this.cacheAdapter.delete(serverCacheKey(workspaceId, r.id)),
             this.cacheAdapter.delete(failureCacheKey(workspaceId, r.id)),
+            mcpConnectionPool?.evict(r.id, 'cache cleared'),
           ])
         )
         logger.debug(`Cleared MCP tool cache for workspace ${workspaceId} (${rows.length} servers)`)

From 3b3bceb0423517c15ffa69d755c601bf84b8b850 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 15:08:49 -0700
Subject: [PATCH 02/13] fix(mcp): make connection pool concurrency-safe and
 auth-scoped

---
 apps/sim/lib/mcp/client.ts               |   8 +-
 apps/sim/lib/mcp/connection-pool.test.ts | 188 ++++++++++-------
 apps/sim/lib/mcp/connection-pool.ts      | 258 ++++++++++++-----------
 apps/sim/lib/mcp/service-pool.test.ts    |  46 ++--
 apps/sim/lib/mcp/service.ts              | 142 ++++++++-----
 5 files changed, 371 insertions(+), 271 deletions(-)

diff --git a/apps/sim/lib/mcp/client.ts b/apps/sim/lib/mcp/client.ts
index a70353a80f8..80931413320 100644
--- a/apps/sim/lib/mcp/client.ts
+++ b/apps/sim/lib/mcp/client.ts
@@ -350,14 +350,18 @@ export class McpClient {
     }
   }
 
-  async ping(): Promise<{ _meta?: Record<string, any> }> {
+  async ping(timeoutMs?: number): Promise<{ _meta?: Record<string, any> }> {
     if (!this.isConnected) {
       throw new McpConnectionError('Not connected to server', this.config.name)
     }
 
     try {
       logger.info(`[${this.config.name}] Sending ping to server`)
-      const response = await this.client.ping()
+      // Bound the ping so a half-open connection (no FIN/RST, so `onclose` never
+      // fires) is detected quickly instead of stalling on the SDK's 60s default.
+      const response = await this.client.ping(
+        timeoutMs !== undefined ? { timeout: timeoutMs } : undefined
+      )
       logger.info(`[${this.config.name}] Ping successful`)
       return response
     } catch (error) {
diff --git a/apps/sim/lib/mcp/connection-pool.test.ts b/apps/sim/lib/mcp/connection-pool.test.ts
index 62311f11285..9e1f3889b6d 100644
--- a/apps/sim/lib/mcp/connection-pool.test.ts
+++ b/apps/sim/lib/mcp/connection-pool.test.ts
@@ -3,7 +3,7 @@
  */
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import type { McpClient } from '@/lib/mcp/client'
-import { McpConnectionPool } from '@/lib/mcp/connection-pool'
+import { type AcquireParams, McpConnectionPool } from '@/lib/mcp/connection-pool'
 
 vi.mock('@sim/logger', () => ({
   createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }),
@@ -40,6 +40,16 @@ function makeFakeClient(init: { connected?: boolean; pingRejects?: boolean } = {
   return client as unknown as FakeClient
 }
 
+/** Acquire + immediately release (models a completed borrow). */
+async function borrow(pool: McpConnectionPool, params: AcquireParams, poison = false): Promise<void> {
+  const lease = await pool.acquire(params)
+  await lease.release(poison)
+}
+
+function params(key: string, create: () => Promise<McpClient>, configUpdatedAt?: string): AcquireParams {
+  return { key, serverId: key.split(':')[0], configUpdatedAt, create }
+}
+
 describe('McpConnectionPool', () => {
   let pool: McpConnectionPool
 
@@ -57,12 +67,11 @@ describe('McpConnectionPool', () => {
     const client = makeFakeClient()
     const create = vi.fn(async () => client)
 
-    const first = await pool.acquire({ key: 's1', create })
-    const second = await pool.acquire({ key: 's1', create })
+    await borrow(pool, params('s1:w1:u1', create))
+    await borrow(pool, params('s1:w1:u1', create))
 
     expect(create).toHaveBeenCalledTimes(1)
-    expect(first).toBe(client)
-    expect(second).toBe(client)
+    expect(client.disconnect).not.toHaveBeenCalled()
   })
 
   it('dedups concurrent creates into a single connect (single-flight)', async () => {
@@ -72,16 +81,31 @@ describe('McpConnectionPool', () => {
     })
     const create = vi.fn(() => created)
 
-    const p1 = pool.acquire({ key: 's1', create })
-    const p2 = pool.acquire({ key: 's1', create })
+    const p1 = pool.acquire(params('s1:w1:u1', create))
+    const p2 = pool.acquire(params('s1:w1:u1', create))
 
-    const client = makeFakeClient()
-    resolveCreate?.(client)
-    const [c1, c2] = await Promise.all([p1, p2])
+    resolveCreate?.(makeFakeClient())
+    const [l1, l2] = await Promise.all([p1, p2])
 
     expect(create).toHaveBeenCalledTimes(1)
-    expect(c1).toBe(client)
-    expect(c2).toBe(client)
+    expect(l1.client).toBe(l2.client)
+    await l1.release()
+    await l2.release()
+  })
+
+  it('keys by (server, workspace, user) so different users do not share a connection', async () => {
+    const a = makeFakeClient()
+    const b = makeFakeClient()
+    const createA = vi.fn(async () => a)
+    const createB = vi.fn(async () => b)
+
+    const la = await pool.acquire(params('s1:w1:userA', createA))
+    const lb = await pool.acquire(params('s1:w1:userB', createB))
+
+    expect(la.client).toBe(a)
+    expect(lb.client).toBe(b)
+    expect(createA).toHaveBeenCalledTimes(1)
+    expect(createB).toHaveBeenCalledTimes(1)
   })
 
   it('rebuilds and disconnects the old connection when the config changes', async () => {
@@ -92,30 +116,16 @@ describe('McpConnectionPool', () => {
       .mockResolvedValueOnce(oldClient)
       .mockResolvedValueOnce(newClient)
 
-    await pool.acquire({ key: 's1', configUpdatedAt: '2026-01-01T00:00:00.000Z', create })
-    const second = await pool.acquire({
-      key: 's1',
-      configUpdatedAt: '2026-01-02T00:00:00.000Z',
-      create,
-    })
+    await borrow(pool, params('s1:w1:u1', create, '2026-01-01T00:00:00.000Z'))
+    const lease = await pool.acquire(params('s1:w1:u1', create, '2026-01-02T00:00:00.000Z'))
 
     expect(create).toHaveBeenCalledTimes(2)
     expect(oldClient.disconnect).toHaveBeenCalledTimes(1)
-    expect(second).toBe(newClient)
+    expect(lease.client).toBe(newClient)
+    await lease.release()
   })
 
-  it('does not rebuild when the config timestamp is unchanged or older', async () => {
-    const client = makeFakeClient()
-    const create = vi.fn(async () => client)
-
-    await pool.acquire({ key: 's1', configUpdatedAt: '2026-01-02T00:00:00.000Z', create })
-    await pool.acquire({ key: 's1', configUpdatedAt: '2026-01-01T00:00:00.000Z', create })
-
-    expect(create).toHaveBeenCalledTimes(1)
-    expect(client.disconnect).not.toHaveBeenCalled()
-  })
-
-  it('rebuilds after the max connection age (SSRF pin re-resolves)', async () => {
+  it('rebuilds after the max connection age', async () => {
     const oldClient = makeFakeClient()
     const newClient = makeFakeClient()
     const create = vi
@@ -123,29 +133,26 @@ describe('McpConnectionPool', () => {
       .mockResolvedValueOnce(oldClient)
       .mockResolvedValueOnce(newClient)
 
-    await pool.acquire({ key: 's1', create })
-    // Jump the clock past the 10-minute max age without firing the idle sweep.
+    await borrow(pool, params('s1:w1:u1', create))
     vi.setSystemTime(Date.now() + 11 * 60 * 1000)
-    const second = await pool.acquire({ key: 's1', create })
+    const lease = await pool.acquire(params('s1:w1:u1', create))
 
     expect(create).toHaveBeenCalledTimes(2)
     expect(oldClient.disconnect).toHaveBeenCalledTimes(1)
-    expect(second).toBe(newClient)
+    await lease.release()
   })
 
-  it('caches liveness for 60s, then pings and rebuilds on ping failure', async () => {
+  it('caches liveness for 60s, then pings before reuse', async () => {
     const client = makeFakeClient()
     const create = vi.fn(async () => client)
 
-    await pool.acquire({ key: 's1', create })
-    // Within the liveness TTL: no ping.
+    await borrow(pool, params('s1:w1:u1', create))
     vi.setSystemTime(Date.now() + 30 * 1000)
-    await pool.acquire({ key: 's1', create })
+    await borrow(pool, params('s1:w1:u1', create))
     expect(client.ping).not.toHaveBeenCalled()
 
-    // Past the TTL: pings once and reuses (ping resolves).
     vi.setSystemTime(Date.now() + 61 * 1000)
-    await pool.acquire({ key: 's1', create })
+    await borrow(pool, params('s1:w1:u1', create))
     expect(client.ping).toHaveBeenCalledTimes(1)
     expect(create).toHaveBeenCalledTimes(1)
   })
@@ -158,16 +165,16 @@ describe('McpConnectionPool', () => {
       .mockResolvedValueOnce(dead)
       .mockResolvedValueOnce(fresh)
 
-    await pool.acquire({ key: 's1', create })
+    await borrow(pool, params('s1:w1:u1', create))
     vi.setSystemTime(Date.now() + 61 * 1000)
-    const second = await pool.acquire({ key: 's1', create })
+    const lease = await pool.acquire(params('s1:w1:u1', create))
 
     expect(dead.disconnect).toHaveBeenCalledTimes(1)
-    expect(second).toBe(fresh)
-    expect(create).toHaveBeenCalledTimes(2)
+    expect(lease.client).toBe(fresh)
+    await lease.release()
   })
 
-  it('rebuilds a connection that reports it is no longer connected', async () => {
+  it('drops a connection from the pool when its transport closes', async () => {
     const client = makeFakeClient()
     const fresh = makeFakeClient()
     const create = vi
@@ -175,63 +182,94 @@ describe('McpConnectionPool', () => {
       .mockResolvedValueOnce(client)
       .mockResolvedValueOnce(fresh)
 
-    await pool.acquire({ key: 's1', create })
-    client.__setConnected(false)
-    const second = await pool.acquire({ key: 's1', create })
+    await borrow(pool, params('s1:w1:u1', create))
+    client.__fireClose()
+    const lease = await pool.acquire(params('s1:w1:u1', create))
 
-    expect(client.disconnect).toHaveBeenCalledTimes(1)
-    expect(second).toBe(fresh)
+    expect(create).toHaveBeenCalledTimes(2)
+    expect(lease.client).toBe(fresh)
+    await lease.release()
   })
 
-  it('drops a connection from the pool when its transport closes', async () => {
+  it('does not disconnect a connection while a borrower still holds it', async () => {
     const client = makeFakeClient()
-    const fresh = makeFakeClient()
-    const create = vi
-      .fn<() => Promise<McpClient>>()
-      .mockResolvedValueOnce(client)
-      .mockResolvedValueOnce(fresh)
+    const create = vi.fn(async () => client)
 
-    await pool.acquire({ key: 's1', create })
-    client.__fireClose()
-    const second = await pool.acquire({ key: 's1', create })
+    const lease = await pool.acquire(params('s1:w1:u1', create))
+    // Retire while borrowed (e.g. server config cleared): must defer the close.
+    await pool.evictServer('s1', 'test')
+    expect(client.disconnect).not.toHaveBeenCalled()
 
-    expect(create).toHaveBeenCalledTimes(2)
-    expect(second).toBe(fresh)
+    // Closes only once the last borrower releases.
+    await lease.release()
+    expect(client.disconnect).toHaveBeenCalledTimes(1)
   })
 
-  it('evict disconnects the pooled connection', async () => {
+  it('does not let one borrower failure disconnect a connection another borrower is using', async () => {
     const client = makeFakeClient()
     const create = vi.fn(async () => client)
 
-    await pool.acquire({ key: 's1', create })
-    await pool.evict('s1', 'test')
+    const a = await pool.acquire(params('s1:w1:u1', create))
+    const b = await pool.acquire(params('s1:w1:u1', create))
+    expect(create).toHaveBeenCalledTimes(1)
+
+    // A fails and poisons the connection while B is still in flight.
+    await a.release(true)
+    expect(client.disconnect).not.toHaveBeenCalled()
 
+    // B finishes → now the retired connection closes.
+    await b.release()
     expect(client.disconnect).toHaveBeenCalledTimes(1)
-    // A follow-up acquire rebuilds.
-    await pool.acquire({ key: 's1', create: vi.fn(async () => makeFakeClient()) })
-    expect(create).toHaveBeenCalledTimes(1)
   })
 
-  it('evicts idle connections on the background sweep', async () => {
+  it('does not idle-evict a connection with an active borrower', async () => {
     const client = makeFakeClient()
-    await pool.acquire({ key: 's1', create: async () => client })
+    const lease = await pool.acquire(params('s1:w1:u1', async () => client))
 
-    // Advance past the 5-minute idle timeout so the 60s sweep evicts it.
     await vi.advanceTimersByTimeAsync(6 * 60 * 1000)
+    expect(client.disconnect).not.toHaveBeenCalled()
+
+    await lease.release()
+  })
+
+  it('idle-evicts a connection once no borrower holds it', async () => {
+    const client = makeFakeClient()
+    await borrow(pool, params('s1:w1:u1', async () => client))
 
+    await vi.advanceTimersByTimeAsync(6 * 60 * 1000)
     expect(client.disconnect).toHaveBeenCalledTimes(1)
   })
 
+  it('does not evict a replacement when a stale connection closes under the same key', async () => {
+    const oldClient = makeFakeClient()
+    const newClient = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockResolvedValueOnce(oldClient)
+      .mockResolvedValueOnce(newClient)
+
+    // Force a rebuild via config change; oldClient is now retired, newClient pooled.
+    await borrow(pool, params('s1:w1:u1', create, '2026-01-01T00:00:00.000Z'))
+    await borrow(pool, params('s1:w1:u1', create, '2026-01-02T00:00:00.000Z'))
+
+    // The old client's late close must NOT drop the replacement.
+    oldClient.__fireClose()
+    await borrow(pool, params('s1:w1:u1', create, '2026-01-02T00:00:00.000Z'))
+
+    // Still 2 creates — the replacement survived the stale close.
+    expect(create).toHaveBeenCalledTimes(2)
+  })
+
   it('bypasses the pool once disposed (connects without caching)', async () => {
     const client = makeFakeClient()
     const create = vi.fn(async () => client)
 
     pool.dispose()
-    const acquired = await pool.acquire({ key: 's1', create })
+    const lease = await pool.acquire(params('s1:w1:u1', create))
+    expect(lease.client).toBe(client)
+    await lease.release()
 
-    expect(acquired).toBe(client)
-    // Not cached: a second acquire connects again.
-    await pool.acquire({ key: 's1', create })
+    await pool.acquire(params('s1:w1:u1', create)).then((l) => l.release())
     expect(create).toHaveBeenCalledTimes(2)
   })
 })
diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index 3f17a031127..32a19fc16ce 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -1,32 +1,20 @@
 /**
- * MCP Connection Pool
+ * Warm MCP connection pool: one reused connection per (server, workspace, user)
+ * for the tool-exec and discovery hot paths, replacing connect-per-operation.
  *
- * Serves warm, reused MCP connections to the tool-execution hot paths
- * (`executeTool` / discovery), replacing the connect-per-operation pattern that
- * pays a full connect + `initialize` + teardown on every call.
+ * The key includes the workspace + user because a server's headers/URL resolve
+ * from the caller's env (`resolveMcpConfigEnvVars`), so two users must never share
+ * a credential-bearing connection. Concurrent borrowers multiplex over one client
+ * (the SDK tracks requests by id); creation is single-flight; liveness is
+ * ping-cached; reconnect is demand-driven (callers' retries re-acquire).
  *
- * Design (verified against LibreChat, Cline, VS Code, and the MCP TS SDK):
- * - **One warm connection per server**, keyed by server id. Concurrent requests
- *   multiplex safely over a single transport (SDK tracks each request by id), so
- *   no per-server sub-pool is needed.
- * - **Demand-driven reconnect, not a background loop**: on a dead/stale
- *   connection the caller's existing retry re-acquires a fresh one. This mirrors
- *   VS Code's model and keeps the pool free of reconnect-storm machinery.
- * - **Single-flight creation** so a burst of concurrent acquires for the same
- *   server performs exactly one connect (the LibreChat app-repo race this fixes).
- * - **Liveness is TTL-cached** — at most one `ping()` per {@link LIVENESS_TTL_MS}
- *   window, never per request.
- * - **Eviction** on: config change (`updatedAt` moved), max age (so the pinned
- *   SSRF `resolvedIP` re-resolves), idle timeout, transport close, and explicit
- *   session-error eviction by the caller.
- *
- * Connections are per-process (no cross-replica sharing on ECS), consistent with
- * every reference client. Callers must NOT disconnect an acquired client — the
- * pool owns its lifecycle.
- *
- * A server that also supports `listChanged` keeps a separate connection in the
- * connection manager (for notifications); the two lifecycles are intentionally
- * distinct — this pool is not the notification path and does not consolidate it.
+ * Connections are ref-counted. Eviction *retires* an entry — removes it from the
+ * pool so no new borrower can take it — but the socket is closed only once the
+ * last in-flight borrower releases, so a sibling failure, idle sweep, or config
+ * change never tears down a connection mid-request. Retirement is identity-checked,
+ * so a stale `onClose` can't disconnect a replacement stored under the same key.
+ * Pools are per-process. Callers must release every acquired lease and must not
+ * disconnect the client themselves.
  */
 
 import { createLogger } from '@sim/logger'
@@ -36,44 +24,44 @@ import type { McpClient } from '@/lib/mcp/client'
 const logger = createLogger('McpConnectionPool')
 
 const MAX_POOL_SIZE = 100
-/** Max connection lifetime; on expiry the pinned SSRF `resolvedIP` re-resolves. */
+/** Max lifetime; on expiry the pinned SSRF `resolvedIP` re-resolves. */
 const MAX_CONNECTION_AGE_MS = 10 * 60 * 1000
-/** Liveness (`ping`) is checked at most once per this window per connection. */
 const LIVENESS_TTL_MS = 60 * 1000
+const LIVENESS_PING_TIMEOUT_MS = 5 * 1000
 const IDLE_TIMEOUT_MS = 5 * 60 * 1000
 const IDLE_CHECK_INTERVAL_MS = 60 * 1000
 
-/** Parameters for acquiring (or lazily creating) a pooled connection. */
 export interface AcquireParams {
-  /** Pool key — the MCP server id. */
+  /** Auth-scoped pool key — `${serverId}:${workspaceId}:${userId}`. */
   key: string
-  /**
-   * The server config's `updatedAt` (ISO string) at acquire time. When it moves
-   * past the pooled connection's creation time the connection is rebuilt, so a
-   * config edit never rides a stale warm connection.
-   */
+  /** Server id, for bulk `evictServer` on config change/delete. */
+  serverId: string
+  /** Server config `updatedAt`; a newer value than the pooled entry rebuilds it. */
   configUpdatedAt?: string
-  /**
-   * Factory that constructs and connects a fresh {@link McpClient}. Invoked only
-   * on a genuine miss; concurrent misses for the same key share one invocation.
-   */
+  /** Builds and connects a fresh client; invoked once per miss (single-flight). */
   create: () => Promise<McpClient>
 }
 
+/** A borrowed connection. `release(poison)` must be called exactly once; pass `poison: true` to retire on failure. */
+export interface ConnectionLease {
+  client: McpClient
+  release: (poison?: boolean) => Promise<void>
+}
+
 interface PoolEntry {
+  key: string
+  serverId: string
   client: McpClient
   createdAt: number
-  /** `configUpdatedAt` (ms epoch) the connection was built against; 0 if unknown. */
   configUpdatedAtMs: number
   lastActivityAt: number
   lastLivenessCheckAt: number
+  borrowers: number
+  retired: boolean
+  closing: boolean
 }
 
-/**
- * Parses an ISO timestamp to epoch ms, or 0 when absent/invalid. A `0` sentinel
- * means "unknown" and never triggers staleness on its own (0 is always ≤ an
- * entry's recorded value), so a missing `updatedAt` degrades to no-op, not churn.
- */
+/** ISO → epoch ms; `0` (unknown) is always ≤ a real stamp, so it never rebuilds. */
 function toEpochMs(iso: string | undefined): number {
   if (!iso) return 0
   const ms = Date.parse(iso)
@@ -82,153 +70,175 @@ function toEpochMs(iso: string | undefined): number {
 
 export class McpConnectionPool {
   private entries = new Map<string, PoolEntry>()
-  private pending = new Map<string, Promise<McpClient>>()
+  private pending = new Map<string, Promise<PoolEntry>>()
   private idleCheckTimer: ReturnType<typeof setInterval> | null = null
   private disposed = false
 
-  /**
-   * Return a warm, live connection for `key`, creating one on a miss. The caller
-   * must not disconnect the returned client; call {@link evict} to drop it (e.g.
-   * on a session error) so the next acquire rebuilds it.
-   */
-  async acquire(params: AcquireParams): Promise<McpClient> {
-    if (this.disposed) return params.create()
-
-    const inFlight = this.pending.get(params.key)
-    if (inFlight) return inFlight
+  /** Borrow a warm connection for `key`, creating one on a miss. Caller must `release`. */
+  async acquire(params: AcquireParams): Promise<ConnectionLease> {
+    if (this.disposed) {
+      const client = await params.create()
+      return { client, release: () => client.disconnect().catch(() => {}) }
+    }
+    const entry = await this.resolveEntry(params)
+    entry.borrowers++
+    entry.lastActivityAt = Date.now()
+    return { client: entry.client, release: (poison) => this.release(entry, poison ?? false) }
+  }
 
-    const reusable = await this.tryReuse(params)
-    if (reusable) return reusable
+  private async resolveEntry(params: AcquireParams): Promise<PoolEntry> {
+    const pending = this.pending.get(params.key)
+    if (pending) {
+      // A joiner may hold a newer config than the create used; rebuild if so.
+      const entry = await pending
+      if (!entry.retired && toEpochMs(params.configUpdatedAt) <= entry.configUpdatedAtMs) {
+        return entry
+      }
+      this.retire(entry)
+      return this.resolveEntry(params)
+    }
 
-    return this.create(params)
+    const current = this.entries.get(params.key)
+    if (current) {
+      const reusable = await this.tryReuse(params, current)
+      if (reusable) return reusable
+    }
+    return this.createEntry(params)
   }
 
-  /**
-   * Returns the pooled client for `key` if it passes every reuse check
-   * (config-fresh, within max age, connected, and live), refreshing its liveness
-   * stamp as a side effect. Evicts and returns `null` on any failed check.
-   */
-  private async tryReuse(params: AcquireParams): Promise<McpClient | null> {
-    const entry = this.entries.get(params.key)
-    if (!entry) return null
-
+  /** Return `entry` if config-fresh, in-age, connected, and live; else retire it and return null. */
+  private async tryReuse(params: AcquireParams, entry: PoolEntry): Promise<PoolEntry | null> {
     const now = Date.now()
-    const configUpdatedAtMs = toEpochMs(params.configUpdatedAt)
-
-    if (configUpdatedAtMs > entry.configUpdatedAtMs) {
-      await this.evict(params.key, 'config changed')
+    if (toEpochMs(params.configUpdatedAt) > entry.configUpdatedAtMs) {
+      this.retire(entry)
       return null
     }
     if (now - entry.createdAt > MAX_CONNECTION_AGE_MS) {
-      await this.evict(params.key, 'max age reached')
+      this.retire(entry)
       return null
     }
     if (!entry.client.getStatus().connected) {
-      await this.evict(params.key, 'not connected')
+      this.retire(entry)
       return null
     }
     if (now - entry.lastLivenessCheckAt > LIVENESS_TTL_MS) {
       const alive = await entry.client
-        .ping()
+        .ping(LIVENESS_PING_TIMEOUT_MS)
         .then(() => true)
         .catch(() => false)
       if (!alive) {
-        await this.evict(params.key, 'ping failed')
+        this.retire(entry)
         return null
       }
       entry.lastLivenessCheckAt = now
     }
-
-    entry.lastActivityAt = now
-    return entry.client
+    return entry
   }
 
-  /** Single-flight creation: concurrent misses for one key share this invocation. */
-  private create(params: AcquireParams): Promise<McpClient> {
-    // Re-check under the synchronous part of this call: two acquires can both
-    // clear the top-of-`acquire` pending check before either reaches here (the
-    // `await tryReuse` between them yields), so the first to arrive registers the
-    // pending promise and any follower joins it instead of connecting again.
+  private createEntry(params: AcquireParams): Promise<PoolEntry> {
+    // Re-check: the `await` in `resolveEntry` yields, so two acquires can both
+    // reach here; the first registers the pending create, the rest join.
     const inFlight = this.pending.get(params.key)
     if (inFlight) return inFlight
 
     const creation = (async () => {
       const client = await params.create()
-      if (this.disposed) {
-        await client.disconnect().catch(() => {})
-        return client
-      }
-      this.evictLruIfFull()
       const now = Date.now()
-      this.entries.set(params.key, {
+      const entry: PoolEntry = {
+        key: params.key,
+        serverId: params.serverId,
         client,
         createdAt: now,
         configUpdatedAtMs: toEpochMs(params.configUpdatedAt),
         lastActivityAt: now,
         lastLivenessCheckAt: now,
-      })
-      client.onClose(this.makeTransportCloseHandler(params.key))
+        borrowers: 0,
+        retired: false,
+        closing: false,
+      }
+      if (this.disposed) {
+        entry.retired = true
+        void client.disconnect().catch(() => {})
+        return entry
+      }
+      this.evictLruIfFull()
+      this.entries.set(params.key, entry)
+      client.onClose(this.makeCloseHandler(entry))
       this.ensureIdleCheck()
-      return client
+      return entry
     })()
 
     this.pending.set(params.key, creation)
     return creation.finally(() => {
-      // Identity guard: only clear our own pending entry, never a newer attempt's.
       if (this.pending.get(params.key) === creation) this.pending.delete(params.key)
     })
   }
 
-  /**
-   * Builds the transport-close handler in its own scope so it captures only the
-   * `key` string — never the `create` params, which transitively retain the
-   * resolved config (and its auth secrets), `resolvedIP`, and the service. Those
-   * must not stay alive for the connection's lifetime.
-   */
-  private makeTransportCloseHandler(key: string): () => void {
-    return () => void this.evict(key, 'transport closed')
+  private async release(entry: PoolEntry, poison: boolean): Promise<void> {
+    entry.borrowers = Math.max(0, entry.borrowers - 1)
+    entry.lastActivityAt = Date.now()
+    if (poison) this.retire(entry)
+    await this.closeIfIdle(entry)
   }
 
-  /** Drop and disconnect the pooled connection for `key`, if any. */
-  async evict(key: string, reason: string): Promise<void> {
-    const entry = this.entries.get(key)
-    if (!entry) return
-    this.entries.delete(key)
-    logger.info(`Evicting pooled MCP connection ${key}: ${reason}`)
+  /** Own scope so the handler captures only `entry` (never the create params / secrets). */
+  private makeCloseHandler(entry: PoolEntry): () => void {
+    return () => {
+      if (this.entries.get(entry.key) === entry) this.retire(entry)
+    }
+  }
+
+  /** Remove `entry` from the pool so no new borrower takes it; close it once idle. */
+  private retire(entry: PoolEntry): void {
+    if (!entry.retired) {
+      entry.retired = true
+      if (this.entries.get(entry.key) === entry) this.entries.delete(entry.key)
+    }
+    void this.closeIfIdle(entry)
+  }
+
+  private async closeIfIdle(entry: PoolEntry): Promise<void> {
+    if (!entry.retired || entry.borrowers > 0 || entry.closing) return
+    entry.closing = true
+    logger.info(`Closing pooled MCP connection ${entry.key}`)
     await entry.client.disconnect().catch((error) => {
-      logger.warn(`Error disconnecting evicted MCP connection ${key}:`, error)
+      logger.warn(`Error disconnecting pooled MCP connection ${entry.key}:`, error)
     })
   }
 
-  /** Evict the least-recently-used connection when the pool is at capacity. */
+  /** Retire every connection for a server (all users) — config changed or deleted. */
+  async evictServer(serverId: string, reason: string): Promise<void> {
+    for (const entry of this.entries.values()) {
+      if (entry.serverId === serverId) {
+        logger.info(`Evicting pooled MCP connection ${entry.key}: ${reason}`)
+        this.retire(entry)
+      }
+    }
+  }
+
   private evictLruIfFull(): void {
     if (this.entries.size < MAX_POOL_SIZE) return
-    let lruKey: string | undefined
-    let oldest = Number.POSITIVE_INFINITY
-    for (const [key, entry] of this.entries) {
-      if (entry.lastActivityAt < oldest) {
-        oldest = entry.lastActivityAt
-        lruKey = key
-      }
+    let lru: PoolEntry | undefined
+    for (const entry of this.entries.values()) {
+      if (!lru || entry.lastActivityAt < lru.lastActivityAt) lru = entry
     }
-    if (lruKey) void this.evict(lruKey, 'pool at capacity (LRU)')
+    // Retiring a still-borrowed LRU frees the map slot now; its socket closes on release.
+    if (lru) this.retire(lru)
   }
 
   private ensureIdleCheck(): void {
     if (this.idleCheckTimer) return
     this.idleCheckTimer = setInterval(() => {
       const now = Date.now()
-      for (const [key, entry] of this.entries) {
-        if (now - entry.lastActivityAt > IDLE_TIMEOUT_MS) {
-          void this.evict(key, 'idle timeout')
-        }
+      for (const entry of this.entries.values()) {
+        if (entry.borrowers === 0 && now - entry.lastActivityAt > IDLE_TIMEOUT_MS)
+          this.retire(entry)
       }
       if (this.entries.size === 0 && this.idleCheckTimer) {
         clearInterval(this.idleCheckTimer)
         this.idleCheckTimer = null
       }
     }, IDLE_CHECK_INTERVAL_MS)
-    // Never keep the process (or a test runner) alive for the sweep.
     this.idleCheckTimer.unref?.()
   }
 
diff --git a/apps/sim/lib/mcp/service-pool.test.ts b/apps/sim/lib/mcp/service-pool.test.ts
index 04947060bde..8c7e7e79615 100644
--- a/apps/sim/lib/mcp/service-pool.test.ts
+++ b/apps/sim/lib/mcp/service-pool.test.ts
@@ -2,9 +2,10 @@
  * @vitest-environment node
  *
  * Integration coverage for the connection-reuse wiring in `McpService`
- * (`withServerClient`): the pooled path reuses without disconnecting, per-request
- * headers bypass the pool, and a failed operation evicts the pooled connection.
- * The pool's own behavior is unit-tested in `connection-pool.test.ts`.
+ * (`withServerClient`): the pooled path leases without disconnecting and skips
+ * env resolution on a hit, per-request headers bypass the pool, and a
+ * dead-connection error poisons the lease. Pool internals are unit-tested in
+ * `connection-pool.test.ts`.
  */
 import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
 import { loggerMock } from '@sim/testing'
@@ -16,7 +17,7 @@ const {
   mockConnect,
   mockDisconnect,
   mockAcquire,
-  mockEvict,
+  mockRelease,
   mockResolveEnvVars,
   mockCacheAdapter,
   poolClient,
@@ -24,15 +25,15 @@ const {
   const mockCallTool = vi.fn()
   const mockConnect = vi.fn()
   const mockDisconnect = vi.fn()
+  const mockRelease = vi.fn(async () => {})
   const poolClient = { callTool: mockCallTool, disconnect: vi.fn() }
-  const cacheStore = new Map<string, unknown>()
   return {
     mockCallTool,
     mockConnect,
     mockDisconnect,
+    mockRelease,
     poolClient,
-    mockAcquire: vi.fn(async () => poolClient),
-    mockEvict: vi.fn(async () => {}),
+    mockAcquire: vi.fn(async () => ({ client: poolClient, release: mockRelease })),
     mockResolveEnvVars: vi.fn(),
     mockCacheAdapter: {
       get: vi.fn(async () => null),
@@ -55,14 +56,13 @@ const {
         }
       }
     ),
-    __cacheStore: cacheStore,
   }
 })
 
 vi.mock('@sim/logger', () => loggerMock)
 vi.mock('@/lib/core/config/env-flags', () => ({ isTest: true }))
 vi.mock('@/lib/mcp/connection-pool', () => ({
-  mcpConnectionPool: { acquire: mockAcquire, evict: mockEvict },
+  mcpConnectionPool: { acquire: mockAcquire, evictServer: vi.fn() },
 }))
 vi.mock('@/lib/mcp/connection-manager', () => ({ mcpConnectionManager: null }))
 vi.mock('@/lib/mcp/client', () => ({ McpClient: MockMcpClient }))
@@ -121,19 +121,22 @@ describe('McpService connection reuse wiring', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     mockResolveEnvVars.mockImplementation(async (config: unknown) => ({ config }))
-    mockAcquire.mockResolvedValue(poolClient)
+    mockAcquire.mockResolvedValue({ client: poolClient, release: mockRelease })
     mockCallTool.mockResolvedValue({ content: [] })
   })
 
-  it('executes a tool through the pool without disconnecting the reused connection', async () => {
+  it('leases from the pool (keyed by server+workspace+user) and never disconnects on a hit', async () => {
     await mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID)
 
     expect(mockAcquire).toHaveBeenCalledTimes(1)
-    expect(mockAcquire).toHaveBeenCalledWith(expect.objectContaining({ key: 'server-1' }))
+    expect(mockAcquire).toHaveBeenCalledWith(
+      expect.objectContaining({ key: `server-1:${WORKSPACE_ID}:${USER_ID}`, serverId: 'server-1' })
+    )
     expect(mockCallTool).toHaveBeenCalledTimes(1)
-    // Pooled connection is returned to the pool, never disconnected here.
+    expect(mockRelease).toHaveBeenCalledWith(false)
     expect(poolClient.disconnect).not.toHaveBeenCalled()
-    expect(mockDisconnect).not.toHaveBeenCalled()
+    // A pool hit must not re-resolve env vars (acquire never invoked `create`).
+    expect(mockResolveEnvVars).not.toHaveBeenCalled()
   })
 
   it('bypasses the pool for calls carrying per-request headers', async () => {
@@ -141,20 +144,29 @@ describe('McpService connection reuse wiring', () => {
       Authorization: 'Bearer per-call',
     })
 
-    // extraHeaders → ephemeral connect-per-op, not the shared pool.
     expect(mockAcquire).not.toHaveBeenCalled()
     expect(mockConnect).toHaveBeenCalledTimes(1)
     expect(mockDisconnect).toHaveBeenCalledTimes(1)
     expect(mockCallTool).toHaveBeenCalledTimes(1)
   })
 
-  it('evicts the pooled connection when the tool call fails', async () => {
+  it('poisons the lease when the tool call hits a dead-connection error', async () => {
     mockCallTool.mockRejectedValue(new StreamableHTTPError(404, 'session gone'))
 
     await expect(
       mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID)
     ).rejects.toThrow()
 
-    expect(mockEvict).toHaveBeenCalledWith('server-1', expect.any(String))
+    expect(mockRelease).toHaveBeenCalledWith(true)
+  })
+
+  it('keeps the pooled connection warm on a benign (non-connection) tool error', async () => {
+    mockCallTool.mockRejectedValue(new Error('tool blew up'))
+
+    await expect(
+      mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID)
+    ).rejects.toThrow()
+
+    expect(mockRelease).toHaveBeenCalledWith(false)
   })
 })
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index abfc43e1443..89c79b7d889 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -101,6 +101,14 @@ function isTimeoutError(error: unknown): boolean {
   return getErrorMessage(error, '').toLowerCase().includes('timed out')
 }
 
+/** A pooled connection is dead and must be retired — a stale session or a closed transport, not a benign tool/consent error. */
+function isDeadConnectionError(error: unknown): boolean {
+  if (error instanceof StreamableHTTPError) {
+    return error.code === 404 || error.code === 400
+  }
+  return error instanceof McpError && error.code === ErrorCode.ConnectionClosed
+}
+
 /** Transient failures a read-only `tools/list` may safely retry (idempotent, unlike `tools/call`); excludes OAuth and terminal 4xx. */
 function isRetryableDiscoveryError(error: unknown): boolean {
   if (isTimeoutError(error)) return true
@@ -303,39 +311,46 @@ class McpService {
     })
   }
 
+  /** Auth-scoped pool key: a server's resolved credentials depend on the (user, workspace) env. */
+  private poolKey(
+    serverId: string,
+    workspaceId: string | undefined,
+    userId: string | undefined
+  ): string {
+    return `${serverId}:${workspaceId ?? ''}:${userId ?? ''}`
+  }
+
   /**
-   * Run `fn` against a connected client for `config`. When allowed, the client is
-   * drawn from the warm pool and returned to it afterward (never disconnected); a
-   * throw from `fn` — which for the MCP transport signals a protocol/session/
-   * transport failure, not a tool-level error (those return `isError`) — evicts
-   * the pooled connection so the caller's retry rebuilds it. Otherwise a one-shot
-   * connection is created and always disconnected. Pooling is skipped for calls
-   * carrying per-request headers, which must not ride a connection shared under
-   * different headers.
+   * Run `fn` against a connected client. When `allowPool`, borrow from the warm
+   * pool (`create` runs only on a miss, so a hit skips env resolution + DNS); a
+   * dead-connection error retires it, benign tool/consent errors keep it warm.
+   * Otherwise connect one-shot and always disconnect.
    */
   private async withServerClient<T>(
-    config: McpServerConfig,
-    resolvedIP: string | null,
-    userId: string | undefined,
-    opts: { allowPool: boolean },
+    opts: { key: string; serverId: string; configUpdatedAt?: string; allowPool: boolean },
+    create: () => Promise<McpClient>,
     fn: (client: McpClient) => Promise<T>
   ): Promise<T> {
     const pool = mcpConnectionPool
     if (opts.allowPool && pool) {
-      const client = await pool.acquire({
-        key: config.id,
-        configUpdatedAt: config.updatedAt,
-        create: () => this.createClient(config, resolvedIP, userId),
+      const lease = await pool.acquire({
+        key: opts.key,
+        serverId: opts.serverId,
+        configUpdatedAt: opts.configUpdatedAt,
+        create,
       })
+      let poison = false
       try {
-        return await fn(client)
+        return await fn(lease.client)
       } catch (error) {
-        await pool.evict(config.id, 'operation failed')
+        poison = isDeadConnectionError(error)
         throw error
+      } finally {
+        await lease.release(poison)
       }
     }
 
-    const client = await this.createClient(config, resolvedIP, userId)
+    const client = await create()
     try {
       return await fn(client)
     } finally {
@@ -368,21 +383,27 @@ class McpService {
           throw new Error(`Server ${serverId} not found or not accessible`)
         }
 
-        const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-          config,
-          userId,
-          workspaceId
-        )
         const hasExtraHeaders = Boolean(extraHeaders && Object.keys(extraHeaders).length > 0)
-        if (hasExtraHeaders) {
-          resolvedConfig.headers = { ...resolvedConfig.headers, ...extraHeaders }
+        const create = async () => {
+          const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
+            config,
+            userId,
+            workspaceId
+          )
+          if (hasExtraHeaders) {
+            resolvedConfig.headers = { ...resolvedConfig.headers, ...extraHeaders }
+          }
+          return this.createClient(resolvedConfig, resolvedIP, userId)
         }
 
         const result = await this.withServerClient(
-          resolvedConfig,
-          resolvedIP,
-          userId,
-          { allowPool: !hasExtraHeaders },
+          {
+            key: this.poolKey(serverId, workspaceId, userId),
+            serverId,
+            configUpdatedAt: config.updatedAt,
+            allowPool: !hasExtraHeaders,
+          },
+          create,
           (client) => client.callTool(toolCall)
         )
         logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
@@ -620,10 +641,13 @@ class McpService {
               workspaceId
             )
             const tools = await this.withServerClient(
-              resolvedConfig,
-              resolvedIP,
-              userId,
-              { allowPool: true },
+              {
+                key: this.poolKey(config.id, workspaceId, userId),
+                serverId: config.id,
+                configUpdatedAt: config.updatedAt,
+                allowPool: true,
+              },
+              () => this.createClient(resolvedConfig, resolvedIP, userId),
               (client) => client.listTools()
             )
             logger.debug(
@@ -827,16 +851,22 @@ class McpService {
         }
         authType = config.authType
 
-        const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-          config,
-          userId,
-          workspaceId
-        )
+        const create = async () => {
+          const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
+            config,
+            userId,
+            workspaceId
+          )
+          return this.createClient(resolvedConfig, resolvedIP, userId)
+        }
         const tools = await this.withServerClient(
-          resolvedConfig,
-          resolvedIP,
-          userId,
-          { allowPool: true },
+          {
+            key: this.poolKey(serverId, workspaceId, userId),
+            serverId,
+            configUpdatedAt: config.updatedAt,
+            allowPool: true,
+          },
+          create,
           (client) => client.listTools()
         )
         logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
@@ -898,16 +928,22 @@ class McpService {
 
       for (const config of servers) {
         try {
-          const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-            config,
-            userId,
-            workspaceId
-          )
+          const create = async () => {
+            const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
+              config,
+              userId,
+              workspaceId
+            )
+            return this.createClient(resolvedConfig, resolvedIP, userId)
+          }
           const tools = await this.withServerClient(
-            resolvedConfig,
-            resolvedIP,
-            userId,
-            { allowPool: true },
+            {
+              key: this.poolKey(config.id, workspaceId, userId),
+              serverId: config.id,
+              configUpdatedAt: config.updatedAt,
+              allowPool: true,
+            },
+            create,
             (client) => client.listTools()
           )
 
@@ -969,7 +1005,7 @@ class McpService {
           rows.flatMap((r) => [
             this.cacheAdapter.delete(serverCacheKey(workspaceId, r.id)),
             this.cacheAdapter.delete(failureCacheKey(workspaceId, r.id)),
-            mcpConnectionPool?.evict(r.id, 'cache cleared'),
+            mcpConnectionPool?.evictServer(r.id, 'cache cleared'),
           ])
         )
         logger.debug(`Cleared MCP tool cache for workspace ${workspaceId} (${rows.length} servers)`)

From 70bd09dcbf8436405f71ba770f48b544d16c7644 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 15:27:01 -0700
Subject: [PATCH 03/13] fix(mcp): decouple pool validity from updatedAt, widen
 dead-connection detection, guard ping race

---
 apps/sim/lib/mcp/connection-pool.test.ts | 44 ++++++++++--------------
 apps/sim/lib/mcp/connection-pool.ts      | 35 +++++++------------
 apps/sim/lib/mcp/service.ts              | 28 ++++++++++-----
 3 files changed, 50 insertions(+), 57 deletions(-)

diff --git a/apps/sim/lib/mcp/connection-pool.test.ts b/apps/sim/lib/mcp/connection-pool.test.ts
index 9e1f3889b6d..97a83c71230 100644
--- a/apps/sim/lib/mcp/connection-pool.test.ts
+++ b/apps/sim/lib/mcp/connection-pool.test.ts
@@ -41,13 +41,17 @@ function makeFakeClient(init: { connected?: boolean; pingRejects?: boolean } = {
 }
 
 /** Acquire + immediately release (models a completed borrow). */
-async function borrow(pool: McpConnectionPool, params: AcquireParams, poison = false): Promise<void> {
+async function borrow(
+  pool: McpConnectionPool,
+  params: AcquireParams,
+  poison = false
+): Promise<void> {
   const lease = await pool.acquire(params)
   await lease.release(poison)
 }
 
-function params(key: string, create: () => Promise<McpClient>, configUpdatedAt?: string): AcquireParams {
-  return { key, serverId: key.split(':')[0], configUpdatedAt, create }
+function params(key: string, create: () => Promise<McpClient>): AcquireParams {
+  return { key, serverId: key.split(':')[0], create }
 }
 
 describe('McpConnectionPool', () => {
@@ -108,23 +112,6 @@ describe('McpConnectionPool', () => {
     expect(createB).toHaveBeenCalledTimes(1)
   })
 
-  it('rebuilds and disconnects the old connection when the config changes', async () => {
-    const oldClient = makeFakeClient()
-    const newClient = makeFakeClient()
-    const create = vi
-      .fn<() => Promise<McpClient>>()
-      .mockResolvedValueOnce(oldClient)
-      .mockResolvedValueOnce(newClient)
-
-    await borrow(pool, params('s1:w1:u1', create, '2026-01-01T00:00:00.000Z'))
-    const lease = await pool.acquire(params('s1:w1:u1', create, '2026-01-02T00:00:00.000Z'))
-
-    expect(create).toHaveBeenCalledTimes(2)
-    expect(oldClient.disconnect).toHaveBeenCalledTimes(1)
-    expect(lease.client).toBe(newClient)
-    await lease.release()
-  })
-
   it('rebuilds after the max connection age', async () => {
     const oldClient = makeFakeClient()
     const newClient = makeFakeClient()
@@ -139,6 +126,7 @@ describe('McpConnectionPool', () => {
 
     expect(create).toHaveBeenCalledTimes(2)
     expect(oldClient.disconnect).toHaveBeenCalledTimes(1)
+    expect(lease.client).toBe(newClient)
     await lease.release()
   })
 
@@ -234,7 +222,10 @@ describe('McpConnectionPool', () => {
 
   it('idle-evicts a connection once no borrower holds it', async () => {
     const client = makeFakeClient()
-    await borrow(pool, params('s1:w1:u1', async () => client))
+    await borrow(
+      pool,
+      params('s1:w1:u1', async () => client)
+    )
 
     await vi.advanceTimersByTimeAsync(6 * 60 * 1000)
     expect(client.disconnect).toHaveBeenCalledTimes(1)
@@ -248,13 +239,14 @@ describe('McpConnectionPool', () => {
       .mockResolvedValueOnce(oldClient)
       .mockResolvedValueOnce(newClient)
 
-    // Force a rebuild via config change; oldClient is now retired, newClient pooled.
-    await borrow(pool, params('s1:w1:u1', create, '2026-01-01T00:00:00.000Z'))
-    await borrow(pool, params('s1:w1:u1', create, '2026-01-02T00:00:00.000Z'))
+    // oldClient's transport closes → retired; the next acquire pools newClient.
+    await borrow(pool, params('s1:w1:u1', create))
+    oldClient.__fireClose()
+    await borrow(pool, params('s1:w1:u1', create))
 
-    // The old client's late close must NOT drop the replacement.
+    // A late duplicate close from the old client must NOT drop the replacement.
     oldClient.__fireClose()
-    await borrow(pool, params('s1:w1:u1', create, '2026-01-02T00:00:00.000Z'))
+    await borrow(pool, params('s1:w1:u1', create))
 
     // Still 2 creates — the replacement survived the stale close.
     expect(create).toHaveBeenCalledTimes(2)
diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index 32a19fc16ce..02ca0e7b270 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -15,6 +15,11 @@
  * so a stale `onClose` can't disconnect a replacement stored under the same key.
  * Pools are per-process. Callers must release every acquired lease and must not
  * disconnect the client themselves.
+ *
+ * Config-change invalidation is the caller's job via `evictServer` (wired to the
+ * service's `clearCache`), not a per-acquire timestamp check — `updatedAt` is also
+ * bumped by status telemetry, so it can't distinguish a config edit. Cross-process,
+ * a config edit is bounded by max age (and self-heals when a stale connection errors).
  */
 
 import { createLogger } from '@sim/logger'
@@ -36,8 +41,6 @@ export interface AcquireParams {
   key: string
   /** Server id, for bulk `evictServer` on config change/delete. */
   serverId: string
-  /** Server config `updatedAt`; a newer value than the pooled entry rebuilds it. */
-  configUpdatedAt?: string
   /** Builds and connects a fresh client; invoked once per miss (single-flight). */
   create: () => Promise<McpClient>
 }
@@ -53,7 +56,6 @@ interface PoolEntry {
   serverId: string
   client: McpClient
   createdAt: number
-  configUpdatedAtMs: number
   lastActivityAt: number
   lastLivenessCheckAt: number
   borrowers: number
@@ -61,13 +63,6 @@ interface PoolEntry {
   closing: boolean
 }
 
-/** ISO → epoch ms; `0` (unknown) is always ≤ a real stamp, so it never rebuilds. */
-function toEpochMs(iso: string | undefined): number {
-  if (!iso) return 0
-  const ms = Date.parse(iso)
-  return Number.isNaN(ms) ? 0 : ms
-}
-
 export class McpConnectionPool {
   private entries = new Map<string, PoolEntry>()
   private pending = new Map<string, Promise<PoolEntry>>()
@@ -89,30 +84,22 @@ export class McpConnectionPool {
   private async resolveEntry(params: AcquireParams): Promise<PoolEntry> {
     const pending = this.pending.get(params.key)
     if (pending) {
-      // A joiner may hold a newer config than the create used; rebuild if so.
       const entry = await pending
-      if (!entry.retired && toEpochMs(params.configUpdatedAt) <= entry.configUpdatedAtMs) {
-        return entry
-      }
-      this.retire(entry)
+      if (!entry.retired) return entry
       return this.resolveEntry(params)
     }
 
     const current = this.entries.get(params.key)
     if (current) {
-      const reusable = await this.tryReuse(params, current)
+      const reusable = await this.tryReuse(current)
       if (reusable) return reusable
     }
     return this.createEntry(params)
   }
 
-  /** Return `entry` if config-fresh, in-age, connected, and live; else retire it and return null. */
-  private async tryReuse(params: AcquireParams, entry: PoolEntry): Promise<PoolEntry | null> {
+  /** Return `entry` if in-age, connected, and live; else retire it and return null. */
+  private async tryReuse(entry: PoolEntry): Promise<PoolEntry | null> {
     const now = Date.now()
-    if (toEpochMs(params.configUpdatedAt) > entry.configUpdatedAtMs) {
-      this.retire(entry)
-      return null
-    }
     if (now - entry.createdAt > MAX_CONNECTION_AGE_MS) {
       this.retire(entry)
       return null
@@ -126,6 +113,9 @@ export class McpConnectionPool {
         .ping(LIVENESS_PING_TIMEOUT_MS)
         .then(() => true)
         .catch(() => false)
+      // A concurrent poison/evict/age eviction may have retired this during the
+      // ping await; don't hand out a connection that's already closing.
+      if (entry.retired) return null
       if (!alive) {
         this.retire(entry)
         return null
@@ -149,7 +139,6 @@ export class McpConnectionPool {
         serverId: params.serverId,
         client,
         createdAt: now,
-        configUpdatedAtMs: toEpochMs(params.configUpdatedAt),
         lastActivityAt: now,
         lastLivenessCheckAt: now,
         borrowers: 0,
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index 89c79b7d889..8b340d308be 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -101,12 +101,29 @@ function isTimeoutError(error: unknown): boolean {
   return getErrorMessage(error, '').toLowerCase().includes('timed out')
 }
 
-/** A pooled connection is dead and must be retired — a stale session or a closed transport, not a benign tool/consent error. */
+/**
+ * A pooled connection is dead and must be retired so the caller's retry rebuilds
+ * fresh: a stale session (400/404), a closed transport, a timeout (no response —
+ * possibly wedged), or a reset socket. Benign tool/consent errors and healthy
+ * upstream responses (429/5xx) keep the connection warm.
+ */
 function isDeadConnectionError(error: unknown): boolean {
   if (error instanceof StreamableHTTPError) {
     return error.code === 404 || error.code === 400
   }
-  return error instanceof McpError && error.code === ErrorCode.ConnectionClosed
+  if (error instanceof McpError && error.code === ErrorCode.ConnectionClosed) {
+    return true
+  }
+  if (isTimeoutError(error)) {
+    return true
+  }
+  const message = getErrorMessage(error, '').toLowerCase()
+  return (
+    message.includes('econnreset') ||
+    message.includes('econnrefused') ||
+    message.includes('epipe') ||
+    message.includes('socket hang up')
+  )
 }
 
 /** Transient failures a read-only `tools/list` may safely retry (idempotent, unlike `tools/call`); excludes OAuth and terminal 4xx. */
@@ -327,7 +344,7 @@ class McpService {
    * Otherwise connect one-shot and always disconnect.
    */
   private async withServerClient<T>(
-    opts: { key: string; serverId: string; configUpdatedAt?: string; allowPool: boolean },
+    opts: { key: string; serverId: string; allowPool: boolean },
     create: () => Promise<McpClient>,
     fn: (client: McpClient) => Promise<T>
   ): Promise<T> {
@@ -336,7 +353,6 @@ class McpService {
       const lease = await pool.acquire({
         key: opts.key,
         serverId: opts.serverId,
-        configUpdatedAt: opts.configUpdatedAt,
         create,
       })
       let poison = false
@@ -400,7 +416,6 @@ class McpService {
           {
             key: this.poolKey(serverId, workspaceId, userId),
             serverId,
-            configUpdatedAt: config.updatedAt,
             allowPool: !hasExtraHeaders,
           },
           create,
@@ -644,7 +659,6 @@ class McpService {
               {
                 key: this.poolKey(config.id, workspaceId, userId),
                 serverId: config.id,
-                configUpdatedAt: config.updatedAt,
                 allowPool: true,
               },
               () => this.createClient(resolvedConfig, resolvedIP, userId),
@@ -863,7 +877,6 @@ class McpService {
           {
             key: this.poolKey(serverId, workspaceId, userId),
             serverId,
-            configUpdatedAt: config.updatedAt,
             allowPool: true,
           },
           create,
@@ -940,7 +953,6 @@ class McpService {
             {
               key: this.poolKey(config.id, workspaceId, userId),
               serverId: config.id,
-              configUpdatedAt: config.updatedAt,
               allowPool: true,
             },
             create,

From 252a4a073908a151d786b64309fcdd5ad393d129 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 15:39:03 -0700
Subject: [PATCH 04/13] fix(mcp): retire pooled connections on auth failure and
 server delete

---
 .../orchestration/server-lifecycle.test.ts    | 35 +++++++++++++++----
 .../lib/mcp/orchestration/server-lifecycle.ts |  4 +++
 apps/sim/lib/mcp/service-pool.test.ts         | 11 ++++++
 apps/sim/lib/mcp/service.ts                   | 12 ++++---
 4 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts
index b95f6103291..ed9db50bed8 100644
--- a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts
+++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts
@@ -14,11 +14,13 @@ import {
 } from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockClearCache, mockOauthCredsChanged, mockRevokeOauthTokens } = vi.hoisted(() => ({
-  mockClearCache: vi.fn(),
-  mockOauthCredsChanged: vi.fn(),
-  mockRevokeOauthTokens: vi.fn(),
-}))
+const { mockClearCache, mockOauthCredsChanged, mockRevokeOauthTokens, mockEvictServer } =
+  vi.hoisted(() => ({
+    mockClearCache: vi.fn(),
+    mockOauthCredsChanged: vi.fn(),
+    mockRevokeOauthTokens: vi.fn(),
+    mockEvictServer: vi.fn(),
+  }))
 
 vi.mock('@sim/audit', () => auditMock)
 vi.mock('@sim/db', () => ({
@@ -47,10 +49,16 @@ vi.mock('@/lib/mcp/oauth', () => ({
 vi.mock('@/lib/mcp/service', () => ({
   mcpService: { clearCache: mockClearCache },
 }))
+vi.mock('@/lib/mcp/connection-pool', () => ({
+  mcpConnectionPool: { evictServer: mockEvictServer },
+}))
 vi.mock('@/lib/mcp/utils', () => ({ generateMcpServerId: vi.fn() }))
 vi.mock('@/lib/posthog/server', () => posthogServerMock)
 
-import { performUpdateMcpServer } from '@/lib/mcp/orchestration/server-lifecycle'
+import {
+  performDeleteMcpServer,
+  performUpdateMcpServer,
+} from '@/lib/mcp/orchestration/server-lifecycle'
 
 describe('MCP server lifecycle orchestration', () => {
   beforeEach(() => {
@@ -140,4 +148,19 @@ describe('MCP server lifecycle orchestration', () => {
     // ...and revoke the now-orphaned OAuth tokens rather than leaving them stored and valid.
     expect(mockRevokeOauthTokens).toHaveBeenCalledWith('server-1')
   })
+
+  it('evicts the deleted server from the connection pool (row is already gone from clearCache)', async () => {
+    dbChainMockFns.returning.mockResolvedValueOnce([
+      { id: 'server-1', workspaceId: 'workspace-1', name: 'Example', transport: 'streamable-http' },
+    ])
+
+    const result = await performDeleteMcpServer({
+      workspaceId: 'workspace-1',
+      userId: 'user-1',
+      serverId: 'server-1',
+    })
+
+    expect(result.success).toBe(true)
+    expect(mockEvictServer).toHaveBeenCalledWith('server-1', expect.any(String))
+  })
 })
diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts
index e0a29488fd3..497137762de 100644
--- a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts
+++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts
@@ -6,6 +6,7 @@ import { generateId } from '@sim/utils/id'
 import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { encryptSecret } from '@/lib/core/security/encryption'
+import { mcpConnectionPool } from '@/lib/mcp/connection-pool'
 import {
   McpDnsResolutionError,
   McpDomainNotAllowedError,
@@ -438,6 +439,9 @@ export async function performDeleteMcpServer(
 
     if (!server) return { success: false, error: 'Server not found', errorCode: 'not_found' }
 
+    // The row is already gone, so clearCache's row-enumeration can't reach it —
+    // evict the deleted server's pooled connections explicitly.
+    await mcpConnectionPool?.evictServer(params.serverId, 'server deleted')
     await mcpService.clearCache(params.workspaceId)
     const source =
       params.source === 'settings' || params.source === 'tool_input' ? params.source : undefined
diff --git a/apps/sim/lib/mcp/service-pool.test.ts b/apps/sim/lib/mcp/service-pool.test.ts
index 8c7e7e79615..61464d56c09 100644
--- a/apps/sim/lib/mcp/service-pool.test.ts
+++ b/apps/sim/lib/mcp/service-pool.test.ts
@@ -7,6 +7,7 @@
  * dead-connection error poisons the lease. Pool internals are unit-tested in
  * `connection-pool.test.ts`.
  */
+import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'
 import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
 import { loggerMock } from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
@@ -169,4 +170,14 @@ describe('McpService connection reuse wiring', () => {
 
     expect(mockRelease).toHaveBeenCalledWith(false)
   })
+
+  it('poisons the lease on an auth failure so a rotated credential is re-resolved', async () => {
+    mockCallTool.mockRejectedValue(new UnauthorizedError('token rejected'))
+
+    await expect(
+      mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID)
+    ).rejects.toThrow()
+
+    expect(mockRelease).toHaveBeenCalledWith(true)
+  })
 })
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index 8b340d308be..8580dacb206 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -103,13 +103,17 @@ function isTimeoutError(error: unknown): boolean {
 
 /**
  * A pooled connection is dead and must be retired so the caller's retry rebuilds
- * fresh: a stale session (400/404), a closed transport, a timeout (no response —
- * possibly wedged), or a reset socket. Benign tool/consent errors and healthy
- * upstream responses (429/5xx) keep the connection warm.
+ * fresh: a stale session (400/404), an auth failure (401 — a rotated/revoked
+ * credential; the rebuild re-resolves it), a closed transport, a timeout (no
+ * response — possibly wedged), or a reset socket. Benign tool/consent errors and
+ * healthy upstream responses (429/5xx) keep the connection warm.
  */
 function isDeadConnectionError(error: unknown): boolean {
+  if (error instanceof UnauthorizedError) {
+    return true
+  }
   if (error instanceof StreamableHTTPError) {
-    return error.code === 404 || error.code === 400
+    return error.code === 404 || error.code === 400 || error.code === 401
   }
   if (error instanceof McpError && error.code === ErrorCode.ConnectionClosed) {
     return true

From b86cfa321840b33ac4317a615701136c5a871efa Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 15:49:44 -0700
Subject: [PATCH 05/13] improvement(mcp): defer bulk-discovery env resolution
 to the pool miss path

---
 apps/sim/lib/mcp/service.ts | 59 +++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index 8580dacb206..1ecc2017452 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -57,12 +57,7 @@ const FAILURE_CACHE_SENTINEL: McpTool[] = []
 
 type DiscoveryOutcome =
   | { kind: 'cached'; tools: McpTool[] }
-  | {
-      kind: 'fetched'
-      tools: McpTool[]
-      resolvedConfig: McpServerConfig
-      resolvedIP: string | null
-    }
+  | { kind: 'fetched'; tools: McpTool[] }
   | { kind: 'oauth-pending' }
   | { kind: 'unhealthy' }
   // originalError preserves the type so markServerUnhealthy's instanceof
@@ -654,24 +649,27 @@ class McpService {
           }
 
           try {
-            const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-              config,
-              userId,
-              workspaceId
-            )
+            const create = async () => {
+              const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
+                config,
+                userId,
+                workspaceId
+              )
+              return this.createClient(resolvedConfig, resolvedIP, userId)
+            }
             const tools = await this.withServerClient(
               {
                 key: this.poolKey(config.id, workspaceId, userId),
                 serverId: config.id,
                 allowPool: true,
               },
-              () => this.createClient(resolvedConfig, resolvedIP, userId),
+              create,
               (client) => client.listTools()
             )
             logger.debug(
               `[${requestId}] Discovered ${tools.length} tools from server ${config.name}`
             )
-            return { kind: 'fetched', tools, resolvedConfig, resolvedIP }
+            return { kind: 'fetched', tools }
           } catch (error) {
             if (isOauthAuthorizationError(error, config.authType)) {
               return { kind: 'oauth-pending' }
@@ -688,10 +686,7 @@ class McpService {
       const allTools: McpTool[] = []
       const cacheWrites: Promise<unknown>[] = []
       const deferredSideEffects: Promise<unknown>[] = []
-      const liveConnections: Array<{
-        resolvedConfig: McpServerConfig
-        resolvedIP: string | null
-      }> = []
+      const liveConnections: McpServerConfig[] = []
       let cachedCount = 0
       let fetchedCount = 0
       let failedCount = 0
@@ -720,10 +715,7 @@ class McpService {
               )
           )
           deferredSideEffects.push(this.clearServerFailure(workspaceId, server.id))
-          liveConnections.push({
-            resolvedConfig: outcome.resolvedConfig,
-            resolvedIP: outcome.resolvedIP,
-          })
+          liveConnections.push(server)
           return
         }
         if (outcome.kind === 'oauth-pending') {
@@ -776,15 +768,24 @@ class McpService {
       for (const p of deferredSideEffects) p.catch(() => {})
 
       if (mcpConnectionManager) {
-        for (const conn of liveConnections) {
-          mcpConnectionManager
-            .connect(conn.resolvedConfig, userId, workspaceId, conn.resolvedIP)
-            .catch((err) => {
-              logger.warn(
-                `[${requestId}] Persistent connection failed for ${conn.resolvedConfig.name}:`,
-                err
+        const manager = mcpConnectionManager
+        for (const config of liveConnections) {
+          // Resolve only for servers the manager isn't already monitoring — a
+          // pooled `listTools` hit above no longer resolves, so this is the sole
+          // remaining resolution cost, and it's skipped in the steady state.
+          if (manager.hasConnection(config.id)) continue
+          void (async () => {
+            try {
+              const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
+                config,
+                userId,
+                workspaceId
               )
-            })
+              await manager.connect(resolvedConfig, userId, workspaceId, resolvedIP)
+            } catch (err) {
+              logger.warn(`[${requestId}] Persistent connection failed for ${config.name}:`, err)
+            }
+          })()
         }
       }
 

From 0362f9f20a0016010e154d623f9caec06c7a5b06 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 15:59:59 -0700
Subject: [PATCH 06/13] fix(mcp): retire in-flight creates evicted mid-connect
 via server generation

---
 apps/sim/lib/mcp/connection-pool.test.ts | 25 ++++++++++++++++++++++++
 apps/sim/lib/mcp/connection-pool.ts      | 15 ++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/apps/sim/lib/mcp/connection-pool.test.ts b/apps/sim/lib/mcp/connection-pool.test.ts
index 97a83c71230..20d3697e23c 100644
--- a/apps/sim/lib/mcp/connection-pool.test.ts
+++ b/apps/sim/lib/mcp/connection-pool.test.ts
@@ -252,6 +252,31 @@ describe('McpConnectionPool', () => {
     expect(create).toHaveBeenCalledTimes(2)
   })
 
+  it('does not pool a connection whose create finished after the server was evicted', async () => {
+    let resolveCreate: ((client: McpClient) => void) | undefined
+    const created = new Promise<McpClient>((resolve) => {
+      resolveCreate = resolve
+    })
+    const staleClient = makeFakeClient()
+    const freshClient = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockImplementationOnce(() => created)
+      .mockImplementation(async () => freshClient)
+
+    const acquireP = pool.acquire(params('s1:w1:u1', create))
+    // Server config changes while the connect is still in flight.
+    await pool.evictServer('s1', 'config changed')
+    resolveCreate?.(staleClient)
+    const lease = await acquireP
+
+    // The stale-config connection is disconnected, not pooled; a fresh one is built.
+    expect(staleClient.disconnect).toHaveBeenCalledTimes(1)
+    expect(lease.client).toBe(freshClient)
+    expect(create).toHaveBeenCalledTimes(2)
+    await lease.release()
+  })
+
   it('bypasses the pool once disposed (connects without caching)', async () => {
     const client = makeFakeClient()
     const create = vi.fn(async () => client)
diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index 02ca0e7b270..e88cb140e38 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -66,6 +66,8 @@ interface PoolEntry {
 export class McpConnectionPool {
   private entries = new Map<string, PoolEntry>()
   private pending = new Map<string, Promise<PoolEntry>>()
+  /** Per-server counter bumped by `evictServer`; an in-flight create built against an older value is retired on completion. */
+  private serverGenerations = new Map<string, number>()
   private idleCheckTimer: ReturnType<typeof setInterval> | null = null
   private disposed = false
 
@@ -94,7 +96,10 @@ export class McpConnectionPool {
       const reusable = await this.tryReuse(current)
       if (reusable) return reusable
     }
-    return this.createEntry(params)
+    const created = await this.createEntry(params)
+    // Evicted (config edit/delete) mid-create → don't borrow the stale connection.
+    if (created.retired && !this.disposed) return this.resolveEntry(params)
+    return created
   }
 
   /** Return `entry` if in-age, connected, and live; else retire it and return null. */
@@ -132,6 +137,7 @@ export class McpConnectionPool {
     if (inFlight) return inFlight
 
     const creation = (async () => {
+      const generation = this.serverGenerations.get(params.serverId) ?? 0
       const client = await params.create()
       const now = Date.now()
       const entry: PoolEntry = {
@@ -145,7 +151,9 @@ export class McpConnectionPool {
         retired: false,
         closing: false,
       }
-      if (this.disposed) {
+      // Disposed, or the server was evicted (config edit/delete) while connecting:
+      // don't pool a connection built against the old config.
+      if (this.disposed || (this.serverGenerations.get(params.serverId) ?? 0) !== generation) {
         entry.retired = true
         void client.disconnect().catch(() => {})
         return entry
@@ -197,6 +205,9 @@ export class McpConnectionPool {
 
   /** Retire every connection for a server (all users) — config changed or deleted. */
   async evictServer(serverId: string, reason: string): Promise<void> {
+    // Bump first so an in-flight create for this server retires on completion
+    // instead of pooling a connection built against the now-stale config.
+    this.serverGenerations.set(serverId, (this.serverGenerations.get(serverId) ?? 0) + 1)
     for (const entry of this.entries.values()) {
       if (entry.serverId === serverId) {
         logger.info(`Evicting pooled MCP connection ${entry.key}: ${reason}`)

From de3527fe1de97b264217bdb3ecee34733b3aead4 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 16:16:20 -0700
Subject: [PATCH 07/13] fix(mcp): use evicted-mid-create connections one-shot
 and decouple pool eviction from cache clear

---
 apps/sim/lib/mcp/connection-pool.test.ts         | 15 +++++++++++----
 apps/sim/lib/mcp/connection-pool.ts              | 16 +++++-----------
 .../mcp/orchestration/server-lifecycle.test.ts   | 14 +++++++-------
 .../lib/mcp/orchestration/server-lifecycle.ts    | 11 ++++++-----
 apps/sim/lib/mcp/service.ts                      | 11 ++++++++++-
 5 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/apps/sim/lib/mcp/connection-pool.test.ts b/apps/sim/lib/mcp/connection-pool.test.ts
index 20d3697e23c..2283d7f2e18 100644
--- a/apps/sim/lib/mcp/connection-pool.test.ts
+++ b/apps/sim/lib/mcp/connection-pool.test.ts
@@ -252,7 +252,7 @@ describe('McpConnectionPool', () => {
     expect(create).toHaveBeenCalledTimes(2)
   })
 
-  it('does not pool a connection whose create finished after the server was evicted', async () => {
+  it('uses an evicted-mid-create connection one-shot and does not pool it', async () => {
     let resolveCreate: ((client: McpClient) => void) | undefined
     const created = new Promise<McpClient>((resolve) => {
       resolveCreate = resolve
@@ -270,11 +270,18 @@ describe('McpConnectionPool', () => {
     resolveCreate?.(staleClient)
     const lease = await acquireP
 
-    // The stale-config connection is disconnected, not pooled; a fresh one is built.
+    // The in-flight request still uses the connection it built (as connect-per-op
+    // would), but it isn't pooled and is disconnected on release.
+    expect(lease.client).toBe(staleClient)
+    expect(staleClient.disconnect).not.toHaveBeenCalled()
+    await lease.release()
     expect(staleClient.disconnect).toHaveBeenCalledTimes(1)
-    expect(lease.client).toBe(freshClient)
+
+    // The next acquire builds fresh — the stale connection was never pooled.
+    const next = await pool.acquire(params('s1:w1:u1', create))
+    expect(next.client).toBe(freshClient)
     expect(create).toHaveBeenCalledTimes(2)
-    await lease.release()
+    await next.release()
   })
 
   it('bypasses the pool once disposed (connects without caching)', async () => {
diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index e88cb140e38..a0631678e24 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -85,21 +85,14 @@ export class McpConnectionPool {
 
   private async resolveEntry(params: AcquireParams): Promise<PoolEntry> {
     const pending = this.pending.get(params.key)
-    if (pending) {
-      const entry = await pending
-      if (!entry.retired) return entry
-      return this.resolveEntry(params)
-    }
+    if (pending) return pending
 
     const current = this.entries.get(params.key)
     if (current) {
       const reusable = await this.tryReuse(current)
       if (reusable) return reusable
     }
-    const created = await this.createEntry(params)
-    // Evicted (config edit/delete) mid-create → don't borrow the stale connection.
-    if (created.retired && !this.disposed) return this.resolveEntry(params)
-    return created
+    return this.createEntry(params)
   }
 
   /** Return `entry` if in-age, connected, and live; else retire it and return null. */
@@ -152,10 +145,11 @@ export class McpConnectionPool {
         closing: false,
       }
       // Disposed, or the server was evicted (config edit/delete) while connecting:
-      // don't pool a connection built against the old config.
+      // don't pool a connection built against the now-stale config. The current
+      // borrower still uses it once (as connect-per-op would for an in-flight
+      // request); it's disconnected on release, and the next acquire builds fresh.
       if (this.disposed || (this.serverGenerations.get(params.serverId) ?? 0) !== generation) {
         entry.retired = true
-        void client.disconnect().catch(() => {})
         return entry
       }
       this.evictLruIfFull()
diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts
index ed9db50bed8..a1861c1ad63 100644
--- a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts
+++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts
@@ -14,12 +14,12 @@ import {
 } from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockClearCache, mockOauthCredsChanged, mockRevokeOauthTokens, mockEvictServer } =
+const { mockClearCache, mockOauthCredsChanged, mockRevokeOauthTokens, mockEvictServerConnections } =
   vi.hoisted(() => ({
     mockClearCache: vi.fn(),
     mockOauthCredsChanged: vi.fn(),
     mockRevokeOauthTokens: vi.fn(),
-    mockEvictServer: vi.fn(),
+    mockEvictServerConnections: vi.fn(),
   }))
 
 vi.mock('@sim/audit', () => auditMock)
@@ -47,10 +47,10 @@ vi.mock('@/lib/mcp/oauth', () => ({
   revokeMcpOauthTokens: mockRevokeOauthTokens,
 }))
 vi.mock('@/lib/mcp/service', () => ({
-  mcpService: { clearCache: mockClearCache },
-}))
-vi.mock('@/lib/mcp/connection-pool', () => ({
-  mcpConnectionPool: { evictServer: mockEvictServer },
+  mcpService: {
+    clearCache: mockClearCache,
+    evictServerConnections: mockEvictServerConnections,
+  },
 }))
 vi.mock('@/lib/mcp/utils', () => ({ generateMcpServerId: vi.fn() }))
 vi.mock('@/lib/posthog/server', () => posthogServerMock)
@@ -161,6 +161,6 @@ describe('MCP server lifecycle orchestration', () => {
     })
 
     expect(result.success).toBe(true)
-    expect(mockEvictServer).toHaveBeenCalledWith('server-1', expect.any(String))
+    expect(mockEvictServerConnections).toHaveBeenCalledWith('server-1', expect.any(String))
   })
 })
diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts
index 497137762de..75c3071ee9e 100644
--- a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts
+++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts
@@ -6,7 +6,6 @@ import { generateId } from '@sim/utils/id'
 import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { encryptSecret } from '@/lib/core/security/encryption'
-import { mcpConnectionPool } from '@/lib/mcp/connection-pool'
 import {
   McpDnsResolutionError,
   McpDomainNotAllowedError,
@@ -208,6 +207,7 @@ export async function performCreateMcpServer(
       })
 
       await mcpService.clearCache(params.workspaceId)
+      await mcpService.evictServerConnections(serverId, 'config changed')
       return { success: true, serverId, updated: true, authType: resolvedAuthType }
     }
 
@@ -397,7 +397,10 @@ export async function performUpdateMcpServer(
       params.timeout !== undefined ||
       params.retries !== undefined
 
-    if (shouldClearCache) await mcpService.clearCache(params.workspaceId)
+    if (shouldClearCache) {
+      await mcpService.clearCache(params.workspaceId)
+      await mcpService.evictServerConnections(params.serverId, 'config changed')
+    }
 
     recordAudit({
       workspaceId: params.workspaceId,
@@ -439,10 +442,8 @@ export async function performDeleteMcpServer(
 
     if (!server) return { success: false, error: 'Server not found', errorCode: 'not_found' }
 
-    // The row is already gone, so clearCache's row-enumeration can't reach it —
-    // evict the deleted server's pooled connections explicitly.
-    await mcpConnectionPool?.evictServer(params.serverId, 'server deleted')
     await mcpService.clearCache(params.workspaceId)
+    await mcpService.evictServerConnections(params.serverId, 'server deleted')
     const source =
       params.source === 'settings' || params.source === 'tool_input' ? params.source : undefined
 
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index 1ecc2017452..f575fc90f1f 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -1008,6 +1008,11 @@ class McpService {
     }
   }
 
+  /**
+   * Invalidate the MCP tool cache. This does NOT evict pooled connections —
+   * pool eviction is tied to config changes (see `evictServerConnections`), so a
+   * refresh or a single-server edit doesn't tear down unrelated warm connections.
+   */
   async clearCache(workspaceId?: string): Promise<void> {
     try {
       if (workspaceId) {
@@ -1022,7 +1027,6 @@ class McpService {
           rows.flatMap((r) => [
             this.cacheAdapter.delete(serverCacheKey(workspaceId, r.id)),
             this.cacheAdapter.delete(failureCacheKey(workspaceId, r.id)),
-            mcpConnectionPool?.evictServer(r.id, 'cache cleared'),
           ])
         )
         logger.debug(`Cleared MCP tool cache for workspace ${workspaceId} (${rows.length} servers)`)
@@ -1034,6 +1038,11 @@ class McpService {
       logger.warn('Failed to clear cache:', error)
     }
   }
+
+  /** Evict a single server's warm pooled connections (all users) — call on config change/delete. */
+  async evictServerConnections(serverId: string, reason: string): Promise<void> {
+    await mcpConnectionPool?.evictServer(serverId, reason)
+  }
 }
 
 export const mcpService = new McpService()

From fc2ee4c1ef51388b87f2d4e08347b472369f54b6 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 16:25:39 -0700
Subject: [PATCH 08/13] improvement(mcp): dedupe create thunks into
 buildClient, harden dispose against liveness race

---
 apps/sim/lib/mcp/connection-pool.ts | 14 +++---
 apps/sim/lib/mcp/service.ts         | 68 ++++++++++++-----------------
 2 files changed, 37 insertions(+), 45 deletions(-)

diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index a0631678e24..0bee05183b2 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -17,9 +17,10 @@
  * disconnect the client themselves.
  *
  * Config-change invalidation is the caller's job via `evictServer` (wired to the
- * service's `clearCache`), not a per-acquire timestamp check — `updatedAt` is also
- * bumped by status telemetry, so it can't distinguish a config edit. Cross-process,
- * a config edit is bounded by max age (and self-heals when a stale connection errors).
+ * service's `evictServerConnections`), not a per-acquire timestamp check — `updatedAt`
+ * is also bumped by status telemetry, so it can't distinguish a config edit.
+ * Cross-process, a config edit is bounded by max age (self-heals when a stale
+ * connection errors).
  */
 
 import { createLogger } from '@sim/logger'
@@ -243,10 +244,13 @@ export class McpConnectionPool {
       clearInterval(this.idleCheckTimer)
       this.idleCheckTimer = null
     }
-    const clients = [...this.entries.values()].map((e) => e.client)
+    const entries = [...this.entries.values()]
+    // Mark retired before disconnecting so an in-flight liveness ping observes it
+    // and can't hand out a torn-down client (the retired-before-disconnect invariant).
+    for (const entry of entries) entry.retired = true
     this.entries.clear()
     this.pending.clear()
-    void Promise.allSettled(clients.map((client) => client.disconnect()))
+    void Promise.allSettled(entries.map((entry) => entry.client.disconnect()))
   }
 }
 
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index f575fc90f1f..cd264e108e5 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -336,6 +336,30 @@ class McpService {
     return `${serverId}:${workspaceId ?? ''}:${userId ?? ''}`
   }
 
+  /**
+   * A `create` thunk for {@link withServerClient} that resolves env vars + SSRF-pins
+   * and connects. Deferred so a pool hit skips this work; `extraHeaders` (per-request)
+   * are merged in and force the caller to bypass the pool.
+   */
+  private buildClient(
+    config: McpServerConfig,
+    userId: string,
+    workspaceId: string,
+    extraHeaders?: Record<string, string>
+  ): () => Promise<McpClient> {
+    return async () => {
+      const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
+        config,
+        userId,
+        workspaceId
+      )
+      if (extraHeaders) {
+        resolvedConfig.headers = { ...resolvedConfig.headers, ...extraHeaders }
+      }
+      return this.createClient(resolvedConfig, resolvedIP, userId)
+    }
+  }
+
   /**
    * Run `fn` against a connected client. When `allowPool`, borrow from the warm
    * pool (`create` runs only on a miss, so a hit skips env resolution + DNS); a
@@ -399,25 +423,13 @@ class McpService {
         }
 
         const hasExtraHeaders = Boolean(extraHeaders && Object.keys(extraHeaders).length > 0)
-        const create = async () => {
-          const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-            config,
-            userId,
-            workspaceId
-          )
-          if (hasExtraHeaders) {
-            resolvedConfig.headers = { ...resolvedConfig.headers, ...extraHeaders }
-          }
-          return this.createClient(resolvedConfig, resolvedIP, userId)
-        }
-
         const result = await this.withServerClient(
           {
             key: this.poolKey(serverId, workspaceId, userId),
             serverId,
             allowPool: !hasExtraHeaders,
           },
-          create,
+          this.buildClient(config, userId, workspaceId, hasExtraHeaders ? extraHeaders : undefined),
           (client) => client.callTool(toolCall)
         )
         logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
@@ -649,21 +661,13 @@ class McpService {
           }
 
           try {
-            const create = async () => {
-              const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-                config,
-                userId,
-                workspaceId
-              )
-              return this.createClient(resolvedConfig, resolvedIP, userId)
-            }
             const tools = await this.withServerClient(
               {
                 key: this.poolKey(config.id, workspaceId, userId),
                 serverId: config.id,
                 allowPool: true,
               },
-              create,
+              this.buildClient(config, userId, workspaceId),
               (client) => client.listTools()
             )
             logger.debug(
@@ -870,21 +874,13 @@ class McpService {
         }
         authType = config.authType
 
-        const create = async () => {
-          const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-            config,
-            userId,
-            workspaceId
-          )
-          return this.createClient(resolvedConfig, resolvedIP, userId)
-        }
         const tools = await this.withServerClient(
           {
             key: this.poolKey(serverId, workspaceId, userId),
             serverId,
             allowPool: true,
           },
-          create,
+          this.buildClient(config, userId, workspaceId),
           (client) => client.listTools()
         )
         logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
@@ -946,21 +942,13 @@ class McpService {
 
       for (const config of servers) {
         try {
-          const create = async () => {
-            const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
-              config,
-              userId,
-              workspaceId
-            )
-            return this.createClient(resolvedConfig, resolvedIP, userId)
-          }
           const tools = await this.withServerClient(
             {
               key: this.poolKey(config.id, workspaceId, userId),
               serverId: config.id,
               allowPool: true,
             },
-            create,
+            this.buildClient(config, userId, workspaceId),
             (client) => client.listTools()
           )
 

From 0edd043d97dd00289d38f6a4add0bb8a277f02d4 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 16:41:00 -0700
Subject: [PATCH 09/13] fix(mcp): close acquire-gap races (re-resolve after
 stale ping, skip closing entries)

---
 apps/sim/lib/mcp/connection-pool.test.ts | 38 ++++++++++++++++++++++++
 apps/sim/lib/mcp/connection-pool.ts      |  8 +++++
 2 files changed, 46 insertions(+)

diff --git a/apps/sim/lib/mcp/connection-pool.test.ts b/apps/sim/lib/mcp/connection-pool.test.ts
index 2283d7f2e18..51be9f6b7a2 100644
--- a/apps/sim/lib/mcp/connection-pool.test.ts
+++ b/apps/sim/lib/mcp/connection-pool.test.ts
@@ -284,6 +284,44 @@ describe('McpConnectionPool', () => {
     await next.release()
   })
 
+  it('reuses a concurrently-pooled replacement rather than orphaning it (recreate race)', async () => {
+    // stale's ping is deferred per-call so we can fully resolve one acquire before
+    // the other's ping settles — the exact interleaving that used to leak.
+    const releasePing: Array<(alive: boolean) => void> = []
+    const stale = makeFakeClient()
+    ;(stale.ping as ReturnType<typeof vi.fn>).mockImplementation(
+      () =>
+        new Promise((resolve, reject) => {
+          releasePing.push((alive) => (alive ? resolve({}) : reject(new Error('dead'))))
+        })
+    )
+    const fresh = makeFakeClient()
+    const create = vi
+      .fn<() => Promise<McpClient>>()
+      .mockResolvedValueOnce(stale)
+      .mockResolvedValueOnce(fresh)
+
+    await borrow(pool, params('s1:w1:u1', create))
+    vi.setSystemTime(Date.now() + 61 * 1000)
+
+    const pA = pool.acquire(params('s1:w1:u1', create))
+    const pB = pool.acquire(params('s1:w1:u1', create))
+    while (releasePing.length < 2) await Promise.resolve()
+
+    // A's ping fails → A retires stale, rebuilds `fresh`, pools it, clears pending.
+    releasePing[0](false)
+    for (let i = 0; i < 20; i++) await Promise.resolve()
+    // B's ping fails → B must reuse the pooled `fresh`, not create a third client.
+    releasePing[1](false)
+
+    const [la, lb] = await Promise.all([pA, pB])
+    expect(create).toHaveBeenCalledTimes(2)
+    expect(la.client).toBe(fresh)
+    expect(lb.client).toBe(fresh)
+    await la.release()
+    await lb.release()
+  })
+
   it('bypasses the pool once disposed (connects without caching)', async () => {
     const client = makeFakeClient()
     const create = vi.fn(async () => client)
diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index 0bee05183b2..9690704fd43 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -79,6 +79,10 @@ export class McpConnectionPool {
       return { client, release: () => client.disconnect().catch(() => {}) }
     }
     const entry = await this.resolveEntry(params)
+    // A concurrent evict/release/idle could have retired + started disconnecting
+    // this entry during the resolve's `await` gap; `closing` (not `retired`, which
+    // a usable one-shot also sets) means the socket is going away — get a fresh one.
+    if (entry.closing) return this.acquire(params)
     entry.borrowers++
     entry.lastActivityAt = Date.now()
     return { client: entry.client, release: (poison) => this.release(entry, poison ?? false) }
@@ -92,6 +96,10 @@ export class McpConnectionPool {
     if (current) {
       const reusable = await this.tryReuse(current)
       if (reusable) return reusable
+      // tryReuse awaited a ping and retired `current`; a concurrent acquire may
+      // have pooled a replacement meanwhile, so re-resolve rather than blindly
+      // creating (which would overwrite and leak that replacement).
+      return this.resolveEntry(params)
     }
     return this.createEntry(params)
   }

From 3ac3d2d8d91711dfd0b178c2de33b8f36c3823d4 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 17:01:16 -0700
Subject: [PATCH 10/13] fix(mcp): retry auth failures in executeTool; simplify
 acquire re-check and clear generations on dispose

---
 apps/sim/lib/mcp/connection-pool.test.ts |  7 +++++-
 apps/sim/lib/mcp/connection-pool.ts      | 30 ++++++++++++------------
 apps/sim/lib/mcp/service-pool.test.ts    | 19 +++++++++++++++
 apps/sim/lib/mcp/service.ts              | 19 +++++++++++++--
 4 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/apps/sim/lib/mcp/connection-pool.test.ts b/apps/sim/lib/mcp/connection-pool.test.ts
index 51be9f6b7a2..4df677f13e0 100644
--- a/apps/sim/lib/mcp/connection-pool.test.ts
+++ b/apps/sim/lib/mcp/connection-pool.test.ts
@@ -54,6 +54,11 @@ function params(key: string, create: () => Promise<McpClient>): AcquireParams {
   return { key, serverId: key.split(':')[0], create }
 }
 
+/** Drain the microtask queue so an in-flight acquire fully settles before the next step. */
+async function flushMicrotasks(turns = 50): Promise<void> {
+  for (let i = 0; i < turns; i++) await Promise.resolve()
+}
+
 describe('McpConnectionPool', () => {
   let pool: McpConnectionPool
 
@@ -310,7 +315,7 @@ describe('McpConnectionPool', () => {
 
     // A's ping fails → A retires stale, rebuilds `fresh`, pools it, clears pending.
     releasePing[0](false)
-    for (let i = 0; i < 20; i++) await Promise.resolve()
+    await flushMicrotasks()
     // B's ping fails → B must reuse the pooled `fresh`, not create a third client.
     releasePing[1](false)
 
diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index 9690704fd43..ebd12cac8af 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -9,7 +9,7 @@
  * ping-cached; reconnect is demand-driven (callers' retries re-acquire).
  *
  * Connections are ref-counted. Eviction *retires* an entry — removes it from the
- * pool so no new borrower can take it — but the socket is closed only once the
+ * pool so later acquires stop reusing it — but the socket is closed only once the
  * last in-flight borrower releases, so a sibling failure, idle sweep, or config
  * change never tears down a connection mid-request. Retirement is identity-checked,
  * so a stale `onClose` can't disconnect a replacement stored under the same key.
@@ -78,30 +78,29 @@ export class McpConnectionPool {
       const client = await params.create()
       return { client, release: () => client.disconnect().catch(() => {}) }
     }
-    const entry = await this.resolveEntry(params)
-    // A concurrent evict/release/idle could have retired + started disconnecting
-    // this entry during the resolve's `await` gap; `closing` (not `retired`, which
-    // a usable one-shot also sets) means the socket is going away — get a fresh one.
-    if (entry.closing) return this.acquire(params)
+    // A concurrent evict/release/idle could have started disconnecting the resolved
+    // entry during an `await` gap; `closing` (not `retired`, which a usable one-shot
+    // also sets) means the socket is going away, so resolve again.
+    let entry = await this.resolveEntry(params)
+    while (entry.closing) entry = await this.resolveEntry(params)
     entry.borrowers++
     entry.lastActivityAt = Date.now()
     return { client: entry.client, release: (poison) => this.release(entry, poison ?? false) }
   }
 
   private async resolveEntry(params: AcquireParams): Promise<PoolEntry> {
-    const pending = this.pending.get(params.key)
-    if (pending) return pending
+    while (true) {
+      const pending = this.pending.get(params.key)
+      if (pending) return pending
+
+      const current = this.entries.get(params.key)
+      if (!current) return this.createEntry(params)
 
-    const current = this.entries.get(params.key)
-    if (current) {
       const reusable = await this.tryReuse(current)
       if (reusable) return reusable
-      // tryReuse awaited a ping and retired `current`; a concurrent acquire may
-      // have pooled a replacement meanwhile, so re-resolve rather than blindly
-      // creating (which would overwrite and leak that replacement).
-      return this.resolveEntry(params)
+      // tryReuse awaited a ping and retired `current`; loop to re-check pending/entries —
+      // a concurrent acquire may have pooled a replacement we must reuse, not overwrite.
     }
-    return this.createEntry(params)
   }
 
   /** Return `entry` if in-age, connected, and live; else retire it and return null. */
@@ -258,6 +257,7 @@ export class McpConnectionPool {
     for (const entry of entries) entry.retired = true
     this.entries.clear()
     this.pending.clear()
+    this.serverGenerations.clear()
     void Promise.allSettled(entries.map((entry) => entry.client.disconnect()))
   }
 }
diff --git a/apps/sim/lib/mcp/service-pool.test.ts b/apps/sim/lib/mcp/service-pool.test.ts
index 61464d56c09..bb614710d05 100644
--- a/apps/sim/lib/mcp/service-pool.test.ts
+++ b/apps/sim/lib/mcp/service-pool.test.ts
@@ -180,4 +180,23 @@ describe('McpService connection reuse wiring', () => {
 
     expect(mockRelease).toHaveBeenCalledWith(true)
   })
+
+  it('retries and recovers when a rotated credential causes a one-off auth failure', async () => {
+    mockCallTool
+      .mockRejectedValueOnce(new UnauthorizedError('stale key'))
+      .mockResolvedValueOnce({ content: [] })
+
+    const result = await mcpService.executeTool(
+      USER_ID,
+      'server-1',
+      { name: 'do', arguments: {} },
+      WORKSPACE_ID
+    )
+
+    expect(result).toEqual({ content: [] })
+    expect(mockCallTool).toHaveBeenCalledTimes(2)
+    // First attempt poisoned the stale lease; the retry re-acquired a fresh one.
+    expect(mockRelease).toHaveBeenCalledWith(true)
+    expect(mockAcquire).toHaveBeenCalledTimes(2)
+  })
 })
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index cd264e108e5..4c8dafa613e 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -125,6 +125,18 @@ function isDeadConnectionError(error: unknown): boolean {
   )
 }
 
+/**
+ * An auth failure (401) from a rotated/revoked credential. Safe for `executeTool`
+ * to retry — auth is rejected *before* the tool runs, so re-acquiring on a fresh
+ * connection (which re-resolves the credential) can't double-execute a tool.
+ */
+function isAuthError(error: unknown): boolean {
+  return (
+    error instanceof UnauthorizedError ||
+    (error instanceof StreamableHTTPError && error.code === 401)
+  )
+}
+
 /** Transient failures a read-only `tools/list` may safely retry (idempotent, unlike `tools/call`); excludes OAuth and terminal 4xx. */
 function isRetryableDiscoveryError(error: unknown): boolean {
   if (isTimeoutError(error)) return true
@@ -435,9 +447,12 @@ class McpService {
         logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
         return result
       } catch (error) {
-        if (this.isSessionError(error) && attempt < maxRetries - 1) {
+        // A stale session (400/404) or a rotated/revoked credential (401) is rejected
+        // before the tool runs, so retrying on a fresh connection is safe and recovers
+        // the request. Timeouts/resets are NOT retried — the tool may have executed.
+        if ((this.isSessionError(error) || isAuthError(error)) && attempt < maxRetries - 1) {
           logger.warn(
-            `[${requestId}] Session error executing tool ${toolCall.name}, retrying (attempt ${attempt + 1}):`,
+            `[${requestId}] Retryable connection error executing tool ${toolCall.name}, retrying (attempt ${attempt + 1}):`,
             error
           )
           await sleep(100)

From abd98481f7bf22e3732a9bf16a3963682120b659 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 17:12:28 -0700
Subject: [PATCH 11/13] fix(mcp): let bulk discovery reconnect a lost
 notification connection with current config

---
 apps/sim/lib/mcp/service.ts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index 4c8dafa613e..b55ce5bde83 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -789,10 +789,10 @@ class McpService {
       if (mcpConnectionManager) {
         const manager = mcpConnectionManager
         for (const config of liveConnections) {
-          // Resolve only for servers the manager isn't already monitoring — a
-          // pooled `listTools` hit above no longer resolves, so this is the sole
-          // remaining resolution cost, and it's skipped in the steady state.
-          if (manager.hasConnection(config.id)) continue
+          // Kick the notification manager for every fetched server; `connect` is
+          // idempotent (skips a live/connecting one) and reconnects a lost one with
+          // this current config — do not pre-gate on `hasConnection`, whose state
+          // survives a transport loss and would block that fresh reconnect.
           void (async () => {
             try {
               const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(

From 65bd1f3e038fa7b5e5561b1ae01b8a99319921bc Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 17:34:15 -0700
Subject: [PATCH 12/13] fix(mcp): recover rotated credentials on discovery via
 shared fetchServerTools auth-retry

---
 apps/sim/lib/mcp/service.test.ts | 17 +++++++--
 apps/sim/lib/mcp/service.ts      | 60 ++++++++++++++++++--------------
 2 files changed, 48 insertions(+), 29 deletions(-)

diff --git a/apps/sim/lib/mcp/service.test.ts b/apps/sim/lib/mcp/service.test.ts
index 7349b39e71f..1fc6e1e4b8f 100644
--- a/apps/sim/lib/mcp/service.test.ts
+++ b/apps/sim/lib/mcp/service.test.ts
@@ -341,7 +341,7 @@ describe('McpService.discoverTools per-server caching', () => {
         statusConfig: { consecutiveFailures: 0, lastSuccessfulDiscovery: null },
       }),
     ])
-    mockListTools.mockRejectedValueOnce(
+    mockListTools.mockRejectedValue(
       new UnauthorizedError(`Rejected Authorization: ${reflectedCredential}`)
     )
 
@@ -491,7 +491,7 @@ describe('McpService.discoverTools per-server caching', () => {
         statusConfig: { consecutiveFailures: 0, lastSuccessfulDiscovery: null },
       }),
     ])
-    mockListTools.mockRejectedValueOnce(
+    mockListTools.mockRejectedValue(
       new UnauthorizedError(`Rejected Authorization: ${reflectedCredential}`)
     )
 
@@ -517,6 +517,19 @@ describe('McpService.discoverTools per-server caching', () => {
     expect(mockListTools).not.toHaveBeenCalled()
   })
 
+  it('recovers a rotated headers-auth credential via a single discovery retry', async () => {
+    mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
+    // Stale key 401s once, then the retry re-resolves and succeeds.
+    mockListTools
+      .mockRejectedValueOnce(new UnauthorizedError('stale key'))
+      .mockResolvedValueOnce([tool('a1', 'mcp-a')])
+
+    const tools = await mcpService.discoverServerTools(USER_ID, 'mcp-a', WORKSPACE_ID)
+
+    expect(tools).toHaveLength(1)
+    expect(mockListTools).toHaveBeenCalledTimes(2)
+  })
+
   it('keeps per-server UnauthorizedError soft-pending for OAuth auth', async () => {
     mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A', { authType: 'oauth' })])
     mockResolveEnvVars.mockRejectedValue(new UnauthorizedError('OAuth token rejected'))
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index b55ce5bde83..dfa4d1b7e5a 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -372,6 +372,36 @@ class McpService {
     }
   }
 
+  /**
+   * Pooled `tools/list` for one server, with a single retry on a non-OAuth auth
+   * failure: a rotated header key throws 401, which retires the pooled connection,
+   * so the retry re-acquires a fresh one that re-resolves the credential. (OAuth
+   * 401s are left to the caller's oauth-pending handling.) `listTools` is
+   * idempotent, so the retry is always safe.
+   */
+  private async fetchServerTools(
+    config: McpServerConfig,
+    userId: string,
+    workspaceId: string
+  ): Promise<McpTool[]> {
+    for (let attempt = 0; ; attempt++) {
+      try {
+        return await this.withServerClient(
+          {
+            key: this.poolKey(config.id, workspaceId, userId),
+            serverId: config.id,
+            allowPool: true,
+          },
+          this.buildClient(config, userId, workspaceId),
+          (client) => client.listTools()
+        )
+      } catch (error) {
+        if (attempt === 0 && isAuthError(error) && config.authType !== 'oauth') continue
+        throw error
+      }
+    }
+  }
+
   /**
    * Run `fn` against a connected client. When `allowPool`, borrow from the warm
    * pool (`create` runs only on a miss, so a hit skips env resolution + DNS); a
@@ -676,15 +706,7 @@ class McpService {
           }
 
           try {
-            const tools = await this.withServerClient(
-              {
-                key: this.poolKey(config.id, workspaceId, userId),
-                serverId: config.id,
-                allowPool: true,
-              },
-              this.buildClient(config, userId, workspaceId),
-              (client) => client.listTools()
-            )
+            const tools = await this.fetchServerTools(config, userId, workspaceId)
             logger.debug(
               `[${requestId}] Discovered ${tools.length} tools from server ${config.name}`
             )
@@ -889,15 +911,7 @@ class McpService {
         }
         authType = config.authType
 
-        const tools = await this.withServerClient(
-          {
-            key: this.poolKey(serverId, workspaceId, userId),
-            serverId,
-            allowPool: true,
-          },
-          this.buildClient(config, userId, workspaceId),
-          (client) => client.listTools()
-        )
+        const tools = await this.fetchServerTools(config, userId, workspaceId)
         logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
         await Promise.allSettled([
           this.cacheAdapter
@@ -957,15 +971,7 @@ class McpService {
 
       for (const config of servers) {
         try {
-          const tools = await this.withServerClient(
-            {
-              key: this.poolKey(config.id, workspaceId, userId),
-              serverId: config.id,
-              allowPool: true,
-            },
-            this.buildClient(config, userId, workspaceId),
-            (client) => client.listTools()
-          )
+          const tools = await this.fetchServerTools(config, userId, workspaceId)
 
           summaries.push({
             id: config.id,

From d74d48dfaf119d273137c6bfa51bdc5e6c55f296 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Sat, 18 Jul 2026 18:50:48 -0700
Subject: [PATCH 13/13] chore(mcp): add debug logging for pool
 hit/miss/eviction observability

---
 apps/sim/lib/mcp/connection-pool.ts | 31 +++++++++++++++++------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/apps/sim/lib/mcp/connection-pool.ts b/apps/sim/lib/mcp/connection-pool.ts
index ebd12cac8af..e367fb7fd68 100644
--- a/apps/sim/lib/mcp/connection-pool.ts
+++ b/apps/sim/lib/mcp/connection-pool.ts
@@ -107,11 +107,11 @@ export class McpConnectionPool {
   private async tryReuse(entry: PoolEntry): Promise<PoolEntry | null> {
     const now = Date.now()
     if (now - entry.createdAt > MAX_CONNECTION_AGE_MS) {
-      this.retire(entry)
+      this.retire(entry, 'max age reached')
       return null
     }
     if (!entry.client.getStatus().connected) {
-      this.retire(entry)
+      this.retire(entry, 'not connected')
       return null
     }
     if (now - entry.lastLivenessCheckAt > LIVENESS_TTL_MS) {
@@ -123,11 +123,12 @@ export class McpConnectionPool {
       // ping await; don't hand out a connection that's already closing.
       if (entry.retired) return null
       if (!alive) {
-        this.retire(entry)
+        this.retire(entry, 'liveness ping failed')
         return null
       }
       entry.lastLivenessCheckAt = now
     }
+    logger.debug(`Reusing pooled MCP connection ${entry.key}`)
     return entry
   }
 
@@ -164,6 +165,9 @@ export class McpConnectionPool {
       this.entries.set(params.key, entry)
       client.onClose(this.makeCloseHandler(entry))
       this.ensureIdleCheck()
+      logger.debug(
+        `Established and pooled MCP connection ${params.key} (${this.entries.size}/${MAX_POOL_SIZE})`
+      )
       return entry
     })()
 
@@ -176,22 +180,26 @@ export class McpConnectionPool {
   private async release(entry: PoolEntry, poison: boolean): Promise<void> {
     entry.borrowers = Math.max(0, entry.borrowers - 1)
     entry.lastActivityAt = Date.now()
-    if (poison) this.retire(entry)
+    if (poison) this.retire(entry, 'poisoned by failed operation')
     await this.closeIfIdle(entry)
   }
 
   /** Own scope so the handler captures only `entry` (never the create params / secrets). */
   private makeCloseHandler(entry: PoolEntry): () => void {
     return () => {
-      if (this.entries.get(entry.key) === entry) this.retire(entry)
+      if (this.entries.get(entry.key) === entry) this.retire(entry, 'transport closed')
     }
   }
 
   /** Remove `entry` from the pool so no new borrower takes it; close it once idle. */
-  private retire(entry: PoolEntry): void {
+  private retire(entry: PoolEntry, reason: string): void {
     if (!entry.retired) {
       entry.retired = true
       if (this.entries.get(entry.key) === entry) this.entries.delete(entry.key)
+      logger.debug(`Retiring pooled MCP connection ${entry.key}: ${reason}`, {
+        borrowers: entry.borrowers,
+        poolSize: this.entries.size,
+      })
     }
     void this.closeIfIdle(entry)
   }
@@ -199,7 +207,7 @@ export class McpConnectionPool {
   private async closeIfIdle(entry: PoolEntry): Promise<void> {
     if (!entry.retired || entry.borrowers > 0 || entry.closing) return
     entry.closing = true
-    logger.info(`Closing pooled MCP connection ${entry.key}`)
+    logger.debug(`Closing pooled MCP connection ${entry.key}`)
     await entry.client.disconnect().catch((error) => {
       logger.warn(`Error disconnecting pooled MCP connection ${entry.key}:`, error)
     })
@@ -211,10 +219,7 @@ export class McpConnectionPool {
     // instead of pooling a connection built against the now-stale config.
     this.serverGenerations.set(serverId, (this.serverGenerations.get(serverId) ?? 0) + 1)
     for (const entry of this.entries.values()) {
-      if (entry.serverId === serverId) {
-        logger.info(`Evicting pooled MCP connection ${entry.key}: ${reason}`)
-        this.retire(entry)
-      }
+      if (entry.serverId === serverId) this.retire(entry, reason)
     }
   }
 
@@ -225,7 +230,7 @@ export class McpConnectionPool {
       if (!lru || entry.lastActivityAt < lru.lastActivityAt) lru = entry
     }
     // Retiring a still-borrowed LRU frees the map slot now; its socket closes on release.
-    if (lru) this.retire(lru)
+    if (lru) this.retire(lru, 'pool at capacity (LRU)')
   }
 
   private ensureIdleCheck(): void {
@@ -234,7 +239,7 @@ export class McpConnectionPool {
       const now = Date.now()
       for (const entry of this.entries.values()) {
         if (entry.borrowers === 0 && now - entry.lastActivityAt > IDLE_TIMEOUT_MS)
-          this.retire(entry)
+          this.retire(entry, 'idle timeout')
       }
       if (this.entries.size === 0 && this.idleCheckTimer) {
         clearInterval(this.idleCheckTimer)