From 5ae41dc89c91a8a1b34c2377f80a4662f4936fbb Mon Sep 17 00:00:00 2001 From: Ali Adnan <165782963+AAliKKhan@users.noreply.github.com> Date: Mon, 24 Aug 2026 03:15:13 +0500 Subject: [PATCH 1/2] fix(mcp): drop cleaned-up servers from active_servers when keeping failed servers cleanup_all() advertised cleaned-up servers as active whenever drop_failed_servers=False, because _refresh_active_servers() returned all servers unconditionally. Derive the post-cleanup active list from the connected set instead: that flag governs connect-phase failures only, and this completes the contract stated in #4586. --- src/agents/mcp/manager.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/agents/mcp/manager.py b/src/agents/mcp/manager.py index 733e744363..54a7d045fd 100644 --- a/src/agents/mcp/manager.py +++ b/src/agents/mcp/manager.py @@ -388,7 +388,12 @@ async def _cleanup_all(self) -> None: ) self._errors[server] = exc finally: - self._refresh_active_servers() + # Cleanup discards every processed server from the connected set, so no cleaned + # server may remain advertised as active regardless of `drop_failed_servers`: + # that flag only governs whether connect-phase failures stay listed. + self._active_servers = [ + server for server in self._all_servers if server in self._connected_servers + ] async def _run_with_timeout( self, func: Callable[[], Awaitable[Any]], timeout_seconds: float | None From ccf573871843c3b3fef01c25b565c15b1b8623c2 Mon Sep 17 00:00:00 2001 From: Ali Adnan <165782963+AAliKKhan@users.noreply.github.com> Date: Mon, 24 Aug 2026 03:15:14 +0500 Subject: [PATCH 2/2] test(mcp): cover active_servers after cleanup with drop_failed_servers=False --- .../test_mcp_server_manager_cleanup_state.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/mcp/test_mcp_server_manager_cleanup_state.py b/tests/mcp/test_mcp_server_manager_cleanup_state.py index f411f464d8..a34464b5f5 100644 --- a/tests/mcp/test_mcp_server_manager_cleanup_state.py +++ b/tests/mcp/test_mcp_server_manager_cleanup_state.py @@ -29,6 +29,25 @@ async def test_cleanup_all_removes_cleaned_servers_from_active_servers() -> None assert server.connect.await_count == 2 +@pytest.mark.asyncio +async def test_cleanup_all_removes_cleaned_servers_from_active_servers_when_keeping_failed() -> ( + None +): + """`drop_failed_servers=False` keeps connect-phase failures listed, but a server that was + cleaned up is disconnected and must not remain advertised as active.""" + server = cast(MCPServer, Mock(spec=MCPServer)) + server.connect = AsyncMock() + server.cleanup = AsyncMock() + + manager = MCPServerManager([server], drop_failed_servers=False) + assert await manager.connect_all() == [server] + + await manager.cleanup_all() + + assert manager.active_servers == [] + assert manager._connected_servers == set() + + @pytest.mark.asyncio async def test_manager_owns_repeated_server_instance_once() -> None: server = cast(MCPServer, Mock(spec=MCPServer))