diff --git a/docs/runtime-decomposition/wave-5a-browser-lifecycle.md b/docs/runtime-decomposition/wave-5a-browser-lifecycle.md new file mode 100644 index 000000000..c33f81bf9 --- /dev/null +++ b/docs/runtime-decomposition/wave-5a-browser-lifecycle.md @@ -0,0 +1,115 @@ +# Wave 5A: deterministic browser lifecycle + +Base: `a46eb7f47abaf15c799275f946d7dfe27bdee516`, branch `feature/browser-lifecycle`. +Scope is browser-specific lifecycle only. Request authority, approvals, +TurnContract, generic process containment (Wave 3-S), effects/provenance +(Wave 4), generic process lifecycle (Wave 5B) and runtime decomposition +(Wave 6) are unchanged. + +## Runtimes + +1. `private_browser` (`src/agent_tools/web_tools.py`, `PrivateBrowserTool`) is the + model-facing browser. It runs the `agent-browser` CLI per action. The CLI is a + short-lived client of a detached daemon; the daemon calls `setsid` and + launches Chrome. Identity is `--session ody-`. + Other entry points: `src/research_navigator.py` (`browser_read`), + `scripts/probe_browser_budget.py`, app shutdown in `app.py`. +2. Playwright MCP (`src/builtin_mcp.py`, server `builtin_browser`) is one global + `npx @playwright/mcp --headless --isolated --no-sandbox` stdio server owned by + `src/mcp_manager.py`. Its tools are hidden from the model unless + `private_browser` is disabled or `ODYSSEUS_EXPOSE_RAW_BROWSER_MCP` is set + (`src/agent_loop.py`, `_should_hide_raw_browser_mcp`). The two runtimes share + no code; only Chromium discovery overlaps. + +## Probe evidence (agent-browser 0.27.0, this host) + +- Runtime files live in `AGENT_BROWSER_SOCKET_DIR`, else + `$XDG_RUNTIME_DIR/agent-browser`, else `$HOME/.agent-browser`, as + `.{pid,sock,stream,version,engine}`. The socket path must stay under + about 103 bytes. +- Every Chrome process shares the daemon's POSIX session id (sid == daemon pid). +- `close` removes the daemon, Chrome, the runtime files and the + `agent-browser-chrome-*` profile. +- SIGKILL of the daemon alone (the previous timeout path) left 13 Chrome + processes, the profile, a Chromium temp directory and stale pid/socket files. +- `close` against a session with no daemon bootstraps one. +- A Chrome launch failure ("No usable sandbox", "Chrome exited early") leaves + the daemon alive; `close` cannot reach a browser. +- There is no `read` command ("Unknown command: read"). +- This host blocks the Chromium sandbox for agent-browser. Tests pass + `AGENT_BROWSER_ARGS=--no-sandbox` in the test environment only; production + launch flags are unchanged. + +## Failure modes found and their resolution + +| # | Failure | Resolution | +|---|---------|------------| +| F1 | Cancellation not handled; CLI, daemon and Chrome survived until idle timeout | `execute` catches `CancelledError`, kills every CLI client of the call and cleans the session tree, then re-raises | +| F2 | Timeout/exception killed only the daemon; Chrome reparented and leaked | `browser_lifecycle.force_cleanup` kills the daemon's whole POSIX session, removes runtime files and the profile, and verifies no survivor | +| F3 | Shutdown force-kill used `os.environ` and only the legacy layout | Shutdown uses each session's recorded launch environment, closes only verified live daemons, then force-cleans and verifies | +| F4 | Missing `session_id` used agent-browser's shared `default` session | A sessionless call gets an ephemeral session that is closed and verified before the call returns | +| F5 | Launch failure left the daemon alive | Launch-failure output triggers forced cleanup and a truthful error | +| F6 | Concurrent actions on one session raced one daemon | Per-session `asyncio.Lock` serializes actions | +| F7 | Observation after a failed navigation silently showed the old page | Sessions track navigation generation, page URL and failed navigation; such observations are prefixed with an explicit stale notice and flagged `stale_observation` | +| F8 | Recovery recursed through `execute` with a model-visible retry flag and no overall deadline | One deadline per call (action timeout + 75s); at most one retry, only for local read-only HTML open; model-supplied `_odysseus_browser_retry` is ignored | +| F9 | `research_navigator` passed `timeout`, which the tool ignored | Passes `timeout_ms` | +| F10 | No lifecycle evidence | Every result carries `browser_lifecycle` with stages, timings, ownership, state and cleanup receipt | +| F11 | Pid lookup assumed `/run/user/`; containers without `XDG_RUNTIME_DIR` were never cleaned | Runtime root follows agent-browser's own resolution from the launch environment | +| F12 | Per-call timeout swept every Chrome under the runtime `TMPDIR`, killing other sessions | Per-call cleanup is limited to the session tree; the `TMPDIR` sweep only runs at runtime shutdown | +| F13 | `read` used a command agent-browser does not have | `read URL` runs `open` and `get text body` in one batch; success requires both rows; `read` without URL extracts the current page | +| F14 | Playwright MCP calls had no time bound | `builtin_browser` calls are bounded by `ODYSSEUS_BROWSER_MCP_CALL_TIMEOUT_S` (default 90) and are not retried | + +## Lifecycle model + +Session states: `idle`, `ready`, `navigation_failed`, `reset`, `timed_out`, +`failed`, `launch_failed`, `bootstrap_failed`, `cancelled`, `closed`. Any state +reached by forced cleanup discards the page URL so nothing earlier remains +observable. Ownership is `retained` for a chat session (bounded by +`AGENT_BROWSER_IDLE_TIMEOUT_MS`, default 300000, and cleaned at shutdown) or +`ephemeral` for a sessionless call. + +The `browser_lifecycle` result field: + +```json +{"session": "ody-...", "ownership": "retained", "state": "ready", + "navigation_generation": 2, "page_url": "file:///...", + "stages": [{"stage": "open", "ms": 210, "ok": true, "cold_start": true}], + "elapsed_ms": 230, "cleanup": {"method": "forced", "verified": true, "...": "..."}, + "recovery_attempts": 1, "stale_observation": true, "closed_page_url": "..."} +``` + +Optional keys appear only when relevant. + +## Ownership boundary + +`src/browser_lifecycle.py` holds the browser-specific process attribution. It +claims processes only through the session's own pid file and the daemon's +POSIX session; once the daemon is gone it claims only Chrome process groups +whose root carries an `agent-browser-chrome-*` profile. Without procfs it kills +nothing. `kill_browser_tree` is the single seam to replace with the shared +process-lifecycle primitives from Wave 3-S/5B. + +## Files + +- New: `src/browser_lifecycle.py`, `tests/test_browser_lifecycle.py`, this document. +- Changed: `src/agent_tools/web_tools.py` (`PrivateBrowserTool` and shutdown), + `src/research_navigator.py` (timeout argument), `src/mcp_manager.py` (bounded + `builtin_browser` call), `scripts/generate_env_reference.py` and + `website/configuration-reference.md` (new variable), + `tests/test_private_browser_tool.py` (shutdown and read fakes). +- Not touched: `src/agent_loop.py`, `src/tool_execution.py`, + `src/agent_runtime/authority.py`, approvals, task and background infrastructure. + +## Limitations + +- A retained session's browser is not closed when its chat session is deleted; + it is bounded by the idle timeout and shutdown cleanup. +- The in-process session registry keeps one small record per chat session that + used the browser until shutdown. +- Chromium temp directories outside the profile (`org.chromium.Chromium.*`) are + not attributable to one session and are not removed by forced cleanup. +- Playwright MCP remains one global browser shared by all sessions. A timed-out + call is abandoned but the server is not restarted, because restarting the npx + server requires its owner task in `builtin_mcp.py`. +- The stale-observation notice marks, but does not block, an observation after + a failed navigation. diff --git a/scripts/generate_env_reference.py b/scripts/generate_env_reference.py index c00131574..35f017e03 100644 --- a/scripts/generate_env_reference.py +++ b/scripts/generate_env_reference.py @@ -557,6 +557,11 @@ VARIABLE_NOTES: dict[str, tuple[str, str, str]] = { "Cache directory handed to the browser MCP server, so its npm download " "survives a container rebuild.", ), + "ODYSSEUS_BROWSER_MCP_CALL_TIMEOUT_S": ( + "Browser automation", USER, + "Upper bound in seconds for one browser MCP tool call. A call that exceeds " + "it fails without being retried.", + ), "ODYSSEUS_BROWSER_MCP_REQUIRE_CACHE": ( "Browser automation", USER, "Truthy refuses to start the browser MCP server unless its npm package is " diff --git a/src/agent_tools/web_tools.py b/src/agent_tools/web_tools.py index 26122e3e0..b18757d37 100644 --- a/src/agent_tools/web_tools.py +++ b/src/agent_tools/web_tools.py @@ -1,6 +1,7 @@ import asyncio import base64 import contextlib +import contextvars import inspect import io import json @@ -10,15 +11,18 @@ import signal import shutil import sys import tempfile +import time import html import hashlib import pwd import urllib.parse import urllib.request +import uuid from pathlib import Path from typing import Dict, Any from core import platform_compat +from src import browser_lifecycle from src.constants import MAX_OUTPUT_CHARS PDF_EXTRACT_MAX_BYTES = 80_000_000 @@ -90,6 +94,28 @@ def _scoped_browser_session(namespace: str, session_id: str) -> str: return f"ody-{_bounded_browser_identity(scope)}" +def _browser_namespace(env: dict[str, str] | None) -> str: + return str( + (env or {}).get("ODYSSEUS_BROWSER_NAMESPACE") + or os.getenv("ODYSSEUS_BROWSER_NAMESPACE", "odysseus-ui") + ).strip() or "odysseus-ui" + + +# Browser CLI processes started by the current private_browser call, so a +# cancellation can stop every client it spawned, not only the main command. +_BROWSER_CALL_PROCS: contextvars.ContextVar[list | None] = contextvars.ContextVar( + "_BROWSER_CALL_PROCS", default=None +) + + +async def _spawn_browser_cli(*command, **kwargs): + proc = await asyncio.create_subprocess_exec(*command, **kwargs) + tracked = _BROWSER_CALL_PROCS.get() + if tracked is not None: + tracked.append(proc) + return proc + + def _browser_pid_file_candidates( runtime_dir: Path, namespace: str, session_id: str | None ) -> list[Path]: @@ -2433,15 +2459,30 @@ class PrivateBrowserTool: @staticmethod def _terminate_owned_daemon( env: dict[str, str], session_id: str | None = None - ) -> None: - """Terminate detached agent-browser daemon(s) for this runtime.""" + ) -> dict[str, Any] | None: + """Terminate the detached agent-browser tree owned by one session. - namespace = str( - env.get("ODYSSEUS_BROWSER_NAMESPACE") - or os.getenv("ODYSSEUS_BROWSER_NAMESPACE", "odysseus-ui") - ).strip() or "odysseus-ui" + Killing only the daemon reparents its Chrome children, so the whole + POSIX session the daemon leads is cleaned together with the session's + runtime files and browser profile. Returns the cleanup receipt. + """ + + namespace = _browser_namespace(env) + receipt = None + pid_files = [] + if session_id: + key = _scoped_browser_session(namespace, session_id) + root = browser_lifecycle.runtime_root(env) + receipt = browser_lifecycle.force_cleanup( + root, key, pid_alive=lambda pid: _process_is_alive(pid) + ).as_dict() + pid_files.append(root / f"{key}.pid") runtime_dir = Path(os.getenv("XDG_RUNTIME_DIR") or f"/run/user/{os.getuid()}") - pid_files = _browser_pid_file_candidates(runtime_dir, namespace, session_id) + pid_files = [ + path + for path in _browser_pid_file_candidates(runtime_dir, namespace, session_id) + if path not in pid_files + ] for pid_file in pid_files: try: pid = int(pid_file.read_text().strip()) @@ -2463,6 +2504,7 @@ class PrivateBrowserTool: os.kill(pid, signal.SIGKILL) with contextlib.suppress(FileNotFoundError, PermissionError, OSError): pid_file.unlink() + return receipt @staticmethod def _owned_daemon_exists(env: dict[str, str], session_id: str | None) -> bool: @@ -2475,12 +2517,17 @@ class PrivateBrowserTool: if not session_id: return False - namespace = str( - env.get("ODYSSEUS_BROWSER_NAMESPACE") - or os.getenv("ODYSSEUS_BROWSER_NAMESPACE", "odysseus-ui") - ).strip() or "odysseus-ui" + namespace = _browser_namespace(env) + key = _scoped_browser_session(namespace, session_id) + root = browser_lifecycle.runtime_root(env) + if browser_lifecycle.has_live_daemon( + root, key, pid_alive=lambda pid: _process_is_alive(pid) + ): + return True runtime_dir = Path(os.getenv("XDG_RUNTIME_DIR") or f"/run/user/{os.getuid()}") for pid_file in _browser_pid_file_candidates(runtime_dir, namespace, session_id): + if pid_file == root / f"{key}.pid": + continue try: pid = int(pid_file.read_text().strip()) except (OSError, ValueError): @@ -2553,10 +2600,184 @@ class PrivateBrowserTool: resolved = cls._resolve_workspace_path(raw_path) return resolved.as_uri() + # Time allowed beyond the action timeout for one bounded recovery attempt. + _RECOVERY_BUDGET_S = 75 + _CLOSE_TIMEOUT_S = 10 + async def execute(self, content: str, ctx: dict) -> dict: + """Run one browser action inside its session's lifecycle. + + Actions on one session are serialized. A call without an owning + Odysseus session gets a browser of its own that is closed before the + call returns; it never falls back to agent-browser's shared default + session. Cancellation stops every CLI client the call started and + cleans the session's browser tree, because its state is unknown. + """ + + ctx = dict(ctx) if isinstance(ctx, dict) else {} + session_id = str(ctx.get("session_id") or "").strip() + ephemeral = not session_id + if ephemeral: + session_id = f"ephemeral-{uuid.uuid4().hex}" + ctx["session_id"] = session_id + runtime_env = ctx.get("subproc_env") if isinstance(ctx.get("subproc_env"), dict) else {} + key = _scoped_browser_session(_browser_namespace(runtime_env), session_id) + browser = browser_lifecycle.session_for(key, ephemeral) + clock = browser_lifecycle.StageClock() + procs: list = [] + token = _BROWSER_CALL_PROCS.set(procs) + lock = browser.lock() + acquired = False + try: + await lock.acquire() + acquired = True + result = await self._execute_unlocked(content, ctx, browser=browser, clock=clock) + if ephemeral: + await self._release_session(browser, session_id, clock) + if isinstance(result, dict) and browser.env is not None: + result["browser_lifecycle"] = browser.receipt(clock) + return result + except asyncio.CancelledError: + if acquired: + for proc in procs: + if getattr(proc, "returncode", None) is None: + self._terminate_subprocess(proc) + if browser.env is not None: + self._terminate_owned_daemon(browser.env, session_id) + browser.discarded("cancelled") + raise + except Exception: + if acquired and ephemeral and browser.env is not None: + self._terminate_owned_daemon(browser.env, session_id) + raise + finally: + if acquired: + lock.release() + _BROWSER_CALL_PROCS.reset(token) + if ephemeral: + browser_lifecycle.forget(key) + _ACTIVE_BROWSER_SESSIONS.discard(key) + + async def _release_session( + self, + browser: browser_lifecycle.BrowserSession, + session_id: str, + clock: browser_lifecycle.StageClock, + ) -> None: + """Close a session gracefully, then verify nothing it owned survives.""" + + if browser.env is None: + return + started = time.monotonic() + graceful = False + if self._owned_daemon_exists(browser.env, session_id): + proc = None + try: + proc = await _spawn_browser_cli( + *browser.command_prefix, + "close", + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.DEVNULL, + env=browser.env, + start_new_session=True, + ) + await asyncio.wait_for(proc.wait(), timeout=self._CLOSE_TIMEOUT_S) + graceful = (proc.returncode or 0) == 0 + except Exception: + if proc is not None: + self._terminate_subprocess(proc) + receipt = self._terminate_owned_daemon(browser.env, session_id) + verified = bool(receipt.get("verified")) if isinstance(receipt, dict) else False + clock.record("close", started, verified or graceful) + clock.extra["cleanup"] = {"graceful_close": graceful, **(receipt or {})} + if browser.page_url: + clock.extra["closed_page_url"] = browser.page_url + browser.discarded("closed") + + def _discard_session( + self, + browser: browser_lifecycle.BrowserSession, + env: dict[str, str], + session_id: str, + clock: browser_lifecycle.StageClock, + state: str, + ) -> None: + """Force-clean a session whose browser state can no longer be trusted.""" + + started = time.monotonic() + receipt = self._terminate_owned_daemon(env, session_id or None) + verified = bool(receipt.get("verified")) if isinstance(receipt, dict) else False + clock.record("forced_cleanup", started, verified, reason=state) + clock.extra["cleanup"] = receipt + browser.discarded(state) + + @staticmethod + def _navigation_target(action: str, args: dict) -> str: + """URL this action navigates the session to, or ``""``.""" + + if action == "read" and any( + str(args.get(key) or "").strip() for key in ("selector", "target", "ref") + ): + return "" + if action in {"open", "read"}: + return str(args.get("url") or "").strip() + if action == "batch" and isinstance(args.get("commands"), list): + target = "" + for command in args["commands"]: + if ( + isinstance(command, list) + and len(command) > 1 + and str(command[0]).lower() in {"open", "goto", "navigate"} + ): + target = str(command[1]).strip() + return target + return "" + + @staticmethod + def _read_page_from_rows(output: str) -> dict[str, Any]: + """Page text from an open + ``get text`` batch, only if both succeeded.""" + + try: + rows = json.loads(output) + except (ValueError, TypeError): + rows = None + if not isinstance(rows, list) or len(rows) != 2 or not all(isinstance(r, dict) for r in rows): + return {"ok": False, "error": "private_browser read returned no structured page result"} + opened, extracted = rows + for row in rows: + if row.get("success") is not True: + return {"ok": False, "error": f"private_browser read failed: {row.get('error') or 'unknown error'}"} + opened_result = opened.get("result") if isinstance(opened.get("result"), dict) else {} + extracted_result = extracted.get("result") if isinstance(extracted.get("result"), dict) else {} + text = extracted_result.get("text") + if not isinstance(text, str): + return {"ok": False, "error": "private_browser read observed no page text"} + url = str(opened_result.get("url") or extracted_result.get("origin") or "") + title = str(opened_result.get("title") or "") + header = "\n".join(part for part in (title, url) if part) + return {"ok": True, "url": url, "text": f"{header}\n\n{text}".strip()} + + @staticmethod + def _navigated_url(output: str) -> str: + """Final URL reported by ``open`` (after redirects), when present.""" + + match = re.search(r"^\s+([a-z][a-z0-9+.-]*:\S+)\s*$", str(output or ""), re.MULTILINE) + return match.group(1) if match else "" + + async def _execute_unlocked( + self, + content: str, + ctx: dict, + *, + browser: browser_lifecycle.BrowserSession, + clock: browser_lifecycle.StageClock, + retry: bool = False, + deadline: float | None = None, + ) -> dict: args, err = self._parse_args(content) if err: return {"error": err, "exit_code": 1} + args.pop("_odysseus_browser_retry", None) action = str(args.get("action") or "").strip().lower() if action not in self._ACTIONS: @@ -2697,8 +2918,23 @@ class PrivateBrowserTool: # only errors emitted by this page. Opening a URL replaces prior page # state anyway; cookies are irrelevant for confined file:// artifacts. session_id = str((ctx or {}).get("session_id") or "").strip() - if verifies_local_html and self._owned_daemon_exists(env, session_id): + browser.bind(env, cmd_prefix) + loop = asyncio.get_running_loop() + if deadline is None: + deadline = loop.time() + timeout_s + self._RECOVERY_BUDGET_S + warm = self._owned_daemon_exists(env, session_id) + if verifies_local_html and warm: + reset_started = time.monotonic() await self._reset_browser_session(cmd_prefix, env, timeout_s) + clock.record("reset", reset_started, True) + browser.discarded("reset") + navigation_url = self._navigation_target(action, command_args) + stale_note = ( + browser.stale_observation_note() + if action in browser_lifecycle.OBSERVATION_ACTIONS and not navigation_url + else "" + ) + command_started = time.monotonic() # agent-browser starts a persistent daemon which can inherit the # client's stdout/stderr descriptors. Pipes therefore never reach @@ -2708,8 +2944,10 @@ class PrivateBrowserTool: # depend on the client process, not its detached daemon. stdout_file = tempfile.TemporaryFile() stderr_file = tempfile.TemporaryFile() + attempt_timeout = max(1.0, min(float(timeout_s), deadline - loop.time())) + proc = None try: - proc = await asyncio.create_subprocess_exec( + proc = await _spawn_browser_cli( *command, stdin=asyncio.subprocess.PIPE if stdin_data is not None else None, stdout=stdout_file, @@ -2719,40 +2957,47 @@ class PrivateBrowserTool: ) await asyncio.wait_for( proc.communicate(stdin_data.encode("utf-8") if stdin_data is not None else None), - timeout=timeout_s, + timeout=attempt_timeout, ) stdout_file.seek(0) stderr_file.seek(0) stdout = stdout_file.read() stderr = stderr_file.read() except asyncio.TimeoutError: - with contextlib.suppress(Exception): - self._terminate_subprocess(proc) - self._terminate_owned_chrome(env) - self._terminate_owned_daemon( - env, str((ctx or {}).get("session_id") or "").strip() or None - ) + if proc is not None: + with contextlib.suppress(Exception): + self._terminate_subprocess(proc) + clock.record(action, command_started, False, cold_start=not warm, failure="timeout") + self._discard_session(browser, env, session_id, clock, "timed_out") # A failed local-page verification can leave agent-browser's # persistent session between a page-error response and the next - # repair attempt. Reopen exactly once after resetting that session; - # never retry mutating browser actions or arbitrary URLs. - if ( - verifies_local_html - and action == "open" - and not args.get("_odysseus_browser_retry") - ): + # repair attempt. The session was cleaned above, so reopen exactly + # once within the call's deadline; never retry mutating browser + # actions or arbitrary URLs. + remaining = deadline - loop.time() + if verifies_local_html and action == "open" and not retry and remaining >= 10: retry_args = dict(args) - retry_args["_odysseus_browser_retry"] = True - retry_args["timeout_ms"] = max(60_000, timeout_s * 1000) - if self._owned_daemon_exists(env, session_id): - await self._reset_browser_session(cmd_prefix, env, timeout_s) - return await self.execute(json.dumps(retry_args), ctx) - return {"error": f"private_browser timed out after {timeout_s}s", "exit_code": 1} + retry_args["timeout_ms"] = int( + min(max(60.0, float(timeout_s)), remaining - 5) * 1000 + ) + clock.extra["recovery_attempts"] = 1 + return await self._execute_unlocked( + json.dumps(retry_args), ctx, + browser=browser, clock=clock, retry=True, deadline=deadline, + ) + return { + "error": f"private_browser timed out after {int(attempt_timeout)}s", + "exit_code": 1, + } except Exception as e: - self._terminate_owned_chrome(env) - self._terminate_owned_daemon( - env, str((ctx or {}).get("session_id") or "").strip() or None - ) + if proc is not None: + with contextlib.suppress(Exception): + self._terminate_subprocess(proc) + clock.record(action, command_started, False, cold_start=not warm, failure=type(e).__name__) + if proc is not None: + # The client reached the daemon, so the session's state is + # unknown. A client that never started left it untouched. + self._discard_session(browser, env, session_id, clock, "failed") return {"error": f"private_browser failed: {type(e).__name__}: {e}", "exit_code": 1} finally: stdout_file.close() @@ -2763,6 +3008,47 @@ class PrivateBrowserTool: combined = out if err_text: combined = f"{combined}\n\n[stderr]\n{err_text}".strip() + command_ok = (proc.returncode or 0) == 0 + read_page = None + if action == "read" and navigation_url: + read_page = self._read_page_from_rows(out) + command_ok = command_ok and read_page.get("ok", False) + clock.record(action, command_started, command_ok, cold_start=not warm) + if not command_ok and browser_lifecycle.LAUNCH_FAILURE_RE.search(combined): + # The browser never became ready. The daemon outlives this failure + # and a later close cannot reach a browser, so clean it here. + self._discard_session(browser, env, session_id, clock, "launch_failed") + return { + "output": combined[:4000], + "error": ( + "private_browser could not launch the browser; no page was " + "opened or observed. The browser session was cleaned up." + ), + "exit_code": 1, + "untrusted_content": True, + } + if navigation_url: + if command_ok: + browser.navigated( + (read_page or {}).get("url") or self._navigated_url(out) or navigation_url + ) + else: + browser.navigation_failed(navigation_url) + if read_page is not None: + if not command_ok: + return { + "output": combined[:4000], + "error": read_page.get("error") or "private_browser read failed; no page text was observed", + "exit_code": 1, + "untrusted_content": True, + } + out = read_page["text"] + combined = out if not err_text else f"{out}\n\n[stderr]\n{err_text}" + elif command_ok and browser.state in {"idle", "closed", "reset"}: + browser.state = "ready" + if stale_note and command_ok: + combined = f"[{stale_note}]\n\n{combined}".strip() + clock.extra["stale_observation"] = True from src.turn_contract import active_turn_contract contract = active_turn_contract() model_choice = getattr(contract, 'routing_experiment', '') == 'recent_model_choice' @@ -2806,13 +3092,15 @@ class PrivateBrowserTool: and action == "open" and (proc.returncode or 0) != 0 and self._retryable_local_open_failure(combined) - and not args.get("_odysseus_browser_retry") + and not retry + and deadline - loop.time() >= 10 ): - self._terminate_owned_chrome(env) - self._terminate_owned_daemon(env, session_id or None) - retry_args = dict(args) - retry_args["_odysseus_browser_retry"] = True - return await self.execute(json.dumps(retry_args), ctx) + self._discard_session(browser, env, session_id, clock, "bootstrap_failed") + clock.extra["recovery_attempts"] = 1 + return await self._execute_unlocked( + json.dumps(args), ctx, + browser=browser, clock=clock, retry=True, deadline=deadline, + ) page_errors = "" if ( verifies_local_html @@ -2893,7 +3181,7 @@ class PrivateBrowserTool: proc = None try: async with asyncio.timeout(max(0, deadline - loop.time())): - proc = await asyncio.create_subprocess_exec( + proc = await _spawn_browser_cli( *cmd_prefix, "batch", "--json", stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, @@ -3044,7 +3332,7 @@ class PrivateBrowserTool: """Best-effort reset of state retained by a persistent browser session.""" proc = None try: - proc = await asyncio.create_subprocess_exec( + proc = await _spawn_browser_cli( *cmd_prefix, "close", stdout=asyncio.subprocess.PIPE, @@ -3070,7 +3358,7 @@ class PrivateBrowserTool: """Return bounded JavaScript errors from the current browser page.""" proc = None try: - proc = await asyncio.create_subprocess_exec( + proc = await _spawn_browser_cli( *cmd_prefix, "errors", stdout=asyncio.subprocess.PIPE, @@ -3114,7 +3402,7 @@ class PrivateBrowserTool: return None proc = None try: - proc = await asyncio.create_subprocess_exec( + proc = await _spawn_browser_cli( *command, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, @@ -3331,10 +3619,13 @@ class PrivateBrowserTool: if target: return [*prefix, "get", "text", target], None, None url = str(args.get("url") or "").strip() - command = [*prefix, "read"] + # agent-browser has no `read` command. Navigate and extract in one + # client call so the text is observed after this navigation. if url: - command.append(url) - return command, None, None + return [*prefix, "batch", "--json"], json.dumps( + [["open", url], ["get", "text", "body"]] + ), None + return [*prefix, "get", "text", "body"], None, None if action == "snapshot": return [*prefix, "snapshot"], None, None if action == "find": @@ -3465,7 +3756,13 @@ class PrivateBrowserTool: async def shutdown_private_browser_sessions() -> None: - """Close browser sessions owned by this Odysseus runtime namespace.""" + """Close and verify every browser session this runtime started. + + Each session is closed with the environment it was launched with, so its + runtime directory resolves to the daemon's own. ``close`` is sent only to + a verified live daemon, because against a missing one it bootstraps a new + browser. Forced cleanup of the session's own browser tree always follows. + """ binary = shutil.which("agent-browser") or PrivateBrowserTool._local_agent_browser_binary() command_prefix = [binary] if binary else ( @@ -3474,26 +3771,37 @@ async def shutdown_private_browser_sessions() -> None: sessions = sorted(_ACTIVE_BROWSER_SESSIONS) if not command_prefix or not sessions: return - env = dict(os.environ) - env["HOME"] = str(_service_home()) - env.setdefault("AGENT_BROWSER_IDLE_TIMEOUT_MS", "300000") + base_env = dict(os.environ) + base_env["HOME"] = str(_service_home()) + base_env.setdefault("AGENT_BROWSER_IDLE_TIMEOUT_MS", "300000") try: for session in sessions: - proc = None - try: - proc = await asyncio.create_subprocess_exec( - *command_prefix, "--session", session, "close", - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - env=env, - start_new_session=True, - ) - await asyncio.wait_for(proc.communicate(), timeout=20) - except Exception: - if proc is not None: - with contextlib.suppress(Exception): - PrivateBrowserTool._terminate_subprocess(proc) + record = browser_lifecycle.registered(session) + env = record.env if record is not None and record.env is not None else base_env + root = browser_lifecycle.runtime_root(env) + if browser_lifecycle.has_live_daemon( + root, session, pid_alive=lambda pid: _process_is_alive(pid) + ): + proc = None + try: + proc = await asyncio.create_subprocess_exec( + *command_prefix, "--session", session, "close", + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.DEVNULL, + env=env, + start_new_session=True, + ) + await asyncio.wait_for(proc.communicate(), timeout=20) + except Exception: + if proc is not None: + with contextlib.suppress(Exception): + PrivateBrowserTool._terminate_subprocess(proc) + browser_lifecycle.force_cleanup( + root, session, method="shutdown", + pid_alive=lambda pid: _process_is_alive(pid), + ) + browser_lifecycle.forget(session) finally: _ACTIVE_BROWSER_SESSIONS.difference_update(sessions) - PrivateBrowserTool._terminate_owned_chrome(env) - PrivateBrowserTool._terminate_owned_daemon(env) + PrivateBrowserTool._terminate_owned_chrome(base_env) + PrivateBrowserTool._terminate_owned_daemon(base_env) diff --git a/src/browser_lifecycle.py b/src/browser_lifecycle.py new file mode 100644 index 000000000..689048309 --- /dev/null +++ b/src/browser_lifecycle.py @@ -0,0 +1,397 @@ +"""Lifecycle ownership for private_browser's agent-browser sessions. + +agent-browser runs a short-lived CLI client against a detached daemon. The +daemon calls ``setsid`` and every Chrome process it launches stays in that +POSIX session, so the daemon pid recorded in the session's own pid file +identifies the complete browser tree. Cleanup here is limited to that tree, +the session's runtime files and its ``agent-browser-chrome-*`` profile. + +This is browser-specific ownership only. Generic process containment and +lifecycle primitives belong to the shared process layer; when those exist, +``kill_browser_tree`` is the single seam to replace. +""" + +from __future__ import annotations + +import asyncio +import os +import re +import shutil +import signal +import time +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Callable + +from core import platform_compat + +PROFILE_PREFIX = "agent-browser-chrome-" +RUNTIME_SUFFIXES = (".pid", ".sock", ".stream", ".version", ".engine") + +# Output that proves the browser never became ready. The daemon survives such +# a failure and a later ``close`` cannot reach a browser to shut down. +LAUNCH_FAILURE_RE = re.compile( + r"Chrome exited early|DevToolsActivePort|No usable sandbox|" + r"Failed to launch (?:the )?browser|Browser (?:process )?exited before", + re.IGNORECASE, +) + +OBSERVATION_ACTIONS = frozenset( + {"snapshot", "read", "find", "evaluate", "screenshot", "scroll", "wait"} +) + + +def runtime_root(env: dict[str, str] | None) -> Path: + """Directory where agent-browser keeps ``.pid`` and its socket. + + Mirrors agent-browser's own resolution for the environment the daemon is + launched with: an explicit socket directory, then the XDG runtime + directory, then ``$HOME/.agent-browser``. + """ + + source = env or {} + + def _get(name: str) -> str: + return str(source.get(name) or os.environ.get(name) or "").strip() + + socket_dir = _get("AGENT_BROWSER_SOCKET_DIR") + if socket_dir: + return Path(socket_dir) + xdg = _get("XDG_RUNTIME_DIR") + if xdg: + return Path(xdg) / "agent-browser" + home = _get("HOME") or str(Path.home()) + return Path(home) / ".agent-browser" + + +def _read_cmdline(pid: int) -> str | None: + try: + return (platform_compat.PROC_ROOT / str(pid) / "cmdline").read_bytes().replace( + b"\0", b" " + ).decode("utf-8", errors="replace") + except (OSError, UnicodeError): + return None + + +def _read_stat(pid: int) -> tuple[str, int, int] | None: + """Return ``(state, pgid, sid)`` for a pid, or ``None`` when unreadable.""" + + try: + raw = (platform_compat.PROC_ROOT / str(pid) / "stat").read_text() + except (OSError, UnicodeError): + return None + _, _, rest = raw.rpartition(")") + fields = rest.split() + if len(fields) < 4: + return None + try: + return fields[0], int(fields[2]), int(fields[3]) + except ValueError: + return None + + +def _live_pids() -> list[int]: + if not platform_compat.has_procfs(): + return [] + pids = [] + for entry in platform_compat.PROC_ROOT.iterdir(): + if entry.name.isdigit(): + pids.append(int(entry.name)) + return pids + + +def daemon_pid(root: Path, key: str) -> int | None: + """Pid recorded in this session's pid file, whether or not it is alive.""" + + try: + return int((root / f"{key}.pid").read_text().strip()) + except (OSError, ValueError): + return None + + +def is_verified_daemon(pid: int | None) -> bool: + """Whether ``pid`` is a live agent-browser process (requires procfs).""" + + if not pid: + return False + stat = _read_stat(pid) + if stat is not None and stat[0] == "Z": + return False + command_line = _read_cmdline(pid) + return bool(command_line) and "agent-browser" in command_line + + +def browser_tree(leader: int) -> list[int]: + """Processes owned by the browser session whose daemon pid is ``leader``. + + While the daemon is verified alive, every member of its POSIX session is + owned. Once the daemon is gone the pid may be reused, so only Chrome + process groups whose root carries an agent-browser profile are claimed. + """ + + members: list[tuple[int, int]] = [] + for pid in _live_pids(): + stat = _read_stat(pid) + if stat is None or stat[0] == "Z" or stat[2] != leader: + continue + members.append((pid, stat[1])) + if not members: + return [] + if is_verified_daemon(leader): + return sorted(pid for pid, _ in members) + owned_groups = { + pgid for pid, pgid in members if _profile_dirs([pid]) + } + return sorted(pid for pid, pgid in members if pgid in owned_groups) + + +def _profile_dirs(pids: list[int]) -> set[Path]: + profiles: set[Path] = set() + for pid in pids: + for token in (_read_cmdline(pid) or "").split(): + if not token.startswith("--user-data-dir="): + continue + path = Path(token.split("=", 1)[1]) + if path.name.startswith(PROFILE_PREFIX): + profiles.add(path) + return profiles + + +def kill_browser_tree(leader: int, *, settle_s: float = 1.0) -> tuple[list[int], list[int], set[Path]]: + """SIGKILL one browser session tree and wait briefly for it to exit. + + Returns ``(killed, survivors, profile_dirs)``. Synchronous so it can run + from cancellation and shutdown paths without awaiting. + """ + + members = browser_tree(leader) + profiles = _profile_dirs(members) + ordered = [pid for pid in members if pid != leader] + if leader in members: + ordered.append(leader) + killed = [] + for pid in ordered: + try: + os.kill(pid, signal.SIGKILL) + killed.append(pid) + except (ProcessLookupError, PermissionError, OSError): + continue + deadline = time.monotonic() + settle_s + survivors = list(killed) + while survivors and time.monotonic() < deadline: + survivors = [pid for pid in survivors if _alive(pid)] + if survivors: + time.sleep(0.02) + return killed, survivors, profiles + + +def _alive(pid: int) -> bool: + stat = _read_stat(pid) + if stat is None: + return False + return stat[0] != "Z" + + +@dataclass +class CleanupReceipt: + method: str + daemon_pid: int | None = None + killed: int = 0 + survivors: list[int] = field(default_factory=list) + removed_files: list[str] = field(default_factory=list) + removed_profiles: int = 0 + verified: bool = False + note: str = "" + + def as_dict(self) -> dict[str, Any]: + return { + "method": self.method, + "daemon_pid": self.daemon_pid, + "killed": self.killed, + "survivors": list(self.survivors), + "removed_files": list(self.removed_files), + "removed_profiles": self.removed_profiles, + "verified": self.verified, + **({"note": self.note} if self.note else {}), + } + + +def force_cleanup( + root: Path, + key: str, + *, + method: str = "forced", + pid_alive: Callable[[int], bool] = platform_compat.pid_alive, +) -> CleanupReceipt: + """Kill this session's browser tree and remove its owned resources. + + Without procfs nothing can be attributed safely, so live processes are + left alone and only the pid file of a dead daemon is forgotten. + """ + + pid = daemon_pid(root, key) + receipt = CleanupReceipt(method=method, daemon_pid=pid) + if not platform_compat.has_procfs(): + if pid and not pid_alive(pid): + _remove_runtime_files(root, key, receipt) + receipt.verified = True + else: + receipt.note = "procfs unavailable; browser ownership could not be verified" + return receipt + profiles: set[Path] = set() + if pid: + killed, survivors, profiles = kill_browser_tree(pid) + receipt.killed = len(killed) + receipt.survivors = survivors + if not receipt.survivors: + _remove_runtime_files(root, key, receipt) + for profile in profiles: + if profile.name.startswith(PROFILE_PREFIX) and profile.is_dir(): + shutil.rmtree(profile, ignore_errors=True) + if not profile.exists(): + receipt.removed_profiles += 1 + receipt.verified = not receipt.survivors and not (pid and browser_tree(pid)) + return receipt + + +def _remove_runtime_files(root: Path, key: str, receipt: CleanupReceipt) -> None: + for suffix in RUNTIME_SUFFIXES: + path = root / f"{key}{suffix}" + try: + path.unlink() + receipt.removed_files.append(path.name) + except FileNotFoundError: + continue + except OSError: + continue + + +class StageClock: + """Ordered stage timings for one browser call.""" + + def __init__(self) -> None: + self.stages: list[dict[str, Any]] = [] + self.extra: dict[str, Any] = {} + self._start = time.monotonic() + + def record(self, stage: str, started: float, ok: bool, **detail: Any) -> None: + entry = { + "stage": stage, + "ms": int((time.monotonic() - started) * 1000), + "ok": bool(ok), + } + entry.update({k: v for k, v in detail.items() if v not in (None, "")}) + self.stages.append(entry) + + def total_ms(self) -> int: + return int((time.monotonic() - self._start) * 1000) + + +@dataclass +class BrowserSession: + """In-process lifecycle record for one owned agent-browser session.""" + + key: str + ephemeral: bool + root: Path | None = None + env: dict[str, str] | None = field(default=None, repr=False) + command_prefix: list[str] = field(default_factory=list, repr=False) + state: str = "idle" + navigation_generation: int = 0 + page_url: str = "" + failed_navigation_url: str = "" + _lock: asyncio.Lock | None = field(default=None, repr=False) + _lock_loop: Any = field(default=None, repr=False) + + def bind(self, env: dict[str, str], command_prefix: list[str]) -> None: + """Record the environment and CLI prefix the daemon is launched with.""" + + self.env = dict(env) + self.root = runtime_root(env) + self.command_prefix = list(command_prefix) + + def lock(self) -> asyncio.Lock: + loop = asyncio.get_running_loop() + if self._lock is None or self._lock_loop is not loop: + self._lock = asyncio.Lock() + self._lock_loop = loop + return self._lock + + def navigated(self, url: str) -> None: + self.navigation_generation += 1 + self.page_url = url + self.failed_navigation_url = "" + self.state = "ready" + + def navigation_failed(self, url: str) -> None: + self.failed_navigation_url = url + self.state = "navigation_failed" + + def discarded(self, state: str) -> None: + """The browser and its page are gone; nothing earlier is observable.""" + + self.page_url = "" + self.failed_navigation_url = "" + self.state = state + + def stale_observation_note(self) -> str: + if not self.failed_navigation_url: + return "" + shown = self.page_url or "an earlier page" + return ( + f"Browser lifecycle: the most recent navigation to {self.failed_navigation_url} " + f"failed. This observation shows {shown} (navigation " + f"#{self.navigation_generation}), not {self.failed_navigation_url}." + ) + + def receipt(self, clock: StageClock) -> dict[str, Any]: + payload = { + "session": self.key, + "ownership": "ephemeral" if self.ephemeral else "retained", + "state": self.state, + "navigation_generation": self.navigation_generation, + "page_url": self.page_url, + "stages": clock.stages, + "elapsed_ms": clock.total_ms(), + } + payload.update({k: v for k, v in clock.extra.items() if v is not None}) + return payload + + +_SESSIONS: dict[str, BrowserSession] = {} + + +def session_for(key: str, ephemeral: bool) -> BrowserSession: + record = _SESSIONS.get(key) + if record is None: + record = BrowserSession(key=key, ephemeral=ephemeral) + _SESSIONS[key] = record + return record + + +def forget(key: str) -> None: + _SESSIONS.pop(key, None) + + +def registered(key: str) -> BrowserSession | None: + return _SESSIONS.get(key) + + +def has_live_daemon( + root: Path, + key: str, + *, + pid_alive: Callable[[int], bool] = platform_compat.pid_alive, +) -> bool: + """Whether this session has a daemon a ``close`` command could reach. + + Without procfs a live pid from our own pid file is treated as a match, + because answering "no daemon" lets ``close`` bootstrap a fresh browser. + """ + + pid = daemon_pid(root, key) + if not pid: + return False + if not platform_compat.has_procfs(): + return pid_alive(pid) + return is_verified_daemon(pid) diff --git a/src/mcp_manager.py b/src/mcp_manager.py index 1ba563170..21d9e9cad 100644 --- a/src/mcp_manager.py +++ b/src/mcp_manager.py @@ -17,6 +17,18 @@ from src.runtime_paths import get_app_root logger = logging.getLogger(__name__) +BROWSER_MCP_SERVER_ID = "builtin_browser" + + +def browser_mcp_call_timeout() -> float: + """Upper bound for one Playwright MCP tool call, in seconds.""" + + try: + value = float(os.environ.get("ODYSSEUS_BROWSER_MCP_CALL_TIMEOUT_S", "90")) + except ValueError: + return 90.0 + return value if value > 0 else 90.0 + def _format_mcp_connection_error(name: str, command: str = "", args: Optional[List[str]] = None, error: Exception = None) -> str: """Return a user-actionable MCP connection error message.""" args = args or [] @@ -521,6 +533,24 @@ class McpManager: return {"error": f"MCP server not connected: {server_id}", "exit_code": 1} try: + if server_id == BROWSER_MCP_SERVER_ID: + # The shared Playwright browser must not hold a turn forever. + # The call is abandoned, not retried: page state is unknown. + limit = browser_mcp_call_timeout() + try: + return await asyncio.wait_for( + self._do_call(session, tool_name, arguments), timeout=limit + ) + except asyncio.TimeoutError: + logger.warning("Browser MCP call %s timed out after %ss", tool_name, limit) + return { + "error": ( + f"Browser call {tool_name} timed out after {limit:g}s and was " + "not retried. The current page state is unknown; navigate " + "again before relying on any observation." + ), + "exit_code": 1, + } result = await self._do_call(session, tool_name, arguments) except Exception as e: # Auto-reconnect for builtin servers whose subprocess may have died diff --git a/src/research_navigator.py b/src/research_navigator.py index c1f26d7ec..5738e0d4d 100644 --- a/src/research_navigator.py +++ b/src/research_navigator.py @@ -211,7 +211,7 @@ class ResearchNavigator: self._progress({"phase": "navigating", "url": url, "title": url}) tool = PrivateBrowserTool() result = await tool.execute( - json.dumps({"action": "read", "url": url, "timeout": timeout}), + json.dumps({"action": "read", "url": url, "timeout_ms": int(timeout * 1000)}), {"session_id": self.session_id or "research"}, ) output = str(result.get("output") or "").strip() diff --git a/tests/test_browser_lifecycle.py b/tests/test_browser_lifecycle.py new file mode 100644 index 000000000..5b8889130 --- /dev/null +++ b/tests/test_browser_lifecycle.py @@ -0,0 +1,581 @@ +"""Wave 5A browser lifecycle: ownership, cleanup, recovery and freshness.""" + +import asyncio +import json +import os +import shutil +import signal +import tempfile +import time +from pathlib import Path + +import pytest + +from core import platform_compat +import src.agent_tools.web_tools as web_tools +from src import browser_lifecycle +from src.agent_tools.web_tools import PrivateBrowserTool + + +def _fake_proc(root: Path, pid: int, *, ppid: int, pgid: int, sid: int, cmdline: str, state: str = "S") -> None: + entry = root / str(pid) + entry.mkdir(parents=True) + (entry / "stat").write_text(f"{pid} (x y) {state} {ppid} {pgid} {sid} 0 0 0") + (entry / "cmdline").write_bytes(cmdline.replace(" ", "\0").encode()) + + +@pytest.fixture +def fake_procfs(monkeypatch, tmp_path): + proc = tmp_path / "proc" + proc.mkdir() + monkeypatch.setattr(platform_compat, "PROC_ROOT", proc) + killed: list[tuple[int, int]] = [] + + def _kill(pid, sig): + entry = proc / str(pid) + if not entry.exists(): + raise ProcessLookupError(pid) + killed.append((pid, sig)) + shutil.rmtree(entry) + + monkeypatch.setattr(browser_lifecycle.os, "kill", _kill) + return proc, killed + + +def _browser_tree(proc: Path, profile: Path, *, daemon: int = 500, extra_sid: int = 900) -> None: + _fake_proc(proc, daemon, ppid=1, pgid=daemon, sid=daemon, cmdline="/x/agent-browser-linux-x64") + _fake_proc(proc, daemon + 1, ppid=daemon, pgid=daemon + 1, sid=daemon, + cmdline=f"chrome --no-sandbox --user-data-dir={profile}") + _fake_proc(proc, daemon + 2, ppid=daemon + 1, pgid=daemon + 1, sid=daemon, cmdline="chrome --type=renderer") + _fake_proc(proc, extra_sid, ppid=1, pgid=extra_sid, sid=extra_sid, cmdline="chrome --type=renderer") + + +def test_runtime_root_follows_agent_browser_resolution(monkeypatch, tmp_path) -> None: + for name in ("AGENT_BROWSER_SOCKET_DIR", "XDG_RUNTIME_DIR"): + monkeypatch.delenv(name, raising=False) + assert browser_lifecycle.runtime_root({"HOME": str(tmp_path)}) == tmp_path / ".agent-browser" + assert browser_lifecycle.runtime_root( + {"HOME": str(tmp_path), "XDG_RUNTIME_DIR": "/run/x"} + ) == Path("/run/x/agent-browser") + assert browser_lifecycle.runtime_root( + {"XDG_RUNTIME_DIR": "/run/x", "AGENT_BROWSER_SOCKET_DIR": "/s"} + ) == Path("/s") + + +def test_live_daemon_owns_its_whole_session_and_nothing_else(fake_procfs, tmp_path) -> None: + proc, _ = fake_procfs + _browser_tree(proc, tmp_path / "agent-browser-chrome-a") + + assert browser_lifecycle.browser_tree(500) == [500, 501, 502] + + +def test_orphaned_tree_is_claimed_only_through_its_browser_profile(fake_procfs, tmp_path) -> None: + proc, _ = fake_procfs + _browser_tree(proc, tmp_path / "agent-browser-chrome-a") + shutil.rmtree(proc / "500") + # A reused pid's session without an agent-browser profile is not ours. + _fake_proc(proc, 700, ppid=1, pgid=700, sid=500, cmdline="bash") + + assert browser_lifecycle.browser_tree(500) == [501, 502] + + +def test_forced_cleanup_kills_tree_and_removes_owned_resources(fake_procfs, tmp_path) -> None: + proc, killed = fake_procfs + root = tmp_path / "rt" + root.mkdir() + profile = tmp_path / "agent-browser-chrome-a" + profile.mkdir() + (profile / "Default").mkdir() + for suffix in browser_lifecycle.RUNTIME_SUFFIXES: + (root / f"ody-k{suffix}").write_text("500") + (root / "ody-other.pid").write_text("900") + _browser_tree(proc, profile) + + receipt = browser_lifecycle.force_cleanup(root, "ody-k") + + assert [pid for pid, _ in killed] == [501, 502, 500] + assert all(sig == signal.SIGKILL for _, sig in killed) + assert receipt.verified and receipt.killed == 3 and receipt.removed_profiles == 1 + assert not profile.exists() + assert sorted(p.name for p in root.iterdir()) == ["ody-other.pid"] + assert (proc / "900").exists() + + +def test_forced_cleanup_without_procfs_never_kills_unverified_processes(monkeypatch, tmp_path) -> None: + monkeypatch.setattr(platform_compat, "PROC_ROOT", tmp_path / "missing") + monkeypatch.setattr(browser_lifecycle.os, "kill", lambda *a: pytest.fail("killed")) + root = tmp_path / "rt" + root.mkdir() + (root / "ody-k.pid").write_text("4242") + + live = browser_lifecycle.force_cleanup(root, "ody-k", pid_alive=lambda pid: True) + assert not live.verified and (root / "ody-k.pid").exists() + + dead = browser_lifecycle.force_cleanup(root, "ody-k", pid_alive=lambda pid: False) + assert dead.verified and not (root / "ody-k.pid").exists() + + +class _Proc: + """Fake agent-browser CLI client driven by a per-test behaviour.""" + + def __init__(self, command, kwargs, behaviour): + self.command = list(command) + self.kwargs = kwargs + self.behaviour = behaviour + self.returncode = None + self.pid = None + + async def communicate(self, stdin=None): + rc, out = await self.behaviour(self.command) + self.returncode = rc + target = self.kwargs.get("stdout") + if hasattr(target, "write"): + target.write(out.encode()) + return None, None + return out.encode(), b"" + + async def wait(self): + await self.communicate() + return self.returncode + + def kill(self): + self.returncode = -9 + + +@pytest.fixture +def browser_env(monkeypatch, tmp_path): + monkeypatch.setattr(web_tools.shutil, "which", lambda name: "/usr/bin/agent-browser") + monkeypatch.setattr(PrivateBrowserTool, "_AUTO_SCREENSHOT_ACTIONS", set()) + monkeypatch.setattr("src.tool_execution.get_active_workspace", lambda: str(tmp_path)) + monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path / "xdg")) + swept = [] + monkeypatch.setattr(PrivateBrowserTool, "_terminate_owned_chrome", staticmethod(lambda env: swept.append(env))) + cleaned = [] + + def _cleanup(env, session_id=None): + cleaned.append(session_id) + return {"method": "forced", "verified": True} + + monkeypatch.setattr(PrivateBrowserTool, "_terminate_owned_daemon", staticmethod(_cleanup)) + calls = [] + state = {"behaviour": None} + + async def _spawn(*command, **kwargs): + calls.append(list(command)) + return _Proc(command, kwargs, state["behaviour"]) + + monkeypatch.setattr(asyncio, "create_subprocess_exec", _spawn) + return state, calls, cleaned, swept + + +def _run(payload, ctx): + return asyncio.run(PrivateBrowserTool().execute(json.dumps(payload), ctx)) + + +def test_timeout_cleans_only_this_sessions_browser(browser_env) -> None: + state, calls, cleaned, swept = browser_env + + async def _hang(command): + raise asyncio.TimeoutError() + + state["behaviour"] = _hang + result = _run({"action": "open", "url": "https://example.com"}, {"session_id": "s-timeout"}) + + assert result["exit_code"] == 1 and "timed out" in result["error"] + assert cleaned == ["s-timeout"] + assert swept == [], "a per-session timeout must not sweep other sessions' Chrome" + lifecycle = result["browser_lifecycle"] + assert lifecycle["state"] == "timed_out" + assert lifecycle["cleanup"]["verified"] is True + assert [stage["stage"] for stage in lifecycle["stages"]] == ["open", "forced_cleanup"] + assert sum(1 for call in calls if "open" in call) == 1, "remote opens are never retried" + + +def test_launch_failure_is_reported_and_cleaned(browser_env) -> None: + state, _, cleaned, _ = browser_env + + async def _no_sandbox(command): + return 1, ("Chrome exited early (exit code: unknown) without writing DevToolsActivePort\n" + "FATAL: No usable sandbox!") + + state["behaviour"] = _no_sandbox + result = _run({"action": "open", "url": "https://example.com"}, {"session_id": "s-launch"}) + + assert result["exit_code"] == 1 + assert "could not launch the browser" in result["error"] + assert cleaned == ["s-launch"] + assert result["browser_lifecycle"]["state"] == "launch_failed" + assert result["browser_lifecycle"]["navigation_generation"] == 0 + + +def test_observation_after_failed_navigation_is_marked_stale(browser_env) -> None: + state, _, _, _ = browser_env + + async def _behaviour(command): + if command[-2:] == ["open", "https://good.example/"]: + return 0, "✓ Good\n https://good.example/\n" + if "open" in command: + return 1, "net::ERR_NAME_NOT_RESOLVED" + return 0, '- heading "Good page" [ref=e1]' + + state["behaviour"] = _behaviour + ctx = {"session_id": "s-stale"} + opened = _run({"action": "open", "url": "https://good.example/"}, ctx) + assert opened["browser_lifecycle"]["navigation_generation"] == 1 + assert opened["browser_lifecycle"]["page_url"] == "https://good.example/" + + failed = _run({"action": "open", "url": "https://bad.example/"}, ctx) + assert failed["exit_code"] == 1 + assert failed["browser_lifecycle"]["state"] == "navigation_failed" + + observed = _run({"action": "snapshot"}, ctx) + assert observed["output"].startswith("[Browser lifecycle: the most recent navigation to https://bad.example/ failed") + assert "shows https://good.example/ (navigation #1)" in observed["output"] + assert observed["browser_lifecycle"]["stale_observation"] is True + + _run({"action": "open", "url": "https://good.example/"}, ctx) + fresh = _run({"action": "snapshot"}, ctx) + assert not fresh["output"].startswith("[Browser lifecycle") + assert "stale_observation" not in fresh["browser_lifecycle"] + + +def test_sessionless_call_gets_its_own_browser_and_closes_it(browser_env, monkeypatch) -> None: + state, calls, cleaned, _ = browser_env + monkeypatch.setattr(PrivateBrowserTool, "_owned_daemon_exists", staticmethod(lambda env, session: True)) + + async def _ok(command): + return 0, "✓ T\n https://example.com/\n" + + state["behaviour"] = _ok + first = _run({"action": "open", "url": "https://example.com/"}, {}) + second = _run({"action": "open", "url": "https://example.com/"}, {}) + + sessions = [call[call.index("--session") + 1] for call in calls if "--session" in call] + assert all(session.startswith("ody-") for session in sessions) + assert len({sessions[0], sessions[-1]}) == 2, "sessionless calls must not share a browser" + assert any(call[-1] == "close" for call in calls) + assert first["browser_lifecycle"]["ownership"] == "ephemeral" + assert first["browser_lifecycle"]["cleanup"]["graceful_close"] is True + assert first["browser_lifecycle"]["state"] == "closed" + assert len(cleaned) == 2 + assert not web_tools._ACTIVE_BROWSER_SESSIONS.intersection(sessions) + assert not any(browser_lifecycle.registered(s) for s in sessions) + assert second["exit_code"] == 0 + + +def test_actions_on_one_session_are_serialized(browser_env) -> None: + state, _, _, _ = browser_env + active = {"now": 0, "peak": 0} + + async def _slow(command): + active["now"] += 1 + active["peak"] = max(active["peak"], active["now"]) + await asyncio.sleep(0.02) + active["now"] -= 1 + return 0, '- heading "x"' + + state["behaviour"] = _slow + + async def _both(): + tool = PrivateBrowserTool() + await asyncio.gather( + tool.execute(json.dumps({"action": "snapshot"}), {"session_id": "s-lock"}), + tool.execute(json.dumps({"action": "snapshot"}), {"session_id": "s-lock"}), + ) + + asyncio.run(_both()) + assert active["peak"] == 1 + + +def test_cancellation_stops_clients_and_cleans_the_session(browser_env, monkeypatch) -> None: + state, calls, cleaned, _ = browser_env + terminated = [] + + async def _forever(command): + await asyncio.sleep(3600) + + state["behaviour"] = _forever + monkeypatch.setattr( + PrivateBrowserTool, "_terminate_subprocess", + staticmethod(lambda proc: terminated.append(proc.command)), + ) + + async def _cancel(): + task = asyncio.create_task(PrivateBrowserTool().execute( + json.dumps({"action": "open", "url": "https://example.com"}), + {"session_id": "s-cancel"}, + )) + while not calls: + await asyncio.sleep(0.01) + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + asyncio.run(_cancel()) + + assert terminated and terminated[0][-1] == "https://example.com" + assert cleaned == ["s-cancel"] + key = web_tools._scoped_browser_session("odysseus-ui", "s-cancel") + assert browser_lifecycle.registered(key).state == "cancelled" + + +def test_local_open_recovery_is_single_and_inside_the_deadline(browser_env, monkeypatch, tmp_path) -> None: + state, calls, cleaned, _ = browser_env + page = tmp_path / "page.html" + page.write_text("x") + + async def _hang(command): + raise asyncio.TimeoutError() + + state["behaviour"] = _hang + payload = {"action": "open", "url": "/workspace/page.html", "_odysseus_browser_retry": True} + result = _run(payload, {"session_id": "s-retry"}) + + opens = [call for call in calls if call[-1] == page.as_uri()] + assert len(opens) == 2, "a model-supplied retry flag must not change recovery" + assert result["browser_lifecycle"]["recovery_attempts"] == 1 + assert cleaned == ["s-retry", "s-retry"] + + calls.clear() + monkeypatch.setattr(PrivateBrowserTool, "_RECOVERY_BUDGET_S", 0) + exhausted = _run({"action": "open", "url": "/workspace/page.html", "timeout_ms": 1000}, {"session_id": "s-budget"}) + assert len([call for call in calls if call[-1] == page.as_uri()]) == 1 + assert "recovery_attempts" not in exhausted["browser_lifecycle"] + + +def test_research_reader_passes_its_timeout_to_the_browser(monkeypatch) -> None: + from src.research_navigator import ResearchNavigator + + seen = {} + + async def _execute(self, content, ctx): + seen.update(json.loads(content)) + return {"output": "", "exit_code": 1} + + monkeypatch.setattr(PrivateBrowserTool, "execute", _execute) + navigator = ResearchNavigator.__new__(ResearchNavigator) + navigator._progress = None + navigator.session_id = "r" + asyncio.run(navigator.browser_read("https://example.com", timeout=12)) + + assert seen["timeout_ms"] == 12000 + + +# Real browser: open/extract a local HTML page, then prove the cleanup paths +# leave no process, profile or runtime file behind. + +def _real_browser(): + binary = shutil.which("agent-browser") or PrivateBrowserTool._local_agent_browser_binary() + chrome = web_tools._browser_executable_candidates() + if not binary or not chrome or not platform_compat.has_procfs(): + return None + return binary, str(chrome[0]) + + +REAL = _real_browser() +real_browser = pytest.mark.skipif(REAL is None, reason="agent-browser and Chromium are not installed") + + +@pytest.fixture +def real_runtime(monkeypatch, tmp_path): + # agent-browser's Unix socket path must stay under ~103 bytes. + runtime = Path(tempfile.mkdtemp(prefix="abt", dir="/tmp")) + workspace = tmp_path / "ws" + workspace.mkdir() + monkeypatch.setattr("src.tool_execution.get_active_workspace", lambda: str(workspace)) + monkeypatch.setattr(web_tools.shutil, "which", lambda name: REAL[0] if name == "agent-browser" else shutil.which(name)) + env = { + "XDG_RUNTIME_DIR": str(runtime), + "TMPDIR": str(runtime / "tmp"), + "AGENT_BROWSER_EXECUTABLE_PATH": REAL[1], + "AGENT_BROWSER_IDLE_TIMEOUT_MS": "60000", + # Hosts that restrict unprivileged user namespaces cannot start + # Chrome's sandbox. Test-only; production launch flags are unchanged. + "AGENT_BROWSER_ARGS": "--no-sandbox", + } + (runtime / "tmp").mkdir() + yield workspace, runtime, env + for pid_file in (runtime / "agent-browser").glob("*.pid"): + browser_lifecycle.force_cleanup(runtime / "agent-browser", pid_file.stem) + shutil.rmtree(runtime, ignore_errors=True) + + +def _owned_processes(runtime: Path) -> list[int]: + owned = [] + for entry in platform_compat.PROC_ROOT.iterdir(): + if not entry.name.isdigit(): + continue + try: + text = (entry / "cmdline").read_bytes().decode(errors="replace") + except OSError: + continue + if str(runtime) in text: + owned.append(int(entry.name)) + return owned + + +@real_browser +def test_real_local_page_open_extract_and_ephemeral_cleanup(real_runtime) -> None: + workspace, runtime, env = real_runtime + (workspace / "page.html").write_text( + "Lifecycle

