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

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

@@ -0,0 +1,46 @@
+# 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; a later stop or explicit disconnect invalidates that queued work. 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. All acquisition paths wait for the previous entry's cleanup; shutdown includes evicted entries still being cleaned within its existing deadline. A shared five-second cooldown covers acquisition and session-acceptance failures. Each model send can demand recovery, including tool-loop continuations and scheduled model turns; there are no background reconnect timers or invocation 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. Refresh tools and ACP prompt commands after recovery; expose success/failure as an ACP diagnostic. Identical notices appear once per logical turn, while each send still refreshes registrations after recovery. If a turn is cancelled while waiting for a shared connection, stop waiting without cancelling that connection or executing a tool; retain its notice until the next send or superseding configuration/management action.
+
+Runtime add and recovery use the same effective transport configuration, including the session cwd for implicit stdio servers. The raw runtime overlay remains unstamped so it can follow a workspace relocation. Retiring a transport settles the SDK's old pending requests and removes its callbacks before reconnecting; a late child-close event cannot clear the SDK's replacement transport. Releasing an unavailable resource handle also detaches its session listener.
+
+Pool-owned tool failures keep an explicit unknown-outcome/no-replay warning, even though they skip invocation-level reconnect. Preventing the SDK from replaying is insufficient if a bare connection error encourages the model to issue the same operation again. This warning does not prove that arbitrary future model decisions will never duplicate a business operation; the client recovery itself performs no tool calls.
+
+A session-acceptance failure can impose the remaining cooldown on another session even if the pool still has a healthy entry. This conservative bound avoids repeated registration failures and concurrent successful returns erasing a failure record. Budget refusals retain the workspace refusal batch and receive budget-specific advice. When pooling is disabled, legacy per-session recovery remains outside this change.
+
+## 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.
+For tools outside the workspace MCP pool (including daemon sessions started with `--no-mcp-pool`), 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
+
+When workspace MCP pooling is enabled, daemon sessions (including Channel sessions) share eligible MCP connections, including stdio processes. 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. Disabling the pool retains the legacy per-session connection behavior.
+
+When a previously connected server disconnects unexpectedly, the next model send in each affected session attempts to restore that session's connection, tools and prompt commands. This includes later tool loops and scheduled model turns. Concurrent sessions share a replacement process. Each model send can request recovery; failures impose a shared five-second cooldown, including failures while accepting the acquired connection into a session. All acquisition paths wait for old transport cleanup. There is no background reconnect timer. The session reports success or failure, with identical diagnostics shown once per user turn.
+
+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,131 @@ describe('Session', () => {
       });
     });
 
+    describe('MCP demand recovery', () => {
+      it.each(['reconnected', 'remains disconnected'])(
+        'deduplicates %s notices across tool loops while refreshing each send',
+        async (state) => {
+          const notice = `MCP server 'counter' ${state}.`;
+          const recover =
+            mockToolRegistry.getMcpClientManager().recoverFailedConnections;
+          recover.mockResolvedValue([notice]);
+          const execute = vi
+            .fn()
+            .mockResolvedValue({ llmContent: 'ok', returnDisplay: 'ok' });
+          mockToolRegistry.getTool.mockReturnValue({
+            name: 'read_file',
+            kind: core.Kind.Read,
+            displayName: 'Read File',
+            description: 'Read file',
+            canUpdateOutput: false,
+            isOutputMarkdown: true,
+            build: vi.fn().mockReturnValue({
+              params: {},
+              execute,
+              getDefaultPermission: vi.fn().mockResolvedValue('allow'),
+              getDescription: vi.fn().mockReturnValue('Read file'),
+              toolLocations: vi.fn().mockReturnValue([]),
+            }),
+          });
+          const send = vi.fn();
+          for (const id of ['read-1', 'read-2']) {
+            send.mockResolvedValueOnce(
+              createStreamWithChunks([
+                {
+                  type: core.StreamEventType.CHUNK,
+                  value: {
+                    functionCalls: [
+                      { id, name: 'read_file', args: { file_path: id } },
+                    ],
+                  },
+                },
+              ]),
+            );
+          }
+          send.mockImplementation(async () => createEmptyStream());
+          mockChat.sendMessageStream = send;
+          const prompt = {
+            sessionId: 'test-session-id',
+            prompt: [{ type: 'text' as const, text: 'read files' }],
+          };
+          const notices = () =>
+            vi
+              .mocked(mockClient.sessionUpdate)
+              .mock.calls.filter(
+                ([update]) =>
+                  update.update.sessionUpdate === 'agent_message_chunk' &&
+                  update.update.content.type === 'text' &&
+                  update.update.content.text === notice,
+              );
+          await session.prompt(prompt);
+          expect(execute).toHaveBeenCalledTimes(2);
+          expect(send).toHaveBeenCalledTimes(3);
+          expect(recover).toHaveBeenCalledTimes(3);
+          expect(notices()).toHaveLength(1);
+          // A second connection loss in the turn can change declarations, even
+          // when its recovery message is identical to the first one's.
+          expect(
+            mockLlmClient.setTools.mock.calls.length,
+          ).toBeGreaterThanOrEqual(3);
+          await session.prompt(prompt);
+          expect(notices()).toHaveLength(2);
+        },
+      );
+
+      it.each(['reconnected', 'remains disconnected'])(
+        'refreshes declarations and reports %s before the model send',
+        async (state) => {
+          const commands = vi.spyOn(session, 'sendAvailableCommandsUpdate');
+          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(commands).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

@@ -1982,6 +1982,8 @@ export class Session implements SessionContext {
    */
   private followupAbort: AbortController | null = null;
   private turn: number = 0;
+  private mcpRecoveryNoticeTurn?: string;
+  private readonly mcpRecoveryNotices = new Set<string>();
   private refreshContextFilesOnWrite = false;
   private activeTodoWorkChainPromptId: string | undefined;
   private readonly createdAt: number = Date.now();
@@ -6753,6 +6755,7 @@ export class Session implements SessionContext {
           nextMessage.parts ?? [],
           pendingSend.signal,
           {
+            recoveryTurnId: toolPromptId,
             skipCompression:
               skipCompression || (guardForThisSend?.attempt ?? 0) > 1,
             getModelOverride: options.getModelOverride,
@@ -7700,6 +7703,7 @@ export class Session implements SessionContext {
     message: Part[],
     abortSignal: AbortSignal,
     options: {
+      recoveryTurnId?: string;
       skipCompression?: boolean;
       modelOverride?: string;
       getModelOverride?: () => string | undefined;
@@ -7725,6 +7729,33 @@ 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) {
+      // A later tool loop may need another connection or fresh declarations,
+      // but identical status messages should appear only once per user turn.
+      await llmClient.setTools();
+      await this.sendAvailableCommandsUpdate();
+      const recoveryTurnId = options.recoveryTurnId ?? promptId;
+      if (this.mcpRecoveryNoticeTurn !== recoveryTurnId) {
+        this.mcpRecoveryNoticeTurn = recoveryTurnId;
+        this.mcpRecoveryNotices.clear();
+      }
+      for (const notice of mcpRecoveryNotices) {
+        if (this.mcpRecoveryNotices.has(notice)) continue;
+        this.mcpRecoveryNotices.add(notice);
+        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),
       }),