mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(browser): preserve agent-browser paths with spaces
This commit is contained in:
parent
a18e5b95ad
commit
085c1c6875
2 changed files with 108 additions and 1 deletions
|
|
@ -152,6 +152,109 @@ class TestFindAgentBrowser:
|
|||
class TestRunBrowserCommandPathConstruction:
|
||||
"""Verify _run_browser_command() includes Homebrew node dirs in subprocess PATH."""
|
||||
|
||||
def test_subprocess_preserves_executable_path_with_spaces(self, tmp_path):
|
||||
"""A local agent-browser path containing spaces must stay one argv entry."""
|
||||
captured_cmd = None
|
||||
|
||||
mock_proc = MagicMock()
|
||||
mock_proc.returncode = 0
|
||||
mock_proc.wait.return_value = 0
|
||||
|
||||
def capture_popen(cmd, **kwargs):
|
||||
nonlocal captured_cmd
|
||||
captured_cmd = cmd
|
||||
return mock_proc
|
||||
|
||||
fake_session = {
|
||||
"session_name": "test-session",
|
||||
"session_id": "test-id",
|
||||
"cdp_url": None,
|
||||
}
|
||||
fake_json = json.dumps({"success": True})
|
||||
browser_path = "/Users/test/Library/Application Support/hermes/node_modules/.bin/agent-browser"
|
||||
hermes_home = str(tmp_path / "hermes-home")
|
||||
|
||||
with patch("tools.browser_tool._find_agent_browser", return_value=browser_path), \
|
||||
patch("tools.browser_tool._get_session_info", return_value=fake_session), \
|
||||
patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \
|
||||
patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
|
||||
patch("hermes_constants.Path.home", return_value=tmp_path), \
|
||||
patch("subprocess.Popen", side_effect=capture_popen), \
|
||||
patch("os.open", return_value=99), \
|
||||
patch("os.close"), \
|
||||
patch("tools.interrupt.is_interrupted", return_value=False), \
|
||||
patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"PATH": "/usr/bin:/bin",
|
||||
"HOME": "/home/test",
|
||||
"HERMES_HOME": hermes_home,
|
||||
},
|
||||
clear=True,
|
||||
):
|
||||
with patch("builtins.open", mock_open(read_data=fake_json)):
|
||||
_run_browser_command("test-task", "navigate", ["https://example.com"])
|
||||
|
||||
assert captured_cmd is not None
|
||||
assert captured_cmd[0] == browser_path
|
||||
assert captured_cmd[1:5] == [
|
||||
"--session",
|
||||
"test-session",
|
||||
"--json",
|
||||
"navigate",
|
||||
]
|
||||
|
||||
def test_subprocess_splits_npx_fallback_into_command_and_package(self, tmp_path):
|
||||
"""The synthetic npx fallback should still expand into separate argv items."""
|
||||
captured_cmd = None
|
||||
|
||||
mock_proc = MagicMock()
|
||||
mock_proc.returncode = 0
|
||||
mock_proc.wait.return_value = 0
|
||||
|
||||
def capture_popen(cmd, **kwargs):
|
||||
nonlocal captured_cmd
|
||||
captured_cmd = cmd
|
||||
return mock_proc
|
||||
|
||||
fake_session = {
|
||||
"session_name": "test-session",
|
||||
"session_id": "test-id",
|
||||
"cdp_url": None,
|
||||
}
|
||||
fake_json = json.dumps({"success": True})
|
||||
hermes_home = str(tmp_path / "hermes-home")
|
||||
|
||||
with patch("tools.browser_tool._find_agent_browser", return_value="npx agent-browser"), \
|
||||
patch("tools.browser_tool._get_session_info", return_value=fake_session), \
|
||||
patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \
|
||||
patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
|
||||
patch("hermes_constants.Path.home", return_value=tmp_path), \
|
||||
patch("subprocess.Popen", side_effect=capture_popen), \
|
||||
patch("os.open", return_value=99), \
|
||||
patch("os.close"), \
|
||||
patch("tools.interrupt.is_interrupted", return_value=False), \
|
||||
patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"PATH": "/usr/bin:/bin",
|
||||
"HOME": "/home/test",
|
||||
"HERMES_HOME": hermes_home,
|
||||
},
|
||||
clear=True,
|
||||
):
|
||||
with patch("builtins.open", mock_open(read_data=fake_json)):
|
||||
_run_browser_command("test-task", "navigate", ["https://example.com"])
|
||||
|
||||
assert captured_cmd is not None
|
||||
assert captured_cmd[:2] == ["npx", "agent-browser"]
|
||||
assert captured_cmd[2:6] == [
|
||||
"--session",
|
||||
"test-session",
|
||||
"--json",
|
||||
"navigate",
|
||||
]
|
||||
|
||||
def test_subprocess_path_includes_homebrew_node_dirs(self, tmp_path):
|
||||
"""When _discover_homebrew_node_dirs returns dirs, they should appear
|
||||
in the subprocess env PATH passed to Popen."""
|
||||
|
|
|
|||
|
|
@ -877,7 +877,11 @@ def _run_browser_command(
|
|||
# Local mode — launch a headless Chromium instance
|
||||
backend_args = ["--session", session_info["session_name"]]
|
||||
|
||||
cmd_parts = browser_cmd.split() + backend_args + [
|
||||
# Keep concrete executable paths intact, even when they contain spaces.
|
||||
# Only the synthetic npx fallback needs to expand into multiple argv items.
|
||||
cmd_prefix = ["npx", "agent-browser"] if browser_cmd == "npx agent-browser" else [browser_cmd]
|
||||
|
||||
cmd_parts = cmd_prefix + backend_args + [
|
||||
"--json",
|
||||
command
|
||||
] + args
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue