diff --git a/apps/desktop/src/components/assistant-ui/tool-fallback-model.test.ts b/apps/desktop/src/components/assistant-ui/tool-fallback-model.test.ts index 142b912e4dad..6c864608c727 100644 --- a/apps/desktop/src/components/assistant-ui/tool-fallback-model.test.ts +++ b/apps/desktop/src/components/assistant-ui/tool-fallback-model.test.ts @@ -77,6 +77,36 @@ describe('buildToolView terminal exit-code status', () => { }) }) +describe('buildToolView browser_navigate title', () => { + it('shows failed title when navigate returns success=false', () => { + const view = buildToolView( + part({ + toolName: 'browser_navigate', + args: { url: 'https://hermes-agent.nousresearch.com/docs' }, + result: { success: false, error: 'Command timed out after 60 seconds' } + }), + '' + ) + + expect(view.status).toBe('error') + expect(view.title).toBe('Failed to open hermes-agent.nousresearch.com') + }) + + it('shows opened title on success', () => { + const view = buildToolView( + part({ + toolName: 'browser_navigate', + args: { url: 'https://hermes-agent.nousresearch.com/docs' }, + result: { success: true, url: 'https://hermes-agent.nousresearch.com/docs', title: 'Docs' } + }), + '' + ) + + expect(view.status).toBe('success') + expect(view.title).toBe('Opened hermes-agent.nousresearch.com') + }) +}) + describe('buildToolView file edit diffs', () => { const patchDiff = '--- a/src/demo.ts\n+++ b/src/demo.ts\n@@ -1 +1 @@\n-old\n+new' diff --git a/apps/desktop/src/components/assistant-ui/tool-fallback-model.ts b/apps/desktop/src/components/assistant-ui/tool-fallback-model.ts index 0cc22b62e48a..b99527f05c40 100644 --- a/apps/desktop/src/components/assistant-ui/tool-fallback-model.ts +++ b/apps/desktop/src/components/assistant-ui/tool-fallback-model.ts @@ -1534,11 +1534,32 @@ function dynamicTitle( if (part.toolName === 'browser_navigate') { const url = findFirstUrl(args, result) + + if (!url) { + return fallback + } + + const failed = + part.isError || + result.success === false || + result.ok === false || + Boolean(firstStringField(result, ['error'])) + + if (failed) { + const failAction = translateNow('assistant.tool.actions.failedToOpen') + + return titledAction( + failAction, + translateNow('assistant.tool.titleTemplates.actionTarget', failAction, hostnameOf(url)) + ) + } + const action = verb(translateNow('assistant.tool.actions.opening'), translateNow('assistant.tool.actions.opened')) - return url - ? titledAction(action, translateNow('assistant.tool.titleTemplates.actionTarget', action, hostnameOf(url))) - : fallback + return titledAction( + action, + translateNow('assistant.tool.titleTemplates.actionTarget', action, hostnameOf(url)) + ) } if (part.toolName === 'web_search') { diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index f7841f760a35..52501ee97395 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -2066,6 +2066,7 @@ export const en: Translations = { reading: 'Reading', opened: 'Opened', opening: 'Opening', + failedToOpen: 'Failed to open', searched: 'Searched', searching: 'Searching', ran: 'Ran', diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index 741712a6d3c5..bdf4e8829299 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -2189,6 +2189,7 @@ export const ja = defineLocale({ reading: '読み取り中', opened: 'オープン済み', opening: 'オープン中', + failedToOpen: 'オープン失敗', searched: '検索完了', searching: '検索中', ran: '実行完了', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index 54534f915e75..c11357d91f6e 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1715,6 +1715,7 @@ export interface Translations { reading: string opened: string opening: string + failedToOpen: string searched: string searching: string ran: string diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index fbf2ed0822b2..24e0ff12dac3 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -2122,6 +2122,7 @@ export const zhHant = defineLocale({ reading: '正在讀取', opened: '已開啟', opening: '正在開啟', + failedToOpen: '開啟失敗', searched: '已搜尋', searching: '正在搜尋', ran: '已執行', diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 6d029367eae7..a528099cc529 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -2235,6 +2235,7 @@ export const zh: Translations = { reading: '正在读取', opened: '已打开', opening: '正在打开', + failedToOpen: '打开失败', searched: '已搜索', searching: '正在搜索', ran: '已运行', diff --git a/tests/tools/test_browser_open_timeout.py b/tests/tools/test_browser_open_timeout.py new file mode 100644 index 000000000000..e6fd45491202 --- /dev/null +++ b/tests/tools/test_browser_open_timeout.py @@ -0,0 +1,112 @@ +"""Tests for browser first-open timeout and timeout diagnostics.""" + +from unittest.mock import patch + +import pytest + +import tools.browser_tool as bt + + +@pytest.fixture(autouse=True) +def _reset_browser_caches(): + bt._cached_command_timeout = None + bt._command_timeout_resolved = False + yield + bt._cached_command_timeout = None + bt._command_timeout_resolved = False + + +class TestOpenCommandTimeout: + def test_first_open_uses_longer_floor(self, monkeypatch): + monkeypatch.setattr(bt, "_get_command_timeout", lambda: 30) + assert bt._get_open_command_timeout(first_open=True) == bt.MIN_FIRST_OPEN_TIMEOUT + assert bt._get_open_command_timeout(first_open=False) == bt.MIN_OPEN_TIMEOUT + + def test_respects_config_above_floor(self, monkeypatch): + monkeypatch.setattr(bt, "_get_command_timeout", lambda: 180) + assert bt._get_open_command_timeout(first_open=True) == 180 + assert bt._get_open_command_timeout(first_open=False) == 180 + + +class TestSandboxBypass: + def test_docker_triggers_bypass(self, monkeypatch): + monkeypatch.setattr(bt, "_running_in_docker", lambda: True) + assert bt._needs_chromium_sandbox_bypass() is True + + def test_apparmor_userns_triggers_bypass(self, monkeypatch, tmp_path): + monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + sysctl = tmp_path / "apparmor_restrict_unprivileged_userns" + sysctl.write_text("1\n", encoding="utf-8") + + import builtins + + real_open = builtins.open + + def _open(path, *args, **kwargs): + if "apparmor_restrict_unprivileged_userns" in str(path): + return real_open(sysctl, *args, **kwargs) + return real_open(path, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", _open) + assert bt._needs_chromium_sandbox_bypass() is True + + +class TestTimeoutErrorFormatting: + def test_includes_stderr_detail(self): + err = bt._format_browser_timeout_error( + "open", + 120, + "", + "Daemon process exited during startup", + ) + assert "120 seconds" in err + assert "Daemon process exited" in err + + def test_sandbox_hint(self): + err = bt._format_browser_timeout_error( + "open", + 60, + "", + "No usable sandbox!", + ) + assert "AGENT_BROWSER_ARGS" in err + + def test_local_install_hint(self, monkeypatch): + monkeypatch.setattr(bt, "_is_local_mode", lambda: True) + monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + err = bt._format_browser_timeout_error("open", 60, "", "") + assert "agent-browser install --with-deps" in err + + +class TestReadCommandOutputFiles: + def test_reads_stdout_and_stderr(self, tmp_path): + stdout_path = tmp_path / "out" + stderr_path = tmp_path / "err" + stdout_path.write_text("ok", encoding="utf-8") + stderr_path.write_text("warn", encoding="utf-8") + stdout, stderr = bt._read_command_output_files(str(stdout_path), str(stderr_path)) + assert stdout == "ok" + assert stderr == "warn" + + +class TestBrowserNavigateOpenTimeout: + def test_first_navigation_uses_first_open_timeout(self, monkeypatch): + captured: dict = {} + + def fake_run(task_id, command, args, timeout=None): + if command == "open": + captured["timeout"] = timeout + return {"success": True, "data": {"title": "t", "url": args[0] if args else ""}} + + monkeypatch.setattr(bt, "_get_open_command_timeout", lambda first_open=False: 120 if first_open else 60) + monkeypatch.setattr(bt, "_run_browser_command", fake_run) + monkeypatch.setattr(bt, "_get_session_info", lambda key: {"_first_nav": True, "features": {}}) + monkeypatch.setattr(bt, "_is_camofox_mode", lambda: False) + monkeypatch.setattr(bt, "_is_local_backend", lambda: True) + monkeypatch.setattr(bt, "_is_local_sidecar_key", lambda key: False) + monkeypatch.setattr(bt, "_navigation_session_key", lambda task_id, url: task_id) + monkeypatch.setattr(bt, "_maybe_start_recording", lambda *a, **kw: None) + monkeypatch.setattr(bt, "check_website_access", lambda url: None) + + bt.browser_navigate("https://example.com", task_id="task-1") + assert captured["timeout"] == 120 diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 018fc008c8d8..3afc63346dad 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -221,6 +221,11 @@ _last_screenshot_cleanup_by_dir: dict[str, float] = {} # Default timeout for browser commands (seconds) DEFAULT_COMMAND_TIMEOUT = 30 +# Floor for ``open`` (navigate) — cold daemon + first Chromium launch can exceed +# the generic command_timeout on slow or library-starved Linux hosts. +MIN_OPEN_TIMEOUT = 60 +MIN_FIRST_OPEN_TIMEOUT = 120 + # Max tokens for snapshot content before summarization SNAPSHOT_SUMMARIZE_THRESHOLD = 8000 @@ -256,6 +261,92 @@ def _get_command_timeout() -> int: return result +def _get_open_command_timeout(*, first_open: bool = False) -> int: + """Timeout for agent-browser ``open`` (navigation / daemon cold start).""" + base = _get_command_timeout() + floor = MIN_FIRST_OPEN_TIMEOUT if first_open else MIN_OPEN_TIMEOUT + return max(base, floor) + + +def _needs_chromium_sandbox_bypass() -> bool: + """Return True when Chromium needs --no-sandbox to start reliably.""" + if hasattr(os, "geteuid") and os.geteuid() == 0: + return True + if _running_in_docker(): + return True + userns_restrict = "/proc/sys/kernel/apparmor_restrict_unprivileged_userns" + try: + with open(userns_restrict, encoding="utf-8") as f: + if f.read().strip() == "1": + return True + except OSError: + pass + return False + + +def _read_command_output_files(stdout_path: str, stderr_path: str) -> tuple[str, str]: + """Best-effort read of agent-browser stdout/stderr temp files.""" + stdout = stderr = "" + for path, slot in ((stdout_path, "stdout"), (stderr_path, "stderr")): + try: + with open(path, "r", encoding="utf-8") as f: + text = f.read().strip() + except OSError: + continue + if slot == "stdout": + stdout = text + else: + stderr = text + return stdout, stderr + + +def _unlink_command_output_files(*paths: str) -> None: + for path in paths: + try: + os.unlink(path) + except OSError: + pass + + +def _format_browser_timeout_error( + command: str, + timeout: int, + stdout: str, + stderr: str, +) -> str: + """Build an actionable timeout message from captured daemon output.""" + parts = [f"Command timed out after {timeout} seconds"] + detail = (stderr or stdout or "").strip() + if detail: + parts.append(detail[:1500]) + + combined = f"{stderr}\n{stdout}".lower() + hints: list[str] = [] + if "sandbox" in combined: + hints.append( + "Chromium sandbox launch failed. Set AGENT_BROWSER_ARGS=" + "'--no-sandbox,--disable-dev-shm-usage' in your environment, " + "or run: npx agent-browser install --with-deps" + ) + elif command == "open" and _is_local_mode(): + if _running_in_docker(): + hints.append( + "The browser daemon may still be starting or Chromium may be " + "missing. Pull the latest image: " + "docker pull ghcr.io/nousresearch/hermes-agent:latest" + ) + else: + hints.append( + "The browser daemon may still be starting, or Chromium may be " + "missing system libraries. Install/repair with: " + "npx agent-browser install --with-deps " + "(or: npx playwright install --with-deps chromium)" + ) + if hints: + parts.extend(hints) + return "\n".join(parts) + + def _get_vision_model() -> Optional[str]: """Model for browser_vision (screenshot analysis — multimodal).""" return os.getenv("AUXILIARY_VISION_MODEL", "").strip() or None @@ -2187,24 +2278,11 @@ def _run_browser_command( "AGENT_BROWSER_ARGS" not in browser_env and "AGENT_BROWSER_CHROME_FLAGS" not in browser_env ): - _needs_sandbox_bypass = False - if hasattr(os, "geteuid") and os.geteuid() == 0: - _needs_sandbox_bypass = True - logger.debug("browser: running as root — injecting --no-sandbox") - else: - # Detect AppArmor user namespace restrictions (Ubuntu 23.10+) - _userns_restrict = "/proc/sys/kernel/apparmor_restrict_unprivileged_userns" - try: - with open(_userns_restrict, encoding="utf-8") as _f: - if _f.read().strip() == "1": - _needs_sandbox_bypass = True - logger.debug( - "browser: AppArmor userns restrictions detected — " - "injecting --no-sandbox" - ) - except OSError: - pass - if _needs_sandbox_bypass: + if _needs_chromium_sandbox_bypass(): + logger.debug( + "browser: sandbox bypass needed (root/docker/AppArmor userns) — " + "injecting --no-sandbox" + ) browser_env["AGENT_BROWSER_ARGS"] = ( "--no-sandbox,--disable-dev-shm-usage" ) @@ -2252,9 +2330,20 @@ def _run_browser_command( except subprocess.TimeoutExpired: proc.kill() proc.wait() + stdout, stderr = _read_command_output_files(stdout_path, stderr_path) + _unlink_command_output_files(stdout_path, stderr_path) + if stderr and stderr.strip(): + logger.warning( + "browser '%s' stderr after timeout: %s", + command, + stderr.strip()[:500], + ) logger.warning("browser '%s' timed out after %ds (task=%s, socket_dir=%s)", command, timeout, task_id, task_socket_dir) - result = {"success": False, "error": f"Command timed out after {timeout} seconds"} + result = { + "success": False, + "error": _format_browser_timeout_error(command, timeout, stdout, stderr), + } # Fall through to fallback check below else: with open(stdout_path, "r", encoding="utf-8") as f: @@ -2554,7 +2643,12 @@ def browser_navigate(url: str, task_id: Optional[str] = None) -> str: session_info["_first_nav"] = False _maybe_start_recording(nav_session_key) - result = _run_browser_command(nav_session_key, "open", [url], timeout=max(_get_command_timeout(), 60)) + result = _run_browser_command( + nav_session_key, + "open", + [url], + timeout=_get_open_command_timeout(first_open=is_first_nav), + ) # Remember which session served this nav so snapshot/click/fill/... # on the same task_id hit it (critical when hybrid routing has both a