Fresh heading

" + ) + + result = _run( + {"action": "batch", "commands": [["open", "/workspace/page.html"], ["snapshot"]]}, + {"subproc_env": env}, + ) + + assert result["exit_code"] == 0, result + assert "Fresh heading" in result["output"] + lifecycle = result["browser_lifecycle"] + assert lifecycle["ownership"] == "ephemeral" + assert lifecycle["navigation_generation"] == 1 + assert lifecycle["state"] == "closed" and lifecycle["page_url"] == "" + assert lifecycle["closed_page_url"].endswith("/page.html") + assert lifecycle["cleanup"]["verified"] is True + assert [stage["stage"] for stage in lifecycle["stages"]] == ["batch", "close"] + time.sleep(0.5) + assert _owned_processes(runtime) == [] + assert list((runtime / "agent-browser").glob("ody-*")) == [] + assert list((runtime / "tmp").glob("agent-browser-chrome-*")) == [] + + +@real_browser +def test_real_retained_session_survives_then_forced_cleanup_leaves_nothing(real_runtime) -> None: + workspace, runtime, env = real_runtime + (workspace / "a.html").write_text("A

Alpha

") + ctx = {"session_id": "retained", "subproc_env": env} + + opened = _run({"action": "open", "url": "/workspace/a.html"}, ctx) + assert opened["exit_code"] == 0, opened + observed = _run({"action": "snapshot"}, ctx) + assert "Alpha" in observed["output"] + assert observed["browser_lifecycle"]["ownership"] == "retained" + assert _owned_processes(runtime), "a retained session keeps its browser" + + receipt = PrivateBrowserTool._terminate_owned_daemon(dict(os.environ, **env), "retained") + + assert receipt["verified"] is True and receipt["killed"] >= 2 + assert receipt["removed_profiles"] == 1 + assert _owned_processes(runtime) == [] + assert list((runtime / "agent-browser").glob("ody-*")) == [] + + +@real_browser +def test_real_cancellation_leaves_no_browser(real_runtime) -> None: + workspace, runtime, env = real_runtime + (workspace / "slow.html").write_text("S

