From cc14151d105af9516cf4bc458c606fde08df9a12 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:25:45 +0100 Subject: [PATCH] fix(browser): clean up owned browser daemons on shutdown after cancellation Shutdown cleanup must not depend on active record.session capability, which is cleared on cancellation. Guard cleanup by socket directory presence so all owned daemons are terminated. --- src/agent_tools/web_tools.py | 3 +-- tests/test_private_browser_tool.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/agent_tools/web_tools.py b/src/agent_tools/web_tools.py index fe33e2da0..007717bb5 100644 --- a/src/agent_tools/web_tools.py +++ b/src/agent_tools/web_tools.py @@ -2708,8 +2708,7 @@ async def shutdown_private_browser_sessions() -> None: _ACTIVE_BROWSER_SESSIONS.discard(session) from src.browser_identity import _REGISTRY for record in tuple(_REGISTRY.values()): - session = record.session - if session is not None and session.observation.daemon.owned(): + if record.env and "AGENT_BROWSER_SOCKET_DIR" in record.env: browser_lifecycle.force_cleanup(Path(record.env["AGENT_BROWSER_SOCKET_DIR"]), record.key, method="shutdown", pid_alive=lambda pid: _process_is_alive(pid)) record.invalidate() diff --git a/tests/test_private_browser_tool.py b/tests/test_private_browser_tool.py index 2c5ecc315..37d9856d6 100644 --- a/tests/test_private_browser_tool.py +++ b/tests/test_private_browser_tool.py @@ -746,3 +746,31 @@ def test_liveness_probe_goes_through_the_platform_safe_helper(monkeypatch) -> No assert web_tools._process_is_alive(4242) is True assert asked == [4242] + + +@pytest.mark.asyncio +async def test_shutdown_cleans_up_invalidated_registered_browser_session(monkeypatch) -> None: + """Shutdown cleanup must terminate owned daemons even if record.session was invalidated.""" + from unittest.mock import MagicMock + from src import browser_identity as browser + + cleaned: list[tuple[Path, str]] = [] + def fake_force_cleanup(root, key, **kwargs): + cleaned.append((Path(root), key)) + + monkeypatch.setattr("src.browser_lifecycle.force_cleanup", fake_force_cleanup) + + record = MagicMock() + record.key = "ody-test1234" + record.env = {"AGENT_BROWSER_SOCKET_DIR": "/tmp/test-socket-dir"} + record.session = None # Simulates cancellation / invalidate() + record.invalidate = MagicMock() + + monkeypatch.setattr(browser, "_REGISTRY", {("alice", "thread"): record}) + + await shutdown_private_browser_sessions() + + assert cleaned == [(Path("/tmp/test-socket-dir"), "ody-test1234")] + assert browser._REGISTRY == {} + record.invalidate.assert_called_once() +