diff --git a/agent/browser_provider.py b/agent/browser_provider.py index 75e88e584f31b..3e96fa9c85e66 100644 --- a/agent/browser_provider.py +++ b/agent/browser_provider.py @@ -26,6 +26,7 @@ Session metadata contract (preserved from the legacy ``CloudBrowserProvider``):: "session_name": str, # unique name for agent-browser --session "bb_session_id": str, # provider session ID (for close/cleanup) "cdp_url": str, # CDP websocket URL + "expires_at": str, # optional provider-authoritative ISO timestamp "features": dict, # feature flags that were enabled "external_call_id": str, # optional, managed-gateway billing key } @@ -96,6 +97,7 @@ class BrowserProvider(abc.ABC): "session_name": str, # unique name for agent-browser --session "bb_session_id": str, # provider session ID (for close/cleanup) "cdp_url": str, # CDP websocket URL + "expires_at": str, # optional provider-authoritative ISO timestamp "features": dict, # feature flags that were enabled } diff --git a/plugins/browser/browser_use/provider.py b/plugins/browser/browser_use/provider.py index 8310862cf7a7e..e098cf814a655 100644 --- a/plugins/browser/browser_use/provider.py +++ b/plugins/browser/browser_use/provider.py @@ -246,6 +246,10 @@ class BrowserUseBrowserProvider(BrowserProvider): "session_name": session_name, "bb_session_id": session_data["id"], "cdp_url": cdp_url, + # Browser Use sessions have a fixed server-side lifetime. Preserve + # the authority returned by the API so the dispatcher can retire an + # expired CDP endpoint instead of reconnecting to it indefinitely. + "expires_at": session_data.get("timeoutAt"), "features": {"browser_use": True}, "external_call_id": external_call_id, } diff --git a/tests/tools/test_browser_use_session_expiry.py b/tests/tools/test_browser_use_session_expiry.py new file mode 100644 index 0000000000000..44a761e1c59bc --- /dev/null +++ b/tests/tools/test_browser_use_session_expiry.py @@ -0,0 +1,93 @@ +"""Regression coverage for provider-authoritative cloud browser expiry.""" + +from unittest.mock import Mock + +import tools.browser_tool as browser_tool +from plugins.browser.browser_use import provider as browser_use_provider + + +def _isolate_browser_state(monkeypatch): + monkeypatch.setattr(browser_tool, "_active_sessions", {}) + monkeypatch.setattr(browser_tool, "_session_last_activity", {}) + monkeypatch.setattr(browser_tool, "_start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr(browser_tool, "_ensure_cdp_supervisor", lambda task_id: None) + + +def test_browser_use_preserves_provider_timeout(monkeypatch): + provider = browser_use_provider.BrowserUseBrowserProvider() + response = Mock( + ok=True, + headers={}, + ) + response.json.return_value = { + "id": "browser-session-1", + "cdpUrl": "ws://browser-use.example/devtools/browser/1", + "timeoutAt": "2030-01-01T00:05:00Z", + } + + monkeypatch.setattr( + provider, + "_get_config", + lambda: { + "api_key": "test-key", + "base_url": "https://api.browser-use.example/api/v3", + "managed_mode": False, + }, + ) + monkeypatch.setattr(browser_use_provider.requests, "post", Mock(return_value=response)) + + session = provider.create_session("task-1") + + assert session["expires_at"] == "2030-01-01T00:05:00Z" + + +def test_live_cloud_session_is_reused(monkeypatch): + _isolate_browser_state(monkeypatch) + existing = { + "session_name": "existing", + "bb_session_id": "browser-session-1", + "cdp_url": "ws://browser-use.example/devtools/browser/1", + "expires_at": "2999-01-01T00:05:00Z", + } + browser_tool._active_sessions["task-1"] = existing + provider = Mock() + monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) + + session = browser_tool._get_session_info("task-1") + + assert session is existing + provider.create_session.assert_not_called() + + +def test_expired_cloud_session_is_replaced_without_reusing_dead_cdp(monkeypatch): + _isolate_browser_state(monkeypatch) + browser_tool._active_sessions["task-1"] = { + "session_name": "expired", + "bb_session_id": "browser-session-old", + "cdp_url": "ws://browser-use.example/devtools/browser/old", + "expires_at": "2020-01-01T00:05:00Z", + } + browser_tool._session_last_activity["task-1"] = 1.0 + + provider = Mock() + provider.create_session.return_value = { + "session_name": "replacement", + "bb_session_id": "browser-session-new", + "cdp_url": "ws://browser-use.example/devtools/browser/new", + "expires_at": "2999-01-01T00:05:00Z", + } + monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr(browser_tool, "_get_cdp_override", lambda: "") + monkeypatch.setattr(browser_tool, "_stop_cdp_supervisor", Mock()) + monkeypatch.setattr(browser_tool, "_maybe_stop_recording", Mock()) + monkeypatch.setattr(browser_tool, "_run_browser_command", Mock()) + monkeypatch.setattr(browser_tool.os.path, "exists", lambda path: False) + + session = browser_tool._get_session_info("task-1") + + assert session["bb_session_id"] == "browser-session-new" + assert browser_tool._active_sessions["task-1"] is session + assert "task-1" in browser_tool._session_last_activity + provider.close_session.assert_called_once_with("browser-session-old") + provider.create_session.assert_called_once_with("task-1") + browser_tool._run_browser_command.assert_not_called() diff --git a/tools/browser_tool.py b/tools/browser_tool.py index d81bab4406096..eff20ed968733 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -61,6 +61,7 @@ import sys import tempfile import threading import time +from datetime import datetime, timezone from typing import Dict, Any, Optional, List, Tuple, Union from pathlib import Path from agent.redact import redact_cdp_url @@ -1568,6 +1569,42 @@ _cleanup_running = False _cleanup_lock = threading.Lock() +def _session_expiry_timestamp(session_info: Dict[str, Any]) -> Optional[float]: + """Return a provider-authoritative session expiry as epoch seconds. + + Cloud providers may omit ``expires_at``. Unknown or malformed values are + therefore treated as having no known expiry, preserving the existing + lifecycle for local browsers and providers without an expiry contract. + """ + value = session_info.get("expires_at") + if isinstance(value, (int, float)) and not isinstance(value, bool): + return float(value) + if not isinstance(value, str) or not value.strip(): + return None + + normalized = value.strip() + if normalized.endswith(("Z", "z")): + normalized = f"{normalized[:-1]}+00:00" + try: + parsed = datetime.fromisoformat(normalized) + except ValueError: + logger.warning("Ignoring invalid cloud browser session expiry timestamp") + return None + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed.timestamp() + + +def _session_has_expired( + session_info: Dict[str, Any], *, now: Optional[float] = None +) -> bool: + """Return whether a cached browser session crossed its provider deadline.""" + expires_at = _session_expiry_timestamp(session_info) + if expires_at is None: + return False + return (time.time() if now is None else now) >= expires_at + + def _emergency_cleanup_all_sessions(): """ Emergency cleanup of all active browser sessions. @@ -2152,7 +2189,22 @@ def _get_session_info(task_id: Optional[str] = None) -> Dict[str, Any]: with _cleanup_lock: # Check if we already have a session for this task if task_id in _active_sessions: - return _active_sessions[task_id] + existing_session = _active_sessions[task_id] + else: + existing_session = None + + if existing_session is not None: + if not _session_has_expired(existing_session): + return existing_session + + logger.info( + "Replacing expired cloud browser session for task %s", + task_id, + ) + _cleanup_single_browser_session(task_id) + # Cleanup removes the activity entry. The replacement session must be + # tracked by the inactivity reaper just like an initial session. + _update_session_activity(task_id) # Hybrid routing: session keys ending with ``::local`` force a local # Chromium regardless of the globally-configured cloud provider. Public @@ -4539,12 +4591,23 @@ def _cleanup_single_browser_session(task_id: str) -> None: # Stop auto-recording before closing (saves the file) _maybe_stop_recording(task_id) - # Try to close via agent-browser first (needs session in _active_sessions) - try: - _run_browser_command(task_id, "close", [], timeout=10) - logger.debug("agent-browser close command completed for task %s", task_id) - except Exception as e: - logger.warning("agent-browser close failed for task %s: %s", task_id, e) + # An expired cloud CDP URL cannot accept an agent-browser close command. + # Avoid feeding it back through _get_session_info(), which would try to + # renew the session recursively while cleanup is still in progress. + if _session_has_expired(session_info): + logger.debug( + "Skipping agent-browser close for expired session %s", + task_id, + ) + else: + try: + _run_browser_command(task_id, "close", [], timeout=10) + logger.debug( + "agent-browser close command completed for task %s", + task_id, + ) + except Exception as e: + logger.warning("agent-browser close failed for task %s: %s", task_id, e) # Now remove from tracking under lock with _cleanup_lock: