fix(browser): keep cloud browser sessions alive during long browser_exec runs - #83739
fix(browser): keep cloud browser sessions alive during long browser_exec runs#83739pierrenode wants to merge 1 commit into
Conversation
…xec runs browser_exec's Browser Use mode is now the default backend (8d8bc85, today) whenever the browser-use CLI is runnable. When a cloud provider (Browserbase, Firecrawl, Nous gateway) is configured, _resolve_backend_cdp() calls browser_tool._get_session_info(task_id) exactly once, before the CLI subprocess starts — that's the only place this call touched the per-task session tracker's activity timestamp. It then hands the CDP endpoint to an external browser-use CLI subprocess that drives the browser directly over CDP for up to 1800s (raised from 120s/600s by 92968a5 specifically to support long, multi-step extractions), making zero further calls back into hermes's Python process. A background thread (_cleanup_inactive_browser_sessions, 30s poll) reaps any session whose last-activity timestamp is older than browser.inactivity_timeout (default 120s) — a real teardown: it sends an `agent-browser close` command, calls the provider's close_session() (destroying the remote browser instance), and kills the local daemon PID. With a cloud provider configured and no explicit session=, any browser_exec call running longer than ~120-150s has its underlying cloud browser destroyed by the reaper while the browser-use CLI subprocess is still actively connected to it over CDP — breaking the connection mid-task, directly undermining 92968a5's own raised timeout and "workspace persists across timeouts" design (this failure is a broken CDP connection, not a clean timeout the workspace-recovery guidance addresses). Fix: _resolve_backend_cdp() now also returns the task-tracker key it registered activity for (None when no cloud-provider session was resolved — local CDP override, no provider, or a Browser Use direct-API config that manages its own session). browser_exec() runs the CLI via a bounded poll loop (subprocess.Popen + background stdout/stderr drain threads to avoid pipe-buffer deadlock during a long run) instead of a single blocking subprocess.run(), and calls browser_tool._update_session_activity() every _CLOUD_SESSION_HEARTBEAT_INTERVAL_S (20s, well under the 120s default threshold) while waiting — refreshing the session's activity timestamp for the whole run instead of only at resolution time. Local Chrome and explicit session=/BU_NAME runs are unaffected (cloud_session_task_id is None there, so no heartbeat calls are made); overall timeout/kill semantics, stdout/stderr capture, and the result shape are unchanged.
7c605a4 to
87bf53f
Compare
fix(browser): keep cloud browser sessions alive during long browser_exec runs — the heartbeat-pump fix is the right shape and the regression test is solid. Observations:
|
Summary
browser_exec's Browser Use mode is now the default browser backend (8d8bc85dca, today) whenever thebrowser-useCLI is runnable. When a cloud provider (Browserbase, Firecrawl, Nous gateway) is configured,_resolve_backend_cdp()callsbrowser_tool._get_session_info(task_id)exactly once, before the CLI subprocess even starts — that's the only place this call path touches the legacy stack's per-task session activity tracker. It then hands the resolved CDP endpoint to an externalbrowser-useCLI subprocess that drives the browser directly over CDP for up to 1800s (raised from 120s/600s by92968a5c7d, specifically to support long, multi-step extractions — "split the work into several calls that append to workspace files… so progress survives timeouts"), making zero further calls back into Hermes's Python process for the whole run.A background thread (
_cleanup_inactive_browser_sessions, 30s poll) reaps any session whose last-activity timestamp is older thanbrowser.inactivity_timeout(default 120s) — and this is a real teardown: it sends anagent-browser closecommand, calls the provider'sclose_session()(destroying the remote browser instance), and kills the local daemon PID.Net effect: with a cloud provider configured and no explicit
session=, anybrowser_execcall running longer than ~120-150s has its underlying cloud browser destroyed by the reaper while the browser-use CLI subprocess is still actively connected to it over CDP — breaking the connection mid-task. This directly undermines92968a5c7d's own raised timeout and "workspace persists across timeouts" design: the failure here is a broken CDP connection, not a clean timeout the workspace-recovery guidance addresses. Local Chrome setups (no cloud provider) are unaffected, since_resolve_backend_cdpreturns early without touching session tracking there.Fix
_resolve_backend_cdp()now returns(error, cloud_session_task_id)instead of justerror.cloud_session_task_idis the key it registered activity for with the session tracker when it successfully resolved a cloud-provider session, orNonewhen no such session was resolved (local CDP override, no provider configured, or a Browser Use direct-API config that manages its own session).browser_exec()now runs the CLI via a bounded poll loop (subprocess.Popen+ background stdout/stderr drain threads, to avoid a pipe-buffer deadlock during a long run) instead of a single blockingsubprocess.run(). While waiting, it callsbrowser_tool._update_session_activity()every_CLOUD_SESSION_HEARTBEAT_INTERVAL_S(20s — well under the 120s default inactivity threshold) whenever a cloud session was resolved, refreshing its activity timestamp for the whole run instead of only at resolution time.session=/BU_NAMEruns are unaffected —cloud_session_task_idisNonethere, so no heartbeat calls are made.Test plan
test_long_exec_keeps_cloud_session_activity_fresh: drives a fake CLI subprocess that sleeps across several (tiny, monkeypatched) heartbeat intervals with a resolved cloud-provider session, and assertsbrowser_tool._update_session_activityis called repeatedly (not just once) with the correct task id.tools/browser_use_cli.pyfix and confirmed the new test fails against pre-fix code (the heartbeat mechanism/constant doesn't exist at all pre-fix)._resolve_backend_cdpunit tests for its new(error, cloud_session_task_id)return shape;test_cloud_provider_session_exportedadditionally asserts the returned key matches the resolved task id.tests/tools/test_browser_use_cli.pysuite (68 tests, including the pre-existing timeout, non-zero-exit, stderr-capture, stdin-piping, session-naming, and screenshot-detection tests, all of which exercise the real subprocess path against a fake CLI script): all pass, confirming thePopenrewrite is behaviorally equivalent to the oldsubprocess.runfor every existing case.test_browser_orphan_reaper.py,test_browser_cleanup.py,test_browser_use_session_expiry.py,test_browser_cloud_provider_cache.py,test_browser_command_timeout_race.py,test_browser_provider_plugins.py): all pass.ruff checkclean on both changed files.Conflict note
Two open PRs touch
tools/browser_use_cli.py:_base_subprocess_env()— no line/function overlap with this PR.browser_exec(), right after the URL-safety check and before this PR's changes begin — no semantic overlap (the Camofox path returns before reaching the CDP-backend/subprocess-execution code this PR touches), but the insertion point is textually close, so a rebase may need light manual reconciliation if both land.