refactor(tools): browser_tool — compact docstrings/comments across browser_tool_* modules (keep every WHY)
This commit is contained in:
@@ -200,11 +200,9 @@ def _browser_cfg(key: str, default, parse, log_label: str):
|
||||
|
||||
|
||||
def _cached_browser_cfg(cache_name: str, flag_name: str, key: str, default, parse, log_label: str):
|
||||
"""Process-cached ``_browser_cfg`` read (cache cleared by ``cleanup_all_browsers``).
|
||||
|
||||
The value is stored BEFORE the resolved flag flips so a concurrent reader can
|
||||
never observe ``resolved=True`` with a ``None`` cache.
|
||||
"""
|
||||
"""Process-cached ``_browser_cfg`` read (cleared by ``cleanup_all_browsers``). The value is
|
||||
stored BEFORE the resolved flag flips so a concurrent reader never sees ``resolved=True``
|
||||
with a ``None`` cache."""
|
||||
g = globals()
|
||||
if g[flag_name] and g[cache_name] is not None:
|
||||
return g[cache_name]
|
||||
@@ -315,11 +313,8 @@ _PRIVATE_HOST_SUFFIXES = (".localhost", ".local", ".lan", ".internal")
|
||||
|
||||
def _url_is_private(url: str) -> bool:
|
||||
"""True when the URL's host is (or resolves to) a private/LAN/loopback/CGNAT address.
|
||||
|
||||
Routing oracle only: DNS failures are NOT private (fall through to the
|
||||
configured backend, which surfaces the DNS error). Obvious names short-circuit
|
||||
the DNS hop; bare ``localhost`` resolves via /etc/hosts otherwise.
|
||||
"""
|
||||
Routing oracle only: DNS failures are NOT private (the configured backend surfaces the
|
||||
error); obvious names short-circuit the DNS hop."""
|
||||
import ipaddress
|
||||
import socket
|
||||
from urllib.parse import urlparse
|
||||
@@ -350,13 +345,10 @@ def _url_is_private(url: str) -> bool:
|
||||
|
||||
|
||||
def _navigation_session_key(task_id: str, url: str) -> str:
|
||||
"""Session key that should handle ``url`` for ``task_id``.
|
||||
|
||||
``f"{task_id}::local"`` (hybrid routing: local Chromium sidecar while the cloud
|
||||
session keeps serving public URLs) only when ALL hold: cloud provider
|
||||
configured, ``browser.auto_local_for_private_urls`` on, private URL, no CDP
|
||||
override (it owns the whole session), Camofox off (already local-only).
|
||||
"""
|
||||
"""Session key that should handle ``url`` for ``task_id``: ``f"{task_id}::local"`` (hybrid
|
||||
local sidecar while the cloud session keeps serving public URLs) only when ALL hold —
|
||||
cloud provider configured, ``browser.auto_local_for_private_urls`` on, private URL, no
|
||||
CDP override (it owns the whole session), Camofox off (already local-only)."""
|
||||
if task_id is None:
|
||||
task_id = "default"
|
||||
hybrid = (
|
||||
@@ -387,10 +379,8 @@ def _session_info_owned_by_task(session_info: Dict[str, Any], task_id: str, sess
|
||||
|
||||
def _last_session_key(task_id: str) -> str:
|
||||
"""Session key a non-nav tool must use: the one that served the task's last navigation.
|
||||
|
||||
If that session was cleaned up or its ownership no longer matches, fail closed by
|
||||
dropping the stale binding rather than recreating or mutating the wrong browser.
|
||||
"""
|
||||
If it was cleaned up or ownership no longer matches, fail closed by dropping the stale
|
||||
binding rather than recreating or mutating the wrong browser."""
|
||||
if task_id is None:
|
||||
task_id = "default"
|
||||
recorded_key = _last_active_session_key.get(task_id)
|
||||
@@ -686,15 +676,11 @@ def _secret_url_error(url: str) -> Optional[dict]:
|
||||
|
||||
|
||||
def _url_policy_error(url: str, *, auto_local: bool = False) -> Optional[dict]:
|
||||
"""Backend-aware URL checks on an already-normalized URL; None if allowed.
|
||||
|
||||
Ordered floors: (1) credential-like query params refused for cloud backends
|
||||
(third-party readers), allowed for local and the hybrid sidecar; (2) cloud
|
||||
metadata / IMDS refused UNCONDITIONALLY for every backend (a local Chromium on
|
||||
a cloud VM still reaches the host IMDS); (3) private addresses refused unless
|
||||
local, auto-routed to the sidecar, or ``browser.allow_private_urls``;
|
||||
(4) website policy allow/deny lists.
|
||||
"""
|
||||
"""Backend-aware URL checks on an already-normalized URL; None if allowed. Ordered floors:
|
||||
(1) credential-like query params refused for cloud backends (third-party readers);
|
||||
(2) cloud metadata / IMDS refused UNCONDITIONALLY (a local Chromium on a cloud VM still
|
||||
reaches the host IMDS); (3) private addresses refused unless local, sidecar-routed, or
|
||||
``browser.allow_private_urls``; (4) website policy allow/deny lists."""
|
||||
local = _is_local_backend()
|
||||
sensitive_query_key = _sensitive_query_param_name(url)
|
||||
if sensitive_query_key and not local and not auto_local:
|
||||
@@ -737,13 +723,10 @@ _BOT_DETECTION_TITLE_PATTERNS = (
|
||||
|
||||
|
||||
def _post_redirect_block(nav_session_key: str, url: str, final_url: str, auto_local_this_nav: bool) -> Optional[str]:
|
||||
"""Post-redirect SSRF check; blocked JSON payload or None.
|
||||
|
||||
A redirect onto a private/internal address would let later snapshots read
|
||||
internal content, so the page is navigated to about:blank first. The metadata
|
||||
floor fires for every backend; the private-address check is skipped for local
|
||||
backends, the hybrid sidecar, and ``browser.allow_private_urls``.
|
||||
"""
|
||||
"""Post-redirect SSRF check; blocked JSON payload or None. The page is moved to about:blank
|
||||
first so later snapshots can't read the internal content. The metadata floor fires for
|
||||
every backend; the private-address check is skipped for local, the sidecar, and
|
||||
``browser.allow_private_urls``."""
|
||||
if not final_url or final_url == url:
|
||||
return None
|
||||
if _is_always_blocked_url(final_url):
|
||||
@@ -762,11 +745,8 @@ def _post_redirect_block(nav_session_key: str, url: str, final_url: str, auto_lo
|
||||
|
||||
|
||||
def _snapshot_fields(snap_result: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""``snapshot`` + ``element_count`` response fields from a successful snapshot result.
|
||||
|
||||
Oversized snapshots truncate at line boundaries; the full tree is stored to
|
||||
cache/web with a read_file paging note (same pattern as web_extract — no LLM).
|
||||
"""
|
||||
"""``snapshot`` + ``element_count`` fields from a successful snapshot result; oversized
|
||||
snapshots truncate at line boundaries with the full tree stored for read_file paging."""
|
||||
data = snap_result.get("data", {})
|
||||
snapshot_text = data.get("snapshot", "")
|
||||
refs = data.get("refs", {})
|
||||
@@ -795,11 +775,8 @@ def _attach_auto_snapshot(response: Dict[str, Any], nav_session_key: str) -> Non
|
||||
|
||||
def browser_navigate(url: str, task_id: Optional[str] = None) -> str:
|
||||
"""Navigate to ``url``; JSON with title, compact snapshot and, on first nav, stealth features.
|
||||
|
||||
Hybrid routing decides BEFORE the safety checks whether this URL goes to a local
|
||||
Chromium sidecar; the cloud provider never sees the URL then, so the
|
||||
private-address checks are relaxed for it.
|
||||
"""
|
||||
Hybrid routing decides BEFORE the safety checks whether this URL goes to a local sidecar
|
||||
(the cloud provider never sees it then, so the private-address checks are relaxed)."""
|
||||
url, safety_error = _secret_url_error_normalized(url)
|
||||
if safety_error is not None:
|
||||
return json.dumps(safety_error)
|
||||
@@ -1103,12 +1080,9 @@ def _eval_result_or_blocked(effective_task_id: str, parsed: Any, result: Dict[st
|
||||
|
||||
def _eval_supervisor_fast_path(effective_task_id: str, expression: str) -> Optional[str]:
|
||||
"""``Runtime.evaluate`` on the CDP supervisor's persistent WebSocket (no subprocess cost).
|
||||
|
||||
Returns tool JSON when the supervisor gave a definitive answer (a value, a
|
||||
blocked private page, or a real JS-side exception — NOT retried through the
|
||||
subprocess, that would just reproduce it slower), or None to fall through to the
|
||||
subprocess path (no supervisor, supervisor-side failure, import error).
|
||||
"""
|
||||
Tool JSON when the supervisor gave a definitive answer (value, blocked page, or a real
|
||||
JS-side exception — NOT retried via subprocess, that would just reproduce it slower);
|
||||
None to fall through to the subprocess path."""
|
||||
try:
|
||||
from tools.browser_supervisor import SUPERVISOR_REGISTRY # type: ignore[import-not-found]
|
||||
supervisor = SUPERVISOR_REGISTRY.get(effective_task_id)
|
||||
@@ -1147,12 +1121,9 @@ def _eval_failure_response(result: Dict[str, Any]) -> str:
|
||||
|
||||
|
||||
def _browser_eval(expression: str, task_id: Optional[str] = None) -> str:
|
||||
"""Evaluate a JavaScript expression in the page context and return the result.
|
||||
|
||||
Private-network guard, both halves gated on the same condition: the literal
|
||||
pre-scan closes direct fetches (``fetch('http://127.0.0.1/...')`` never updates
|
||||
``location.href``); the post-eval page-URL recheck closes navigate-then-read.
|
||||
"""
|
||||
"""Evaluate JS in the page context. Private-network guard in two halves: the literal
|
||||
pre-scan closes direct fetches (they never update ``location.href``); the post-eval
|
||||
page-URL recheck closes navigate-then-read."""
|
||||
effective_task_id = _last_session_key(task_id or "default")
|
||||
|
||||
if _eval_ssrf_guard_active(effective_task_id):
|
||||
@@ -1309,13 +1280,9 @@ def _capture_vision_screenshot(effective_task_id: str, annotate: bool, screensho
|
||||
|
||||
|
||||
def browser_vision(question: str, annotate: bool = False, task_id: Optional[str] = None) -> Union[str, Dict[str, Any]]:
|
||||
"""Screenshot the current page for visual inspection (CAPTCHAs, images, layouts).
|
||||
|
||||
Native-vision models get the screenshot attached to the conversation (multimodal
|
||||
tool-result envelope); otherwise the auxiliary vision model returns a text
|
||||
analysis. The file is saved persistently and its path returned (MEDIA:<path>).
|
||||
``annotate`` overlays numbered [N] labels on interactive elements.
|
||||
"""
|
||||
"""Screenshot the current page for visual inspection. Native-vision models get the image
|
||||
attached to the conversation; otherwise the auxiliary vision model returns a text
|
||||
analysis. The file is kept and its path returned (MEDIA:<path>)."""
|
||||
if _is_camofox_mode():
|
||||
return _camofox("camofox_vision", question, annotate, task_id)
|
||||
|
||||
@@ -1324,14 +1291,12 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str]
|
||||
screenshots_dir = get_hermes_dir("cache/screenshots", "browser_screenshots")
|
||||
screenshot_path = screenshots_dir / f"browser_screenshot_{uuid_mod.uuid4().hex}.png"
|
||||
effective_task_id = _last_session_key(task_id or "default")
|
||||
|
||||
blocked = _blocked_private_page_content(effective_task_id)
|
||||
if blocked is not None:
|
||||
return blocked
|
||||
|
||||
_lp_prerouted, _lp_fallback_warning, screenshot_path = _lightpanda_vision_preroute(
|
||||
effective_task_id, annotate, screenshot_path,
|
||||
)
|
||||
effective_task_id, annotate, screenshot_path)
|
||||
result: Dict[str, Any] = {}
|
||||
try:
|
||||
screenshots_dir.mkdir(parents=True, exist_ok=True)
|
||||
@@ -1340,11 +1305,9 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str]
|
||||
effective_task_id, annotate, screenshot_path, _lp_prerouted)
|
||||
if error is not None:
|
||||
return error
|
||||
|
||||
# Native image routing for the active main model: attach the screenshot
|
||||
# directly instead of describing it through an aux vision LLM (no information loss).
|
||||
# Native image routing: attach the screenshot directly instead of describing it
|
||||
# through an aux vision LLM (no information loss).
|
||||
from tools.vision_tools import _should_use_native_vision_fast_path
|
||||
|
||||
if _should_use_native_vision_fast_path():
|
||||
return _native_vision_result(screenshot_path, question, annotate, result, _lp_fallback_warning)
|
||||
|
||||
@@ -1355,10 +1318,9 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str]
|
||||
if annotate and result.get("data", {}).get("annotations"):
|
||||
response_data["annotations"] = result["data"]["annotations"]
|
||||
return _dumps(response_data)
|
||||
|
||||
except Exception as e:
|
||||
# Keep a captured screenshot — the failure is in the analysis, not the
|
||||
# capture, and deleting it loses evidence. The 24-hour cleanup bounds disk growth.
|
||||
# Keep a captured screenshot — the failure is in the analysis, not the capture,
|
||||
# and deleting it loses evidence. The 24-hour cleanup bounds disk growth.
|
||||
logger.warning("browser_vision failed: %s", e, exc_info=True)
|
||||
error_info = _err(f"Error during vision analysis: {str(e)}")
|
||||
if screenshot_path.exists():
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
"""Browser session lifecycle: inactivity janitor, orphan reaper, per-session teardown, atexit emergency cleanup.
|
||||
"""Browser session lifecycle: inactivity janitor, orphan reaper, per-session teardown, atexit cleanup.
|
||||
|
||||
Split out of ``tools/browser_tool.py``; every name is re-imported there. Origin
|
||||
symbols and module state are read/written through ``_bt`` (the origin module,
|
||||
the :data:`tools.browser_tool_origin.origin` proxy) so
|
||||
``patch("tools.browser_tool.X")`` is honoured and no import cycle exists.
|
||||
Split out of ``tools/browser_tool.py`` (every name re-imported there); origin symbols
|
||||
are read through the ``_bt`` proxy so ``patch("tools.browser_tool.X")`` is honoured.
|
||||
"""
|
||||
|
||||
import contextlib
|
||||
@@ -189,14 +187,12 @@ def _write_owner_pid(socket_dir: str, session_name: str) -> None:
|
||||
|
||||
def _verify_reapable_browser_daemon(daemon_pid: int, socket_dir: str,
|
||||
session_name: str) -> bool:
|
||||
"""Confirm a live PID is genuinely *this* session's agent-browser daemon.
|
||||
"""Confirm a live PID is genuinely *this* session's agent-browser daemon (fail-closed).
|
||||
|
||||
The ``.pid`` file lives in a world-writable temp dir and is written by the
|
||||
daemon: a same-user actor can plant one pointing at a victim PID, or a recycled
|
||||
PID can land on an unrelated process — and reaping is a *tree* kill. Two
|
||||
checks must pass: (1) identity — ``agent-browser`` in name or cmdline;
|
||||
(2) binding — the socket dir in the cmdline or ``AGENT_BROWSER_SOCKET_DIR`` in
|
||||
its environ (the real spoof defense). Fail-closed on any ambiguity.
|
||||
The ``.pid`` file sits in a world-writable temp dir: a planted or recycled PID
|
||||
would turn the tree-kill into an arbitrary-process DoS. Both must pass:
|
||||
(1) identity — ``agent-browser`` in name/cmdline; (2) binding — the socket dir in
|
||||
the cmdline or ``AGENT_BROWSER_SOCKET_DIR`` in its environ (the real spoof defense).
|
||||
"""
|
||||
def refuse(reason: str, *args) -> bool:
|
||||
_bt.logger.warning("Refusing to reap browser daemon PID %d (session %s): " + reason,
|
||||
@@ -236,11 +232,8 @@ def _verify_reapable_browser_daemon(daemon_pid: int, socket_dir: str,
|
||||
|
||||
def _socket_dir_idle_seconds(socket_dir: str) -> Optional[float]:
|
||||
"""Seconds since anything in ``socket_dir`` was last written; None if unknown (fail safe).
|
||||
|
||||
Every command writes ``_stdout_<cmd>`` there, so the newest mtime is a
|
||||
last-activity marker surviving hermes restarts. The dir's own mtime is not
|
||||
enough — rewriting an existing file doesn't touch it — so entries are scanned.
|
||||
"""
|
||||
Every command rewrites ``_stdout_<cmd>`` there — a restart-proof activity marker — and
|
||||
rewriting doesn't touch the dir mtime, so entries are scanned too."""
|
||||
try:
|
||||
latest = os.path.getmtime(socket_dir)
|
||||
except OSError:
|
||||
@@ -295,13 +288,11 @@ def _terminate_verified_daemon(daemon_pid: int, session_name: str, log) -> bool:
|
||||
def _reap_socket_dir(socket_dir: str, session_name: str, tracked_names: set) -> bool:
|
||||
"""Reap one ``agent-browser-<session>`` dir if orphaned; True when a daemon was killed.
|
||||
|
||||
Ownership priority: (1) a live ``owner_pid`` means another hermes process owns
|
||||
it — leave it alone UNLESS it is untracked here and idle past
|
||||
``BROWSER_ORPHAN_GRACE_SECONDS`` (owner-alive alone made leaked daemons
|
||||
immortal); (2) no owner_pid (legacy) falls back to this process's tracking.
|
||||
A pidless dir is only stale after the grace period — deleting it immediately
|
||||
races the creator's first stdout open. The daemon PID is identity-verified
|
||||
before a tree-kill and refused without a start-time fingerprint.
|
||||
A live ``owner_pid`` means another hermes process owns it — leave it UNLESS untracked
|
||||
here and idle past ``BROWSER_ORPHAN_GRACE_SECONDS`` (owner-alive alone made leaked
|
||||
daemons immortal); no owner_pid (legacy) falls back to this process's tracking. A
|
||||
pidless dir is only stale after the grace period (deleting it immediately races the
|
||||
creator's first stdout open). The PID is identity-verified before any tree-kill.
|
||||
"""
|
||||
owner_pid, owner_alive = _bt._owner_pid_alive(socket_dir, session_name)
|
||||
if owner_alive is True:
|
||||
@@ -349,13 +340,9 @@ def _reap_socket_dir(socket_dir: str, session_name: str, tracked_names: set) ->
|
||||
|
||||
|
||||
def _reap_orphaned_browser_sessions():
|
||||
"""Kill agent-browser daemons whose owning hermes process is gone.
|
||||
|
||||
An unclean exit (SIGKILL, crash, gateway restart) loses ``_active_sessions``
|
||||
but node + Chromium keep running. Scans the tmp dir for ``agent-browser-*``
|
||||
socket dirs and applies ``_reap_socket_dir``'s ownership rules. Safe from any
|
||||
context — atexit, cleanup thread, or on demand.
|
||||
"""
|
||||
"""Kill agent-browser daemons whose owning hermes process is gone (an unclean exit loses
|
||||
``_active_sessions`` but node + Chromium keep running). Scans the tmp dir for
|
||||
``agent-browser-*`` socket dirs; safe from any context."""
|
||||
import glob
|
||||
|
||||
# Lightpanda servers keep their own records (no socket dir); sweep them with the
|
||||
@@ -429,9 +416,8 @@ def _stop_browser_cleanup_thread():
|
||||
|
||||
|
||||
def _update_session_activity(task_id: str):
|
||||
"""Touch the activity timestamp; records the owning Hermes home on first sight so
|
||||
the process-global janitor tears down under the owner's scope. Deliberately does
|
||||
NOT reset ``_cleanup_failures`` — only a successful cleanup does."""
|
||||
"""Touch the activity timestamp and record the owning Hermes home on first sight (the
|
||||
janitor tears down under the owner's scope). Does NOT reset ``_cleanup_failures``."""
|
||||
with _bt._cleanup_lock:
|
||||
_bt._session_last_activity[task_id] = time.time()
|
||||
_bt._session_owner_homes.setdefault(task_id, str(get_hermes_home()))
|
||||
@@ -440,12 +426,10 @@ def _update_session_activity(task_id: str):
|
||||
def _kill_process_tree(proc: "subprocess.Popen") -> None:
|
||||
"""Best-effort kill of *proc* and every descendant; never raises.
|
||||
|
||||
``Popen.kill()`` only signals the direct child; npm/npx helpers and
|
||||
agent-browser's detached daemon grandchild keep a capture pipe open so
|
||||
``communicate()`` never sees EOF (no non-blocking read on Windows), so the whole
|
||||
tree must go. No grace period: the caller already burned its timeout.
|
||||
Delegates to :func:`agent.deadline.kill_process_tree`, falling back to
|
||||
:func:`_legacy_kill_process_tree` on any failure.
|
||||
``Popen.kill()`` only signals the direct child; npm/npx helpers and the detached
|
||||
daemon grandchild keep a capture pipe open so ``communicate()`` never sees EOF, so
|
||||
the whole tree must go (no grace: the caller already burned its timeout). Delegates
|
||||
to :func:`agent.deadline.kill_process_tree`, falling back to the legacy kill.
|
||||
"""
|
||||
try:
|
||||
from agent.deadline import kill_process_tree as _deadline_kill_tree
|
||||
@@ -457,8 +441,7 @@ def _kill_process_tree(proc: "subprocess.Popen") -> None:
|
||||
|
||||
def _legacy_kill_process_tree(proc: "subprocess.Popen") -> None:
|
||||
"""Local tree-kill (SIGTERM then SIGKILL to the process group) — fallback when
|
||||
agent.deadline is unavailable. Differs from hermes_cli._subprocess_compat's
|
||||
group-leader-only variant, and tests pin this sequence."""
|
||||
agent.deadline is unavailable; tests pin this signal sequence."""
|
||||
if os.name == "nt":
|
||||
try:
|
||||
subprocess.run(["taskkill", "/PID", str(proc.pid), "/T", "/F"],
|
||||
@@ -530,12 +513,9 @@ def _cleanup_old_recordings(max_age_hours=72):
|
||||
|
||||
|
||||
def _drop_last_active_binding(task_id: str) -> None:
|
||||
"""Drop stale last-active ownership after cleaning ``task_id``.
|
||||
|
||||
A bare task drops its binding; a sidecar drops it only if that sidecar was
|
||||
still the recorded owner — a later click can't resurrect a cleaned sidecar on
|
||||
about:blank while a primary-session binding is preserved.
|
||||
"""
|
||||
"""Drop stale last-active ownership after cleaning ``task_id``: a bare task always, a
|
||||
sidecar only if it was still the recorded owner (a later click must not resurrect a
|
||||
cleaned sidecar while a primary-session binding is preserved)."""
|
||||
bare_task_id = _bt._bare_task_id_for_session_key(task_id)
|
||||
if bare_task_id == task_id or _bt._last_active_session_key.get(bare_task_id) == task_id:
|
||||
_bt._last_active_session_key.pop(bare_task_id, None)
|
||||
@@ -558,8 +538,8 @@ def cleanup_browser(task_id: Optional[str] = None) -> None:
|
||||
|
||||
|
||||
def _kill_verified_daemon(socket_dir: str, session_name: str) -> bool:
|
||||
"""Tree-kill the daemon in ``<socket_dir>/<session>.pid`` if verifiably ours
|
||||
(identity check + start-time fingerprint). True when a kill was issued. Never raises."""
|
||||
"""Tree-kill the daemon in ``<socket_dir>/<session>.pid`` if verifiably ours; True when
|
||||
a kill was issued. Never raises."""
|
||||
pid_file = os.path.join(socket_dir, f"{session_name}.pid")
|
||||
if not os.path.isfile(pid_file):
|
||||
return False
|
||||
@@ -579,11 +559,8 @@ def _kill_verified_daemon(socket_dir: str, session_name: str) -> bool:
|
||||
|
||||
|
||||
def _release_session_resources(task_id: str, session_info: Dict[str, Any]) -> None:
|
||||
"""Untrack ``task_id``, close its cloud provider session, kill its daemon.
|
||||
|
||||
Unconditional tail of ``_cleanup_single_browser_session``; also the whole of the
|
||||
janitor's force-reap path, which skips the polite ``close`` that kept failing.
|
||||
"""
|
||||
"""Untrack ``task_id``, close its cloud provider session, kill its daemon — the
|
||||
unconditional tail of a teardown, and the whole of the janitor's force-reap path."""
|
||||
bb_session_id = session_info.get("bb_session_id", "unknown")
|
||||
_forget_session_tracking(task_id, session=True)
|
||||
|
||||
@@ -604,8 +581,7 @@ def _release_session_resources(task_id: str, session_info: Dict[str, Any]) -> No
|
||||
|
||||
|
||||
def _force_reap_browser_session(task_id: str) -> None:
|
||||
"""Janitor last resort after repeated failures: skip the failing ``close``
|
||||
round-trips, go straight to ``_release_session_resources``."""
|
||||
"""Janitor last resort: skip the failing ``close`` round-trips, release resources directly."""
|
||||
_bt._stop_cdp_supervisor(task_id)
|
||||
with _bt._cleanup_lock:
|
||||
session_info = _bt._active_sessions.get(task_id)
|
||||
@@ -618,12 +594,10 @@ def _force_reap_browser_session(task_id: str) -> None:
|
||||
|
||||
def _cleanup_single_browser_session(task_id: str) -> None:
|
||||
"""Reap a single browser session by its exact session key."""
|
||||
# Stop the CDP supervisor FIRST so our WebSocket closes before the backend
|
||||
# tears down the CDP endpoint.
|
||||
_bt._stop_cdp_supervisor(task_id)
|
||||
_bt._stop_cdp_supervisor(task_id) # close our WebSocket BEFORE the backend tears down the endpoint
|
||||
|
||||
# Camofox: skip the full close when managed persistence is on — the profile
|
||||
# (session cookies) must survive across tasks; the inactivity reaper still frees idle resources.
|
||||
# Camofox: managed persistence keeps the profile (cookies) across tasks; skip the full
|
||||
# close then — the inactivity reaper still frees idle resources.
|
||||
if _bt._is_camofox_mode():
|
||||
def _camofox_cleanup():
|
||||
from tools.browser_camofox import camofox_close, camofox_soft_cleanup
|
||||
@@ -634,8 +608,7 @@ def _cleanup_single_browser_session(task_id: str) -> None:
|
||||
_bt.logger.debug("cleanup_browser called for task_id: %s", task_id)
|
||||
_bt.logger.debug("Active sessions: %s", list(_bt._active_sessions.keys()))
|
||||
|
||||
# Look up (under lock) but don't remove yet — _run_browser_command needs the
|
||||
# entry to build the close command.
|
||||
# Look up but don't remove yet — _run_browser_command needs the entry for ``close``.
|
||||
with _bt._cleanup_lock:
|
||||
session_info = _bt._active_sessions.get(task_id)
|
||||
|
||||
@@ -646,9 +619,8 @@ def _cleanup_single_browser_session(task_id: str) -> None:
|
||||
_bt.logger.debug("Found session for task %s: bb_session_id=%s", task_id, session_info.get("bb_session_id", "unknown"))
|
||||
_bt._maybe_stop_recording(task_id) # saves the file before close
|
||||
|
||||
# Lightpanda sessions are processes Hermes spawned (no daemon to ``close``).
|
||||
# An expired cloud CDP URL cannot accept a close, and feeding it through
|
||||
# _get_session_info() would try to renew the session recursively mid-cleanup.
|
||||
# Lightpanda sessions have no daemon to ``close``; an expired cloud CDP URL cannot
|
||||
# accept one and would make _get_session_info() renew the session mid-cleanup.
|
||||
if (session_info.get("features") or {}).get("lightpanda"):
|
||||
try:
|
||||
from tools.browser_lightpanda import stop_lightpanda
|
||||
@@ -675,8 +647,7 @@ def cleanup_all_browsers() -> None:
|
||||
for task_id in task_ids:
|
||||
_bt.cleanup_browser(task_id)
|
||||
|
||||
# Tear down CDP supervisors for all tasks so background threads exit.
|
||||
try:
|
||||
try: # tear down CDP supervisors so background threads exit
|
||||
from tools.browser_supervisor import SUPERVISOR_REGISTRY # type: ignore[import-not-found]
|
||||
SUPERVISOR_REGISTRY.stop_all()
|
||||
except Exception:
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
"""Origin-module lookup shared by the ``tools.browser_tool_*`` extraction modules.
|
||||
|
||||
Extracted code must keep reading its origin's symbols (helpers, module state,
|
||||
``logger``) *through* ``tools.browser_tool`` so ``patch("tools.browser_tool.X")``
|
||||
in tests is honoured. The extraction modules must not import ``tools.browser_tool``
|
||||
at import time (cycle), so they call :func:`origin_module` lazily per call.
|
||||
Extracted code must read its origin's symbols *through* ``tools.browser_tool`` so
|
||||
``patch("tools.browser_tool.X")`` is honoured, and must not import it at import time
|
||||
(cycle) — hence the lazy :func:`origin_module` and the :data:`origin` proxy.
|
||||
"""
|
||||
|
||||
import sys
|
||||
@@ -13,9 +12,8 @@ _ORIGIN_NAME = "tools.browser_tool"
|
||||
|
||||
|
||||
class _NamespaceView:
|
||||
"""Live attribute view over a module namespace whose module object is gone (a
|
||||
test purged ``sys.modules`` after importing the origin): the old code still runs
|
||||
with its own globals, so reads/writes must hit *those*, not a fresh re-import."""
|
||||
"""Live view over a module namespace whose module object is gone (a test purged
|
||||
``sys.modules``): the old code still runs with its own globals, so hit *those*."""
|
||||
|
||||
__slots__ = ("_g",)
|
||||
|
||||
@@ -43,15 +41,11 @@ def _module_for_globals(g: dict):
|
||||
|
||||
|
||||
def origin_module(_depth: int = 2):
|
||||
"""Return the ``tools.browser_tool`` instance the *calling* moved function belongs to.
|
||||
|
||||
Resolution order, mirroring what in-file code would see: (1) the nearest
|
||||
enclosing frame executing ``tools.browser_tool`` code — binds to that exact
|
||||
module copy even after a test purged/reloaded ``sys.modules``; (2) a
|
||||
``tools.browser_tool`` module referenced from a calling frame's globals;
|
||||
(3) ``sys.modules`` / the ``tools`` package attribute / a fresh import.
|
||||
``_depth`` is the frame of the moved function's caller.
|
||||
"""
|
||||
"""The ``tools.browser_tool`` instance the *calling* moved function belongs to, mirroring
|
||||
what in-file code would see: (1) the nearest enclosing frame executing origin code (the
|
||||
exact module copy, even after a test purged/reloaded ``sys.modules``); (2) an origin
|
||||
module referenced from a calling frame's globals; (3) ``sys.modules`` / ``tools``
|
||||
package attribute / fresh import. ``_depth`` is the frame of the moved function's caller."""
|
||||
try:
|
||||
start = sys._getframe(_depth)
|
||||
except ValueError: # called directly by the interpreter (atexit callback)
|
||||
@@ -78,9 +72,8 @@ def origin_module(_depth: int = 2):
|
||||
|
||||
|
||||
class _OriginProxy:
|
||||
"""Module-level ``_bt`` stand-in: every attribute read/write forwards to the
|
||||
origin module resolved at that moment, so moved functions need no per-call
|
||||
``_bt = _origin()`` line."""
|
||||
"""Module-level ``_bt`` stand-in: every attribute read/write forwards to the origin
|
||||
module resolved at that moment."""
|
||||
|
||||
__slots__ = ()
|
||||
|
||||
|
||||
@@ -1,11 +1,8 @@
|
||||
"""agent-browser session management: daemon spawn, per-backend session creation
|
||||
(local/lightpanda/cdp/cloud), cached session lookup, command execution with timeout
|
||||
handling and output interpretation.
|
||||
(local/lightpanda/cdp/cloud), cached lookup, command execution + output interpretation.
|
||||
|
||||
Split out of ``tools/browser_tool.py``; every name is re-imported there. Origin
|
||||
symbols and module state are read/written through ``_bt`` (the origin module,
|
||||
the :data:`tools.browser_tool_origin.origin` proxy) so
|
||||
``patch("tools.browser_tool.X")`` is honoured and no import cycle exists.
|
||||
Split out of ``tools/browser_tool.py`` (every name re-imported there); origin symbols
|
||||
are read through the ``_bt`` proxy so ``patch("tools.browser_tool.X")`` is honoured.
|
||||
"""
|
||||
|
||||
import json
|
||||
@@ -21,9 +18,8 @@ from tools.browser_tool_origin import origin as _bt
|
||||
|
||||
_DOCKER_PULL = "docker pull ghcr.io/nousresearch/hermes-agent:latest"
|
||||
_CHROMIUM_INSTALL = "npx agent-browser install --with-deps (or: npx playwright install --with-deps chromium)"
|
||||
_CHROMIUM_MISSING_DOCKER_HINT = (
|
||||
f"Chromium browser is missing. You're running in Docker — pull the latest image to get the bundled Chromium: {_DOCKER_PULL}"
|
||||
)
|
||||
_CHROMIUM_MISSING_DOCKER_HINT = ("Chromium browser is missing. You're running in Docker — pull the latest image "
|
||||
f"to get the bundled Chromium: {_DOCKER_PULL}")
|
||||
_CHROMIUM_MISSING_HINT = f"Chromium browser is missing. Install it with: {_CHROMIUM_INSTALL}"
|
||||
|
||||
|
||||
@@ -42,11 +38,8 @@ def _needs_chromium_sandbox_bypass() -> bool:
|
||||
|
||||
def _apply_chromium_sandbox_args(browser_env: Dict[str, str]) -> None:
|
||||
"""Add required Chromium sandbox flags without overriding user settings."""
|
||||
if (
|
||||
"AGENT_BROWSER_ARGS" not in browser_env
|
||||
and "AGENT_BROWSER_CHROME_FLAGS" not in browser_env
|
||||
and _bt._needs_chromium_sandbox_bypass()
|
||||
):
|
||||
if ("AGENT_BROWSER_ARGS" not in browser_env and "AGENT_BROWSER_CHROME_FLAGS" not in browser_env
|
||||
and _bt._needs_chromium_sandbox_bypass()):
|
||||
_bt.logger.debug("browser: sandbox bypass needed (root/docker/AppArmor userns) — injecting --no-sandbox")
|
||||
browser_env["AGENT_BROWSER_ARGS"] = "--no-sandbox,--disable-dev-shm-usage"
|
||||
|
||||
@@ -95,14 +88,12 @@ def _format_browser_timeout_error(
|
||||
|
||||
|
||||
def _agent_browser_argv(browser_cmd: str) -> list:
|
||||
"""Command prefix to invoke agent-browser (concrete binary or npx sentinel).
|
||||
"""Command prefix to invoke agent-browser (concrete binary, or the npx sentinel expanded).
|
||||
|
||||
Concrete paths stay a single argv item; only the npx sentinel expands. npx is
|
||||
resolved through the same PATH cascade as ``_find_agent_browser`` (a bare
|
||||
``shutil.which("npx")`` would let a broken system npx shadow a healthy managed
|
||||
one); if absent the bare name gives a readable ``FileNotFoundError: 'npx'``.
|
||||
``--ignore-scripts``: AGENT_BROWSER_NPX_SPEC is a floating range — a compromised
|
||||
future patch must not run install-time scripts.
|
||||
npx is resolved through the same PATH cascade as ``_find_agent_browser`` (a bare
|
||||
``which("npx")`` would let a broken system npx shadow a healthy managed one); if
|
||||
absent the bare name gives a readable ``FileNotFoundError``. ``--ignore-scripts``:
|
||||
the spec is a floating range — a compromised future patch must not run install scripts.
|
||||
"""
|
||||
if _bt._is_npx_agent_browser_sentinel(browser_cmd):
|
||||
_npx_bin = _bt._resolve_npx_bin() or "npx"
|
||||
@@ -111,12 +102,9 @@ def _agent_browser_argv(browser_cmd: str) -> list:
|
||||
|
||||
|
||||
def _prepare_session_socket_dir(session_name: str) -> str:
|
||||
"""Create the per-session socket dir and claim it with our PID.
|
||||
|
||||
Per-session dirs keep parallel workers from fighting over the default socket
|
||||
path. The owner_pid file is written BEFORE first use: another hermes process's
|
||||
orphan reaper rmtree's any ownerless agent-browser-* dir in the shared tmpdir.
|
||||
"""
|
||||
"""Create the per-session socket dir (parallel workers must not share one) and claim it
|
||||
with our PID BEFORE first use — another hermes process's orphan reaper rmtree's any
|
||||
ownerless agent-browser-* dir in the shared tmpdir."""
|
||||
socket_dir = os.path.join(_bt._socket_safe_tmpdir(), f"agent-browser-{session_name}")
|
||||
os.makedirs(socket_dir, mode=0o700, exist_ok=True)
|
||||
_bt._write_owner_pid(socket_dir, session_name)
|
||||
@@ -124,10 +112,9 @@ def _prepare_session_socket_dir(session_name: str) -> str:
|
||||
|
||||
|
||||
def _agent_browser_command_env(socket_dir: str) -> Dict[str, str]:
|
||||
"""Credential-scrubbed env for one agent-browser command: discovery-time PATH
|
||||
fallbacks, the session socket dir, and daemon-side idle self-termination
|
||||
(``AGENT_BROWSER_IDLE_TIMEOUT_MS``, agent-browser 0.24+) mirroring the Python
|
||||
janitor — unless the user set it explicitly."""
|
||||
"""Credential-scrubbed env for one command: PATH fallbacks, the session socket dir, and
|
||||
daemon-side idle self-termination (agent-browser 0.24+) mirroring the Python janitor
|
||||
unless the user set ``AGENT_BROWSER_IDLE_TIMEOUT_MS`` explicitly."""
|
||||
env = _bt._build_browser_env()
|
||||
env["PATH"] = _bt._merge_browser_path(env.get("PATH", ""))
|
||||
env["AGENT_BROWSER_SOCKET_DIR"] = socket_dir
|
||||
@@ -137,14 +124,12 @@ def _agent_browser_command_env(socket_dir: str) -> Dict[str, str]:
|
||||
|
||||
|
||||
def _popen_agent_browser(argv: List[str], env: Dict[str, str], socket_dir: str, tag: str) -> "subprocess.Popen":
|
||||
"""Spawn agent-browser with stdout/stderr redirected to ``socket_dir/_stdout_<tag>``.
|
||||
"""Spawn agent-browser with stdout/stderr redirected to ``socket_dir/_std{out,err}_<tag>``.
|
||||
|
||||
Temp files instead of pipes: the CLI forks a daemon that inherits its fds, so
|
||||
with pipes ``communicate()`` never sees EOF until the timeout. Windows:
|
||||
CREATE_NO_WINDOW only (NOT CREATE_NEW_PROCESS_GROUP — on 3.11 it cancels
|
||||
asyncio's running task and surfaces as KeyboardInterrupt), STARTF_USESTDHANDLES
|
||||
so the child gets ONLY our three handles (leaked console handles make the Rust
|
||||
daemon grandchild die silently), close_fds=True for the rest.
|
||||
Temp files, not pipes: the CLI forks a daemon that inherits its fds, so pipes never
|
||||
see EOF until the timeout. Windows: CREATE_NO_WINDOW only (CREATE_NEW_PROCESS_GROUP
|
||||
cancels asyncio's running task on 3.11), STARTF_USESTDHANDLES + close_fds so the child
|
||||
gets ONLY our three handles (leaked console handles kill the Rust daemon grandchild).
|
||||
"""
|
||||
fds = [os.open(os.path.join(socket_dir, f"_{slot}_{tag}"), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
|
||||
for slot in ("stdout", "stderr")]
|
||||
@@ -169,10 +154,10 @@ def _session_record(prefix: str, cdp_url: Optional[str], features: Dict[str, Any
|
||||
def _create_local_session(task_id: str, allow_real_profile: bool = True) -> Dict[str, str]:
|
||||
"""Local Chromium session; consented real-profile CDP attach when allowed.
|
||||
|
||||
Real-profile fails closed on resolver/launch errors — a consented user must
|
||||
never be silently downgraded to a throwaway. The hybrid private-URL sidecar
|
||||
passes ``allow_real_profile=False``: handing the user's cookie jar to an
|
||||
arbitrary internal host the model chose is a larger, unconsented exposure.
|
||||
Real-profile fails closed on resolver/launch errors (a consented user must never be
|
||||
silently downgraded to a throwaway). The hybrid private-URL sidecar passes
|
||||
``allow_real_profile=False``: the user's cookie jar must not reach an arbitrary
|
||||
internal host the model chose.
|
||||
"""
|
||||
if allow_real_profile:
|
||||
cdp_url, err = _bt._real_profile_cdp()
|
||||
@@ -183,9 +168,8 @@ def _create_local_session(task_id: str, allow_real_profile: bool = True) -> Dict
|
||||
_bt.logger.info("Created real-profile local session %s for task %s", info["session_name"], task_id)
|
||||
return info
|
||||
|
||||
# Browser Use mode drives whatever CDP endpoint it is handed; with
|
||||
# ``browser.engine: lightpanda`` that is a Hermes-spawned ``lightpanda serve``.
|
||||
# The built-in tools never reach this branch (hidden in Browser Use mode).
|
||||
# Browser Use mode + ``browser.engine: lightpanda`` drives a Hermes-spawned
|
||||
# ``lightpanda serve`` (the built-in tools are hidden in that mode).
|
||||
if _bt._is_browser_use_cli_mode() and _bt._using_lightpanda_engine():
|
||||
return _bt._create_lightpanda_session(task_id)
|
||||
|
||||
@@ -226,11 +210,8 @@ def _create_cdp_session(task_id: str, cdp_url: str) -> Dict[str, str]:
|
||||
|
||||
|
||||
def _create_cloud_session_or_fallback(task_id: str, provider) -> Dict[str, Any]:
|
||||
"""Cloud session; fall back to local Chromium (marked degraded) on failure.
|
||||
|
||||
Some providers (Browser-Use v3) return an HTTP CDP discovery URL instead of a
|
||||
raw websocket endpoint, so ``cdp_url`` is resolved here.
|
||||
"""
|
||||
"""Cloud session; fall back to local Chromium (marked degraded) on failure. ``cdp_url``
|
||||
is resolved here because some providers return an HTTP discovery URL, not a websocket."""
|
||||
try:
|
||||
session_info = provider.create_session(task_id)
|
||||
if not session_info or not isinstance(session_info, dict):
|
||||
@@ -256,10 +237,7 @@ def _create_cloud_session_or_fallback(task_id: str, provider) -> Dict[str, Any]:
|
||||
|
||||
def _create_session_for_key(task_id: str, force_local: bool) -> Dict[str, Any]:
|
||||
"""Fresh session for ``task_id`` (runs OUTSIDE the lock: cloud mode makes a network call).
|
||||
|
||||
Precedence: CDP override > hybrid local sidecar > cloud provider > local. The
|
||||
hybrid sidecar NEVER gets the real profile (see ``_create_local_session``).
|
||||
"""
|
||||
Precedence: CDP override > hybrid local sidecar (never real-profile) > cloud > local."""
|
||||
cdp_override = _bt._get_cdp_override()
|
||||
if cdp_override and not force_local:
|
||||
return _bt._create_cdp_session(task_id, cdp_override)
|
||||
@@ -272,13 +250,9 @@ def _create_session_for_key(task_id: str, force_local: bool) -> Dict[str, Any]:
|
||||
|
||||
|
||||
def _get_session_info(task_id: Optional[str] = None) -> Dict[str, Any]:
|
||||
"""Get or create session info for a session key (thread-safe).
|
||||
|
||||
A ``::local``-suffixed key (hybrid sidecar) forces local Chromium even with a
|
||||
cloud provider configured. Also starts the inactivity thread and touches
|
||||
activity tracking. Returns ``session_name`` (always) plus ``bb_session_id`` /
|
||||
``cdp_url`` for cloud sessions.
|
||||
"""
|
||||
"""Get or create session info for a session key (thread-safe); also starts the
|
||||
inactivity thread and touches activity. A ``::local`` key forces local Chromium
|
||||
even with a cloud provider configured."""
|
||||
if task_id is None:
|
||||
task_id = "default"
|
||||
|
||||
@@ -289,20 +263,16 @@ def _get_session_info(task_id: Optional[str] = None) -> Dict[str, Any]:
|
||||
existing_session = _bt._active_sessions.get(task_id)
|
||||
|
||||
def _replacement_after_teardown() -> Optional[Dict[str, Any]]:
|
||||
# Teardown removes the activity entry; the replacement must be tracked by
|
||||
# the reaper like an initial session. Another thread may already have
|
||||
# recycled and re-created it — return that live one instead of a third.
|
||||
# Teardown removes the activity entry; re-touch so the reaper tracks the
|
||||
# replacement. Another thread may already have re-created it — reuse that.
|
||||
_bt._update_session_activity(task_id)
|
||||
with _bt._cleanup_lock:
|
||||
replacement = _bt._active_sessions.get(task_id)
|
||||
if replacement is not None and replacement is not existing_session:
|
||||
return replacement
|
||||
return None
|
||||
return replacement if replacement is not None and replacement is not existing_session else None
|
||||
|
||||
if existing_session is not None:
|
||||
# Suspect-session recycle: a previous command timeout marked this cached
|
||||
# session via the SuspectableBackend adapter; the expensive recycle lives
|
||||
# here at next use, not on the timeout path (mark must stay cheap).
|
||||
# Suspect recycle: a command timeout marked this session; the expensive recycle
|
||||
# lives here at next use, not on the timeout path (mark must stay cheap).
|
||||
if not _bt._browser_session_backend(task_id).ensure_healthy():
|
||||
replacement = _replacement_after_teardown()
|
||||
if replacement is not None:
|
||||
@@ -321,20 +291,16 @@ def _get_session_info(task_id: Optional[str] = None) -> Dict[str, Any]:
|
||||
session_info = _bt._create_session_for_key(task_id, force_local)
|
||||
|
||||
with _bt._cleanup_lock:
|
||||
# Another thread may have created a session during the network call; use
|
||||
# it to avoid leaking orphan cloud sessions.
|
||||
if task_id in _bt._active_sessions:
|
||||
if task_id in _bt._active_sessions: # created concurrently during the network call — don't leak ours
|
||||
return _bt._active_sessions[task_id]
|
||||
session_info = dict(session_info)
|
||||
session_info.setdefault("session_key", task_id)
|
||||
session_info.setdefault("owner_task_id", _bt._bare_task_id_for_session_key(task_id))
|
||||
_bt._active_sessions[task_id] = session_info
|
||||
# A brand-new session is healthy by definition — drop any stale suspect flag.
|
||||
_bt._suspect_browser_sessions.pop(task_id, None)
|
||||
_bt._suspect_browser_sessions.pop(task_id, None) # brand-new session is healthy by definition
|
||||
|
||||
# Lazy-start the CDP supervisor (idempotent; swallows errors). Skip for local
|
||||
# sidecars (no CDP URL) and Lightpanda sessions (Browser Use mode hides the
|
||||
# browser_* tools that consume supervisor state; it would just idle a second CDP connection).
|
||||
# Lazy-start the CDP supervisor (idempotent). Skip local sidecars (no CDP URL) and
|
||||
# Lightpanda sessions (Browser Use mode hides the tools that consume supervisor state).
|
||||
if not force_local and not (session_info.get("features") or {}).get("lightpanda"):
|
||||
_bt._ensure_cdp_supervisor(task_id)
|
||||
|
||||
@@ -389,12 +355,9 @@ def _read_browser_daemon_pid(task_socket_dir: str, session_name: str) -> Optiona
|
||||
|
||||
|
||||
def _browser_daemon_responsive(task_socket_dir: str, probe_timeout_s: float = 1.0) -> bool:
|
||||
"""Cheap liveness probe: connect to the daemon's unix control socket.
|
||||
|
||||
A successful connect proves the accept loop is alive (the command wedged on the
|
||||
page/CDP side). Windows uses named pipes — no probe possible, so report
|
||||
unresponsive (tree-kill + respawn is the safe recovery).
|
||||
"""
|
||||
"""Cheap liveness probe: a connect to the daemon's unix control socket proves the accept
|
||||
loop is alive (the command wedged page/CDP-side). Windows named pipes can't be probed →
|
||||
report unresponsive (tree-kill + respawn is the safe recovery)."""
|
||||
if os.name == "nt":
|
||||
return False
|
||||
import socket as socket_mod
|
||||
@@ -417,17 +380,14 @@ def _browser_daemon_responsive(task_socket_dir: str, probe_timeout_s: float = 1.
|
||||
|
||||
|
||||
def _handle_browser_command_timeout(task_id: str, session_info: Dict[str, Any], task_socket_dir: str) -> None:
|
||||
"""Recover session state after a browser command timeout.
|
||||
"""Recover session state after a command timeout.
|
||||
|
||||
* Cloud / CDP: no local daemon to probe — replace the stuck client generation
|
||||
now (fresh ``session_name``, same ``bb_session_id`` so cloud cleanup works).
|
||||
* Local daemon alive (PID live, identity-verified, control socket accepts): only
|
||||
the *command* wedged; mark suspect and let next use recycle via ``ensure_healthy``.
|
||||
* Local daemon wedged/dead: it cannot service a clean close and its Chromium
|
||||
children would leak — tree-kill and evict now.
|
||||
|
||||
Both local branches ``mark_suspect`` first (cheap, lock-free) so the
|
||||
poisoned-cache invariant holds even if eviction races another thread's replacement.
|
||||
Cloud/CDP: no daemon to probe — replace the stuck client generation now (same
|
||||
``bb_session_id`` so cloud cleanup works). Local daemon alive (PID live, verified,
|
||||
socket accepts): only the command wedged — mark suspect, recycle at next use.
|
||||
Local daemon wedged/dead: tree-kill and evict now (Chromium children would leak).
|
||||
Both local branches ``mark_suspect`` first so the poisoned-cache invariant holds
|
||||
even if eviction races another thread's replacement.
|
||||
"""
|
||||
if session_info.get("bb_session_id") or session_info.get("cdp_url"):
|
||||
_bt._discard_timed_out_browser_session(task_id, session_info, task_socket_dir)
|
||||
@@ -457,13 +417,9 @@ def _handle_browser_command_timeout(task_id: str, session_info: Dict[str, Any],
|
||||
|
||||
|
||||
def _interpret_browser_command_output(command: str, stdout: str, stderr: str, returncode: int) -> Dict[str, Any]:
|
||||
"""Turn a finished agent-browser process's output into a result dict.
|
||||
|
||||
Empty stdout with rc=0 is a broken state (stale daemon) and is reported as
|
||||
failure rather than a silent success — except for ``_EMPTY_OK_COMMANDS``.
|
||||
Non-JSON output is an error, except ``screenshot`` where the saved path is
|
||||
recovered from the prose.
|
||||
"""
|
||||
"""Finished agent-browser process output → result dict. Empty stdout with rc=0 is a
|
||||
broken state (stale daemon) reported as failure except for ``_EMPTY_OK_COMMANDS``;
|
||||
non-JSON output is an error except ``screenshot``, whose path is recovered from prose."""
|
||||
if stderr and stderr.strip():
|
||||
level = logging.WARNING if returncode != 0 else logging.DEBUG
|
||||
_bt.logger.log(level, "browser '%s' stderr: %s", command, stderr.strip()[:500])
|
||||
@@ -502,9 +458,8 @@ def _interpret_browser_command_output(command: str, stdout: str, stderr: str, re
|
||||
|
||||
|
||||
def _browser_command_preflight() -> Dict[str, Any]:
|
||||
"""Fail fast before spawning: missing CLI, Termux install gap, interrupt, or no
|
||||
Chromium in local mode (else every call hangs for command_timeout). Returns an
|
||||
error result, or ``{"browser_cmd": path}`` on success."""
|
||||
"""Fail fast before spawning (missing CLI, Termux gap, interrupt, no Chromium in local
|
||||
mode — else every call hangs for command_timeout). Error result, or ``{"browser_cmd": path}``."""
|
||||
try:
|
||||
browser_cmd = _bt._find_agent_browser()
|
||||
except FileNotFoundError as e:
|
||||
@@ -587,11 +542,8 @@ def _run_browser_command(
|
||||
_engine_override: Optional[str] = None,
|
||||
) -> Dict[str, Any]:
|
||||
"""Run one agent-browser CLI command against the task's session; returns its parsed JSON.
|
||||
|
||||
``timeout=None`` reads ``browser.command_timeout`` (default 30s).
|
||||
``_engine_override`` forces an engine for this call only (the Lightpanda
|
||||
fallback uses it to retry with Chrome without touching global state).
|
||||
"""
|
||||
``timeout=None`` reads ``browser.command_timeout``; ``_engine_override`` forces an engine
|
||||
for this call only (Lightpanda fallback retries with Chrome without touching global state)."""
|
||||
if timeout is None:
|
||||
timeout = _bt._safe_command_timeout()
|
||||
args = args or []
|
||||
|
||||
Reference in New Issue
Block a user