From 2f9d88caee267aefcf19cb8c9cf1a514bf1fed6c Mon Sep 17 00:00:00 2001 From: Tianqing Yun Date: Mon, 20 Jul 2026 19:39:53 -0700 Subject: [PATCH] fix: resolve cua-driver across computer-use surfaces --- hermes_cli/main.py | 15 +++--- hermes_cli/tools_config.py | 21 +++++--- .../computer_use/test_cua_cli_fallback_env.py | 10 +++- tests/computer_use/test_doctor.py | 23 +++++++++ .../test_permissions_resolution.py | 32 ++++++++++++ tests/tools/test_computer_use.py | 50 ++++++++++++++++++- tools/computer_use/cua_backend.py | 29 ++++++++--- tools/computer_use/doctor.py | 18 +++---- tools/computer_use/permissions.py | 17 +++---- 9 files changed, 169 insertions(+), 46 deletions(-) create mode 100644 tests/computer_use/test_permissions_resolution.py diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 9bdb87691df1..01eb6a758a57 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -14456,14 +14456,14 @@ def main(): install_cua_driver(upgrade=bool(getattr(args, "upgrade", False))) return if action == "status": - import shutil import subprocess - from hermes_cli.tools_config import _cua_driver_cmd - # Honor HERMES_CUA_DRIVER_CMD for local-build testing — same - # resolver `install_cua_driver` and the runtime backend use, - # so `status` reports what `computer_use` will actually invoke. - driver_cmd = _cua_driver_cmd() - path = shutil.which(driver_cmd) + from tools.computer_use.cua_backend import ( + cua_driver_update_check, + resolve_cua_driver_cmd, + ) + # Must match the runtime resolver: Desktop/TUI processes can omit + # ~/.local/bin even though the official installer put the driver there. + path = resolve_cua_driver_cmd() if path: version = "" try: @@ -14480,7 +14480,6 @@ def main(): else: print(f"cua-driver: installed at {path}") try: - from tools.computer_use.cua_backend import cua_driver_update_check st = cua_driver_update_check() if st and st.get("update_available"): latest = st.get("latest_version") or "?" diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 9c82d5a17257..f3abfb4d9a94 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -649,10 +649,17 @@ TOOLSET_ENV_REQUIREMENTS = { def _cua_driver_cmd() -> str: - """Return the cua-driver executable name/path, honoring non-empty overrides.""" + """Return the configured cua-driver override, or the bare default name.""" return os.environ.get("HERMES_CUA_DRIVER_CMD", "").strip() or "cua-driver" +def _resolved_cua_driver_cmd() -> Optional[str]: + """Resolve cua-driver exactly as the runtime and Desktop status do.""" + from tools.computer_use.cua_backend import resolve_cua_driver_cmd + + return resolve_cua_driver_cmd() + + def _cua_driver_env() -> dict: """cua-driver child env with the Hermes telemetry policy applied. @@ -824,7 +831,7 @@ def install_cua_driver(upgrade: bool = False) -> bool: fetch_tool = "powershell" if is_windows else "curl" driver_cmd = _cua_driver_cmd() - binary = shutil.which(driver_cmd) + binary = _resolved_cua_driver_cmd() # Not installed → fresh install path (only when caller asked for it). if not binary and not upgrade: @@ -849,7 +856,7 @@ def install_cua_driver(upgrade: bool = False) -> bool: if binary and not upgrade: try: version = subprocess.run( - [driver_cmd, "--version"], + [binary, "--version"], capture_output=True, text=True, timeout=5, env=_cua_driver_env(), creationflags=_post_setup_no_window_flags(), ).stdout.strip() @@ -907,7 +914,7 @@ def install_cua_driver(upgrade: bool = False) -> bool: # Show before/after version when we have a baseline. Best-effort. try: before = subprocess.run( - [driver_cmd, "--version"], + [binary, "--version"], capture_output=True, text=True, timeout=5, env=_cua_driver_env(), creationflags=_post_setup_no_window_flags(), ).stdout.strip() @@ -920,7 +927,7 @@ def install_cua_driver(upgrade: bool = False) -> bool: if ok and before: try: after = subprocess.run( - [driver_cmd, "--version"], + [binary, "--version"], capture_output=True, text=True, timeout=5, env=_cua_driver_env(), creationflags=_post_setup_no_window_flags(), ).stdout.strip() @@ -2672,7 +2679,7 @@ _POST_SETUP_INSTALLED: dict = { # entry when (a) the post_setup is the ONLY install side-effect for # a no-key provider, and (b) an installed-state check is cheap and # doesn't trigger a heavy import. - "cua_driver": lambda: bool(shutil.which(_cua_driver_cmd())), + "cua_driver": lambda: _resolved_cua_driver_cmd() is not None, } @@ -2730,7 +2737,7 @@ _POST_SETUP_READY: dict = { "agent_browser": lambda: _agent_browser_installed(), "browserbase": lambda: _cloud_agent_browser_installed(), "camofox": lambda: _camofox_installed(), - "cua_driver": lambda: bool(shutil.which(_cua_driver_cmd())), + "cua_driver": lambda: _resolved_cua_driver_cmd() is not None, } diff --git a/tests/computer_use/test_cua_cli_fallback_env.py b/tests/computer_use/test_cua_cli_fallback_env.py index 1f3180dfe4fc..5a4b04fdda32 100644 --- a/tests/computer_use/test_cua_cli_fallback_env.py +++ b/tests/computer_use/test_cua_cli_fallback_env.py @@ -31,8 +31,12 @@ def _fake_completed_process(stdout: str) -> MagicMock: def test_cli_fallback_strips_provider_secret_from_subprocess_env(monkeypatch): - monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-super-secret-should-not-leak") + monkeypatch.setenv("ANTHROPIC_API_KEY", "«redacted:sk-…»") monkeypatch.setenv("PATH", "/usr/bin:/bin") + monkeypatch.setattr( + "tools.computer_use.cua_backend.resolve_cua_driver_cmd", + lambda: "/resolved/cua-driver", + ) captured = {} @@ -56,6 +60,10 @@ def test_cli_fallback_applies_telemetry_policy(monkeypatch): """The env should also go through cua_driver_child_env(), like every other cua-driver spawn site, not just _sanitize_subprocess_env alone.""" monkeypatch.delenv("HERMES_CUA_TELEMETRY", raising=False) + monkeypatch.setattr( + "tools.computer_use.cua_backend.resolve_cua_driver_cmd", + lambda: "/resolved/cua-driver", + ) captured = {} def fake_run(cmd, **kwargs): diff --git a/tests/computer_use/test_doctor.py b/tests/computer_use/test_doctor.py index edd2b24b20db..a912176138e7 100644 --- a/tests/computer_use/test_doctor.py +++ b/tests/computer_use/test_doctor.py @@ -20,9 +20,12 @@ shape. from __future__ import annotations import json +import sys from io import StringIO from unittest.mock import MagicMock, patch +import pytest + # ── helpers ──────────────────────────────────────────────────────────────── @@ -323,3 +326,23 @@ class TestDriverCmdResolution: doctor.run_doctor() # First (and only) which call should have used the env var. which_mock.assert_called_with("/env/path/cua-driver") + + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX user-local path regression") + def test_user_local_driver_is_found_when_path_omits_it(self, tmp_path, monkeypatch): + """Doctor must inspect the same user-local driver as the runtime.""" + from tools.computer_use import doctor + + driver = tmp_path / ".local" / "bin" / "cua-driver" + driver.parent.mkdir(parents=True) + driver.write_text("#!/bin/sh\nexit 0\n") + driver.chmod(0o755) + + monkeypatch.delenv("HERMES_CUA_DRIVER_CMD", raising=False) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("PATH", "/usr/bin:/bin:/usr/sbin:/sbin") + + with patch("tools.computer_use.doctor._drive_health_report", return_value=_ok_report()) as health, \ + patch("sys.stdout", new_callable=StringIO): + assert doctor.run_doctor() == 0 + + health.assert_called_once_with(str(driver), include=(), skip=()) diff --git a/tests/computer_use/test_permissions_resolution.py b/tests/computer_use/test_permissions_resolution.py new file mode 100644 index 000000000000..c51fc9987f2c --- /dev/null +++ b/tests/computer_use/test_permissions_resolution.py @@ -0,0 +1,32 @@ +"""Regression tests for Computer Use readiness under a thin GUI PATH.""" + +from __future__ import annotations + +import sys +from unittest.mock import MagicMock, patch + +import pytest + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX user-local path regression") +def test_status_finds_user_local_driver_when_path_omits_it(tmp_path, monkeypatch): + """Desktop status must agree with the runtime resolver, not bare PATH.""" + from tools.computer_use import permissions + + driver = tmp_path / ".local" / "bin" / "cua-driver" + driver.parent.mkdir(parents=True) + driver.write_text("#!/bin/sh\nexit 0\n") + driver.chmod(0o755) + + monkeypatch.delenv("HERMES_CUA_DRIVER_CMD", raising=False) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("PATH", "/usr/bin:/bin:/usr/sbin:/sbin") + + with patch("tools.computer_use.permissions.sys.platform", "darwin"), \ + patch("tools.computer_use.cua_backend.sys.platform", "darwin"), \ + patch.object(permissions, "_run", return_value=MagicMock(stdout="0.0.0")), \ + patch.object(permissions, "_doctor", return_value={"ok": True, "checks": []}), \ + patch.object(permissions, "_mac_permissions"): + status = permissions.computer_use_status() + + assert status["installed"] is True diff --git a/tests/tools/test_computer_use.py b/tests/tools/test_computer_use.py index 530254b7dd7c..a715f14da658 100644 --- a/tests/tools/test_computer_use.py +++ b/tests/tools/test_computer_use.py @@ -1654,13 +1654,18 @@ class TestCuaDriverSessionReconnect: assert cli_calls == [("get_window_state", {"pid": 1, "window_id": 2})] assert result["images"] == ["B64PNG"] - def test_cli_fallback_reads_screenshot_from_file(self, tmp_path): + def test_cli_fallback_reads_screenshot_from_file(self, tmp_path, monkeypatch): """_call_tool_via_cli must base64-read a screenshot written to disk (screenshot_out_file path) when no inline base64 is present.""" import base64 as _b64 from typing import Any, cast from tools.computer_use.cua_backend import _CuaDriverSession + monkeypatch.setattr( + "tools.computer_use.cua_backend.resolve_cua_driver_cmd", + lambda: "/resolved/cua-driver", + ) + png_bytes = b"\x89PNG\r\n\x1a\nFAKEDATA" shot = tmp_path / "shot.png" shot.write_bytes(png_bytes) @@ -1701,6 +1706,10 @@ class TestCuaDriverSessionReconnect: from tools.computer_use.cua_backend import _CuaDriverSession session = cast(Any, _CuaDriverSession.__new__(_CuaDriverSession)) + monkeypatch.setattr( + "tools.computer_use.cua_backend.resolve_cua_driver_cmd", + lambda: "/resolved/cua-driver", + ) class FakeProc: stdout = '{"isError": true, "message": "bad target"}' @@ -2425,6 +2434,30 @@ class TestCuaEnvironmentScrubbing: "At least one safe environment variable should be preserved" +class TestCuaCliFallbackResolution: + def test_cli_fallback_uses_resolved_driver_under_thin_path(self): + """CLI transport must use the same resolved path as MCP startup. + + The CLI fallback runs after an MCP bridge error, precisely when a + Finder/Dock-launched Desktop process may have a PATH without + ``~/.local/bin``. Falling back to the bare ``cua-driver`` command + would reintroduce the original bug at runtime. + """ + from tools.computer_use.cua_backend import _AsyncBridge, _CuaDriverSession + + proc = MagicMock(stdout="{}", stderr="", returncode=0) + session = _CuaDriverSession(_AsyncBridge()) + with patch( + "tools.computer_use.cua_backend.resolve_cua_driver_cmd", + return_value="/Users/example/.local/bin/cua-driver", + ), patch("subprocess.run", return_value=proc) as run: + session._call_tool_via_cli("click", {"x": 1, "y": 2}, timeout=0.1) + + assert run.call_args.args[0][:3] == [ + "/Users/example/.local/bin/cua-driver", "call", "click" + ] + + class TestClickButtonPassthrough: """Surface 5 (NousResearch/hermes-agent#47072) — `middle_click` must actually reach cua-driver as a middle button, not silently degrade to @@ -2856,6 +2889,21 @@ class TestMcpInvocationResolution: assert cmd == "/opt/cua-driver" assert args == ["mcp"] + def test_manifest_bare_command_keeps_resolved_driver_path(self): + """A bare manifest command must not reintroduce the narrow-PATH bug.""" + from unittest.mock import patch + from tools.computer_use.cua_backend import _resolve_mcp_invocation + + manifest = ( + '{"mcp_invocation":' + '{"command":"cua-driver","args":["mcp-stdio","--strict"]}}' + ) + driver = "/Users/example/.local/bin/cua-driver" + with patch("subprocess.run", new=self._fake_run(stdout=manifest)): + cmd, args = _resolve_mcp_invocation(driver) + assert cmd == driver + assert args == ["mcp-stdio", "--strict"] + def test_future_renamed_subcommand_is_honored(self): """The whole point: a future cua-driver that exposes `mcp-stdio` instead of `mcp` keeps working without a Hermes patch.""" diff --git a/tools/computer_use/cua_backend.py b/tools/computer_use/cua_backend.py index 641929748a2b..6f75f15922ef 100644 --- a/tools/computer_use/cua_backend.py +++ b/tools/computer_use/cua_backend.py @@ -335,6 +335,11 @@ def _resolve_mcp_invocation( # The driver knows the subcommand but didn't surface its own path. # Keep our resolved driver_cmd; the args are still authoritative. return driver_cmd, args + if not _has_path_separator(command): + # A manifest may legitimately retain the generic ``cua-driver`` name. + # Under a GUI's thin PATH that would lose the resolved user-local path + # and fail at MCP spawn, so preserve the concrete command we verified. + return driver_cmd, args return command, args # Regex to parse element lines from get_window_state AX tree markdown. @@ -380,16 +385,20 @@ def _has_path_separator(value: str) -> bool: return os.sep in value or (os.altsep is not None and os.altsep in value) -def _candidate_cua_driver_commands() -> List[str]: +def _candidate_cua_driver_commands(override: Optional[str] = None) -> List[str]: """Return candidate cua-driver commands in resolution order. + ``override`` is authoritative when supplied. Otherwise a non-empty + ``HERMES_CUA_DRIVER_CMD`` is authoritative; only when neither is set do we + use PATH and canonical install locations. + Desktop apps launched from Finder/Dock often inherit a narrow PATH that omits user-local install directories. The upstream cua-driver installer commonly places the binary under ``~/.local/bin`` on POSIX systems, so a Hermes Desktop/TUI session can otherwise filter out the `computer_use` tool even though `hermes computer-use doctor` succeeds from a login shell. """ - configured = os.environ.get(_CUA_DRIVER_CMD_ENV) + configured = (override if override is not None else os.environ.get(_CUA_DRIVER_CMD_ENV, "")).strip() if configured: # An explicit override is authoritative: if it is wrong, report the # driver missing instead of silently picking a different binary. @@ -412,9 +421,14 @@ def _candidate_cua_driver_commands() -> List[str]: return candidates -def resolve_cua_driver_cmd() -> Optional[str]: - """Resolve the cua-driver executable for PATH-constrained surfaces.""" - for candidate in _candidate_cua_driver_commands(): +def resolve_cua_driver_cmd(override: Optional[str] = None) -> Optional[str]: + """Resolve the cua-driver executable for every runtime/status surface. + + A supplied override (or ``HERMES_CUA_DRIVER_CMD``) is never silently + replaced by another binary. Otherwise resolve PATH first, then canonical + user-local installation locations used by the official installer. + """ + for candidate in _candidate_cua_driver_commands(override): expanded = os.path.expanduser(candidate) if _has_path_separator(expanded): if shutil.which(expanded): @@ -1118,7 +1132,10 @@ class _CuaDriverSession: os.close(fd) call_args["screenshot_out_file"] = shot_file - cmd = [_CUA_DRIVER_CMD, "call", name, json.dumps(call_args)] + driver_cmd = resolve_cua_driver_cmd() + if not driver_cmd: + raise RuntimeError(cua_driver_install_hint()) + cmd = [driver_cmd, "call", name, json.dumps(call_args)] attempts = 4 backoff = 0.5 parsed: Any = None diff --git a/tools/computer_use/doctor.py b/tools/computer_use/doctor.py index 38c1b43e91d8..54fad0666844 100644 --- a/tools/computer_use/doctor.py +++ b/tools/computer_use/doctor.py @@ -17,7 +17,6 @@ from __future__ import annotations import json import os -import shutil import subprocess import sys from typing import Any, Dict, List, Optional, Sequence @@ -240,9 +239,8 @@ def run_doctor( ) -> int: """Resolve the cua-driver binary, call `health_report`, render the result. - Honors `HERMES_CUA_DRIVER_CMD` via the same `_cua_driver_cmd()` resolver - that `install_cua_driver` + the runtime backend use, so the doctor - diagnoses what your `computer_use` toolset will actually invoke. + Honors `HERMES_CUA_DRIVER_CMD` via the shared runtime resolver, so the + doctor diagnoses what your `computer_use` toolset will actually invoke. """ # Windows ships stdout/stderr wrapped with the system ANSI codec # (`cp1252` on a US locale, `cp936` on zh-CN, etc.). The check-matrix @@ -255,16 +253,12 @@ def run_doctor( stream.reconfigure(encoding="utf-8", errors="replace") # type: ignore[union-attr] except (AttributeError, OSError): pass - if driver_cmd is None: - try: - from hermes_cli.tools_config import _cua_driver_cmd - driver_cmd = _cua_driver_cmd() - except Exception: - driver_cmd = os.environ.get("HERMES_CUA_DRIVER_CMD") or "cua-driver" + from tools.computer_use.cua_backend import resolve_cua_driver_cmd - binary = shutil.which(driver_cmd) + binary = resolve_cua_driver_cmd(driver_cmd) if not binary: - print(f"cua-driver: not installed (looked for {driver_cmd!r}).") + looked_for = driver_cmd or "cua-driver (PATH and canonical install paths)" + print(f"cua-driver: not installed (looked for {looked_for!r}).") print(" Run: hermes computer-use install") return 2 diff --git a/tools/computer_use/permissions.py b/tools/computer_use/permissions.py index d6fd0f3f91fd..4cccf2d683d5 100644 --- a/tools/computer_use/permissions.py +++ b/tools/computer_use/permissions.py @@ -25,7 +25,6 @@ from __future__ import annotations import json import os -import shutil import subprocess import sys from typing import Any, Dict, List, Optional @@ -35,15 +34,11 @@ _RUNTIME_PLATFORMS = frozenset({"darwin", "win32", "linux"}) _BOOLS = ("accessibility", "screen_recording", "screen_recording_capturable") -def _driver_cmd(override: Optional[str]) -> str: - if override: - return override - try: - from hermes_cli.tools_config import _cua_driver_cmd +def _resolve_driver_cmd(override: Optional[str]) -> Optional[str]: + """Use the runtime resolver for UI status and permission commands too.""" + from tools.computer_use.cua_backend import resolve_cua_driver_cmd - return _cua_driver_cmd() - except Exception: - return os.environ.get("HERMES_CUA_DRIVER_CMD", "").strip() or "cua-driver" + return resolve_cua_driver_cmd(override) def _child_env() -> Dict[str, str]: @@ -128,7 +123,7 @@ def computer_use_status(driver_cmd: Optional[str] = None) -> Dict[str, Any]: unknown (binary missing / probe failed). ``can_grant`` is macOS-only. """ plat = sys.platform - binary = shutil.which(_driver_cmd(driver_cmd)) + binary = _resolve_driver_cmd(driver_cmd) out: Dict[str, Any] = { "platform": plat, "platform_supported": plat in _RUNTIME_PLATFORMS, @@ -175,7 +170,7 @@ def request_permissions_grant(driver_cmd: Optional[str] = None) -> int: print("Computer Use permissions are a macOS concept; nothing to grant here.") return 64 - binary = shutil.which(_driver_cmd(driver_cmd)) + binary = _resolve_driver_cmd(driver_cmd) if not binary: print("cua-driver: not installed. Run: hermes computer-use install") return 2