Slow

") + ctx = {"session_id": "cancelled", "subproc_env": env} + assert _run({"action": "open", "url": "/workspace/slow.html"}, ctx)["exit_code"] == 0 + + async def _cancel_wait(): + task = asyncio.create_task(PrivateBrowserTool().execute( + json.dumps({"action": "wait", "timeout_ms": 30000}), ctx, + )) + await asyncio.sleep(1.5) + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + asyncio.run(_cancel_wait()) + time.sleep(0.5) + assert _owned_processes(runtime) == [] + assert list((runtime / "agent-browser").glob("ody-*")) == [] + + +def test_browser_mcp_call_is_bounded_and_never_replayed(monkeypatch) -> None: + from src.mcp_manager import McpManager + + manager = McpManager() + calls = [] + + class _Session: + async def call_tool(self, name, arguments): + calls.append(name) + await asyncio.sleep(3600) + + manager._sessions["builtin_browser"] = _Session() + monkeypatch.setenv("ODYSSEUS_BROWSER_MCP_CALL_TIMEOUT_S", "0.05") + + result = asyncio.run(manager.call_tool( + "mcp__builtin_browser__browser_navigate", {"url": "https://example.com"} + )) + + assert result["exit_code"] == 1 + assert "timed out after 0.05s and was not retried" in result["error"] + assert calls == ["browser_navigate"] + + +def test_read_url_navigates_and_extracts_in_one_observation(browser_env) -> None: + state, calls, _, _ = browser_env + + async def _batch(command): + return 0, json.dumps([ + {"command": ["open", "https://example.com/"], "success": True, + "result": {"title": "Example", "url": "https://example.com/final"}}, + {"command": ["get", "text", "body"], "success": True, + "result": {"text": "Example body"}}, + ]) + + state["behaviour"] = _batch + result = _run({"action": "read", "url": "https://example.com/"}, {"session_id": "s-read"}) + + assert calls[-1][-2:] == ["batch", "--json"] + assert result["exit_code"] == 0 + assert result["output"] == "Example\nhttps://example.com/final\n\nExample body" + assert result["browser_lifecycle"]["page_url"] == "https://example.com/final" + + +def test_read_url_without_extracted_text_is_a_failure(browser_env) -> None: + state, _, _, _ = browser_env + + async def _no_text(command): + return 0, json.dumps([ + {"success": True, "result": {"url": "https://example.com/"}}, + {"success": False, "error": "Timeout waiting for body", "result": None}, + ]) + + state["behaviour"] = _no_text + result = _run({"action": "read", "url": "https://example.com/"}, {"session_id": "s-read-fail"}) + + assert result["exit_code"] == 1 + assert "Timeout waiting for body" in result["error"] + assert result["browser_lifecycle"]["state"] == "navigation_failed" + + +@real_browser +def test_real_read_url_extracts_text_after_navigation(real_runtime) -> None: + import functools + import http.server + import threading + + workspace, runtime, env = real_runtime + (workspace / "doc.html").write_text("Doc

