From 920facdc4c4ffab9242203d601e43605b67066bc Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:06:11 -0700 Subject: [PATCH] fix(update): never blind-reinstall cua-driver during hermes update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit '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. --- hermes_cli/main.py | 9 +- hermes_cli/tools_config.py | 48 +++++-- tests/hermes_cli/test_install_cua_driver.py | 135 ++++++++++++++++++++ tools/computer_use/cua_backend.py | 10 +- 4 files changed, 190 insertions(+), 12 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index a5d175102efc..75a64c256965 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -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) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index ace4dc3a7270..c07379710624 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -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. diff --git a/tests/hermes_cli/test_install_cua_driver.py b/tests/hermes_cli/test_install_cua_driver.py index bf69f7fae89d..59fceb1b91a6 100644 --- a/tests/hermes_cli/test_install_cua_driver.py +++ b/tests/hermes_cli/test_install_cua_driver.py @@ -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`. diff --git a/tools/computer_use/cua_backend.py b/tools/computer_use/cua_backend.py index 98b98ec0be6f..79912814e468 100644 --- a/tools/computer_use/cua_backend.py +++ b/tools/computer_use/cua_backend.py @@ -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