diff --git a/tests/tools/test_browser_homebrew_paths.py b/tests/tools/test_browser_homebrew_paths.py index 3e2e766694..33b725604c 100644 --- a/tests/tools/test_browser_homebrew_paths.py +++ b/tests/tools/test_browser_homebrew_paths.py @@ -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.""" diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 7e52ed78d9..012b8eb020 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -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