fix(mcp): recover failed pooled connections without replaying calls

docs/design/2026-09-08-pooled-mcp-recovery.md

@@ -0,0 +1,40 @@
+# Recovering lost MCP connections in active ACP sessions
+
+Issue: https://github.com/QwenLM/qwen-code/issues/11272
+
+## Reproduction and ownership
+
+Baseline `bb79319ce5e0e2bbe64307c50dc3621621afc25c` was exercised through a real loopback `qwen serve` daemon and ACP WebSocket sessions, with a deterministic model endpoint and a counted stdio MCP fixture.
+
+Ordinary cancellation delivered `notifications/cancelled`, left the child alive, and allowed both sessions to continue. A fixture that deliberately exited on cancellation exposed a stale CONNECTED status; a fixture that emitted an invalid JSON-RPC frame triggered `silent_drop`, removed session tool registrations, and left later turns unable to call tools. These are controlled fault injections, not proof of the original reporter's teardown mechanism.
+
+There are three lifetimes:
+
+- A tool invocation owns arguments, an abort signal, and one permission decision. It may already have applied a side effect when its response disappears.
+- The pool owns the transport and stdio process, shared by connection fingerprint. Its cleanup owns process and descendant teardown.
+- Each live session owns a pool reference and filtered tool/prompt/resource registrations. Removing an entry does not close the session, but previously nothing restored these registrations before its next model send.
+
+The SDK's clean-close callback was not reflected in McpClient status. Separately, the invocation reconnect path could use the shared snapshot's bootstrap Config to spawn a private client while the real sessions retained stale handles.
+
+## Design
+
+1. Reflect unexpected SDK close in the existing disconnected status path. An intentional disconnect remains guarded by `isDisconnecting`.
+2. Record a recovery candidate only when a held connection emits `failed`. Retain its transport fingerprint, not a tool invocation. Explicit disconnect/stop removes that candidate; initial discovery failure does not create one.
+3. Before an ACP model send, recover eligible candidates through the pool and refresh declarations. Preserve the normal folder trust, disabled-server, approval, configuration fingerprint, transport authentication and per-tool permission checks. Recheck eligibility after acquisition so a late result cannot override revocation or shutdown.
+4. Serialize a session's recovery with its discovery work. A configuration refresh queued behind recovery runs a full reconciliation afterward; stop invalidates queued refreshes. Reapply current trust and tool filters before attaching a recovered handle, since these fields are excluded from transport identity. Use the pool's existing spawn-in-flight coordination across sessions, wait for the previous entry's cleanup, and impose a shared five-second cooldown after acquisition failure. Each demand makes at most one attempt; there are no background reconnect timers or unlimited per-call retries. Explicit management discovery is not throttled by this cooldown.
+5. Pool-projected tools delegate recovery to their session. They do not invoke the standalone reconnect/replay path, regardless of annotations. Copying a tool preserves this ownership.
+6. Expose recovery success/failure as an ACP diagnostic message. If a turn is cancelled while waiting for a shared connection, stop waiting without cancelling that connection or executing a tool; retain the notice for the next send.
+
+## Existing management interfaces and scope
+
+PR #7309 remains open at `b5f111c02300e39c2c1563af7abbf229e8455341` at investigation time. Its workspace runtime management, authentication, approval and restart routes overlap the management surface, not this demand recovery implementation. Current main already owns a workspace MCP pool and the restart route. This change adds no route, chat command, management platform, authentication bypass, or generic retry framework. PR #11145 concerns initial workspace discovery after preheat; recovery here is limited to previously acquired connections.
+
+## Validation contract
+
+Use counted tools and PID logs, not model prose or `end_turn`, to judge success. Cover ordinary cancellation, cancellation plus controlled exit/error, independent child exit, simultaneous sessions, concurrent acquisition and failure cooldown, explicit disable/disconnect, late shutdown/revocation, discovery timeout and permission denial. Keep the cancelled invocation count at one. Distinguish local macOS daemon evidence from untested production DingTalk infrastructure, Windows/Linux process semantics and real OAuth providers.
+
+## Review follow-up
+
+Session projection deliberately clones a discovered tool to disable standalone reconnect; it is not a zero-allocation path. Tests use an unprojected discovery snapshot and verify that the shared snapshot keeps standalone behavior. Cooldown records expire after five seconds even if their configuration is removed. These expiry timers perform bookkeeping, never connection attempts.
+
+Recovery before model send can delay a turn: stdio discovery defaults to 30 seconds (remote discovery to five seconds), in addition to waiting for old-entry cleanup. The five-second retry cooldown is not a turn deadline. Cancellation stops the caller waiting. This change preserves existing discovery timeout configuration rather than adding another timeout policy.

