fix(browser): reset sessions after command timeout

tests/tools/test_browser_open_timeout.py

@@ -1,6 +1,7 @@
 """Tests for browser first-open timeout and timeout diagnostics."""
 
-from unittest.mock import patch
+import subprocess
+from unittest.mock import Mock, patch
 
 import pytest
 
@@ -11,9 +12,15 @@ import tools.browser_tool as bt
 def _reset_browser_caches():
     bt._cached_command_timeout = None
     bt._command_timeout_resolved = False
+    bt._active_sessions.clear()
+    bt._session_last_activity.clear()
+    bt._last_active_session_key.clear()
     yield
     bt._cached_command_timeout = None
     bt._command_timeout_resolved = False
+    bt._active_sessions.clear()
+    bt._session_last_activity.clear()
+    bt._last_active_session_key.clear()
 
 
 class TestOpenCommandTimeout:
@@ -81,6 +88,66 @@ class TestReadCommandOutputFiles:
         assert stderr == "warn"
 
 
+class TestCommandTimeoutRecovery:
+    @pytest.mark.parametrize("cloud", [False, True])
+    def test_timeout_replaces_only_stuck_client(self, monkeypatch, tmp_path, cloud):
+        task_id = "stuck-command"
+        session_info = {
+            "session_name": "stuck-session",
+            "bb_session_id": "cloud-session-1" if cloud else None,
+            "cdp_url": "ws://cloud.invalid/devtools/browser/1" if cloud else None,
+        }
+        bt._active_sessions[task_id] = session_info
+        bt._session_last_activity[task_id] = 1.0
+        bt._last_active_session_key[task_id] = task_id
+
+        process = Mock()
+        process.returncode = 0
+        process.wait.side_effect = [subprocess.TimeoutExpired("agent-browser", 1), -9, 0]
+        supervisor_events = []
+
+        monkeypatch.setattr(bt, "_find_agent_browser", lambda: "agent-browser")
+        monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _cmd: False)
+        monkeypatch.setattr(bt, "_start_browser_cleanup_thread", lambda: None)
+        monkeypatch.setattr(bt, "_ensure_cdp_supervisor", lambda _: supervisor_events.append("ensure"))
+        monkeypatch.setattr(bt, "_stop_cdp_supervisor", lambda _: supervisor_events.append("stop"))
+        monkeypatch.setattr(bt, "_socket_safe_tmpdir", lambda: str(tmp_path))
+        monkeypatch.setattr(bt, "_write_owner_pid", lambda *_args: None)
+        monkeypatch.setattr(bt, "_build_browser_env", lambda: {})
+        monkeypatch.setattr(bt, "_merge_browser_path", lambda value: value)
+        monkeypatch.setattr(subprocess, "Popen", lambda *_args, **_kwargs: process)
+        monkeypatch.setattr("tools.interrupt.is_interrupted", lambda: False)
+
+        bt._run_browser_command(task_id, "click", ["@e1"], timeout=1)
+
+        assert task_id not in bt._last_active_session_key
+        assert not (tmp_path / "agent-browser-stuck-session").exists()
+        if not cloud:
+            assert task_id not in bt._active_sessions and task_id not in bt._session_last_activity
+            return
+
+        replacement = bt._active_sessions[task_id]
+        assert replacement is not session_info
+        assert replacement["session_name"] != "stuck-session"
+        assert replacement["bb_session_id"] == "cloud-session-1"
+        assert bt._get_session_info(task_id) is replacement
+
+        provider = Mock()
+        monkeypatch.setattr(bt, "_get_cloud_provider", lambda: provider)
+        bt.cleanup_browser(task_id)
+        provider.close_session.assert_called_once_with("cloud-session-1")
+        assert supervisor_events == ["ensure", "stop", "stop"]
+
+    def test_stale_timeout_cannot_remove_concurrent_replacement(self, tmp_path):
+        stale, replacement = {"session_name": "stale"}, {"session_name": "replacement"}
+        bt._active_sessions["race"] = replacement
+
+        bt._discard_timed_out_browser_session("race", stale, str(tmp_path))
+
+        assert bt._active_sessions["race"] is replacement
+        assert tmp_path.exists()
+
+
 class TestBrowserNavigateOpenTimeout:
     def test_first_navigation_uses_first_open_timeout(self, monkeypatch):
         captured: dict = {}

tools/browser_tool.py

@@ -3142,6 +3142,47 @@ def _extract_screenshot_path_from_text(text: str) -> Optional[str]:
     return None
 
 
+def _discard_timed_out_browser_session(
+    task_id: str,
+    session_info: Dict[str, Any],
+    task_socket_dir: str,
+) -> None:
+    """Drop a stuck client generation without losing cloud cleanup state."""
+    with _cleanup_lock:
+        if _active_sessions.get(task_id) is not session_info:
+            return
+        _stop_cdp_supervisor(task_id)
+        if session_info.get("bb_session_id") or session_info.get("cdp_url"):
+            import uuid
+            replacement = dict(session_info)
+            replacement["session_name"] = f"h_{uuid.uuid4().hex[:10]}"
+            replacement.pop("_first_nav", None)
+            _active_sessions[task_id] = replacement
+        else:
+            _active_sessions.pop(task_id, None)
+            _session_last_activity.pop(task_id, None)
+
+        bare_task_id = _bare_task_id_for_session_key(task_id)
+        if _last_active_session_key.get(bare_task_id) == task_id:
+            _last_active_session_key.pop(bare_task_id, None)
+
+    session_name = str(session_info.get("session_name") or "")
+    if session_name:
+        pid_file = os.path.join(task_socket_dir, f"{session_name}.pid")
+        if os.path.isfile(pid_file):
+            try:
+                from tools.process_registry import ProcessRegistry
+
+                daemon_pid = int(Path(pid_file).read_text(encoding="utf-8").strip())
+                if not _verify_reapable_browser_daemon(daemon_pid, task_socket_dir, session_name):
+                    return
+                ProcessRegistry._terminate_host_pid(daemon_pid)
+            except (ProcessLookupError, ValueError, PermissionError, OSError):
+                logger.debug("Could not kill timed-out browser daemon for %s", session_name)
+                return
+    shutil.rmtree(task_socket_dir, ignore_errors=True)
+
+
 def _run_browser_command(
     task_id: str,
     command: str,
@@ -3215,6 +3256,9 @@ def _run_browser_command(
     except Exception as e:
         logger.warning("Failed to create browser session for task=%s: %s", task_id, e)
         return {"success": False, "error": f"Failed to create browser session: {str(e)}"}
+    # Cleanup stops the supervisor before closing the backend; keep it stopped.
+    if command != "close" and session_info.get("cdp_url"):
+        _ensure_cdp_supervisor(task_id)
 
     # Build the command with the appropriate backend flag.
     # Cloud mode: --cdp <websocket_url> connects to Browserbase.
@@ -3353,6 +3397,7 @@ def _run_browser_command(
             proc.wait()
             stdout, stderr = _read_command_output_files(stdout_path, stderr_path)
             _unlink_command_output_files(stdout_path, stderr_path)
+            _discard_timed_out_browser_session(task_id, session_info, task_socket_dir)
             if stderr and stderr.strip():
                 logger.warning(
                     "browser '%s' stderr after timeout: %s",