diff --git a/docs/runtime-decomposition/wave-5a-browser-lifecycle.md b/docs/runtime-decomposition/wave-5a-browser-lifecycle.md index c33f81bf9..3b02b4457 100644 --- a/docs/runtime-decomposition/wave-5a-browser-lifecycle.md +++ b/docs/runtime-decomposition/wave-5a-browser-lifecycle.md @@ -50,7 +50,7 @@ TurnContract, generic process containment (Wave 3-S), effects/provenance | 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` | +| 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`. A batch's navigation outcome comes from its per-command rows; when it cannot be determined the page is treated as unknown | | 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 | @@ -61,7 +61,7 @@ TurnContract, generic process containment (Wave 3-S), effects/provenance ## Lifecycle model -Session states: `idle`, `ready`, `navigation_failed`, `reset`, `timed_out`, +Session states: `idle`, `ready`, `navigation_failed`, `navigation_unknown`, `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 @@ -113,3 +113,8 @@ process-lifecycle primitives from Wave 3-S/5B. server requires its owner task in `builtin_mcp.py`. - The stale-observation notice marks, but does not block, an observation after a failed navigation. +- Forced cleanup waits synchronously, at most one second, for killed processes + to exit, so it can run from cancellation without awaiting. +- The recovery deadline covers the action and its retry. Post-action + observations (page errors, settled snapshot, screenshot) keep their own + 20 second bounds outside it. diff --git a/src/agent_tools/web_tools.py b/src/agent_tools/web_tools.py index b18757d37..bff68ef66 100644 --- a/src/agent_tools/web_tools.py +++ b/src/agent_tools/web_tools.py @@ -2757,6 +2757,33 @@ class PrivateBrowserTool: 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 _batch_navigation_outcome(output: str, command_ok: bool) -> tuple[str, str]: + """Outcome of a batch's last navigation: ``ok``, ``failed`` or ``unknown``. + + A later command failing does not undo a navigation that succeeded, + so the per-command rows decide, not the batch exit status. + """ + + try: + rows = json.loads(output) + except (ValueError, TypeError): + rows = None + if isinstance(rows, list): + for row in reversed(rows): + command = row.get("command") if isinstance(row, dict) else None + if not ( + isinstance(command, list) + and command + and str(command[0]).lower() in {"open", "goto", "navigate"} + ): + continue + if row.get("success") is True: + result = row.get("result") if isinstance(row.get("result"), dict) else {} + return "ok", str(result.get("url") or "") + return "failed", "" + return ("ok", "") if command_ok else ("unknown", "") + @staticmethod def _navigated_url(output: str) -> str: """Final URL reported by ``open`` (after redirects), when present.""" @@ -3028,12 +3055,20 @@ class PrivateBrowserTool: "untrusted_content": True, } if navigation_url: - if command_ok: + outcome, final_url = "ok" if command_ok else "failed", "" + if action == "batch": + outcome, final_url = self._batch_navigation_outcome(out, command_ok) + if outcome == "ok": browser.navigated( - (read_page or {}).get("url") or self._navigated_url(out) or navigation_url + final_url + or (read_page or {}).get("url") + or self._navigated_url(out) + or navigation_url ) - else: + elif outcome == "failed": browser.navigation_failed(navigation_url) + else: + browser.navigation_unknown(navigation_url) if read_page is not None: if not command_ok: return { diff --git a/src/browser_lifecycle.py b/src/browser_lifecycle.py index 689048309..7d266ed63 100644 --- a/src/browser_lifecycle.py +++ b/src/browser_lifecycle.py @@ -300,6 +300,7 @@ class BrowserSession: navigation_generation: int = 0 page_url: str = "" failed_navigation_url: str = "" + navigation_outcome_unknown: bool = False _lock: asyncio.Lock | None = field(default=None, repr=False) _lock_loop: Any = field(default=None, repr=False) @@ -321,22 +322,39 @@ class BrowserSession: self.navigation_generation += 1 self.page_url = url self.failed_navigation_url = "" + self.navigation_outcome_unknown = False self.state = "ready" def navigation_failed(self, url: str) -> None: self.failed_navigation_url = url + self.navigation_outcome_unknown = False self.state = "navigation_failed" + def navigation_unknown(self, url: str) -> None: + """A navigation was attempted but whether it happened is unknown.""" + + self.page_url = "" + self.failed_navigation_url = url + self.navigation_outcome_unknown = True + self.state = "navigation_unknown" + 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.navigation_outcome_unknown = False self.state = state def stale_observation_note(self) -> str: if not self.failed_navigation_url: return "" + if self.navigation_outcome_unknown: + return ( + f"Browser lifecycle: the outcome of the most recent navigation to " + f"{self.failed_navigation_url} is unknown. This observation may not " + f"show {self.failed_navigation_url}." + ) shown = self.page_url or "an earlier page" return ( f"Browser lifecycle: the most recent navigation to {self.failed_navigation_url} " diff --git a/tests/test_browser_lifecycle.py b/tests/test_browser_lifecycle.py index 5b8889130..55450de38 100644 --- a/tests/test_browser_lifecycle.py +++ b/tests/test_browser_lifecycle.py @@ -579,3 +579,41 @@ def test_selector_read_is_an_observation_not_a_navigation() -> None: assert PrivateBrowserTool._navigation_target( "batch", {"commands": [["open", "file:///a.html"], ["snapshot"], ["open", "file:///b.html"]]} ) == "file:///b.html" + + +def test_batch_navigation_outcome_comes_from_its_rows(browser_env) -> None: + state, _, _, _ = browser_env + responses = {} + + async def _batch(command): + if command[-2:] == ["batch", "--json"]: + return responses["batch"] + return 0, '- heading "x"' + + state["behaviour"] = _batch + ctx = {"session_id": "s-batch"} + + # The open succeeded; a later click failing must not mark it failed. + responses["batch"] = (1, json.dumps([ + {"command": ["open", "https://a.example/"], "success": True, + "result": {"url": "https://a.example/landing"}}, + {"command": ["click", "@e9"], "success": False, "error": "no element"}, + ])) + result = _run({"action": "batch", "commands": [["open", "https://a.example/"], ["click", "@e9"]]}, ctx) + assert result["browser_lifecycle"]["page_url"] == "https://a.example/landing" + assert result["browser_lifecycle"]["state"] == "ready" + assert "stale_observation" not in _run({"action": "snapshot"}, ctx)["browser_lifecycle"] + + responses["batch"] = (1, json.dumps([ + {"command": ["open", "https://b.example/"], "success": False, "error": "net::ERR"}, + ])) + failed = _run({"action": "batch", "commands": [["open", "https://b.example/"]]}, ctx) + assert failed["browser_lifecycle"]["state"] == "navigation_failed" + note = _run({"action": "snapshot"}, ctx)["output"] + assert "shows https://a.example/landing (navigation #1), not https://b.example/" in note + + responses["batch"] = (1, "daemon connection lost") + _run({"action": "batch", "commands": [["open", "https://c.example/"]]}, ctx) + unknown = _run({"action": "snapshot"}, ctx) + assert "outcome of the most recent navigation to https://c.example/ is unknown" in unknown["output"] + assert unknown["browser_lifecycle"]["page_url"] == "" diff --git a/website/configuration-reference.md b/website/configuration-reference.md index b5d8b7a6b..cf21c8a19 100644 --- a/website/configuration-reference.md +++ b/website/configuration-reference.md @@ -90,7 +90,7 @@ The source tree reads **109** `ODYSSEUS_*` variables: 79 an operator may want to | `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: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:3423` | 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:3458` | Where private-browser screenshots are written. Falls back to the container path, then the system temp directory. | ### Container and workspace mounts