docs/users/features/mcp.md

@@ -297,12 +297,22 @@ environment. This is kept as an escape hatch for at least one release.
 
 ### Connection-loss replay
 
-Qwen Code only reconnects and replays the current MCP tool call when the server has `trust: true`, the workspace is trusted, and the tool explicitly declares either `idempotentHint: true` or a consistent read-only annotation. Read-only annotations conflict with `destructiveHint: true` or `idempotentHint: false` and are not replayed.
+Outside shared daemon/ACP sessions, Qwen Code only reconnects and replays the current MCP tool call when the server has `trust: true`, the workspace is trusted, and the tool explicitly declares either `idempotentHint: true` or a consistent read-only annotation. Read-only annotations conflict with `destructiveHint: true` or `idempotentHint: false` and are not replayed.
 
 Calls with missing annotations, conflicting annotations, an untrusted server, or an untrusted workspace are not replayed after a connection failure. Qwen Code reports that the result may be unknown because the server could have completed the operation before the response was lost. Verify the outcome before trying again. This conservative behavior can differ from earlier releases that transparently retried unannotated tools.
 
 Annotations are server-provided behavior hints, not permissions or an authorization boundary. Only configure `trust: true` for servers you control and whose annotations you have verified.
 
+### Shared daemon / ACP connection recovery
+
+Daemon sessions (including Channel sessions) share MCP connections. Cancelling a tool call sends a request cancellation; it does not request a server restart and the cancelled call is never replayed. A server may nevertheless exit or lose its transport while handling cancellation.
+
+When a previously connected server disconnects unexpectedly, the next model send in each affected session attempts to restore that session's connection and tools. Concurrent sessions share a replacement process. Recovery waits for the old transport cleanup, makes one connection attempt per demand, and applies a shared five-second cooldown after a failed attempt. There is no background retry loop. The session reports whether recovery succeeded or the server remains disconnected.
+
+Shared tools do not reconnect or replay a failed invocation themselves, even when annotated read-only or idempotent. A lost response may hide a completed operation: check its outcome before requesting that operation again. Recovery does not execute any tool or reuse a previous permission decision.
+
+Disabled, removed, explicitly disconnected, untrusted, or pending-approval servers are not started by this recovery path. Initial discovery failures and timeouts do not become recovery candidates; correct the configuration and use the existing management controls. Authentication continues through the existing MCP transport and authentication flow.
+
 ### OAuth authentication
 
 Qwen Code supports OAuth 2.0 authentication for MCP servers. This is useful when accessing remote servers that require authentication.

packages/cli/src/acp-integration/session/Session.review-lease.test.ts

@@ -136,6 +136,9 @@ describe('Session review-worktree lease sweep', () => {
         setSessionPrBoundCallback: vi.fn(),
       }),
       getToolRegistry: vi.fn().mockReturnValue({
+        getMcpClientManager: vi.fn().mockReturnValue({
+          recoverFailedConnections: vi.fn().mockResolvedValue([]),
+        }),
         getTool: vi.fn(),
         ensureTool: vi.fn().mockResolvedValue(true),
       }),

packages/cli/src/acp-integration/session/Session.test.ts