Served heading

Body text

") + handler = functools.partial(http.server.SimpleHTTPRequestHandler, directory=str(workspace)) + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + url = f"http://127.0.0.1:{server.server_address[1]}/doc.html" + result = _run({"action": "read", "url": url}, {"subproc_env": env}) + finally: + server.shutdown() + server.server_close() + + assert result["exit_code"] == 0, result + assert result["output"].startswith(f"Doc\n{url}") + assert "Served heading" in result["output"] and "Body text" in result["output"] + assert result["browser_lifecycle"]["closed_page_url"] == url + assert result["browser_lifecycle"]["cleanup"]["verified"] is True + time.sleep(0.5) + assert _owned_processes(runtime) == [] + + +def test_selector_read_is_an_observation_not_a_navigation() -> None: + assert PrivateBrowserTool._navigation_target( + "read", {"selector": "#main", "url": "https://elsewhere.example/"} + ) == "" + assert PrivateBrowserTool._navigation_target( + "batch", {"commands": [["open", "file:///a.html"], ["snapshot"], ["open", "file:///b.html"]]} + ) == "file:///b.html" diff --git a/tests/test_private_browser_tool.py b/tests/test_private_browser_tool.py index b212b2f47..8163c43df 100644 --- a/tests/test_private_browser_tool.py +++ b/tests/test_private_browser_tool.py @@ -951,12 +951,19 @@ def test_private_browser_reuses_host_npx_cache_when_task_home_is_isolated( class _FakeProc: returncode = 0 + def __init__(self, stdout): + self.stdout = stdout + async def communicate(self, stdin=None): - return b"page text", b"" + self.stdout.write(json.dumps([ + {"success": True, "result": {"title": "T", "url": "https://example.com/"}}, + {"success": True, "result": {"text": "page text"}}, + ]).encode()) + return None, None async def _fake_create_subprocess_exec(*command, **kwargs): calls["env"] = kwargs["env"] - return _FakeProc() + return _FakeProc(kwargs["stdout"]) monkeypatch.setattr( asyncio, "create_subprocess_exec", _fake_create_subprocess_exec @@ -1049,7 +1056,7 @@ def test_browser_executable_discovery_supports_chromium_snapshot_cache( assert web_tools._browser_executable_candidates() == [chrome] -def test_private_browser_shutdown_is_namespace_scoped(monkeypatch) -> None: +def test_private_browser_shutdown_is_namespace_scoped(monkeypatch, tmp_path) -> None: calls = {} class _FakeProc: @@ -1058,9 +1065,18 @@ def test_private_browser_shutdown_is_namespace_scoped(monkeypatch) -> None: monkeypatch.setattr(web_tools.shutil, "which", lambda name: "/bin/agent-browser") monkeypatch.setenv("ODYSSEUS_BROWSER_NAMESPACE", "clawmm-test") + monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path)) + monkeypatch.delenv("AGENT_BROWSER_SOCKET_DIR", raising=False) web_tools._ACTIVE_BROWSER_SESSIONS.clear() session = web_tools._scoped_browser_session("clawmm-test", "session-1") web_tools._ACTIVE_BROWSER_SESSIONS.add(session) + (tmp_path / "agent-browser").mkdir() + (tmp_path / "agent-browser" / f"{session}.pid").write_text("7001") + proc = tmp_path / "proc" + (proc / "7001").mkdir(parents=True) + (proc / "7001" / "cmdline").write_bytes(b"agent-browser-linux-x64\0") + monkeypatch.setattr(platform_compat, "PROC_ROOT", proc) + monkeypatch.setattr(web_tools.os, "kill", lambda pid, sig: None) async def _fake_create_subprocess_exec(*command, **kwargs): calls["command"] = command @@ -1076,6 +1092,27 @@ def test_private_browser_shutdown_is_namespace_scoped(monkeypatch) -> None: assert not web_tools._ACTIVE_BROWSER_SESSIONS +def test_private_browser_shutdown_never_bootstraps_a_missing_daemon( + monkeypatch, tmp_path +) -> None: + monkeypatch.setattr(web_tools.shutil, "which", lambda name: "/bin/agent-browser") + monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path)) + monkeypatch.setattr(platform_compat, "PROC_ROOT", tmp_path / "proc") + (tmp_path / "proc").mkdir() + web_tools._ACTIVE_BROWSER_SESSIONS.clear() + web_tools._ACTIVE_BROWSER_SESSIONS.add( + web_tools._scoped_browser_session("odysseus-ui", "never-started") + ) + + async def _no_spawn(*command, **kwargs): + pytest.fail(f"close would start a fresh daemon: {command}") + + monkeypatch.setattr(asyncio, "create_subprocess_exec", _no_spawn) + asyncio.run(shutdown_private_browser_sessions()) + + assert not web_tools._ACTIVE_BROWSER_SESSIONS + + def test_browser_pid_candidates_include_upstream_root_session() -> None: runtime = Path("/run/user/1000") namespace = "clawmm-test" diff --git a/website/configuration-reference.md b/website/configuration-reference.md index 69b3bea60..b5d8b7a6b 100644 --- a/website/configuration-reference.md +++ b/website/configuration-reference.md @@ -21,7 +21,7 @@ described as a switch that turns something off, the read rejects `0`, `false`, `no` and `off` and treats everything else as on. The `Default` column is the value the code falls back to when the variable is unset, quoted from the source. -The source tree reads **108** `ODYSSEUS_*` variables: 78 an operator may want to set, and 30 that are internal - sentinels, fixture switches, capture hooks and development tooling. The internal ones are listed too, in their own section, so this page can be checked against the source mechanically. +The source tree reads **109** `ODYSSEUS_*` variables: 79 an operator may want to set, and 30 that are internal - sentinels, fixture switches, capture hooks and development tooling. The internal ones are listed too, in their own section, so this page can be checked against the source mechanically. > This page is generated. Edit `scripts/generate_env_reference.py` and > re-run it; `tests/test_env_reference.py` enforces that the committed page @@ -86,10 +86,11 @@ The source tree reads **108** `ODYSSEUS_*` variables: 78 an operator may want to | `ODYSSEUS_BROWSER_EXECUTABLE` | `''` | `src/builtin_mcp.py:114` | Absolute path to the Chrome or Chromium binary. Empty searches the usual names, then lets Playwright MCP pick its own browser. | | `ODYSSEUS_BROWSER_ISOLATED` | `'1'` | `src/builtin_mcp.py:139` | Security-relevant. On by default, adding `--isolated` so each browser session starts clean. Set 0, false or no to keep a persistent profile. | | `ODYSSEUS_BROWSER_MCP_CACHE` | `os.path.join(base_dir, 'data', 'local', 'playwright-mcp-cache')` | `src/builtin_mcp.py:229` | Cache directory handed to the browser MCP server, so its npm download survives a container rebuild. | +| `ODYSSEUS_BROWSER_MCP_CALL_TIMEOUT_S` | `'90'` | `src/mcp_manager.py:27` | Upper bound in seconds for one browser MCP tool call. A call that exceeds it fails without being retried. | | `ODYSSEUS_BROWSER_MCP_REQUIRE_CACHE` | `''` | `src/builtin_mcp.py:90` | Truthy refuses to start the browser MCP server unless its npm package is already in the npx cache, instead of installing it at startup. | -| `ODYSSEUS_BROWSER_NAMESPACE` | `'odysseus-ui'` | `src/agent_tools/web_tools.py:2441` (+5 more) | Namespace for the detached agent-browser daemon's pid files, so two runtimes on one machine do not terminate each other's browsers. | +| `ODYSSEUS_BROWSER_NAMESPACE` | `'odysseus-ui'` | `src/agent_tools/web_tools.py:100` (+3 more) | Namespace for the detached agent-browser daemon's pid files, so two runtimes on one machine do not terminate each other's browsers. | | `ODYSSEUS_BROWSER_NO_SANDBOX` | `'1'` | `src/builtin_mcp.py:142` | Security-relevant. On by default, adding `--no-sandbox` because the Docker image cannot use the Chromium sandbox. Set 0, false or no to keep it. | -| `ODYSSEUS_BROWSER_SCREENSHOT_DIR` | *unset* | `src/agent_tools/web_tools.py:3135` | Where private-browser screenshots are written. Falls back to the container path, then the system temp directory. | +| `ODYSSEUS_BROWSER_SCREENSHOT_DIR` | *unset* | `src/agent_tools/web_tools.py:3423` | Where private-browser screenshots are written. Falls back to the container path, then the system temp directory. | ### Container and workspace mounts @@ -256,7 +257,7 @@ reads three ways, because no single pattern covers the codebase: lines, so one read lives inside a string literal. The three passes are not redundancy. A line-based grep for a direct -`os.environ.get("ODYSSEUS_...` call finds 80 of the 108 variables on this +`os.environ.get("ODYSSEUS_...` call finds 81 of the 109 variables on this page. What it misses is reads through an env-reader helper, reads whose call spans more than one line, reads whose variable name is held in a module constant, and reads through a mapping passed in as an argument - which is the