mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(update): never blind-reinstall cua-driver during hermes update
'Refreshing cua-driver (Computer Use)...' could hang for minutes on Windows: when the driver's native check-update verb returned an indeterminate result (old driver without the verb, offline, GitHub rate-limited, or the probe timing out), install_cua_driver(upgrade=True) fell through to the full upstream installer — a silent, output-captured run with a 660s ceiling, plus install.ps1's 600s concurrency-lock wait on Windows on top. Every 'hermes update' paid that cost. Two changes: - install_cua_driver() grows require_confirmed_update: with it set, an indeterminate check keeps the installed version and returns fast, printing the force path (hermes computer-use install --upgrade). 'hermes update' passes it; the explicit --upgrade CLI keeps the old fall-through so a force refresh still works when the check can't answer. - cua_driver_update_check() default timeout is now 25s on Windows (8s unchanged on POSIX): first-spawn of the exe under Defender / SmartScreen routinely exceeds 8s, and a false timeout is exactly the indeterminate result that used to trigger the multi-minute reinstall.
This commit is contained in:
parent
3dc2decd43
commit
920facdc4c
4 changed files with 190 additions and 12 deletions
|
|
@ -12533,7 +12533,14 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
|
||||
print()
|
||||
print("→ Refreshing cua-driver (Computer Use)...")
|
||||
install_cua_driver(upgrade=True)
|
||||
# require_confirmed_update: only run the (multi-minute,
|
||||
# silent) upstream installer when the driver's native
|
||||
# check-update verb positively reports a newer release.
|
||||
# An indeterminate check (offline, rate-limited, old
|
||||
# driver) keeps the installed version — `hermes update`
|
||||
# must stay fast; `hermes computer-use install --upgrade`
|
||||
# remains the force path.
|
||||
install_cua_driver(upgrade=True, require_confirmed_update=True)
|
||||
except Exception as e:
|
||||
logger.debug("cua-driver refresh failed: %s", e)
|
||||
|
||||
|
|
|
|||
|
|
@ -795,7 +795,7 @@ def _cua_install_target_writable() -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def install_cua_driver(upgrade: bool = False) -> bool:
|
||||
def install_cua_driver(upgrade: bool = False, require_confirmed_update: bool = False) -> bool:
|
||||
"""Install or refresh the cua-driver binary used by Computer Use.
|
||||
|
||||
The upstream installer always pulls the latest release tag, so re-running
|
||||
|
|
@ -808,6 +808,19 @@ def install_cua_driver(upgrade: bool = False) -> bool:
|
|||
update`` if the binary supports it). Used by ``hermes update`` and
|
||||
by ``hermes computer-use install --upgrade``.
|
||||
|
||||
``require_confirmed_update`` (only meaningful with ``upgrade=True`` and
|
||||
an installed binary): when the driver's native ``check-update`` verb
|
||||
can't positively confirm that a newer release exists — the driver is
|
||||
too old for the verb, the GitHub check failed, we're offline, or the
|
||||
probe timed out — keep the installed version and return instead of
|
||||
falling through to the full upstream installer. ``hermes update`` sets
|
||||
this so a broken update check costs seconds, not a multi-minute silent
|
||||
reinstall on every update (the upstream installer runs up to
|
||||
``_CUA_INSTALLER_TIMEOUT`` and install.ps1's concurrency lock can add
|
||||
a further ~600s wait on Windows). ``hermes computer-use install
|
||||
--upgrade`` leaves it False — an explicit upgrade request should still
|
||||
reinstall when the check is indeterminate.
|
||||
|
||||
Returns True iff cua-driver is installed (or successfully refreshed)
|
||||
when the function returns. Supported on macOS, Windows, and Linux
|
||||
(Linux is alpha). Silently returns False on unsupported platforms.
|
||||
|
|
@ -897,20 +910,35 @@ def install_cua_driver(upgrade: bool = False) -> bool:
|
|||
|
||||
# Skip the (network) re-install when the driver itself reports it's already
|
||||
# on the latest release. Best-effort: an older driver (no check-update
|
||||
# verb) or an offline check returns None, in which case we fall through and
|
||||
# re-run the installer as before.
|
||||
# verb) or an offline check returns None. What happens then depends on the
|
||||
# caller: `hermes update` (require_confirmed_update=True) keeps the
|
||||
# installed version — an indeterminate check must never cost the user a
|
||||
# multi-minute silent reinstall on every update. An explicit
|
||||
# `hermes computer-use install --upgrade` falls through and re-runs the
|
||||
# installer as before.
|
||||
if binary:
|
||||
_state = None
|
||||
try:
|
||||
from tools.computer_use.cua_backend import cua_driver_update_check
|
||||
_state = cua_driver_update_check()
|
||||
if _state is not None and not _state.get("update_available"):
|
||||
_print_success(
|
||||
f" {driver_cmd} is already on the latest release "
|
||||
f"({_state.get('current_version') or 'unknown'})."
|
||||
)
|
||||
return True
|
||||
except Exception:
|
||||
pass
|
||||
_state = None
|
||||
if _state is not None and not _state.get("update_available"):
|
||||
_print_success(
|
||||
f" {driver_cmd} is already on the latest release "
|
||||
f"({_state.get('current_version') or 'unknown'})."
|
||||
)
|
||||
return True
|
||||
if _state is None and require_confirmed_update:
|
||||
_print_info(
|
||||
f" Could not confirm a newer {driver_cmd} release "
|
||||
"(offline, rate-limited, or driver too old to check); "
|
||||
"keeping the installed version."
|
||||
)
|
||||
_print_info(
|
||||
" Force a refresh with: hermes computer-use install --upgrade"
|
||||
)
|
||||
return True
|
||||
|
||||
if binary:
|
||||
# Show before/after version when we have a baseline. Best-effort.
|
||||
|
|
|
|||
|
|
@ -126,6 +126,141 @@ class TestInstallCuaDriverUpgrade:
|
|||
runner.assert_called_once()
|
||||
|
||||
|
||||
class TestRequireConfirmedUpdate:
|
||||
"""`hermes update` passes require_confirmed_update=True: the full
|
||||
upstream installer (multi-minute, output captured, plus install.ps1's
|
||||
600s lock window on Windows) may only run when the driver's native
|
||||
``check-update`` verb positively confirms a newer release. An
|
||||
indeterminate check (old driver, offline, GitHub rate-limited, probe
|
||||
timeout) keeps the installed version and returns fast.
|
||||
|
||||
Explicit `hermes computer-use install --upgrade` keeps the old
|
||||
fall-through (require_confirmed_update=False): a force-refresh should
|
||||
still reinstall when the check can't answer.
|
||||
"""
|
||||
|
||||
def _install(self, system, check_state, require_confirmed):
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from hermes_cli import tools_config
|
||||
|
||||
exe = "cua-driver" + (".exe" if system == "Windows" else "")
|
||||
with patch("platform.system", return_value=system), \
|
||||
patch.object(tools_config.shutil, "which",
|
||||
side_effect=lambda n: "/x/" + n
|
||||
if n in {"cua-driver", "curl", "powershell"} else None), \
|
||||
patch.object(tools_config, "_resolved_cua_driver_cmd",
|
||||
return_value="/x/" + exe), \
|
||||
patch.object(tools_config, "_cua_install_target_writable",
|
||||
return_value=True), \
|
||||
patch("tools.computer_use.cua_backend.cua_driver_update_check",
|
||||
return_value=check_state), \
|
||||
patch.object(tools_config, "_run_cua_driver_installer",
|
||||
return_value=True) as runner, \
|
||||
patch("subprocess.run",
|
||||
return_value=MagicMock(stdout="cua-driver 0.5.0", returncode=0)), \
|
||||
patch.object(tools_config, "_print_success"), \
|
||||
patch.object(tools_config, "_print_warning"), \
|
||||
patch.object(tools_config, "_print_info") as info:
|
||||
ok = tools_config.install_cua_driver(
|
||||
upgrade=True, require_confirmed_update=require_confirmed
|
||||
)
|
||||
return ok, runner, info
|
||||
|
||||
def test_indeterminate_check_keeps_installed_version(self):
|
||||
ok, runner, info = self._install("Windows", None, require_confirmed=True)
|
||||
assert ok is True
|
||||
runner.assert_not_called()
|
||||
assert any(
|
||||
"keeping the installed version" in call.args[0]
|
||||
for call in info.call_args_list
|
||||
)
|
||||
|
||||
def test_indeterminate_check_points_at_force_path(self):
|
||||
ok, runner, info = self._install("Darwin", None, require_confirmed=True)
|
||||
assert ok is True
|
||||
runner.assert_not_called()
|
||||
assert any(
|
||||
"computer-use install --upgrade" in call.args[0]
|
||||
for call in info.call_args_list
|
||||
)
|
||||
|
||||
def test_confirmed_update_still_runs_installer(self):
|
||||
state = {"current_version": "0.5.0", "latest_version": "0.6.0",
|
||||
"update_available": True}
|
||||
ok, runner, _ = self._install("Windows", state, require_confirmed=True)
|
||||
assert ok is True
|
||||
runner.assert_called_once()
|
||||
|
||||
def test_up_to_date_short_circuits(self):
|
||||
state = {"current_version": "0.6.0", "latest_version": "0.6.0",
|
||||
"update_available": False}
|
||||
ok, runner, _ = self._install("Windows", state, require_confirmed=True)
|
||||
assert ok is True
|
||||
runner.assert_not_called()
|
||||
|
||||
def test_explicit_upgrade_still_falls_through_on_indeterminate(self):
|
||||
# `hermes computer-use install --upgrade` (default flag): the old
|
||||
# behaviour — indeterminate check re-runs the installer.
|
||||
ok, runner, _ = self._install("Darwin", None, require_confirmed=False)
|
||||
assert ok is True
|
||||
runner.assert_called_once()
|
||||
|
||||
|
||||
class TestUpdateCheckTimeoutDefaults:
|
||||
"""cua_driver_update_check: platform-sensitive default timeout.
|
||||
|
||||
8s is fine on POSIX but too tight for Windows first-spawn (Defender /
|
||||
SmartScreen scanning), and a false timeout is what used to trigger the
|
||||
full reinstall fall-through during `hermes update`.
|
||||
"""
|
||||
|
||||
def _captured_timeout(self, platform_name):
|
||||
from unittest.mock import MagicMock
|
||||
from tools.computer_use import cua_backend
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_run(cmd, **kw):
|
||||
captured["timeout"] = kw.get("timeout")
|
||||
m = MagicMock()
|
||||
m.stdout = '{"update_available": false, "current_version": "1.0"}'
|
||||
return m
|
||||
|
||||
with patch("tools.computer_use.cua_backend.resolve_cua_driver_cmd",
|
||||
return_value="/x/cua-driver"), \
|
||||
patch("tools.computer_use.cua_backend.sys.platform", platform_name), \
|
||||
patch("tools.computer_use.cua_backend.subprocess.run",
|
||||
side_effect=fake_run):
|
||||
cua_backend.cua_driver_update_check()
|
||||
return captured.get("timeout")
|
||||
|
||||
def test_windows_default_is_generous(self):
|
||||
assert self._captured_timeout("win32") == 25.0
|
||||
|
||||
def test_posix_default_unchanged(self):
|
||||
assert self._captured_timeout("linux") == 8.0
|
||||
|
||||
def test_explicit_timeout_wins(self):
|
||||
from unittest.mock import MagicMock
|
||||
from tools.computer_use import cua_backend
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_run(cmd, **kw):
|
||||
captured["timeout"] = kw.get("timeout")
|
||||
m = MagicMock()
|
||||
m.stdout = "{}"
|
||||
return m
|
||||
|
||||
with patch("tools.computer_use.cua_backend.resolve_cua_driver_cmd",
|
||||
return_value="/x/cua-driver"), \
|
||||
patch("tools.computer_use.cua_backend.subprocess.run",
|
||||
side_effect=fake_run):
|
||||
cua_backend.cua_driver_update_check(timeout=3.0)
|
||||
assert captured.get("timeout") == 3.0
|
||||
|
||||
|
||||
class TestArchProbeRemoval:
|
||||
"""Regression tests for the deletion of `_check_cua_driver_asset_for_arch`.
|
||||
|
||||
|
|
|
|||
|
|
@ -557,18 +557,26 @@ def cua_driver_binary_available() -> bool:
|
|||
return resolve_cua_driver_cmd() is not None
|
||||
|
||||
|
||||
def cua_driver_update_check(*, timeout: float = 8.0) -> Optional[Dict[str, Any]]:
|
||||
def cua_driver_update_check(*, timeout: Optional[float] = None) -> Optional[Dict[str, Any]]:
|
||||
"""Run ``cua-driver check-update --json`` and return its parsed state.
|
||||
|
||||
The payload mirrors the ``check_for_update`` MCP tool:
|
||||
``{current_version, latest_version, update_available, ...}``.
|
||||
|
||||
``timeout`` defaults to 8s on POSIX and 25s on Windows — first-spawn of
|
||||
the exe there routinely eats several seconds in Defender/SmartScreen
|
||||
scanning, and a false timeout is expensive: callers treat ``None`` as
|
||||
indeterminate, and the ``install_cua_driver(upgrade=True)`` path used to
|
||||
fall through to a full multi-minute reinstall on it.
|
||||
|
||||
Returns ``None`` (callers should stay quiet) when the result is
|
||||
indeterminate: the binary is missing, the driver is too old to support
|
||||
the verb (it predates trycua/cua#1734), the GitHub check failed (an
|
||||
``error`` field is set), or the output didn't parse. Best-effort; never
|
||||
raises.
|
||||
"""
|
||||
if timeout is None:
|
||||
timeout = 25.0 if sys.platform == "win32" else 8.0
|
||||
driver_cmd = resolve_cua_driver_cmd()
|
||||
if not driver_cmd:
|
||||
return None
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue