From a5c940da61f8d694b41fcd127958180cf0c43c26 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sun, 19 Apr 2026 20:16:52 +0200 Subject: [PATCH 01/13] fix(browser): add MCP timeout guard to prevent fs hangs --- .../cli_anything/browser/README.md | 8 +++ .../cli_anything/browser/skills/SKILL.md | 1 + .../cli_anything/browser/tests/test_core.py | 44 +++++++++++- .../browser/utils/domshell_backend.py | 69 ++++++++++++++++--- registry.json | 2 +- 5 files changed, 111 insertions(+), 13 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/README.md b/browser/agent-harness/cli_anything/browser/README.md index 8bee2284f1..1feebe9fe0 100644 --- a/browser/agent-harness/cli_anything/browser/README.md +++ b/browser/agent-harness/cli_anything/browser/README.md @@ -241,10 +241,18 @@ Run: `npx @apireno/domshell --version` (first run downloads the package) - Ensure Chrome is running - Install DOMShell extension from Chrome Web Store - Check that DOMShell is enabled in Chrome +- Verify `DOMSHELL_TOKEN` matches the token used by the running DOMShell server ### Commands hang on first use First `npx` call downloads DOMShell package (10-50 MB). Subsequent calls are faster. Use `--daemon` mode for persistent connection. +If the server is unreachable or auth token is wrong, MCP calls now fail with timeout instead of hanging forever. +Set timeout (seconds) via: + +```bash +export CLI_ANYTHING_BROWSER_MCP_TIMEOUT=20 +``` + ## Architecture This CLI follows the [CLI-Anything harness methodology](https://github.com/HKUDS/CLI-Anything/tree/main/cli-anything-plugin/HARNESS.md): diff --git a/browser/agent-harness/cli_anything/browser/skills/SKILL.md b/browser/agent-harness/cli_anything/browser/skills/SKILL.md index 73b9e4d4a2..640ef234f9 100644 --- a/browser/agent-harness/cli_anything/browser/skills/SKILL.md +++ b/browser/agent-harness/cli_anything/browser/skills/SKILL.md @@ -158,6 +158,7 @@ The CLI provides clear error messages for common issues: - **npx not found**: Install Node.js from https://nodejs.org/ - **DOMShell not found**: Run `npx @apireno/domshell --version` - **MCP call failed**: Install DOMShell Chrome extension +- **MCP timeout / hanging calls**: Verify `DOMSHELL_TOKEN`, ensure DOMShell server is running, and optionally tune timeout via `CLI_ANYTHING_BROWSER_MCP_TIMEOUT` (default: 20s) Check `is_available()` return value before running commands. diff --git a/browser/agent-harness/cli_anything/browser/tests/test_core.py b/browser/agent-harness/cli_anything/browser/tests/test_core.py index cb807a283a..3723eb32cf 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_core.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_core.py @@ -7,10 +7,12 @@ """ import pytest -from unittest.mock import AsyncMock, MagicMock, patch +import asyncio +from unittest.mock import patch from cli_anything.browser.core.session import Session from cli_anything.browser.core import page, fs +from cli_anything.browser.utils import domshell_backend as backend_mod # ── Session Tests ──────────────────────────────────────────────── @@ -387,3 +389,43 @@ def test_normal_mode_does_not_use_daemon(self): result = fs.list_elements(sess) mock_ls.assert_called_once_with("/", use_daemon=False) + + +class TestBackendTimeouts: + """Test backend timeout parsing and behavior.""" + + def test_timeout_default_value(self, monkeypatch): + """Timeout defaults to safe value when env var is unset.""" + monkeypatch.delenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", raising=False) + assert backend_mod._get_tool_timeout_seconds() == 20.0 + + def test_timeout_invalid_env_falls_back_to_default(self, monkeypatch): + """Invalid timeout env var falls back to default.""" + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "not-a-number") + assert backend_mod._get_tool_timeout_seconds() == 20.0 + + def test_timeout_is_clamped_to_minimum(self, monkeypatch): + """Timeout values below 1 second are clamped to 1 second.""" + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "0") + assert backend_mod._get_tool_timeout_seconds() == 1.0 + + def test_await_with_timeout_passes_fast_calls(self, monkeypatch): + """Fast operations should complete without timeout errors.""" + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "5") + + async def _fast(): + return {"ok": True} + + result = asyncio.run(backend_mod._await_with_timeout(_fast(), "unit-test")) + assert result == {"ok": True} + + def test_await_with_timeout_raises_runtime_error_on_timeout(self, monkeypatch): + """Slow operations should raise actionable RuntimeError.""" + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "1") + + async def _slow(): + await asyncio.sleep(2) + return {"ok": True} + + with pytest.raises(RuntimeError, match="timed out"): + asyncio.run(backend_mod._await_with_timeout(_slow(), "unit-test")) diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index eafc25767b..7d064bfc8e 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -16,7 +16,7 @@ import os import subprocess import shutil -from typing import Any, Optional +from typing import Any, Awaitable, Optional from mcp import ClientSession, StdioServerParameters from mcp.client.stdio import stdio_client @@ -26,6 +26,7 @@ # DOMSHELL_TOKEN — auth token (required, must match the running server) # DOMSHELL_PORT — MCP HTTP port of the running server (default: 3001) DEFAULT_SERVER_CMD = "npx" +DEFAULT_TOOL_TIMEOUT_SECONDS = 20.0 def _build_server_args() -> list[str]: @@ -45,6 +46,32 @@ def _build_server_args() -> list[str]: "--token", token, ] + +def _get_tool_timeout_seconds() -> float: + """Read MCP tool timeout (seconds) from env with safe bounds.""" + raw = os.environ.get( + "CLI_ANYTHING_BROWSER_MCP_TIMEOUT", + str(DEFAULT_TOOL_TIMEOUT_SECONDS), + ) + try: + parsed = float(raw) + except (TypeError, ValueError): + return DEFAULT_TOOL_TIMEOUT_SECONDS + return max(1.0, parsed) + + +async def _await_with_timeout(coro: Awaitable[Any], operation: str) -> Any: + """Await an MCP operation with timeout and actionable error text.""" + timeout_seconds = _get_tool_timeout_seconds() + try: + return await asyncio.wait_for(coro, timeout=timeout_seconds) + except asyncio.TimeoutError as e: + raise RuntimeError( + "DOMShell MCP request timed out after " + f"{timeout_seconds:.1f}s during {operation}. " + "Verify DOMSHELL_TOKEN and that the DOMShell server is reachable." + ) from e + # Daemon mode: persistent MCP connection _daemon_session: Optional[ClientSession] = None _daemon_read: Optional[Any] = None @@ -133,9 +160,12 @@ async def _call_tool( if use_daemon and _daemon_session is not None: # Use persistent daemon connection try: - result = await _daemon_session.call_tool(tool_name, arguments) + result = await _await_with_timeout( + _daemon_session.call_tool(tool_name, arguments), + f"{tool_name} (daemon)", + ) return result - except Exception as e: + except Exception: # Daemon died, fall back to spawning new server await _stop_daemon() @@ -148,9 +178,14 @@ async def _call_tool( try: async with stdio_client(server_params) as (read, write): async with ClientSession(read, write) as session: - await session.initialize() - result = await session.call_tool(tool_name, arguments) + await _await_with_timeout(session.initialize(), "session initialize") + result = await _await_with_timeout( + session.call_tool(tool_name, arguments), + tool_name, + ) return result + except RuntimeError: + raise except Exception as e: raise RuntimeError( f"DOMShell MCP call failed: {e}\n" @@ -188,7 +223,7 @@ async def _start_daemon() -> bool: _daemon_read, _daemon_write = await _daemon_client_context.__aenter__() _daemon_session = ClientSession(_daemon_read, _daemon_write) await _daemon_session.__aenter__() - await _daemon_session.initialize() + await _await_with_timeout(_daemon_session.initialize(), "daemon initialize") return True except Exception as e: _daemon_session = None @@ -395,8 +430,14 @@ def type_text(path: str, text: str, use_daemon: bool = False) -> dict: async def _focus_and_type(): global _daemon_session if use_daemon and _daemon_session is not None: - await _daemon_session.call_tool("domshell_focus", {"name": path}) - return await _daemon_session.call_tool("domshell_type", {"text": text}) + await _await_with_timeout( + _daemon_session.call_tool("domshell_focus", {"name": path}), + "domshell_focus (daemon)", + ) + return await _await_with_timeout( + _daemon_session.call_tool("domshell_type", {"text": text}), + "domshell_type (daemon)", + ) server_params = StdioServerParameters( command=DEFAULT_SERVER_CMD, @@ -404,9 +445,15 @@ async def _focus_and_type(): ) async with stdio_client(server_params) as (read, write): async with ClientSession(read, write) as session: - await session.initialize() - await session.call_tool("domshell_focus", {"name": path}) - return await session.call_tool("domshell_type", {"text": text}) + await _await_with_timeout(session.initialize(), "session initialize") + await _await_with_timeout( + session.call_tool("domshell_focus", {"name": path}), + "domshell_focus", + ) + return await _await_with_timeout( + session.call_tool("domshell_type", {"text": text}), + "domshell_type", + ) return asyncio.run(_focus_and_type()) diff --git a/registry.json b/registry.json index ddcba59135..37fb1386c5 100644 --- a/registry.json +++ b/registry.json @@ -103,7 +103,7 @@ { "name": "browser", "display_name": "Browser", - "version": "1.0.0", + "version": "1.0.1", "description": "Browser automation via DOMShell MCP server. Maps Chrome's Accessibility Tree to a virtual filesystem for agent-native navigation.", "requires": "Node.js, npx, Chrome + DOMShell extension", "homepage": "https://github.com/apireno/DOMShell", From 3bcd80c7e90d868395a694ea20567b69a077b8a9 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sun, 19 Apr 2026 21:39:39 +0200 Subject: [PATCH 02/13] fix(browser): avoid retrying daemon timeouts --- .../cli_anything/browser/tests/test_core.py | 30 ++++++++++++++++++- .../browser/utils/domshell_backend.py | 3 ++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_core.py b/browser/agent-harness/cli_anything/browser/tests/test_core.py index 3723eb32cf..59e9d6e2e0 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_core.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_core.py @@ -8,7 +8,7 @@ import pytest import asyncio -from unittest.mock import patch +from unittest.mock import AsyncMock, patch from cli_anything.browser.core.session import Session from cli_anything.browser.core import page, fs @@ -429,3 +429,31 @@ async def _slow(): with pytest.raises(RuntimeError, match="timed out"): asyncio.run(backend_mod._await_with_timeout(_slow(), "unit-test")) + + def test_daemon_timeout_is_not_retried_in_non_daemon_mode(self): + """Daemon timeout should bubble up and avoid duplicate tool reissue.""" + class _DummyDaemonSession: + def call_tool(self, _tool_name, _arguments): + return object() + + original_daemon = backend_mod._daemon_session + try: + backend_mod._daemon_session = _DummyDaemonSession() + + with patch( + "cli_anything.browser.utils.domshell_backend._await_with_timeout", + side_effect=RuntimeError("timed out"), + ) as mock_await, patch( + "cli_anything.browser.utils.domshell_backend._stop_daemon", + new_callable=AsyncMock, + ) as mock_stop, patch( + "cli_anything.browser.utils.domshell_backend.stdio_client", + ) as mock_stdio: + with pytest.raises(RuntimeError, match="timed out"): + asyncio.run(backend_mod._call_tool("domshell_click", {"path": "/"}, use_daemon=True)) + + mock_await.assert_called_once() + mock_stop.assert_not_awaited() + mock_stdio.assert_not_called() + finally: + backend_mod._daemon_session = original_daemon diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 7d064bfc8e..bd4f46f03c 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -165,6 +165,9 @@ async def _call_tool( f"{tool_name} (daemon)", ) return result + except RuntimeError: + # Timeout/runtime errors should not re-run potentially non-idempotent tools. + raise except Exception: # Daemon died, fall back to spawning new server await _stop_daemon() From cd5524759531dfc4064a6260d40240e42171fb8b Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sun, 19 Apr 2026 22:07:45 +0200 Subject: [PATCH 03/13] fix(browser): restrict daemon no-retry to timeout errors --- .../cli_anything/browser/tests/test_core.py | 60 ++++++++++++++++++- .../browser/utils/domshell_backend.py | 10 +++- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_core.py b/browser/agent-harness/cli_anything/browser/tests/test_core.py index 59e9d6e2e0..fca060d711 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_core.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_core.py @@ -442,7 +442,7 @@ def call_tool(self, _tool_name, _arguments): with patch( "cli_anything.browser.utils.domshell_backend._await_with_timeout", - side_effect=RuntimeError("timed out"), + side_effect=backend_mod.MCPToolTimeoutError("timed out"), ) as mock_await, patch( "cli_anything.browser.utils.domshell_backend._stop_daemon", new_callable=AsyncMock, @@ -457,3 +457,61 @@ def call_tool(self, _tool_name, _arguments): mock_stdio.assert_not_called() finally: backend_mod._daemon_session = original_daemon + + def test_daemon_runtime_error_falls_back_to_non_daemon_mode(self, monkeypatch): + """Non-timeout daemon RuntimeError should stop daemon and fallback once.""" + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + class _BrokenDaemonSession: + def call_tool(self, _tool_name, _arguments): + raise RuntimeError("loop mismatch") + + class _DummyStdioContext: + async def __aenter__(self): + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + return False + + class _DummyClientSession: + def __init__(self, _read, _write): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + def initialize(self): + return object() + + def call_tool(self, _tool_name, _arguments): + return object() + + original_daemon = backend_mod._daemon_session + try: + backend_mod._daemon_session = _BrokenDaemonSession() + + with patch( + "cli_anything.browser.utils.domshell_backend._await_with_timeout", + side_effect=[None, {"ok": True}], + ) as mock_await, patch( + "cli_anything.browser.utils.domshell_backend._stop_daemon", + new_callable=AsyncMock, + ) as mock_stop, patch( + "cli_anything.browser.utils.domshell_backend.stdio_client", + return_value=_DummyStdioContext(), + ) as mock_stdio, patch( + "cli_anything.browser.utils.domshell_backend.ClientSession", + _DummyClientSession, + ): + result = asyncio.run( + backend_mod._call_tool("domshell_click", {"path": "/"}, use_daemon=True) + ) + + assert result == {"ok": True} + mock_stop.assert_awaited_once() + assert mock_await.call_count == 2 + mock_stdio.assert_called_once() + finally: + backend_mod._daemon_session = original_daemon diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index bd4f46f03c..5355be13db 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -29,6 +29,10 @@ DEFAULT_TOOL_TIMEOUT_SECONDS = 20.0 +class MCPToolTimeoutError(RuntimeError): + """Raised when a DOMShell MCP operation exceeds configured timeout.""" + + def _build_server_args() -> list[str]: """Build server args at call time so env var changes are honored.""" token = os.environ.get("DOMSHELL_TOKEN", "") @@ -66,7 +70,7 @@ async def _await_with_timeout(coro: Awaitable[Any], operation: str) -> Any: try: return await asyncio.wait_for(coro, timeout=timeout_seconds) except asyncio.TimeoutError as e: - raise RuntimeError( + raise MCPToolTimeoutError( "DOMShell MCP request timed out after " f"{timeout_seconds:.1f}s during {operation}. " "Verify DOMSHELL_TOKEN and that the DOMShell server is reachable." @@ -165,8 +169,8 @@ async def _call_tool( f"{tool_name} (daemon)", ) return result - except RuntimeError: - # Timeout/runtime errors should not re-run potentially non-idempotent tools. + except MCPToolTimeoutError: + # Timeout errors should not re-run potentially non-idempotent tools. raise except Exception: # Daemon died, fall back to spawning new server From 5338d01d82ccfbbb3c2bc8a80978c3c2a2bc9ffe Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sun, 19 Apr 2026 22:14:22 +0200 Subject: [PATCH 04/13] fix(browser): reset daemon on timeout without retrying tool --- browser/agent-harness/cli_anything/browser/tests/test_core.py | 4 ++-- .../cli_anything/browser/utils/domshell_backend.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_core.py b/browser/agent-harness/cli_anything/browser/tests/test_core.py index fca060d711..86dfaf513e 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_core.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_core.py @@ -431,7 +431,7 @@ async def _slow(): asyncio.run(backend_mod._await_with_timeout(_slow(), "unit-test")) def test_daemon_timeout_is_not_retried_in_non_daemon_mode(self): - """Daemon timeout should bubble up and avoid duplicate tool reissue.""" + """Daemon timeout should reset daemon, bubble up, and avoid duplicate reissue.""" class _DummyDaemonSession: def call_tool(self, _tool_name, _arguments): return object() @@ -453,7 +453,7 @@ def call_tool(self, _tool_name, _arguments): asyncio.run(backend_mod._call_tool("domshell_click", {"path": "/"}, use_daemon=True)) mock_await.assert_called_once() - mock_stop.assert_not_awaited() + mock_stop.assert_awaited_once() mock_stdio.assert_not_called() finally: backend_mod._daemon_session = original_daemon diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 5355be13db..209c26d673 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -170,7 +170,8 @@ async def _call_tool( ) return result except MCPToolTimeoutError: - # Timeout errors should not re-run potentially non-idempotent tools. + # Do not auto-retry this tool call, but reset daemon for future recovery. + await _stop_daemon() raise except Exception: # Daemon died, fall back to spawning new server From 7426cbb45dc2ad3ffd70d2e03740458b2da0b7b4 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Thu, 23 Apr 2026 16:28:31 +0200 Subject: [PATCH 05/13] fix(browser): bound _stop_daemon, narrow exception, type_text recovery, sync version Addresses Copilot/codex review feedback: - Bound _stop_daemon() __aexit__ calls with asyncio.wait_for so timeout recovery cannot itself hang on a wedged daemon transport (DOMSHELL_DAEMON_STOP_TIMEOUT, default 5s). - Narrow 'except RuntimeError: raise' to only pass through MCPToolTimeoutError; other RuntimeErrors are still wrapped with the helpful 'Ensure Chrome is running...' context. - Add MCPToolTimeoutError handling to type_text() daemon branch so a timeout there also resets the daemon, matching _call_tool behavior. - Bump browser package version 1.0.0 -> 1.0.1 to match registry.json (setup.py, browser/__init__.py, browser_cli.py REPL banner). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cli_anything/browser/__init__.py | 2 +- .../cli_anything/browser/browser_cli.py | 2 +- .../browser/utils/domshell_backend.py | 53 +++++++++++++------ browser/agent-harness/setup.py | 2 +- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/__init__.py b/browser/agent-harness/cli_anything/browser/__init__.py index ebd15be9da..d4b40d44a5 100644 --- a/browser/agent-harness/cli_anything/browser/__init__.py +++ b/browser/agent-harness/cli_anything/browser/__init__.py @@ -4,4 +4,4 @@ Accessibility Tree exposed through DOMShell's MCP server. """ -__version__ = "1.0.0" +__version__ = "1.0.1" diff --git a/browser/agent-harness/cli_anything/browser/browser_cli.py b/browser/agent-harness/cli_anything/browser/browser_cli.py index 54fbc565c0..86111b97b2 100644 --- a/browser/agent-harness/cli_anything/browser/browser_cli.py +++ b/browser/agent-harness/cli_anything/browser/browser_cli.py @@ -367,7 +367,7 @@ def repl(): global _repl_mode _repl_mode = True - skin = ReplSkin("browser", version="1.0.0") + skin = ReplSkin("browser", version="1.0.1") skin.print_banner() pt_session = skin.create_prompt_session() diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 209c26d673..5c65ee843c 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -192,8 +192,16 @@ async def _call_tool( tool_name, ) return result - except RuntimeError: - raise + except RuntimeError as e: + # Pass through MCPToolTimeoutError unchanged so callers can recover; + # but wrap other RuntimeErrors with actionable context below. + if isinstance(e, MCPToolTimeoutError): + raise + raise RuntimeError( + f"DOMShell MCP call failed: {e}\n" + f"Ensure Chrome is running with DOMShell extension installed.\n" + f"Chrome Web Store: https://chromewebstore.google.com/detail/domshell" + ) from e except Exception as e: raise RuntimeError( f"DOMShell MCP call failed: {e}\n" @@ -242,18 +250,28 @@ async def _start_daemon() -> bool: async def _stop_daemon() -> None: - """Stop persistent daemon mode.""" + """Stop persistent daemon mode. + + Cleanup is bounded to avoid hanging if the daemon transport is wedged. + """ global _daemon_session, _daemon_read, _daemon_write, _daemon_client_context if _daemon_session is None: return + cleanup_timeout = float(os.environ.get("DOMSHELL_DAEMON_STOP_TIMEOUT", "5.0")) try: - await _daemon_session.__aexit__(None, None, None) + await asyncio.wait_for( + _daemon_session.__aexit__(None, None, None), + timeout=cleanup_timeout, + ) if _daemon_client_context: - await _daemon_client_context.__aexit__(None, None, None) - except Exception: - pass # Ignore cleanup errors + await asyncio.wait_for( + _daemon_client_context.__aexit__(None, None, None), + timeout=cleanup_timeout, + ) + except (Exception, asyncio.TimeoutError): + pass # Ignore cleanup errors; we're discarding the session anyway finally: _daemon_session = None _daemon_read = None @@ -438,14 +456,19 @@ def type_text(path: str, text: str, use_daemon: bool = False) -> dict: async def _focus_and_type(): global _daemon_session if use_daemon and _daemon_session is not None: - await _await_with_timeout( - _daemon_session.call_tool("domshell_focus", {"name": path}), - "domshell_focus (daemon)", - ) - return await _await_with_timeout( - _daemon_session.call_tool("domshell_type", {"text": text}), - "domshell_type (daemon)", - ) + try: + await _await_with_timeout( + _daemon_session.call_tool("domshell_focus", {"name": path}), + "domshell_focus (daemon)", + ) + return await _await_with_timeout( + _daemon_session.call_tool("domshell_type", {"text": text}), + "domshell_type (daemon)", + ) + except MCPToolTimeoutError: + # Reset daemon so subsequent commands can reconnect cleanly. + await _stop_daemon() + raise server_params = StdioServerParameters( command=DEFAULT_SERVER_CMD, diff --git a/browser/agent-harness/setup.py b/browser/agent-harness/setup.py index 0c14a3ea4b..7ca9731c8c 100644 --- a/browser/agent-harness/setup.py +++ b/browser/agent-harness/setup.py @@ -13,7 +13,7 @@ def read_readme(): setup( name="cli-anything-browser", - version="1.0.0", + version="1.0.1", author="CLI Anything Contributors", description="CLI harness for browser automation via DOMShell MCP server", From 7bad81d19842c70bb391742c6ebd980382babc29 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 01:33:15 +0200 Subject: [PATCH 06/13] fix(browser): split MCP init/tool-call timeouts, fix undocumented daemon-stop override Addresses maintainer blockers on PR #233: - Add CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT (default 120s) for every session.initialize()/daemon initialize() path, independent from CLI_ANYTHING_BROWSER_MCP_TIMEOUT (default 20s) which now applies only to tool calls. Previously a single timeout governed both, so a slow but healthy handshake could be mistaken for a hung tool call. - Remove the undocumented DOMSHELL_DAEMON_STOP_TIMEOUT env override; _stop_daemon() cleanup now uses a fixed DAEMON_STOP_TIMEOUT_SECONDS (5s) internal safety bound instead of exposing an untested, undocumented knob. - _call_tool() and type_text()'s _focus_and_type() now capture the tool/init timeouts once per operation via _get_tool_timeout_seconds() / _get_init_timeout_seconds() and pass them explicitly into _await_with_timeout(), instead of re-reading env vars on every await within the same logical operation. - Document CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT in README.md and the packaged SKILL.md alongside the existing tool-call timeout docs. - Add regression tests for init-timeout defaults/clamping, init vs. tool timeout usage in _call_tool/_start_daemon, and single-capture timeout behavior under mid-operation env mutation. No existing tests were modified. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cli_anything/browser/README.md | 9 +- .../cli_anything/browser/skills/SKILL.md | 2 +- .../cli_anything/browser/tests/test_core.py | 191 ++++++++++++++++++ .../browser/utils/domshell_backend.py | 76 ++++++- 4 files changed, 265 insertions(+), 13 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/README.md b/browser/agent-harness/cli_anything/browser/README.md index 1feebe9fe0..b83467084f 100644 --- a/browser/agent-harness/cli_anything/browser/README.md +++ b/browser/agent-harness/cli_anything/browser/README.md @@ -247,12 +247,19 @@ Run: `npx @apireno/domshell --version` (first run downloads the package) First `npx` call downloads DOMShell package (10-50 MB). Subsequent calls are faster. Use `--daemon` mode for persistent connection. If the server is unreachable or auth token is wrong, MCP calls now fail with timeout instead of hanging forever. -Set timeout (seconds) via: +Set the per-tool-call timeout (seconds) via: ```bash export CLI_ANYTHING_BROWSER_MCP_TIMEOUT=20 ``` +Session initialization (spawning `npx`/`domshell-proxy` and completing the MCP handshake) +can legitimately take longer than a regular tool call, so it has its own timeout: + +```bash +export CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT=120 +``` + ## Architecture This CLI follows the [CLI-Anything harness methodology](https://github.com/HKUDS/CLI-Anything/tree/main/cli-anything-plugin/HARNESS.md): diff --git a/browser/agent-harness/cli_anything/browser/skills/SKILL.md b/browser/agent-harness/cli_anything/browser/skills/SKILL.md index 640ef234f9..0185fed9c9 100644 --- a/browser/agent-harness/cli_anything/browser/skills/SKILL.md +++ b/browser/agent-harness/cli_anything/browser/skills/SKILL.md @@ -158,7 +158,7 @@ The CLI provides clear error messages for common issues: - **npx not found**: Install Node.js from https://nodejs.org/ - **DOMShell not found**: Run `npx @apireno/domshell --version` - **MCP call failed**: Install DOMShell Chrome extension -- **MCP timeout / hanging calls**: Verify `DOMSHELL_TOKEN`, ensure DOMShell server is running, and optionally tune timeout via `CLI_ANYTHING_BROWSER_MCP_TIMEOUT` (default: 20s) +- **MCP timeout / hanging calls**: Verify `DOMSHELL_TOKEN`, ensure DOMShell server is running, and optionally tune timeouts via `CLI_ANYTHING_BROWSER_MCP_TIMEOUT` (default: 20s, per tool call) and `CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT` (default: 120s, session initialize) Check `is_available()` return value before running commands. diff --git a/browser/agent-harness/cli_anything/browser/tests/test_core.py b/browser/agent-harness/cli_anything/browser/tests/test_core.py index 86dfaf513e..2af39f268d 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_core.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_core.py @@ -409,6 +409,197 @@ def test_timeout_is_clamped_to_minimum(self, monkeypatch): monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "0") assert backend_mod._get_tool_timeout_seconds() == 1.0 + def test_init_timeout_default_value(self, monkeypatch): + """Init timeout defaults to 120s when env var is unset.""" + monkeypatch.delenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", raising=False) + assert backend_mod._get_init_timeout_seconds() == 120.0 + + def test_init_timeout_invalid_env_falls_back_to_default(self, monkeypatch): + """Invalid init timeout env var falls back to default.""" + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "not-a-number") + assert backend_mod._get_init_timeout_seconds() == 120.0 + + def test_init_timeout_is_clamped_to_minimum(self, monkeypatch): + """Init timeout values below 1 second are clamped to 1 second.""" + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "0") + assert backend_mod._get_init_timeout_seconds() == 1.0 + + def test_init_timeout_is_independent_of_tool_timeout(self, monkeypatch): + """Init and tool timeouts are configured independently.""" + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "5") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "120") + assert backend_mod._get_tool_timeout_seconds() == 5.0 + assert backend_mod._get_init_timeout_seconds() == 120.0 + + def test_call_tool_uses_init_timeout_for_session_initialize(self, monkeypatch): + """Non-daemon _call_tool must await session.initialize() with the init timeout.""" + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "5") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "120") + + class _DummyStdioContext: + async def __aenter__(self): + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + return False + + class _DummyClientSession: + def __init__(self, _read, _write): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + def initialize(self): + return object() + + def call_tool(self, _tool_name, _arguments): + return object() + + recorded_timeouts = [] + + async def _fake_await_with_timeout(_coro, operation, timeout_seconds=None): + if timeout_seconds is None: + timeout_seconds = ( + backend_mod._get_init_timeout_seconds() + if "initialize" in operation + else backend_mod._get_tool_timeout_seconds() + ) + recorded_timeouts.append((operation, timeout_seconds)) + return {"ok": True} + + with patch( + "cli_anything.browser.utils.domshell_backend._await_with_timeout", + side_effect=_fake_await_with_timeout, + ), patch( + "cli_anything.browser.utils.domshell_backend.stdio_client", + return_value=_DummyStdioContext(), + ), patch( + "cli_anything.browser.utils.domshell_backend.ClientSession", + _DummyClientSession, + ): + asyncio.run( + backend_mod._call_tool("domshell_click", {"path": "/"}, use_daemon=False) + ) + + assert ("session initialize", 120.0) in recorded_timeouts + assert ("domshell_click", 5.0) in recorded_timeouts + + def test_start_daemon_uses_init_timeout(self, monkeypatch): + """_start_daemon must await daemon initialize() with the init timeout, not the tool timeout.""" + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "5") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "120") + + class _DummyStdioContext: + async def __aenter__(self): + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + return False + + class _DummyClientSession: + def __init__(self, _read, _write): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + def initialize(self): + return object() + + recorded_timeouts = [] + + async def _fake_await_with_timeout(_coro, operation, timeout_seconds=None): + recorded_timeouts.append((operation, timeout_seconds)) + return None + + original_daemon = backend_mod._daemon_session + try: + backend_mod._daemon_session = None + with patch( + "cli_anything.browser.utils.domshell_backend._await_with_timeout", + side_effect=_fake_await_with_timeout, + ), patch( + "cli_anything.browser.utils.domshell_backend.stdio_client", + return_value=_DummyStdioContext(), + ), patch( + "cli_anything.browser.utils.domshell_backend.ClientSession", + _DummyClientSession, + ): + asyncio.run(backend_mod._start_daemon()) + + assert recorded_timeouts == [("daemon initialize", 120.0)] + finally: + backend_mod._daemon_session = original_daemon + + def test_call_tool_captures_timeout_once_per_operation(self, monkeypatch): + """Timeout values are captured once per _call_tool invocation, not re-read per await.""" + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "5") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "120") + + class _DummyStdioContext: + async def __aenter__(self): + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + return False + + class _DummyClientSession: + def __init__(self, _read, _write): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + def initialize(self): + return object() + + def call_tool(self, _tool_name, _arguments): + return object() + + recorded_timeouts = [] + + async def _fake_await_with_timeout(_coro, operation, timeout_seconds=None): + # Simulate the env var mutating mid-operation (e.g. another thread/test). + # A per-call env read would pick this up; a captured-once value would not. + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "999") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "999") + recorded_timeouts.append((operation, timeout_seconds)) + return {"ok": True} + + with patch( + "cli_anything.browser.utils.domshell_backend._await_with_timeout", + side_effect=_fake_await_with_timeout, + ), patch( + "cli_anything.browser.utils.domshell_backend.stdio_client", + return_value=_DummyStdioContext(), + ), patch( + "cli_anything.browser.utils.domshell_backend.ClientSession", + _DummyClientSession, + ): + asyncio.run( + backend_mod._call_tool("domshell_click", {"path": "/"}, use_daemon=False) + ) + + # Both calls within this single _call_tool invocation must use the timeouts + # captured before the env var was mutated mid-operation. + assert recorded_timeouts == [ + ("session initialize", 120.0), + ("domshell_click", 5.0), + ] + def test_await_with_timeout_passes_fast_calls(self, monkeypatch): """Fast operations should complete without timeout errors.""" monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "5") diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 5c65ee843c..65c8451159 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -27,6 +27,8 @@ # DOMSHELL_PORT — MCP HTTP port of the running server (default: 3001) DEFAULT_SERVER_CMD = "npx" DEFAULT_TOOL_TIMEOUT_SECONDS = 20.0 +DEFAULT_INIT_TIMEOUT_SECONDS = 120.0 +DAEMON_STOP_TIMEOUT_SECONDS = 5.0 class MCPToolTimeoutError(RuntimeError): @@ -52,7 +54,7 @@ def _build_server_args() -> list[str]: def _get_tool_timeout_seconds() -> float: - """Read MCP tool timeout (seconds) from env with safe bounds.""" + """Read MCP tool-call timeout (seconds) from env with safe bounds.""" raw = os.environ.get( "CLI_ANYTHING_BROWSER_MCP_TIMEOUT", str(DEFAULT_TOOL_TIMEOUT_SECONDS), @@ -64,9 +66,37 @@ def _get_tool_timeout_seconds() -> float: return max(1.0, parsed) -async def _await_with_timeout(coro: Awaitable[Any], operation: str) -> Any: - """Await an MCP operation with timeout and actionable error text.""" - timeout_seconds = _get_tool_timeout_seconds() +def _get_init_timeout_seconds() -> float: + """Read MCP session-initialize timeout (seconds) from env with safe bounds. + + Initialize can legitimately take much longer than a regular tool call + (e.g. spawning npx/domshell-proxy and completing the MCP handshake), so it + is configured separately from CLI_ANYTHING_BROWSER_MCP_TIMEOUT. + """ + raw = os.environ.get( + "CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", + str(DEFAULT_INIT_TIMEOUT_SECONDS), + ) + try: + parsed = float(raw) + except (TypeError, ValueError): + return DEFAULT_INIT_TIMEOUT_SECONDS + return max(1.0, parsed) + + +async def _await_with_timeout( + coro: Awaitable[Any], + operation: str, + timeout_seconds: Optional[float] = None, +) -> Any: + """Await an MCP operation with timeout and actionable error text. + + Callers that need to distinguish init vs. tool-call timeouts (or capture + the timeout once for a whole operation) should pass timeout_seconds + explicitly; it otherwise defaults to the tool-call timeout. + """ + if timeout_seconds is None: + timeout_seconds = _get_tool_timeout_seconds() try: return await asyncio.wait_for(coro, timeout=timeout_seconds) except asyncio.TimeoutError as e: @@ -161,12 +191,18 @@ async def _call_tool( """ global _daemon_session, _daemon_read, _daemon_write + # Capture timeouts once per operation so env mutations mid-call (e.g. from + # another thread) can't apply inconsistent timeouts across the awaits below. + tool_timeout = _get_tool_timeout_seconds() + init_timeout = _get_init_timeout_seconds() + if use_daemon and _daemon_session is not None: # Use persistent daemon connection try: result = await _await_with_timeout( _daemon_session.call_tool(tool_name, arguments), f"{tool_name} (daemon)", + tool_timeout, ) return result except MCPToolTimeoutError: @@ -186,10 +222,13 @@ async def _call_tool( try: async with stdio_client(server_params) as (read, write): async with ClientSession(read, write) as session: - await _await_with_timeout(session.initialize(), "session initialize") + await _await_with_timeout( + session.initialize(), "session initialize", init_timeout + ) result = await _await_with_timeout( session.call_tool(tool_name, arguments), tool_name, + tool_timeout, ) return result except RuntimeError as e: @@ -239,7 +278,11 @@ async def _start_daemon() -> bool: _daemon_read, _daemon_write = await _daemon_client_context.__aenter__() _daemon_session = ClientSession(_daemon_read, _daemon_write) await _daemon_session.__aenter__() - await _await_with_timeout(_daemon_session.initialize(), "daemon initialize") + await _await_with_timeout( + _daemon_session.initialize(), + "daemon initialize", + _get_init_timeout_seconds(), + ) return True except Exception as e: _daemon_session = None @@ -252,23 +295,24 @@ async def _start_daemon() -> bool: async def _stop_daemon() -> None: """Stop persistent daemon mode. - Cleanup is bounded to avoid hanging if the daemon transport is wedged. + Cleanup is bounded to a fixed timeout (DAEMON_STOP_TIMEOUT_SECONDS) to + avoid hanging if the daemon transport is wedged. Not configurable via + env var: this is an internal safety bound, not a user-tunable setting. """ global _daemon_session, _daemon_read, _daemon_write, _daemon_client_context if _daemon_session is None: return - cleanup_timeout = float(os.environ.get("DOMSHELL_DAEMON_STOP_TIMEOUT", "5.0")) try: await asyncio.wait_for( _daemon_session.__aexit__(None, None, None), - timeout=cleanup_timeout, + timeout=DAEMON_STOP_TIMEOUT_SECONDS, ) if _daemon_client_context: await asyncio.wait_for( _daemon_client_context.__aexit__(None, None, None), - timeout=cleanup_timeout, + timeout=DAEMON_STOP_TIMEOUT_SECONDS, ) except (Exception, asyncio.TimeoutError): pass # Ignore cleanup errors; we're discarding the session anyway @@ -455,15 +499,21 @@ def type_text(path: str, text: str, use_daemon: bool = False) -> dict: """ async def _focus_and_type(): global _daemon_session + # Capture timeouts once per operation, consistent with _call_tool. + tool_timeout = _get_tool_timeout_seconds() + init_timeout = _get_init_timeout_seconds() + if use_daemon and _daemon_session is not None: try: await _await_with_timeout( _daemon_session.call_tool("domshell_focus", {"name": path}), "domshell_focus (daemon)", + tool_timeout, ) return await _await_with_timeout( _daemon_session.call_tool("domshell_type", {"text": text}), "domshell_type (daemon)", + tool_timeout, ) except MCPToolTimeoutError: # Reset daemon so subsequent commands can reconnect cleanly. @@ -476,14 +526,18 @@ async def _focus_and_type(): ) async with stdio_client(server_params) as (read, write): async with ClientSession(read, write) as session: - await _await_with_timeout(session.initialize(), "session initialize") + await _await_with_timeout( + session.initialize(), "session initialize", init_timeout + ) await _await_with_timeout( session.call_tool("domshell_focus", {"name": path}), "domshell_focus", + tool_timeout, ) return await _await_with_timeout( session.call_tool("domshell_type", {"text": text}), "domshell_type", + tool_timeout, ) return asyncio.run(_focus_and_type()) From 00624fc9c03c58aba98af27465e5c2c73304d83c Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 01:52:52 +0200 Subject: [PATCH 07/13] docs(browser): sync canonical SKILL.md with MCP timeout troubleshooting line Regenerated skills/cli-anything-browser/SKILL.md via .github/scripts/sync_root_skills.py so it picks up the CLI_ANYTHING_BROWSER_MCP_TIMEOUT / CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT troubleshooting line already present in the packaged compatibility copy (browser/agent-harness/cli_anything/browser/skills/SKILL.md), which is the actual sync source per .github/scripts/sync_root_skills.py and validate_root_skills.py (root skills/ is generated from the harness-local SKILL.md files, not hand-authored). python3 .github/scripts/validate_root_skills.py now passes (previously failed with 'stale root skill' for this file). No other root skill needed regeneration (67 already up to date). Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skills/cli-anything-browser/SKILL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/skills/cli-anything-browser/SKILL.md b/skills/cli-anything-browser/SKILL.md index 73b9e4d4a2..0185fed9c9 100644 --- a/skills/cli-anything-browser/SKILL.md +++ b/skills/cli-anything-browser/SKILL.md @@ -158,6 +158,7 @@ The CLI provides clear error messages for common issues: - **npx not found**: Install Node.js from https://nodejs.org/ - **DOMShell not found**: Run `npx @apireno/domshell --version` - **MCP call failed**: Install DOMShell Chrome extension +- **MCP timeout / hanging calls**: Verify `DOMSHELL_TOKEN`, ensure DOMShell server is running, and optionally tune timeouts via `CLI_ANYTHING_BROWSER_MCP_TIMEOUT` (default: 20s, per tool call) and `CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT` (default: 120s, session initialize) Check `is_available()` return value before running commands. From a767396fd0c08f58838158e2d9f36344c957f610 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 02:11:38 +0200 Subject: [PATCH 08/13] fix(browser): close orphaned daemon contexts on init failure; restore lane cwd after wrapper-op timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two quality-review findings, both fixed with strict TDD (red tests added first, confirmed failing, then minimal implementation): 1. _start_daemon() entered the stdio_client transport and ClientSession as manually-driven contexts (not `async with`), so it could hold them open for the daemon's lifetime. On an initialize() timeout/ error, the except block only nulled the module globals — it never called __aexit__ on either context, orphaning the underlying npx/ DOMShell subprocess and stdio pipes. Extracted the existing bounded cleanup logic from _stop_daemon into a shared _close_daemon_transport() helper (best-effort, bounded by DAEMON_STOP_TIMEOUT_SECONDS, swallows its own exceptions) and call it from _start_daemon's except block before nulling globals and re-raising. _stop_daemon is refactored to call the same helper — same behavior, no duplication. Handles both partial-entry cases: ClientSession never entered (only stdio needs closing) and both contexts entered (both need closing). 2. The absolute-path wrappers (ls, cat, click, grep, type_text) each anchor a `cd` before their real operation and restore afterward, but the restore call only ran on normal return. An operation that raised (e.g. MCPToolTimeoutError) skipped the restore entirely, leaving the DOMShell lane's cwd stuck at the anchor and silently corrupting every later relative-path call against that lane. Added a shared _best_effort_restore() helper and wrapped each wrapper's operation call in try/except: on exception, attempt the restore (swallowing any restore-side failure so it can't mask the original error) then re-raise the original exception unchanged. The restore-on-success path is untouched — a restore failure there still propagates as before (intentional, pre-existing behavior). type_text needed two such guards (focus and type are separate calls in its safety-chain design). Evaluated reviewer claims against current code before editing: both were confirmed accurate against the current _call_execute-based architecture (post upstream/main merge) — no nuance changed the minimal design. No existing tests were modified, only added. Red: 9 new tests added to test_domshell_backend.py, all failing before the fix (2 for _start_daemon context leaks, 7 for the 5 wrappers' restore-on-raise + one restore-failure-does-not-mask case). Green: same 9 tests pass after the fix. Full browser suite: 216 passed, 11 skipped (up from 207 baseline), 0 failures, 0 warnings (verified with -W error::RuntimeWarning to catch unawaited coroutines). Root skill validation (.github/scripts/validate_root_skills.py) still passes — untouched by this change. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/tests/test_domshell_backend.py | 237 ++++++++++++++++++ .../browser/utils/domshell_backend.py | 165 ++++++++---- 2 files changed, 360 insertions(+), 42 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py index 54cee04d3e..f49cb63415 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py @@ -1642,3 +1642,240 @@ async def _fake_await_with_timeout(coro, operation, timeout_seconds=None): finally: backend._daemon_session = original_daemon + + +# ── _start_daemon: entered contexts must be closed on init failure ─── +# +# _start_daemon manually drives stdio_client and ClientSession as +# entered context managers (not via `async with`) so it can hold them +# open for the life of the daemon. If `initialize()` times out or +# raises after both contexts are entered, the previous code only +# nulled the module globals — it never called `__aexit__` on either +# context, orphaning the underlying npx/DOMShell subprocess and stdio +# pipes. This must close both entered contexts (best-effort, bounded) +# before re-raising, without masking the original failure. + + +def test_start_daemon_closes_contexts_on_initialize_failure(monkeypatch): + """A daemon-initialize failure must exit both the entered + ClientSession and the entered stdio_client context — not just null + the globals — so the underlying npx/DOMShell process isn't + orphaned.""" + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT", "120") + + session_exit_calls = [] + stdio_exit_calls = [] + + class _DummyMcpSession: + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + session_exit_calls.append((exc_type, exc)) + return False + + def initialize(self): + return object() + + class _DummyStdio: + async def __aenter__(self): + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + stdio_exit_calls.append((exc_type, exc)) + return False + + async def _fake_await_with_timeout(coro, operation, timeout_seconds=None): + if hasattr(coro, "close"): + coro.close() + raise backend.MCPToolTimeoutError("daemon initialize timed out") + + original_daemon = backend._daemon_session + original_context = backend._daemon_client_context + try: + backend._daemon_session = None + backend._daemon_client_context = None + with patch.object(backend, "stdio_client", return_value=_DummyStdio()), \ + patch.object(backend, "ClientSession", return_value=_DummyMcpSession()), \ + patch.object(backend, "_build_server_args", return_value=[]), \ + patch.object(backend, "_await_with_timeout", side_effect=_fake_await_with_timeout): + import asyncio as _aio + with pytest.raises(RuntimeError, match="Failed to start DOMShell daemon"): + _aio.run(backend._start_daemon()) + + # Both entered contexts must have been exited exactly once. + assert len(session_exit_calls) == 1 + assert len(stdio_exit_calls) == 1 + # Globals still end up cleared (pre-existing behavior, preserved). + assert backend._daemon_session is None + assert backend._daemon_read is None + assert backend._daemon_write is None + assert backend._daemon_client_context is None + finally: + backend._daemon_session = original_daemon + backend._daemon_client_context = original_context + + +def test_start_daemon_closes_stdio_context_when_session_never_entered(monkeypatch): + """If ClientSession.__aenter__ itself raises (session never fully + entered), the already-entered stdio_client context must still be + exited.""" + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + + stdio_exit_calls = [] + + class _BrokenMcpSession: + async def __aenter__(self): + raise RuntimeError("session enter failed") + + async def __aexit__(self, exc_type, exc, tb): + return False + + class _DummyStdio: + async def __aenter__(self): + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + stdio_exit_calls.append((exc_type, exc)) + return False + + original_daemon = backend._daemon_session + original_context = backend._daemon_client_context + try: + backend._daemon_session = None + backend._daemon_client_context = None + with patch.object(backend, "stdio_client", return_value=_DummyStdio()), \ + patch.object(backend, "ClientSession", return_value=_BrokenMcpSession()), \ + patch.object(backend, "_build_server_args", return_value=[]): + import asyncio as _aio + with pytest.raises(RuntimeError, match="Failed to start DOMShell daemon"): + _aio.run(backend._start_daemon()) + + assert len(stdio_exit_calls) == 1 + assert backend._daemon_client_context is None + finally: + backend._daemon_session = original_daemon + backend._daemon_client_context = original_context + + +# ── Absolute-path wrappers: restore must be attempted even when the +# operation raises (not just on normal return) ───────────────────── +# +# ls/cat/click/grep/type_text all anchor a `cd` before the real +# operation and restore afterward. Previously the restore call only +# ran on normal return — an operation that raised (e.g. an +# MCPToolTimeoutError) skipped it entirely, leaving the DOMShell +# lane's cwd stuck at the anchor and silently corrupting every later +# relative-path call against that lane. The fix must still let the +# original exception propagate (the restore is best-effort — its own +# failure must never mask the operation's failure). + + +@patch.object(backend, "_call_execute", new_callable=AsyncMock) +def test_ls_absolute_restores_after_op_raises(mock_call): + sess = _make_session(working_dir="/main") + mock_call.side_effect = [ + _make_result("✓ Entered\n[lane: 1]"), # anchor ok + backend.MCPToolTimeoutError("ls timed out"), # op raises + _make_result("✓\n[lane: 1]"), # restore + ] + with pytest.raises(backend.MCPToolTimeoutError, match="ls timed out"): + backend.ls("/main", session=sess) + assert mock_call.call_count == 3 + assert mock_call.call_args_list[0].args[0] == "cd %here%/main" + assert mock_call.call_args_list[2].args[0] == "cd %here%/main" + + +@patch.object(backend, "_call_execute", new_callable=AsyncMock) +def test_cat_absolute_restores_after_op_raises(mock_call): + sess = _make_session(working_dir="/") + mock_call.side_effect = [ + _make_result("✓ Entered\n[lane: 1]"), + backend.MCPToolTimeoutError("cat timed out"), + _make_result("✓\n[lane: 1]"), + ] + with pytest.raises(backend.MCPToolTimeoutError, match="cat timed out"): + backend.cat("/main/btn", session=sess) + assert mock_call.call_count == 3 + assert mock_call.call_args_list[2].args[0] == "cd %here%" + + +@patch.object(backend, "_call_execute", new_callable=AsyncMock) +def test_click_absolute_restores_after_op_raises(mock_call): + sess = _make_session(working_dir="/main") + mock_call.side_effect = [ + _make_result("✓ Entered\n[lane: 1]"), + backend.MCPToolTimeoutError("click timed out"), + _make_result("✓\n[lane: 1]"), + ] + with pytest.raises(backend.MCPToolTimeoutError, match="click timed out"): + backend.click("/main/button[0]", session=sess) + assert mock_call.call_count == 3 + assert mock_call.call_args_list[2].args[0] == "cd %here%/main" + + +@patch.object(backend, "_call_execute", new_callable=AsyncMock) +def test_grep_rooted_absolute_restores_after_op_raises(mock_call): + mock_call.side_effect = [ + _make_result("✓ Entered\n[lane: 1]"), + backend.MCPToolTimeoutError("grep timed out"), + _make_result("✓\n[lane: 1]"), + ] + with pytest.raises(backend.MCPToolTimeoutError, match="grep timed out"): + backend.grep( + "Login", path="/main", prev="/", session=_make_session(working_dir="/"), + ) + assert mock_call.call_count == 3 + assert mock_call.call_args_list[2].args[0] == "cd %here%" + + +@patch.object(backend, "_call_execute", new_callable=AsyncMock) +def test_type_text_absolute_restores_after_focus_raises(mock_call): + """focus itself raising (not just returning an error dict) must + still trigger the restore — the anchor already moved the lane + cwd.""" + sess = _make_session(working_dir="/main") + mock_call.side_effect = [ + _make_result("✓ Entered\n[lane: 1]"), # anchor ok + backend.MCPToolTimeoutError("focus timed out"), # focus raises + _make_result("✓\n[lane: 1]"), # restore + ] + with pytest.raises(backend.MCPToolTimeoutError, match="focus timed out"): + backend.type_text("/main/input", "hello", session=sess) + assert mock_call.call_count == 3 + assert mock_call.call_args_list[2].args[0] == "cd %here%/main" + + +@patch.object(backend, "_call_execute", new_callable=AsyncMock) +def test_type_text_absolute_restores_after_type_raises(mock_call): + """type itself raising after a successful focus must still trigger + the restore — the lane cwd was moved by the anchor and must not + stay parked there.""" + sess = _make_session(working_dir="/main") + mock_call.side_effect = [ + _make_result("✓ Entered\n[lane: 1]"), # anchor ok + _make_result("✓ Focused\n[lane: 1]"), # focus ok + backend.MCPToolTimeoutError("type timed out"), # type raises + _make_result("✓\n[lane: 1]"), # restore + ] + with pytest.raises(backend.MCPToolTimeoutError, match="type timed out"): + backend.type_text("/main/input", "hello", session=sess) + assert mock_call.call_count == 4 + assert mock_call.call_args_list[3].args[0] == "cd %here%/main" + + +@patch.object(backend, "_call_execute", new_callable=AsyncMock) +def test_ls_absolute_restore_failure_does_not_mask_op_error(mock_call): + """If both the operation and the best-effort restore fail, the + original operation error must be what propagates — not the + restore's own failure.""" + sess = _make_session(working_dir="/main") + mock_call.side_effect = [ + _make_result("✓ Entered\n[lane: 1]"), + backend.MCPToolTimeoutError("ls timed out"), + RuntimeError("restore also failed"), + ] + with pytest.raises(backend.MCPToolTimeoutError, match="ls timed out"): + backend.ls("/main", session=sess) + assert mock_call.call_count == 3 diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 54cd818697..1f4d2c7329 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -360,6 +360,33 @@ def _restore_cwd_cmd(session: Any) -> str: return f"cd {_q(stripped)}" if stripped else f"cd {_here_path('')}" +def _best_effort_restore( + restore_cmd: str, use_daemon: bool, session: Any, +) -> None: + """Attempt a restore-cwd call after an operation raised. + + Absolute-path wrappers (ls/cat/click/grep/type_text) anchor a `cd` + before their real operation and restore afterward. If the operation + itself raises (e.g. an MCP timeout), the restore must still be + attempted — otherwise the lane's cwd stays parked at the anchor and + silently corrupts every later relative-path call in that lane. + + Any exception from THIS restore attempt is swallowed: it must never + mask the original operation failure that's already propagating past + this cleanup call. This is deliberately narrower than the + restore-on-success call each wrapper makes directly — a restore + failure on the normal-return path is not swallowed, since there's no + prior error it could mask. + """ + try: + asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) + except Exception: + log.warning( + "DOMShell lane cwd restore failed after an operation error", + exc_info=True, + ) + + def _anchor_path_cmd(deeper: str = "") -> str: """Build a single ``cd %here%[/]`` command line, shell-quoted. @@ -696,6 +723,35 @@ async def _call_execute( # (like the daemon session) may have issues when accessed from subsequent calls that # create new loops. This is a documented limitation for v1; future work should use # a single long-lived event loop (e.g., background thread + run_coroutine_threadsafe). +async def _close_daemon_transport(session: Any, client_context: Any) -> None: + """Best-effort, bounded exit of whichever daemon contexts were entered. + + Shared by ``_start_daemon`` (init failure after partial entry) and + ``_stop_daemon`` (normal shutdown). Each ``__aexit__`` is bounded by + ``DAEMON_STOP_TIMEOUT_SECONDS`` and failures are swallowed — this + always runs either during cleanup-on-error or intentional shutdown, + so it must never raise over/mask whatever triggered it. Either + argument may be ``None`` (e.g. ``ClientSession.__aenter__`` never + even ran) and is skipped. + """ + if session is not None: + try: + await asyncio.wait_for( + session.__aexit__(None, None, None), + timeout=DAEMON_STOP_TIMEOUT_SECONDS, + ) + except (Exception, asyncio.TimeoutError): + pass + if client_context is not None: + try: + await asyncio.wait_for( + client_context.__aexit__(None, None, None), + timeout=DAEMON_STOP_TIMEOUT_SECONDS, + ) + except (Exception, asyncio.TimeoutError): + pass + + async def _start_daemon() -> bool: """Start persistent daemon mode. @@ -728,6 +784,12 @@ async def _start_daemon() -> bool: ) return True except Exception as e: + # An initialize() timeout/error can land after the stdio + # transport and/or the ClientSession have already been + # __aenter__'d. Exiting them here (best-effort, bounded) is + # required — otherwise the underlying npx/DOMShell subprocess + # and stdio pipes are orphaned, not just the Python globals. + await _close_daemon_transport(_daemon_session, _daemon_client_context) _daemon_session = None _daemon_read = None _daemon_write = None @@ -747,23 +809,11 @@ async def _stop_daemon() -> None: if _daemon_session is None: return - try: - await asyncio.wait_for( - _daemon_session.__aexit__(None, None, None), - timeout=DAEMON_STOP_TIMEOUT_SECONDS, - ) - if _daemon_client_context: - await asyncio.wait_for( - _daemon_client_context.__aexit__(None, None, None), - timeout=DAEMON_STOP_TIMEOUT_SECONDS, - ) - except (Exception, asyncio.TimeoutError): - pass # Ignore cleanup errors; we're discarding the session anyway - finally: - _daemon_session = None - _daemon_read = None - _daemon_write = None - _daemon_client_context = None + await _close_daemon_transport(_daemon_session, _daemon_client_context) + _daemon_session = None + _daemon_read = None + _daemon_write = None + _daemon_client_context = None def daemon_started() -> bool: @@ -816,12 +866,18 @@ def ls(path: str = "/", use_daemon: bool = False, *, session: Any = None) -> dic )) if _is_error(anchor): return _parse_execute_result(anchor, "ls") - op = asyncio.run(_call_execute("ls", use_daemon, session=session)) + restore_cmd = _restore_cwd_cmd(session) + try: + op = asyncio.run(_call_execute("ls", use_daemon, session=session)) + except Exception: + # ls raised (e.g. a timeout) instead of returning an error + # dict — restore must still be attempted so the lane cwd + # doesn't stay parked at the anchor. + _best_effort_restore(restore_cmd, use_daemon, session) + raise # Best-effort restore — ls already ran; restore failure is # cosmetic (next harness cd corrects any drift). - asyncio.run(_call_execute( - _restore_cwd_cmd(session), use_daemon, session=session, - )) + asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) return _parse_execute_result(op, "ls") if translated: op = asyncio.run(_call_execute( @@ -908,12 +964,15 @@ def cat(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: )) if _is_error(anchor): return _parse_execute_result(anchor, "cat") - op = asyncio.run(_call_execute( - f"cat {_q(translated)}", use_daemon, session=session, - )) - asyncio.run(_call_execute( - _restore_cwd_cmd(session), use_daemon, session=session, - )) + restore_cmd = _restore_cwd_cmd(session) + try: + op = asyncio.run(_call_execute( + f"cat {_q(translated)}", use_daemon, session=session, + )) + except Exception: + _best_effort_restore(restore_cmd, use_daemon, session) + raise + asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) return _parse_execute_result(op, "cat") op = asyncio.run(_call_execute( f"cat {_q(translated)}", use_daemon, session=session, @@ -1029,9 +1088,13 @@ def grep( return _parse_execute_result(anchor, "grep") # `-r` preserves the pre-migration recursive default (see unrooted # branch above for the full rationale). - op = asyncio.run(_call_execute( - f"grep -r {_q(pattern)}", use_daemon, session=session, - )) + try: + op = asyncio.run(_call_execute( + f"grep -r {_q(pattern)}", use_daemon, session=session, + )) + except Exception: + _best_effort_restore(restore_cmd, use_daemon, session) + raise asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) return _parse_execute_result(op, "grep") @@ -1067,12 +1130,15 @@ def click(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: )) if _is_error(anchor): return _parse_execute_result(anchor, "click") - op = asyncio.run(_call_execute( - f"click {_q(translated)}", use_daemon, session=session, - )) - asyncio.run(_call_execute( - _restore_cwd_cmd(session), use_daemon, session=session, - )) + restore_cmd = _restore_cwd_cmd(session) + try: + op = asyncio.run(_call_execute( + f"click {_q(translated)}", use_daemon, session=session, + )) + except Exception: + _best_effort_restore(restore_cmd, use_daemon, session) + raise + asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) return _parse_execute_result(op, "click") op = asyncio.run(_call_execute( f"click {_q(translated)}", use_daemon, session=session, @@ -1257,9 +1323,17 @@ def type_text( # Anchor failed — we never moved, so no restore is needed. return _parse_execute_result(anchor_result, "focus") - focus_result = asyncio.run(_call_execute( - f"focus {_q(translated_path)}", use_daemon, session=session, - )) + try: + focus_result = asyncio.run(_call_execute( + f"focus {_q(translated_path)}", use_daemon, session=session, + )) + except Exception: + # focus raised (e.g. a timeout) instead of returning an error + # dict. Only relevant to restore when we actually moved (the + # absolute path branch) — mirrors the _is_error restore below. + if is_absolute: + _best_effort_restore(_restore_cwd_cmd(session), use_daemon, session) + raise if _is_error(focus_result): # Focus failed — restore cwd before returning so the lane # doesn't stay parked at the anchor (only relevant when we @@ -1270,9 +1344,16 @@ def type_text( )) return _parse_execute_result(focus_result, "focus") - type_result = asyncio.run(_call_execute( - f"type {_q(text)}", use_daemon, session=session, - )) + try: + type_result = asyncio.run(_call_execute( + f"type {_q(text)}", use_daemon, session=session, + )) + except Exception: + # type raised after a successful focus — the anchor already + # moved the lane cwd, so restore must still be attempted. + if is_absolute: + _best_effort_restore(_restore_cwd_cmd(session), use_daemon, session) + raise if is_absolute: # Best-effort restore — type already succeeded, so a restore From a2460fd8ccb90f3c5c571a256e88d3d716229cbe Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 16:50:21 +0200 Subject: [PATCH 09/13] fix(browser): bound cleanup and restore after anchor timeouts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/tests/test_domshell_backend.py | 109 ++++++++++++++ .../browser/utils/domshell_backend.py | 133 +++++++++++------- 2 files changed, 192 insertions(+), 50 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py index f49cb63415..530945b83d 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py @@ -1559,6 +1559,61 @@ async def __aexit__(self, exc_type, exc, tb): ] +def test_call_execute_bounds_context_exit_after_tool_timeout(monkeypatch): + """A wedged context exit must not hide the original tool timeout.""" + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + monkeypatch.setattr(backend, "DAEMON_STOP_TIMEOUT_SECONDS", 0.01) + exit_started = [] + + class _DummyMcpSession: + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + exit_started.append("session") + import asyncio as _aio + await _aio.sleep(0.5) + + def initialize(self): + return object() + + def call_tool(self, _tool_name, _arguments): + return object() + + class _DummyStdio: + async def __aenter__(self): + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + exit_started.append("stdio") + import asyncio as _aio + await _aio.sleep(0.5) + + async def _fake_await_with_timeout(coro, operation, timeout_seconds=None): + if hasattr(coro, "close"): + coro.close() + if operation == "domshell_execute": + raise backend.MCPToolTimeoutError("tool timed out") + return None + + async def _run(): + import asyncio as _aio + with pytest.raises(backend.MCPToolTimeoutError, match="tool timed out"): + await _aio.wait_for( + backend._call_execute("ls /", session=Session()), + timeout=0.2, + ) + + with patch.object(backend, "stdio_client", return_value=_DummyStdio()), \ + patch.object(backend, "ClientSession", return_value=_DummyMcpSession()), \ + patch.object(backend, "_build_server_args", return_value=[]), \ + patch.object(backend, "_await_with_timeout", side_effect=_fake_await_with_timeout): + import asyncio as _aio + _aio.run(_run()) + + assert exit_started == ["session", "stdio"] + + def test_daemon_timeout_is_not_retried_and_resets_daemon(): """A daemon-mode timeout must not be retried against the same tool call, but must reset the daemon so later commands can recover.""" @@ -1879,3 +1934,57 @@ def test_ls_absolute_restore_failure_does_not_mask_op_error(mock_call): with pytest.raises(backend.MCPToolTimeoutError, match="ls timed out"): backend.ls("/main", session=sess) assert mock_call.call_count == 3 + + +@pytest.mark.parametrize( + ("invoke", "anchor_cmd", "restore_cmd"), + [ + pytest.param( + lambda sess: backend.ls("/main", session=sess), + "cd %here%/main", + "cd %here%/main", + id="ls", + ), + pytest.param( + lambda sess: backend.cat("/main/btn", session=sess), + "cd %here%", + "cd %here%/main", + id="cat", + ), + pytest.param( + lambda sess: backend.grep("Login", path="/main", session=sess), + "cd %here%/main", + "cd %here%/main", + id="grep", + ), + pytest.param( + lambda sess: backend.click("/main/button[0]", session=sess), + "cd %here%", + "cd %here%/main", + id="click", + ), + pytest.param( + lambda sess: backend.type_text("/main/input", "hello", session=sess), + "cd %here%", + "cd %here%/main", + id="type-text", + ), + ], +) +def test_absolute_wrappers_restore_after_anchor_timeout( + invoke, anchor_cmd, restore_cmd, +): + sess = _make_session(working_dir="/main") + mock_call = AsyncMock(side_effect=[ + backend.MCPToolTimeoutError("anchor timed out"), + _make_result("restored\n[lane: 1]"), + ]) + + with patch.object(backend, "_call_execute", mock_call): + with pytest.raises(backend.MCPToolTimeoutError, match="anchor timed out"): + invoke(sess) + + assert [item.args[0] for item in mock_call.call_args_list] == [ + anchor_cmd, + restore_cmd, + ] diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 1f4d2c7329..9f085ad8ce 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -387,6 +387,19 @@ def _best_effort_restore( ) +def _anchor_with_restore_on_error( + anchor_cmd: str, restore_cmd: str, use_daemon: bool, session: Any, +) -> Any: + """Run an anchor command and restore cwd if it raises.""" + try: + return asyncio.run( + _call_execute(anchor_cmd, use_daemon, session=session) + ) + except Exception: + _best_effort_restore(restore_cmd, use_daemon, session) + raise + + def _anchor_path_cmd(deeper: str = "") -> str: """Build a single ``cd %here%[/]`` command line, shell-quoted. @@ -679,28 +692,34 @@ async def _call_execute( args=_build_server_args() ) + client_context = None + session_context = None try: - async with stdio_client(server_params) as (read, write): - async with ClientSession(read, write) as mcp_session: - await _await_with_timeout( - mcp_session.initialize(), "session initialize", init_timeout - ) - result = await _await_with_timeout( - mcp_session.call_tool("domshell_execute", arguments), - "domshell_execute", - tool_timeout, - ) - _capture_lane(session, result) - # See daemon-path capture above for rationale — same - # logic applies on the fresh-spawn fall-back path, - # including the lock-guarded test-and-set. - if use_daemon and session is None: - with _daemon_lane_lock: - if _daemon_lane_id is None: - captured = _extract_lane_id(result) - if captured: - _daemon_lane_id = captured - return result + client_manager = stdio_client(server_params) + read, write = await client_manager.__aenter__() + client_context = client_manager + session_manager = ClientSession(read, write) + mcp_session = await session_manager.__aenter__() + session_context = session_manager + await _await_with_timeout( + mcp_session.initialize(), "session initialize", init_timeout + ) + result = await _await_with_timeout( + mcp_session.call_tool("domshell_execute", arguments), + "domshell_execute", + tool_timeout, + ) + _capture_lane(session, result) + # See daemon-path capture above for rationale — same + # logic applies on the fresh-spawn fall-back path, + # including the lock-guarded test-and-set. + if use_daemon and session is None: + with _daemon_lane_lock: + if _daemon_lane_id is None: + captured = _extract_lane_id(result) + if captured: + _daemon_lane_id = captured + return result except RuntimeError as e: # Pass through MCPToolTimeoutError unchanged so callers can recover; # but wrap other RuntimeErrors with actionable context below. @@ -717,6 +736,8 @@ async def _call_execute( f"Ensure Chrome is running with DOMShell extension installed.\n" f"Chrome Web Store: https://chromewebstore.google.com/detail/domshell" ) from e + finally: + await _close_daemon_transport(session_context, client_context) # NOTE: Known limitation - Daemon mode uses asyncio.run() per tool call (in sync wrappers). # Each asyncio.run() creates a new event loop. Async IO objects created in one loop @@ -724,15 +745,12 @@ async def _call_execute( # create new loops. This is a documented limitation for v1; future work should use # a single long-lived event loop (e.g., background thread + run_coroutine_threadsafe). async def _close_daemon_transport(session: Any, client_context: Any) -> None: - """Best-effort, bounded exit of whichever daemon contexts were entered. - - Shared by ``_start_daemon`` (init failure after partial entry) and - ``_stop_daemon`` (normal shutdown). Each ``__aexit__`` is bounded by - ``DAEMON_STOP_TIMEOUT_SECONDS`` and failures are swallowed — this - always runs either during cleanup-on-error or intentional shutdown, - so it must never raise over/mask whatever triggered it. Either - argument may be ``None`` (e.g. ``ClientSession.__aenter__`` never - even ran) and is skipped. + """Best-effort, bounded exit of entered MCP contexts. + + Shared by per-command calls and daemon startup/shutdown. Each + ``__aexit__`` is bounded by ``DAEMON_STOP_TIMEOUT_SECONDS`` and + failures are swallowed so cleanup cannot hide the operation result. + Either argument may be ``None`` and is skipped. """ if session is not None: try: @@ -861,12 +879,15 @@ def ls(path: str = "/", use_daemon: bool = False, *, session: Any = None) -> dic # wrong-target results. Three separate _call_execute calls so # we can _is_error-gate after the anchor and skip the operation # cleanly. All share the persisted lane via session. - anchor = asyncio.run(_call_execute( - _anchor_path_cmd(translated), use_daemon, session=session, - )) + restore_cmd = _restore_cwd_cmd(session) + anchor = _anchor_with_restore_on_error( + _anchor_path_cmd(translated), + restore_cmd, + use_daemon, + session, + ) if _is_error(anchor): return _parse_execute_result(anchor, "ls") - restore_cmd = _restore_cwd_cmd(session) try: op = asyncio.run(_call_execute("ls", use_daemon, session=session)) except Exception: @@ -959,12 +980,15 @@ def cat(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: # otherwise run relative cat, restore. Anchor success is # load-bearing — without it cat resolves the relative path # against the wrong cwd. - anchor = asyncio.run(_call_execute( - _anchor_path_cmd(""), use_daemon, session=session, - )) + restore_cmd = _restore_cwd_cmd(session) + anchor = _anchor_with_restore_on_error( + _anchor_path_cmd(""), + restore_cmd, + use_daemon, + session, + ) if _is_error(anchor): return _parse_execute_result(anchor, "cat") - restore_cmd = _restore_cwd_cmd(session) try: op = asyncio.run(_call_execute( f"cat {_q(translated)}", use_daemon, session=session, @@ -1083,7 +1107,9 @@ def grep( else _anchor_path_cmd("") ) - anchor = asyncio.run(_call_execute(anchor_cmd, use_daemon, session=session)) + anchor = _anchor_with_restore_on_error( + anchor_cmd, restore_cmd, use_daemon, session, + ) if _is_error(anchor): return _parse_execute_result(anchor, "grep") # `-r` preserves the pre-migration recursive default (see unrooted @@ -1125,12 +1151,15 @@ def click(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: # otherwise click the relative path, restore. Anchor success is # load-bearing — clicking the wrong element if cwd has drifted # could trigger an unintended action. - anchor = asyncio.run(_call_execute( - _anchor_path_cmd(""), use_daemon, session=session, - )) + restore_cmd = _restore_cwd_cmd(session) + anchor = _anchor_with_restore_on_error( + _anchor_path_cmd(""), + restore_cmd, + use_daemon, + session, + ) if _is_error(anchor): return _parse_execute_result(anchor, "click") - restore_cmd = _restore_cwd_cmd(session) try: op = asyncio.run(_call_execute( f"click {_q(translated)}", use_daemon, session=session, @@ -1316,9 +1345,13 @@ def type_text( # we can _is_error-check, then type, then restore as a separate # best-effort call. All four share the persisted lane via session. if is_absolute: - anchor_result = asyncio.run(_call_execute( - _anchor_path_cmd(""), use_daemon, session=session, - )) + restore_cmd = _restore_cwd_cmd(session) + anchor_result = _anchor_with_restore_on_error( + _anchor_path_cmd(""), + restore_cmd, + use_daemon, + session, + ) if _is_error(anchor_result): # Anchor failed — we never moved, so no restore is needed. return _parse_execute_result(anchor_result, "focus") @@ -1332,7 +1365,7 @@ def type_text( # dict. Only relevant to restore when we actually moved (the # absolute path branch) — mirrors the _is_error restore below. if is_absolute: - _best_effort_restore(_restore_cwd_cmd(session), use_daemon, session) + _best_effort_restore(restore_cmd, use_daemon, session) raise if _is_error(focus_result): # Focus failed — restore cwd before returning so the lane @@ -1340,7 +1373,7 @@ def type_text( # actually moved, i.e. the absolute path branch). if is_absolute: asyncio.run(_call_execute( - _restore_cwd_cmd(session), use_daemon, session=session, + restore_cmd, use_daemon, session=session, )) return _parse_execute_result(focus_result, "focus") @@ -1352,7 +1385,7 @@ def type_text( # type raised after a successful focus — the anchor already # moved the lane cwd, so restore must still be attempted. if is_absolute: - _best_effort_restore(_restore_cwd_cmd(session), use_daemon, session) + _best_effort_restore(restore_cmd, use_daemon, session) raise if is_absolute: @@ -1360,7 +1393,7 @@ def type_text( # failure is cosmetic. The next harness cd will correct any # drift. asyncio.run(_call_execute( - _restore_cwd_cmd(session), use_daemon, session=session, + restore_cmd, use_daemon, session=session, )) return _parse_execute_result(type_result, "type") From d91bd26808262485447a19483558adb2ffd111f3 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:21:48 +0200 Subject: [PATCH 10/13] fix(browser): restore cwd after cd timeout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/tests/test_domshell_backend.py | 17 +++++++++++++++++ .../browser/utils/domshell_backend.py | 15 ++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py index 530945b83d..3b6f84ac34 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py @@ -127,6 +127,23 @@ def test_cd_relative_quoted(mock_call): assert mock_call.call_args.args[0] == "cd main" +def test_cd_restores_tracked_cwd_after_timeout(): + sess = _make_session(working_dir="/main") + mock_call = AsyncMock(side_effect=[ + backend.MCPToolTimeoutError("cd timed out"), + _make_result("restored\n[lane: 1]"), + ]) + + with patch.object(backend, "_call_execute", mock_call): + with pytest.raises(backend.MCPToolTimeoutError, match="cd timed out"): + backend.cd("/dialog", session=sess) + + assert [item.args[0] for item in mock_call.call_args_list] == [ + "cd %here%/dialog", + "cd %here%/main", + ] + + @patch.object(backend, "_call_execute", new_callable=AsyncMock) def test_cat_absolute_path_uses_three_separate_calls(mock_call): """`cat /main/btn`: anchor at tab root → cat main/btn → restore.""" diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 9f085ad8ce..0e6f8e9b0b 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -935,10 +935,10 @@ def cd(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: "output": "cd: /missing: No such directory"} """ translated, is_absolute = _translate_path(path) - # cd is the one wrapper where the operation IS the new state — no - # following operation needs the anchored cwd, so no split-and-check - # and no restore. Absolute targets anchor via `cd %here%/` so - # the result is independent of the lane's current cwd. + # cd is the one wrapper where the operation IS the new state, so no + # restore follows a successful call. Absolute targets anchor via + # `cd %here%/` so the result is independent of the lane's + # current cwd. if is_absolute: command = _anchor_path_cmd(translated) elif translated: @@ -946,7 +946,12 @@ def cd(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: else: # Bare/empty `cd` → back to tab root. command = _anchor_path_cmd("") - result = asyncio.run(_call_execute(command, use_daemon, session=session)) + if session is not None: + result = _anchor_with_restore_on_error( + command, _restore_cwd_cmd(session), use_daemon, session + ) + else: + result = asyncio.run(_call_execute(command, use_daemon, session=session)) return _parse_execute_result(result, "cd") From c3c21eb503d5dbe9b81d322f8344e92ab0b51fa4 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:50:31 +0200 Subject: [PATCH 11/13] fix(browser): close MCP contexts in entering task Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/tests/test_domshell_backend.py | 67 ++++++++++++++++++- .../browser/utils/domshell_backend.py | 29 ++++---- browser/agent-harness/setup.py | 1 + 3 files changed, 84 insertions(+), 13 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py index 3b6f84ac34..654318dfd1 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py @@ -1424,8 +1424,11 @@ async def _slow(): import asyncio as _aio - with pytest.raises(RuntimeError, match="timed out"): + with pytest.raises(RuntimeError, match="timed out") as exc_info: _aio.run(backend._await_with_timeout(_slow(), "unit-test")) + message = str(exc_info.value) + assert "CLI_ANYTHING_BROWSER_MCP_TIMEOUT" in message + assert "CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT" in message def test_call_execute_uses_init_timeout_for_session_initialize(monkeypatch): @@ -1631,6 +1634,68 @@ async def _run(): assert exit_started == ["session", "stdio"] +def test_close_transport_exits_contexts_in_entering_task(): + """AnyIO-backed MCP contexts must exit in the task that entered them.""" + import asyncio as _aio + + class _TaskBoundContext: + entered_task = None + exited_task = None + + async def __aenter__(self): + self.entered_task = _aio.current_task() + return self + + async def __aexit__(self, exc_type, exc, tb): + self.exited_task = _aio.current_task() + return False + + async def _run(): + real_wait_for = _aio.wait_for + + async def _scheduled_wait_for(awaitable, timeout): + return await real_wait_for(_aio.create_task(awaitable), timeout) + + session = _TaskBoundContext() + client = _TaskBoundContext() + await client.__aenter__() + await session.__aenter__() + with patch.object( + backend.asyncio, "wait_for", new=_scheduled_wait_for + ): + await backend._close_daemon_transport(session, client) + assert session.exited_task is session.entered_task + assert client.exited_task is client.entered_task + + _aio.run(_run()) + + +def test_close_transport_preserves_anyio_cancel_scope_order(): + """Cleanup timeout must not nest a cancel scope inside the MCP context.""" + import asyncio as _aio + import anyio + + class _AnyioContext: + closed = False + + async def __aenter__(self): + self.task_group = anyio.create_task_group() + await self.task_group.__aenter__() + return self + + async def __aexit__(self, exc_type, exc, tb): + await self.task_group.__aexit__(exc_type, exc, tb) + self.closed = True + + async def _run(): + context = _AnyioContext() + await context.__aenter__() + await backend._close_daemon_transport(context, None) + assert context.closed is True + + _aio.run(_run()) + + def test_daemon_timeout_is_not_retried_and_resets_daemon(): """A daemon-mode timeout must not be retried against the same tool call, but must reset the daemon so later commands can recover.""" diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 0e6f8e9b0b..590298fafd 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -115,7 +115,9 @@ async def _await_with_timeout( raise MCPToolTimeoutError( "DOMShell MCP request timed out after " f"{timeout_seconds:.1f}s during {operation}. " - "Verify DOMSHELL_TOKEN and that the DOMShell server is reachable." + "Verify DOMSHELL_TOKEN and that the DOMShell server is reachable. " + "Adjust CLI_ANYTHING_BROWSER_MCP_TIMEOUT for tool calls or " + "CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT for initialization." ) from e # Daemon mode: persistent MCP connection @@ -748,24 +750,27 @@ async def _close_daemon_transport(session: Any, client_context: Any) -> None: """Best-effort, bounded exit of entered MCP contexts. Shared by per-command calls and daemon startup/shutdown. Each - ``__aexit__`` is bounded by ``DAEMON_STOP_TIMEOUT_SECONDS`` and - failures are swallowed so cleanup cannot hide the operation result. - Either argument may be ``None`` and is skipped. + ``__aexit__`` runs in the task that entered the AnyIO-backed context + and is bounded by ``DAEMON_STOP_TIMEOUT_SECONDS``. Failures are + swallowed so cleanup cannot hide the operation result. Either argument + may be ``None`` and is skipped. """ + def cleanup_timeout(): + if hasattr(asyncio, "timeout"): + return asyncio.timeout(DAEMON_STOP_TIMEOUT_SECONDS) + from async_timeout import timeout + return timeout(DAEMON_STOP_TIMEOUT_SECONDS) + if session is not None: try: - await asyncio.wait_for( - session.__aexit__(None, None, None), - timeout=DAEMON_STOP_TIMEOUT_SECONDS, - ) + async with cleanup_timeout(): + await session.__aexit__(None, None, None) except (Exception, asyncio.TimeoutError): pass if client_context is not None: try: - await asyncio.wait_for( - client_context.__aexit__(None, None, None), - timeout=DAEMON_STOP_TIMEOUT_SECONDS, - ) + async with cleanup_timeout(): + await client_context.__aexit__(None, None, None) except (Exception, asyncio.TimeoutError): pass diff --git a/browser/agent-harness/setup.py b/browser/agent-harness/setup.py index 7ca9731c8c..c3e1d205c7 100644 --- a/browser/agent-harness/setup.py +++ b/browser/agent-harness/setup.py @@ -32,6 +32,7 @@ def read_readme(): python_requires=">=3.10", install_requires=[ + "async-timeout>=4.0,<6.0; python_version < '3.11'", "click>=8.1,<9.0", "prompt-toolkit>=3.0,<4.0", "mcp>=0.1.0,<1.0.0", From 47742537bbb5181b0f2b9d5eaaeec19369e1c930 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 18:07:08 +0200 Subject: [PATCH 12/13] fix: harden DOMShell MCP context lifecycle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/tests/test_domshell_backend.py | 144 ++++++++++++++++++ .../browser/utils/domshell_backend.py | 112 ++++++++------ 2 files changed, 207 insertions(+), 49 deletions(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py index 654318dfd1..e6ac19db13 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py @@ -1696,6 +1696,150 @@ async def _run(): _aio.run(_run()) +@pytest.mark.parametrize( + "operation", + ["ls", "cat", "grep", "click", "focus-error", "type"], +) +def test_restore_failure_does_not_mask_primary_result(operation): + sess = _make_session(working_dir="/main") + success = _make_result("primary result\n[lane: 1]") + restore_timeout = backend.MCPToolTimeoutError("restore timed out") + + if operation == "focus-error": + focus_error = _make_result("Error: focus failed\n[lane: 1]") + responses = [_make_result("anchored\n[lane: 1]"), focus_error, restore_timeout] + elif operation == "type": + responses = [ + _make_result("anchored\n[lane: 1]"), + _make_result("focused\n[lane: 1]"), + success, + restore_timeout, + ] + else: + responses = [ + _make_result("anchored\n[lane: 1]"), + success, + restore_timeout, + ] + + with patch.object( + backend, "_call_execute", AsyncMock(side_effect=responses) + ): + if operation == "ls": + result = backend.ls("/main", session=sess) + elif operation == "cat": + result = backend.cat("/main/button", session=sess) + elif operation == "grep": + result = backend.grep("needle", path="/main", session=sess) + elif operation == "click": + result = backend.click("/main/button", session=sess) + else: + result = backend.type_text("/main/input", "hello", session=sess) + + if operation == "focus-error": + assert "focus failed" in result["error"] + elif operation in {"ls", "grep"}: + assert "primary result" in result["raw"] + else: + assert "primary result" in result["output"] + + +@pytest.mark.parametrize("blocked_stage", ["stdio", "session"]) +def test_call_execute_bounds_context_entry(monkeypatch, blocked_stage): + monkeypatch.setenv("DOMSHELL_TOKEN", "test-token") + monkeypatch.setattr(backend, "_get_init_timeout_seconds", lambda: 0.01) + + class _Stdio: + async def __aenter__(self): + if blocked_stage == "stdio": + import asyncio as _aio + await _aio.sleep(0.5) + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + return False + + class _Session: + async def __aenter__(self): + if blocked_stage == "session": + import asyncio as _aio + await _aio.sleep(0.5) + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + async def _run(): + import asyncio as _aio + with pytest.raises( + backend.MCPToolTimeoutError, match=f"{blocked_stage}.*enter" + ): + await _aio.wait_for( + backend._call_execute("ls /", session=Session()), + timeout=0.2, + ) + + with patch.object(backend, "stdio_client", return_value=_Stdio()), \ + patch.object(backend, "ClientSession", return_value=_Session()), \ + patch.object(backend, "_build_server_args", return_value=[]): + import asyncio as _aio + _aio.run(_run()) + + +@pytest.mark.parametrize("blocked_stage", ["stdio", "session"]) +def test_start_daemon_bounds_context_entry(monkeypatch, blocked_stage): + monkeypatch.setattr(backend, "_get_init_timeout_seconds", lambda: 0.01) + + class _Stdio: + async def __aenter__(self): + if blocked_stage == "stdio": + import asyncio as _aio + await _aio.sleep(0.5) + return object(), object() + + async def __aexit__(self, exc_type, exc, tb): + return False + + class _Session: + async def __aenter__(self): + if blocked_stage == "session": + import asyncio as _aio + await _aio.sleep(0.5) + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + async def _run(): + import asyncio as _aio + with pytest.raises(RuntimeError, match=f"{blocked_stage}.*enter"): + await _aio.wait_for(backend._start_daemon(), timeout=0.2) + + original_state = ( + backend._daemon_session, + backend._daemon_read, + backend._daemon_write, + backend._daemon_client_context, + ) + try: + backend._daemon_session = None + backend._daemon_read = None + backend._daemon_write = None + backend._daemon_client_context = None + with patch.object(backend, "stdio_client", return_value=_Stdio()), \ + patch.object(backend, "ClientSession", return_value=_Session()), \ + patch.object(backend, "_build_server_args", return_value=[]): + import asyncio as _aio + _aio.run(_run()) + finally: + ( + backend._daemon_session, + backend._daemon_read, + backend._daemon_write, + backend._daemon_client_context, + ) = original_state + + def test_daemon_timeout_is_not_retried_and_resets_daemon(): """A daemon-mode timeout must not be retried against the same tool call, but must reset the daemon so later commands can recover.""" diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 590298fafd..7abacb7eb2 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -96,6 +96,24 @@ def _get_init_timeout_seconds() -> float: return max(1.0, parsed) +def _same_task_timeout(timeout_seconds: float) -> Any: + """Return a timeout context that does not move work to another task.""" + if hasattr(asyncio, "timeout"): + return asyncio.timeout(timeout_seconds) + from async_timeout import timeout + return timeout(timeout_seconds) + + +def _timeout_error(operation: str, timeout_seconds: float) -> str: + return ( + "DOMShell MCP request timed out after " + f"{timeout_seconds:.1f}s during {operation}. " + "Verify DOMSHELL_TOKEN and that the DOMShell server is reachable. " + "Adjust CLI_ANYTHING_BROWSER_MCP_TIMEOUT for tool calls or " + "CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT for initialization." + ) + + async def _await_with_timeout( coro: Awaitable[Any], operation: str, @@ -113,11 +131,22 @@ async def _await_with_timeout( return await asyncio.wait_for(coro, timeout=timeout_seconds) except asyncio.TimeoutError as e: raise MCPToolTimeoutError( - "DOMShell MCP request timed out after " - f"{timeout_seconds:.1f}s during {operation}. " - "Verify DOMSHELL_TOKEN and that the DOMShell server is reachable. " - "Adjust CLI_ANYTHING_BROWSER_MCP_TIMEOUT for tool calls or " - "CLI_ANYTHING_BROWSER_MCP_INIT_TIMEOUT for initialization." + _timeout_error(operation, timeout_seconds) + ) from e + + +async def _enter_context_with_timeout( + context: Any, + operation: str, + timeout_seconds: float, +) -> Any: + """Enter an AnyIO-backed context with a timeout in the current task.""" + try: + async with _same_task_timeout(timeout_seconds): + return await context.__aenter__() + except asyncio.TimeoutError as e: + raise MCPToolTimeoutError( + _timeout_error(operation, timeout_seconds) ) from e # Daemon mode: persistent MCP connection @@ -365,26 +394,19 @@ def _restore_cwd_cmd(session: Any) -> str: def _best_effort_restore( restore_cmd: str, use_daemon: bool, session: Any, ) -> None: - """Attempt a restore-cwd call after an operation raised. + """Attempt a restore-cwd call without masking the primary result. Absolute-path wrappers (ls/cat/click/grep/type_text) anchor a `cd` before their real operation and restore afterward. If the operation - itself raises (e.g. an MCP timeout), the restore must still be - attempted — otherwise the lane's cwd stays parked at the anchor and - silently corrupts every later relative-path call in that lane. - - Any exception from THIS restore attempt is swallowed: it must never - mask the original operation failure that's already propagating past - this cleanup call. This is deliberately narrower than the - restore-on-success call each wrapper makes directly — a restore - failure on the normal-return path is not swallowed, since there's no - prior error it could mask. + raises or returns, restore must still be attempted so the lane's cwd + does not stay parked at the anchor. Restore errors are logged and + swallowed because cleanup must not replace the operation's result. """ try: asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) except Exception: log.warning( - "DOMShell lane cwd restore failed after an operation error", + "DOMShell lane cwd restore failed", exc_info=True, ) @@ -697,12 +719,14 @@ async def _call_execute( client_context = None session_context = None try: - client_manager = stdio_client(server_params) - read, write = await client_manager.__aenter__() - client_context = client_manager - session_manager = ClientSession(read, write) - mcp_session = await session_manager.__aenter__() - session_context = session_manager + client_context = stdio_client(server_params) + read, write = await _enter_context_with_timeout( + client_context, "stdio client enter", init_timeout + ) + session_context = ClientSession(read, write) + mcp_session = await _enter_context_with_timeout( + session_context, "session enter", init_timeout + ) await _await_with_timeout( mcp_session.initialize(), "session initialize", init_timeout ) @@ -755,21 +779,15 @@ async def _close_daemon_transport(session: Any, client_context: Any) -> None: swallowed so cleanup cannot hide the operation result. Either argument may be ``None`` and is skipped. """ - def cleanup_timeout(): - if hasattr(asyncio, "timeout"): - return asyncio.timeout(DAEMON_STOP_TIMEOUT_SECONDS) - from async_timeout import timeout - return timeout(DAEMON_STOP_TIMEOUT_SECONDS) - if session is not None: try: - async with cleanup_timeout(): + async with _same_task_timeout(DAEMON_STOP_TIMEOUT_SECONDS): await session.__aexit__(None, None, None) except (Exception, asyncio.TimeoutError): pass if client_context is not None: try: - async with cleanup_timeout(): + async with _same_task_timeout(DAEMON_STOP_TIMEOUT_SECONDS): await client_context.__aexit__(None, None, None) except (Exception, asyncio.TimeoutError): pass @@ -796,14 +814,19 @@ async def _start_daemon() -> bool: try: # Store the context manager so we can properly clean it up later + init_timeout = _get_init_timeout_seconds() _daemon_client_context = stdio_client(server_params) - _daemon_read, _daemon_write = await _daemon_client_context.__aenter__() + _daemon_read, _daemon_write = await _enter_context_with_timeout( + _daemon_client_context, "daemon stdio enter", init_timeout + ) _daemon_session = ClientSession(_daemon_read, _daemon_write) - await _daemon_session.__aenter__() + await _enter_context_with_timeout( + _daemon_session, "daemon session enter", init_timeout + ) await _await_with_timeout( _daemon_session.initialize(), "daemon initialize", - _get_init_timeout_seconds(), + init_timeout, ) return True except Exception as e: @@ -901,9 +924,7 @@ def ls(path: str = "/", use_daemon: bool = False, *, session: Any = None) -> dic # doesn't stay parked at the anchor. _best_effort_restore(restore_cmd, use_daemon, session) raise - # Best-effort restore — ls already ran; restore failure is - # cosmetic (next harness cd corrects any drift). - asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) + _best_effort_restore(restore_cmd, use_daemon, session) return _parse_execute_result(op, "ls") if translated: op = asyncio.run(_call_execute( @@ -1006,7 +1027,7 @@ def cat(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: except Exception: _best_effort_restore(restore_cmd, use_daemon, session) raise - asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) + _best_effort_restore(restore_cmd, use_daemon, session) return _parse_execute_result(op, "cat") op = asyncio.run(_call_execute( f"cat {_q(translated)}", use_daemon, session=session, @@ -1131,7 +1152,7 @@ def grep( except Exception: _best_effort_restore(restore_cmd, use_daemon, session) raise - asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) + _best_effort_restore(restore_cmd, use_daemon, session) return _parse_execute_result(op, "grep") @@ -1177,7 +1198,7 @@ def click(path: str, use_daemon: bool = False, *, session: Any = None) -> dict: except Exception: _best_effort_restore(restore_cmd, use_daemon, session) raise - asyncio.run(_call_execute(restore_cmd, use_daemon, session=session)) + _best_effort_restore(restore_cmd, use_daemon, session) return _parse_execute_result(op, "click") op = asyncio.run(_call_execute( f"click {_q(translated)}", use_daemon, session=session, @@ -1382,9 +1403,7 @@ def type_text( # doesn't stay parked at the anchor (only relevant when we # actually moved, i.e. the absolute path branch). if is_absolute: - asyncio.run(_call_execute( - restore_cmd, use_daemon, session=session, - )) + _best_effort_restore(restore_cmd, use_daemon, session) return _parse_execute_result(focus_result, "focus") try: @@ -1399,12 +1418,7 @@ def type_text( raise if is_absolute: - # Best-effort restore — type already succeeded, so a restore - # failure is cosmetic. The next harness cd will correct any - # drift. - asyncio.run(_call_execute( - restore_cmd, use_daemon, session=session, - )) + _best_effort_restore(restore_cmd, use_daemon, session) return _parse_execute_result(type_result, "type") From 9495a76dc69bace08eef3fe078ff3121eed3e70b Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 18:25:15 +0200 Subject: [PATCH 13/13] fix: keep MCP timeout awaits in task Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/tests/test_domshell_backend.py | 18 ++++++++++++++++++ .../browser/utils/domshell_backend.py | 3 ++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py index e6ac19db13..9d7612370f 100644 --- a/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/tests/test_domshell_backend.py @@ -1413,6 +1413,24 @@ async def _fast(): assert result == {"ok": True} +def test_await_with_timeout_runs_operation_in_calling_task(): + async def _run(): + import asyncio as _aio + + calling_task = _aio.current_task() + + async def _operation(): + return _aio.current_task() + + operation_task = await backend._await_with_timeout( + _operation(), "unit-test", 1 + ) + assert operation_task is calling_task + + import asyncio as _aio + _aio.run(_run()) + + def test_await_with_timeout_raises_on_timeout(monkeypatch): """Slow operations should raise an actionable MCPToolTimeoutError.""" monkeypatch.setenv("CLI_ANYTHING_BROWSER_MCP_TIMEOUT", "1") diff --git a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py index 7abacb7eb2..c0b2141192 100644 --- a/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py +++ b/browser/agent-harness/cli_anything/browser/utils/domshell_backend.py @@ -128,7 +128,8 @@ async def _await_with_timeout( if timeout_seconds is None: timeout_seconds = _get_tool_timeout_seconds() try: - return await asyncio.wait_for(coro, timeout=timeout_seconds) + async with _same_task_timeout(timeout_seconds): + return await coro except asyncio.TimeoutError as e: raise MCPToolTimeoutError( _timeout_error(operation, timeout_seconds)