mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix: resolve cua-driver across computer-use surfaces
This commit is contained in:
parent
030822b68d
commit
2f9d88caee
9 changed files with 169 additions and 46 deletions
|
|
@ -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 "?"
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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=())
|
||||
|
|
|
|||
32
tests/computer_use/test_permissions_resolution.py
Normal file
32
tests/computer_use/test_permissions_resolution.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue