mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(computer_use): refine auto-detect to headless/WSL2 only, add driver version probe
Address review feedback from cross-vendor review (Flash + GPT-OSS): 1. Auto-detect now checks for headless Linux (no DISPLAY), WSL2 (/proc/version contains 'microsoft'), instead of all Linux. Desktop Linux with a compositor keeps the overlay. 2. Add _cua_driver_supports_no_overlay() that probes cua-driver --help to check if the flag is supported. Older drivers (< 0.6.x) reject unknown flags, so passing --no-overlay would crash the MCP spawn. 3. Update tests to cover headless vs desktop Linux, WSL2 detection, version probe, and the unsupported-driver fallback path.
This commit is contained in:
parent
f43ff5b4bb
commit
8d4f7a0002
2 changed files with 122 additions and 38 deletions
|
|
@ -2,24 +2,43 @@
|
|||
|
||||
cua-driver's cursor overlay rendering loop can consume CPU indefinitely when
|
||||
idle (#28152, #47032). Hermes passes ``--no-overlay`` to suppress it when the
|
||||
``computer_use.no_overlay`` config is enabled (or auto-detected on Linux).
|
||||
``computer_use.no_overlay`` config is enabled (or auto-detected on headless
|
||||
Linux / WSL2).
|
||||
|
||||
These assert the behavior contract (auto-detect on Linux, explicit override,
|
||||
config failure fails safe toward overlay enabled), not specific config
|
||||
snapshots.
|
||||
These assert the behavior contract (auto-detect on headless/WSL2, explicit
|
||||
override, version probe), not specific config snapshots.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import mock_open, patch
|
||||
|
||||
from tools.computer_use import cua_backend
|
||||
|
||||
|
||||
class TestNoOverlayFlag:
|
||||
def test_default_linux_disables(self):
|
||||
"""Auto-detect: Linux => overlay disabled."""
|
||||
def test_default_linux_headless_disables(self):
|
||||
"""Auto-detect: Linux without DISPLAY => overlay disabled."""
|
||||
with patch("hermes_cli.config.load_config", return_value={}), \
|
||||
patch.object(sys, "platform", "linux"):
|
||||
patch.object(sys, "platform", "linux"), \
|
||||
patch.dict(os.environ, {}, clear=False):
|
||||
os.environ.pop("DISPLAY", None)
|
||||
assert cua_backend._cua_no_overlay() is True
|
||||
|
||||
def test_default_linux_desktop_enables(self):
|
||||
"""Auto-detect: Linux with DISPLAY => overlay enabled."""
|
||||
with patch("hermes_cli.config.load_config", return_value={}), \
|
||||
patch.object(sys, "platform", "linux"), \
|
||||
patch.dict(os.environ, {"DISPLAY": ":0"}):
|
||||
assert cua_backend._cua_no_overlay() is False
|
||||
|
||||
def test_default_linux_wsl2_disables(self):
|
||||
"""Auto-detect: WSL2 (microsoft in /proc/version) => overlay disabled."""
|
||||
fake_version = "Linux version 5.15.0 (Microsoft@Microsoft.com)"
|
||||
with patch("hermes_cli.config.load_config", return_value={}), \
|
||||
patch.object(sys, "platform", "linux"), \
|
||||
patch.dict(os.environ, {"DISPLAY": ":0"}), \
|
||||
patch("builtins.open", mock_open(read_data=fake_version)):
|
||||
assert cua_backend._cua_no_overlay() is True
|
||||
|
||||
def test_default_macos_enables(self):
|
||||
|
|
@ -42,39 +61,72 @@ class TestNoOverlayFlag:
|
|||
def test_explicit_false_overrides(self):
|
||||
with patch("hermes_cli.config.load_config",
|
||||
return_value={"computer_use": {"no_overlay": False}}), \
|
||||
patch.object(sys, "platform", "linux"):
|
||||
# Explicit False overrides auto-detect on Linux.
|
||||
patch.object(sys, "platform", "linux"), \
|
||||
patch.dict(os.environ, {}, clear=False):
|
||||
os.environ.pop("DISPLAY", None)
|
||||
# Explicit False overrides auto-detect on headless Linux.
|
||||
assert cua_backend._cua_no_overlay() is False
|
||||
|
||||
def test_config_load_failure_fails_safe(self):
|
||||
"""Unreadable config => auto-detect (platform-dependent)."""
|
||||
def test_config_load_failure_falls_through_to_auto_detect(self):
|
||||
"""Unreadable config => auto-detect."""
|
||||
with patch("hermes_cli.config.load_config",
|
||||
side_effect=RuntimeError("boom")):
|
||||
expected = sys.platform == "linux"
|
||||
assert cua_backend._cua_no_overlay() is expected
|
||||
side_effect=RuntimeError("boom")), \
|
||||
patch.object(sys, "platform", "darwin"):
|
||||
assert cua_backend._cua_no_overlay() is False
|
||||
|
||||
def test_missing_section_enables(self):
|
||||
def test_missing_section_falls_through_to_auto_detect(self):
|
||||
with patch("hermes_cli.config.load_config",
|
||||
return_value={"other": {}}):
|
||||
expected = sys.platform == "linux"
|
||||
assert cua_backend._cua_no_overlay() is expected
|
||||
return_value={"other": {}}), \
|
||||
patch.object(sys, "platform", "linux"), \
|
||||
patch.dict(os.environ, {"DISPLAY": ":0"}):
|
||||
assert cua_backend._cua_no_overlay() is False
|
||||
|
||||
|
||||
class TestDriverSupportsNoOverlay:
|
||||
def test_returns_true_when_help_shows_flag(self):
|
||||
fake_help = "Usage: cua-driver [OPTIONS] COMMAND\n --no-overlay Disable cursor overlay\n"
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value.stdout = fake_help
|
||||
mock_run.return_value.stderr = ""
|
||||
assert cua_backend._cua_driver_supports_no_overlay("cua-driver") is True
|
||||
|
||||
def test_returns_false_when_help_lacks_flag(self):
|
||||
fake_help = "Usage: cua-driver [OPTIONS] COMMAND\n"
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value.stdout = fake_help
|
||||
mock_run.return_value.stderr = ""
|
||||
cua_backend._cua_driver_supports_no_overlay.cache_clear()
|
||||
assert cua_backend._cua_driver_supports_no_overlay("cua-driver") is False
|
||||
|
||||
def test_returns_false_on_subprocess_error(self):
|
||||
with patch("subprocess.run", side_effect=FileNotFoundError("no such file")):
|
||||
cua_backend._cua_driver_supports_no_overlay.cache_clear()
|
||||
assert cua_backend._cua_driver_supports_no_overlay("cua-driver") is False
|
||||
|
||||
|
||||
class TestMcpArgsOverlayFlag:
|
||||
def test_no_overlay_appended_when_enabled(self):
|
||||
with patch.object(cua_backend, "_cua_no_overlay", return_value=True):
|
||||
def test_appended_when_enabled_and_supported(self):
|
||||
with patch.object(cua_backend, "_cua_no_overlay", return_value=True), \
|
||||
patch.object(cua_backend, "_cua_driver_supports_no_overlay", return_value=True):
|
||||
result = cua_backend._mcp_args_with_overlay_flag(["mcp"])
|
||||
assert result == ["mcp", "--no-overlay"]
|
||||
|
||||
def test_no_overlay_not_appended_when_disabled(self):
|
||||
with patch.object(cua_backend, "_cua_no_overlay", return_value=False):
|
||||
def test_not_appended_when_disabled(self):
|
||||
with patch.object(cua_backend, "_cua_no_overlay", return_value=False), \
|
||||
patch.object(cua_backend, "_cua_driver_supports_no_overlay", return_value=True):
|
||||
result = cua_backend._mcp_args_with_overlay_flag(["mcp"])
|
||||
assert result == ["mcp"]
|
||||
|
||||
def test_not_appended_when_driver_unsupported(self):
|
||||
with patch.object(cua_backend, "_cua_no_overlay", return_value=True), \
|
||||
patch.object(cua_backend, "_cua_driver_supports_no_overlay", return_value=False):
|
||||
result = cua_backend._mcp_args_with_overlay_flag(["mcp"])
|
||||
assert result == ["mcp"]
|
||||
|
||||
def test_does_not_mutate_original_list(self):
|
||||
"""The original args list must not be mutated."""
|
||||
original = ["mcp"]
|
||||
with patch.object(cua_backend, "_cua_no_overlay", return_value=True):
|
||||
with patch.object(cua_backend, "_cua_no_overlay", return_value=True), \
|
||||
patch.object(cua_backend, "_cua_driver_supports_no_overlay", return_value=True):
|
||||
result = cua_backend._mcp_args_with_overlay_flag(original)
|
||||
assert "--no-overlay" in result
|
||||
assert "--no-overlay" not in original
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ from __future__ import annotations
|
|||
import asyncio
|
||||
import base64
|
||||
import concurrent.futures
|
||||
import functools
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
|
|
@ -175,12 +176,10 @@ def _cua_no_overlay() -> bool:
|
|||
"""True when Hermes should pass ``--no-overlay`` to cua-driver.
|
||||
|
||||
Reads ``computer_use.no_overlay`` from config.yaml. Default is
|
||||
``None`` (auto-detect): disable the overlay on Linux / headless / WSL2
|
||||
where it serves no visual purpose and can consume CPU indefinitely
|
||||
(#28152, #47032). Explicit ``True`` / ``False`` in config overrides
|
||||
auto-detection. Fails SAFE — toward enabling the overlay (False)
|
||||
when config is unreadable, because the flag only takes effect on
|
||||
cua-driver ≥ 0.6.x and an older driver ignores unknown flags.
|
||||
``None`` (auto-detect): disable the overlay on headless Linux / WSL2 /
|
||||
containers where it serves no visual purpose and can consume CPU
|
||||
indefinitely (#28152, #47032). Explicit ``True`` / ``False`` in config
|
||||
overrides auto-detection.
|
||||
"""
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
|
|
@ -192,10 +191,20 @@ def _cua_no_overlay() -> bool:
|
|||
return bool(val)
|
||||
except Exception:
|
||||
pass
|
||||
# Auto-detect: disable on Linux (covers WSL2, headless, containers).
|
||||
# macOS and Windows get the overlay by default — it's visually useful
|
||||
# there, even though macOS has the vImage redraw bug on some versions.
|
||||
return sys.platform == "linux"
|
||||
# Auto-detect: disable on headless Linux (no DISPLAY), WSL2, or
|
||||
# containers. Desktop Linux with a running compositor keeps the
|
||||
# overlay — it's visually useful there.
|
||||
if sys.platform != "linux":
|
||||
return False
|
||||
if not os.environ.get("DISPLAY"):
|
||||
return True
|
||||
try:
|
||||
with open("/proc/version") as f:
|
||||
if "microsoft" in f.read().lower():
|
||||
return True
|
||||
except Exception:
|
||||
pass
|
||||
return False
|
||||
|
||||
|
||||
def _cua_telemetry_disabled() -> bool:
|
||||
|
|
@ -371,12 +380,35 @@ def _resolve_mcp_invocation(
|
|||
return command, _mcp_args_with_overlay_flag(args)
|
||||
|
||||
|
||||
def _mcp_args_with_overlay_flag(args: List[str]) -> List[str]:
|
||||
"""Return *args* with ``--no-overlay`` appended when the config says to."""
|
||||
if _cua_no_overlay():
|
||||
def _mcp_args_with_overlay_flag(
|
||||
args: List[str],
|
||||
driver_cmd: str = _CUA_DRIVER_CMD,
|
||||
) -> List[str]:
|
||||
"""Return *args* with ``--no-overlay`` appended when configured and supported."""
|
||||
if _cua_no_overlay() and _cua_driver_supports_no_overlay(driver_cmd):
|
||||
return [*args, "--no-overlay"]
|
||||
return list(args)
|
||||
|
||||
|
||||
@functools.lru_cache(maxsize=1)
|
||||
def _cua_driver_supports_no_overlay(driver_cmd: str) -> bool:
|
||||
"""True if the installed cua-driver recognises ``--no-overlay``.
|
||||
|
||||
Probes ``<driver> --help`` once and caches the result. Older
|
||||
drivers (< 0.6.x) reject unknown flags, so passing ``--no-overlay``
|
||||
would crash the MCP spawn.
|
||||
"""
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
[driver_cmd, "--help"],
|
||||
capture_output=True, text=True, timeout=3.0,
|
||||
stdin=subprocess.DEVNULL,
|
||||
)
|
||||
help_text = (proc.stdout or "") + (proc.stderr or "")
|
||||
return "--no-overlay" in help_text
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
# Regex to parse element lines from get_window_state AX tree markdown.
|
||||
#
|
||||
# cua-driver renders each actionable node as one of:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue