fix(mcp): reject initial tools/list failure before close

Detached and dropped the connection synchronously, then closed the transport in the background, so a slow or hung close no longer keeps the tool-load promise pending past the startup race and no longer strands pending state that skips future connects.

Applied the same reject-fast teardown to the stale initial-connect and reconnect bailouts.

Fixes #8112

packages/coding-agent/src/mcp/manager.ts

@@ -498,7 +498,8 @@ export class MCPManager {
 					}
 
 					if (this.#epoch !== connectionEpoch || this.#pendingConnections.get(name) !== connectionPromise) {
-						await this.#discardConnection(name, connection).catch(() => {});
+						this.#detachConnection(name, connection);
+						void disconnectServer(connection).catch(() => {});
 						throw new Error(`Server "${name}" was disconnected during initial connection`);
 					}
 
@@ -546,7 +547,13 @@ export class MCPManager {
 					const serverTools = await listTools(connection);
 					return { connection, serverTools };
 				} catch (error) {
-					await this.#discardConnection(name, connection).catch(() => {});
+					// Detach and delete synchronously, then close in the background:
+					// awaiting a slow HTTP close (session DELETE) here would keep
+					// toolsPromise pending past the startup race, so connectServers
+					// would return with no error while #pendingToolLoads stayed set
+					// and future connects for this server were skipped.
+					this.#detachConnection(name, connection);
+					void disconnectServer(connection).catch(() => {});
 					throw error;
 				}
 			});
@@ -864,11 +871,31 @@ export class MCPManager {
 		);
 	}
 
-	async #discardConnection(name: string, connection: MCPServerConnection): Promise<void> {
+	/**
+	 * Drop a connection from the active map and detach its lifecycle hooks.
+	 *
+	 * Synchronous and identity-guarded: only removes the entry when it is still
+	 * the connection registered under `name`, so a stale cleanup never evicts a
+	 * newer connection for the same server. Detaching `onClose` first prevents
+	 * the transport's own `close()` from re-arming reconnect.
+	 */
+	#detachConnection(name: string, connection: MCPServerConnection): void {
 		connection.transport.onClose = undefined;
 		if (this.#connections.get(name) === connection) {
 			this.#connections.delete(name);
 		}
+	}
+
+	/**
+	 * Detach a connection and await its transport close.
+	 *
+	 * Use only where blocking on the close is acceptable (owned disconnects,
+	 * dispose). On reject-fast paths detach synchronously and close in the
+	 * background so a slow `close()` (HTTP session DELETE) cannot delay the
+	 * rejection — see the `tools/list` failure handler in `connectServers`.
+	 */
+	async #discardConnection(name: string, connection: MCPServerConnection): Promise<void> {
+		this.#detachConnection(name, connection);
 		await disconnectServer(connection);
 	}
 
@@ -1103,7 +1130,8 @@ export class MCPManager {
 		// Bail out if the server was disconnected or the manager was reset
 		// while we were connecting (e.g. /mcp reload called disconnectAll).
 		if (!this.#serverConfigs.has(name) || this.#epoch !== reconnectEpoch) {
-			await this.#discardConnection(name, connection).catch(() => {});
+			this.#detachConnection(name, connection);
+			void disconnectServer(connection).catch(() => {});
 			throw new Error(`Server "${name}" was disconnected during reconnection`);
 		}
 
@@ -1134,8 +1162,10 @@ export class MCPManager {
 			void this.#loadServerResourcesAndPrompts(name, connection);
 			return connection;
 		} catch (error) {
-			// Clean up the connection to avoid zombie transports
-			await this.#discardConnection(name, connection).catch(() => {});
+			// Detach synchronously and close in the background so a slow close
+			// cannot delay the rejection (and the retry backoff that follows).
+			this.#detachConnection(name, connection);
+			void disconnectServer(connection).catch(() => {});
 			throw error;
 		}
 	}

packages/coding-agent/test/mcp-manager-initial-connection-cleanup.test.ts

@@ -12,6 +12,12 @@ class FakeTransport implements MCPTransport {
 	connected = true;
 	closeCalls = 0;
 	onClose?: () => void;
+	#closeGate?: Promise<void>;
+
+	/** Make `close()` hang on the given gate to simulate a slow HTTP session DELETE. */
+	gateClose(gate: Promise<void>): void {
+		this.#closeGate = gate;
+	}
 
 	request<T>(): Promise<T> {
 		throw new Error("Unexpected transport request");
@@ -22,6 +28,7 @@ class FakeTransport implements MCPTransport {
 	async close(): Promise<void> {
 		this.closeCalls += 1;
 		this.connected = false;
+		if (this.#closeGate) await this.#closeGate;
 	}
 }
 
@@ -113,4 +120,29 @@ describe("MCPManager initial connection ownership", () => {
 		expect(manager.getConnectedServers()).toEqual(["server"]);
 		await manager.disconnectAll();
 	});
+
+	it("reports a tools/list failure and re-enables connects even when close hangs", async () => {
+		const manager = new MCPManager(process.cwd());
+		const failed = fakeConnection("server");
+		const stuckClose = Promise.withResolvers<void>();
+		failed.transport.gateClose(stuckClose.promise);
+		const connectSpy = vi
+			.spyOn(mcpClient, "connectToServer")
+			.mockResolvedValueOnce(failed.connection)
+			.mockRejectedValue(new Error("second connect refused"));
+		vi.spyOn(mcpClient, "listTools").mockRejectedValueOnce(new Error("initial tools/list failed"));
+
+		// close() never settles, but the failure must still surface and clear
+		// pending state so the server is not silently skipped forever.
+		const result = await manager.connectServers({ server: CONFIG }, {});
+		expect(result.errors.get("server")).toBe("initial tools/list failed");
+		expect(failed.transport.closeCalls).toBe(1);
+		expect(manager.getConnectedServers()).toEqual([]);
+
+		// A subsequent connect is attempted rather than skipped on stale pending state.
+		await manager.connectServers({ server: CONFIG }, {});
+		expect(connectSpy).toHaveBeenCalledTimes(2);
+
+		stuckClose.resolve();
+	});
 });