mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
Merge pull request #54353 from NousResearch/bb/browser-first-open-timeout
fix(browser): extend first-open timeout & surface daemon errors on Linux (salvage #52575)
This commit is contained in:
commit
27868e5b55
9 changed files with 285 additions and 23 deletions
|
|
@ -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'
|
||||
|
||||
|
|
|
|||
|
|
@ -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') {
|
||||
|
|
|
|||
|
|
@ -2066,6 +2066,7 @@ export const en: Translations = {
|
|||
reading: 'Reading',
|
||||
opened: 'Opened',
|
||||
opening: 'Opening',
|
||||
failedToOpen: 'Failed to open',
|
||||
searched: 'Searched',
|
||||
searching: 'Searching',
|
||||
ran: 'Ran',
|
||||
|
|
|
|||
|
|
@ -2189,6 +2189,7 @@ export const ja = defineLocale({
|
|||
reading: '読み取り中',
|
||||
opened: 'オープン済み',
|
||||
opening: 'オープン中',
|
||||
failedToOpen: 'オープン失敗',
|
||||
searched: '検索完了',
|
||||
searching: '検索中',
|
||||
ran: '実行完了',
|
||||
|
|
|
|||
|
|
@ -1715,6 +1715,7 @@ export interface Translations {
|
|||
reading: string
|
||||
opened: string
|
||||
opening: string
|
||||
failedToOpen: string
|
||||
searched: string
|
||||
searching: string
|
||||
ran: string
|
||||
|
|
|
|||
|
|
@ -2122,6 +2122,7 @@ export const zhHant = defineLocale({
|
|||
reading: '正在讀取',
|
||||
opened: '已開啟',
|
||||
opening: '正在開啟',
|
||||
failedToOpen: '開啟失敗',
|
||||
searched: '已搜尋',
|
||||
searching: '正在搜尋',
|
||||
ran: '已執行',
|
||||
|
|
|
|||
|
|
@ -2235,6 +2235,7 @@ export const zh: Translations = {
|
|||
reading: '正在读取',
|
||||
opened: '已打开',
|
||||
opening: '正在打开',
|
||||
failedToOpen: '打开失败',
|
||||
searched: '已搜索',
|
||||
searching: '正在搜索',
|
||||
ran: '已运行',
|
||||
|
|
|
|||
112
tests/tools/test_browser_open_timeout.py
Normal file
112
tests/tools/test_browser_open_timeout.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue