From 80085085d9968ddce22f5d5ad676db317cbd43f6 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Wed, 22 Jul 2026 15:13:18 +0800 Subject: [PATCH] fix(exec): retain failed owner session cleanup --- nanobot/agent/tools/exec_session.py | 17 ++++++++++++++++- tests/tools/test_exec_session_tools.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/nanobot/agent/tools/exec_session.py b/nanobot/agent/tools/exec_session.py index 2fad2e0b..c00c2fd2 100644 --- a/nanobot/agent/tools/exec_session.py +++ b/nanobot/agent/tools/exec_session.py @@ -341,10 +341,25 @@ class ExecSessionManager: for sid, s in list(self._sessions.items()): if s.owner_session_key == owner_session_key: victims.append(self._sessions.pop(sid)) - await asyncio.gather( + results = await asyncio.gather( *(s.kill() for s in victims), return_exceptions=True, ) + failures = [ + (session, result) + for session, result in zip(victims, results, strict=True) + if isinstance(result, BaseException) + ] + if failures: + async with self._lock: + for session, _ in failures: + self._sessions[session.session_id] = session + if len(failures) == 1: + raise failures[0][1] + raise BaseExceptionGroup( + "failed to terminate exec sessions by owner", + [result for _, result in failures], + ) return len(victims) async def _cleanup_locked(self) -> None: diff --git a/tests/tools/test_exec_session_tools.py b/tests/tools/test_exec_session_tools.py index ca7aafad..148baf5b 100644 --- a/tests/tools/test_exec_session_tools.py +++ b/tests/tools/test_exec_session_tools.py @@ -679,6 +679,29 @@ def test_terminate_by_owner_returns_zero_for_no_match(tmp_path): asyncio.run(run()) +def test_terminate_by_owner_retains_failed_sessions(): + async def run() -> None: + manager = ExecSessionManager() + session = SimpleNamespace( + session_id="failed", + owner_session_key="cli:a", + kill=AsyncMock(side_effect=OSError("termination failed")), + ) + manager._sessions[session.session_id] = session + + with pytest.raises(OSError, match="termination failed"): + await manager.terminate_by_owner("cli:a") + + assert manager._sessions == {session.session_id: session} + session.kill.assert_awaited_once() + + session.kill.side_effect = None + assert await manager.terminate_by_owner("cli:a") == 1 + assert manager._sessions == {} + + asyncio.run(run()) + + def test_terminate_by_owner_skips_sessions_without_owner_key(tmp_path): async def run() -> None: manager = ExecSessionManager()