From af2f3436f78701ec5ab3eeb301b96ed56ca23052 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Tue, 21 Jul 2026 14:03:54 -0700
Subject: [PATCH] fix(mcp): keep the pooled connection warm on a request
 timeout
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Empirically: our client's first connect to these servers consistently hangs to
our 30s timeout, then a retry connects in ~100ms (the server answers initialize
in ~120ms from a normal client — proven via A/B). Treating that request timeout
as a dead connection (isDeadConnectionError) retired the pooled connection, so
the next discovery reconnected and paid the stall again — a connect/stall/
reconnect churn loop, and the cause of the intermittent 'Failed to discover' /
disconnected states.

A streamable-HTTP request timeout aborts only that request's own POST stream;
the session stays healthy for the next request. So stop poisoning the lease on a
timeout (matches the MCP SDK, OpenCode, and LibreChat, none of which retire a
connection on a request timeout). Only a real transport close/404/reset retires.
---
 apps/sim/lib/mcp/service-pool.test.ts | 13 +++++++++++++
 apps/sim/lib/mcp/service.ts           | 16 ++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/apps/sim/lib/mcp/service-pool.test.ts b/apps/sim/lib/mcp/service-pool.test.ts
index bb614710d05..f2de2cab54e 100644
--- a/apps/sim/lib/mcp/service-pool.test.ts
+++ b/apps/sim/lib/mcp/service-pool.test.ts
@@ -171,6 +171,19 @@ describe('McpService connection reuse wiring', () => {
     expect(mockRelease).toHaveBeenCalledWith(false)
   })
 
+  it('keeps the pooled connection warm on a request timeout (does not retire the session)', async () => {
+    // A streamable-HTTP request timeout aborts only that request's stream; the session stays
+    // healthy for the next request, so a timeout must NOT poison the lease (matches every
+    // production MCP client and avoids a connect/stall/reconnect churn loop).
+    mockCallTool.mockRejectedValue(new Error('Request timed out'))
+
+    await expect(
+      mcpService.executeTool(USER_ID, 'server-1', { name: 'do', arguments: {} }, WORKSPACE_ID)
+    ).rejects.toThrow()
+
+    expect(mockRelease).not.toHaveBeenCalledWith(true)
+  })
+
   it('poisons the lease on an auth failure so a rotated credential is re-resolved', async () => {
     mockCallTool.mockRejectedValue(new UnauthorizedError('token rejected'))
 
diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts
index e95d3269437..ac595424e8e 100644
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -99,9 +99,16 @@ 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), 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.
+ * credential; the rebuild re-resolves it), a closed transport, or a reset socket.
+ *
+ * A request timeout is deliberately NOT dead: streamable-HTTP aborts only that
+ * request's own POST stream, leaving the session healthy for the next request, so
+ * every production MCP client (SDK, OpenCode, LibreChat) rejects the request and
+ * keeps the connection rather than tearing it down. Retiring on a timeout instead
+ * forced a full reconnect on the next discovery — and a fresh connect can stall on
+ * our end far longer than a warm request — turning one slow response into a
+ * connect/stall/reconnect churn loop. Benign tool/consent errors and healthy
+ * upstream responses (429/5xx) also keep the connection warm.
  */
 function isDeadConnectionError(error: unknown): boolean {
   if (error instanceof UnauthorizedError) {
@@ -113,9 +120,6 @@ function isDeadConnectionError(error: unknown): boolean {
   if (error instanceof McpError && error.code === ErrorCode.ConnectionClosed) {
     return true
   }
-  if (isTimeoutError(error)) {
-    return true
-  }
   const message = getErrorMessage(error, '').toLowerCase()
   return (
     message.includes('econnreset') ||