@@ -566,6 +566,7 @@ describe('Session', () => {
     get: ReturnType<typeof vi.fn>;
   };
   let mockToolRegistry: {
+    getMcpClientManager: ReturnType<typeof vi.fn>;
     getTool: ReturnType<typeof vi.fn>;
     ensureTool: ReturnType<typeof vi.fn>;
     registerTool: ReturnType<typeof vi.fn>;
@@ -893,6 +894,9 @@ describe('Session', () => {
     };
 
     mockToolRegistry = {
+      getMcpClientManager: vi.fn().mockReturnValue({
+        recoverFailedConnections: vi.fn().mockResolvedValue([]),
+      }),
       getTool: vi.fn(),
       ensureTool: vi.fn().mockResolvedValue(true),
       registerTool: vi.fn(),
@@ -16015,6 +16019,59 @@ describe('Session', () => {
       });
     });
 
+    describe('MCP demand recovery', () => {
+      it.each(['reconnected', 'remains disconnected'])(
+        'refreshes declarations and reports %s before the model send',
+        async (state) => {
+          const notice = `MCP server 'counter' ${state}.`;
+          const recover =
+            mockToolRegistry.getMcpClientManager().recoverFailedConnections;
+          recover.mockResolvedValue([notice]);
+          mockChat.sendMessageStream = vi
+            .fn()
+            .mockResolvedValue(createEmptyStream());
+          await session.prompt({
+            sessionId: 'test-session-id',
+            prompt: [{ type: 'text', text: 'use counter' }],
+          });
+          expect(recover).toHaveBeenCalledWith(expect.any(AbortSignal));
+          expect(mockLlmClient.setTools).toHaveBeenCalled();
+          expect(
+            mockLlmClient.setTools.mock.invocationCallOrder[0],
+          ).toBeLessThan(
+            (mockChat.sendMessageStream as ReturnType<typeof vi.fn>).mock
+              .invocationCallOrder[0],
+          );
+          expect(mockClient.sessionUpdate).toHaveBeenCalledWith(
+            expect.objectContaining({
+              update: expect.objectContaining({
+                sessionUpdate: 'agent_message_chunk',
+                content: { type: 'text', text: notice },
+              }),
+            }),
+          );
+        },
+      );
+
+      it('does not send a model request after cancellation during recovery', async () => {
+        const recover =
+          mockToolRegistry.getMcpClientManager().recoverFailedConnections;
+        recover.mockImplementation(async () => {
+          await session.cancelPendingPrompt();
+          return [];
+        });
+        mockChat.sendMessageStream = vi
+          .fn()
+          .mockResolvedValue(createEmptyStream());
+        const result = await session.prompt({
+          sessionId: 'test-session-id',
+          prompt: [{ type: 'text', text: 'use counter' }],
+        });
+        expect(result.stopReason).toBe('cancelled');
+        expect(mockChat.sendMessageStream).not.toHaveBeenCalled();
+      });
+    });
+
     describe('auto-compress', () => {
       it('runs automatic compression before sending an ACP prompt', async () => {
         mockChat.sendMessageStream = vi

packages/cli/src/acp-integration/session/Session.ts

@@ -7734,6 +7734,23 @@ export class Session implements SessionContext {
       return { responseStream: null, stopReason: 'cancelled' };
     }
 
+    const mcpRecoveryNotices = await this.config
+      .getToolRegistry()
+      .getMcpClientManager()
+      .recoverFailedConnections(abortSignal);
+    if (abortSignal.aborted) {
+      return { responseStream: null, stopReason: 'cancelled' };
+    }
+    if (mcpRecoveryNotices.length > 0) {
+      await llmClient.setTools();
+      for (const notice of mcpRecoveryNotices) {
+        await this.#emitAgentDiagnosticMessageSafely(
+          notice,
+          'Failed to emit MCP recovery status',
+        );
+      }
+    }
+
     let compressionDiagnostic: string | null = null;
     let compressionInfo: ChatCompressionInfo | null = null;
     let compressionFailed = false;

packages/cli/src/acp-integration/session/Session.worktree.test.ts

@@ -146,6 +146,9 @@ describe('Session.pendingWorktreeNotice', () => {
         setSessionPrBoundCallback: vi.fn(),
       }),
       getToolRegistry: vi.fn().mockReturnValue({
+        getMcpClientManager: vi.fn().mockReturnValue({
+          recoverFailedConnections: vi.fn().mockResolvedValue([]),
+        }),
         getTool: vi.fn(),
         ensureTool: vi.fn().mockResolvedValue(true),
       }),