fix(computer_use): pass --no-overlay to cua-driver on Linux/WSL2 to prevent idle CPU - #53841
fix(computer_use): pass --no-overlay to cua-driver on Linux/WSL2 to prevent idle CPU#53841DavidMetcalfe wants to merge 5 commits into
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
cua-driver --no-overlay flag on Linux/WSL2 (4 files). Platform-specific CPU fix:
- New
computer_use.no_overlayconfig option (None=True/False/auto-detect) - Auto-detect: disables overlay on Linux without DISPLAY (headless/WSL2)
- Config version bumped from 31 to 32
- Comprehensive test suite: 132 lines covering auto-detect, explicit override, version probe
- Tests use proper mocking of platform, env, and config
Good fix for idle CPU consumption. The auto-detect approach is sensible.
|
@DavidMetcalfe are you able to merge this now it's been approved by @tonydwb please |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the documented idle CPU issue. The current origin/main MCP path still launches the resolved driver without an overlay policy (tools/computer_use/cua_backend.py:144-195, :617-624), so the underlying fix remains needed.
Problems
tools/computer_use/cua_backend.py:260adds acua-driver --helpsubprocess withoutenv=. Current main sanitizes every third-party cua-driver spawn, including manifest discovery (:163-171) and update checking (:253-264); this new probe must use the same sanitized environment.tools/computer_use/cua_backend.py:238returns the manifest'scommand, but_mcp_args_with_overlay_flag()defaults its probe to_CUA_DRIVER_CMD. When manifest discovery relocates the executable, the support check can inspect a different binary from the one launched.hermes_cli/config.py:3178bumps_config_versionfor an additive default.AGENTS.md:586-590specifies that deep merge handles new keys in existing sections without a version bump.
Suggested changes
- Sanitize the help-probe environment, pass the effective resolved command into the support check, and cover a manifest command differing from
driver_cmd. - Remove the schema-version bump and document
computer_use.no_overlaybeside the existing telemetry configuration.
Automated hermes-sweeper review.
| return driver_cmd, args | ||
| return command, args | ||
| return driver_cmd, _mcp_args_with_overlay_flag(args) | ||
| return command, _mcp_args_with_overlay_flag(args) |
There was a problem hiding this comment.
This manifest branch launches command, but the helper defaults to _CUA_DRIVER_CMD; a relocated manifest executable can therefore be probed for --no-overlay on the wrong binary. Pass command here (and driver_cmd on fallback paths) and add a differing-command regression case.
There was a problem hiding this comment.
Good catch — addressed in the latest push. Both findings fixed in commit 176e0876f on the same PR.
1. --help subprocess env sanitization (cua_backend.py:260): Added env=_sanitize_subprocess_env(cua_driver_child_env()) with the same lazy import + rationale comment as the manifest probe at :214. The --help subprocess now matches the policy of every other cua-driver spawn site (manifest probe, MCP spawn, install probe).
2. Manifest command not in support check (cua_backend.py:238): The command returned by the manifest is now passed into _mcp_args_with_overlay_flag via driver_cmd=command. The fallback path (no command in manifest) uses driver_cmd=driver_cmd to keep the existing input-driver semantics. The lru_cache on _cua_driver_supports_no_overlay is keyed on driver_cmd, so each binary gets its own cached verdict — no cross-binary cache leakage.
3. _config_version bump: Resolved by cherry-picking the PR onto current origin/main (already at 33). The original commit's bump to 32 was dropped during the conflict resolution — hermes_cli/config.py:3394 correctly stays at 33 with no bump for the additive no_overlay default.
Four new tests in tests/computer_use/test_cua_no_overlay.py:
test_help_probe_passes_sanitized_env— verifiessubprocess.runis called withenv=.test_manifest_command_drives_support_probe— verifies the probe uses the manifest command when it differs from the input driver_cmd.test_fallback_uses_input_driver_cmd_for_support_probe— verifies the fallback path uses the input driver_cmd.test_probe_distinguishes_support_between_binaries— sanity check on the lru_cache key.
File-revert negative test confirmed all three of the new "manifest/probe" tests are load-bearing: they fail pre-fix and pass post-fix. 20/20 in tests/computer_use/test_cua_no_overlay.py, 50/50 in tests/computer_use/, and the existing TestMcpInvocationResolution (8/8) in tests/tools/test_computer_use.py still green.
| would crash the MCP spawn. | ||
| """ | ||
| try: | ||
| proc = subprocess.run( |
There was a problem hiding this comment.
This new third-party subprocess inherits the full parent environment. Match the current manifest/update-check policy with env=_sanitize_subprocess_env(cua_driver_child_env()) so provider credentials are not exposed to cua-driver --help.
There was a problem hiding this comment.
Good catch — addressed in the latest push (commit 176e0876f on this PR).
The cua-driver --help subprocess at :260 now passes env=_sanitize_subprocess_env(cua_driver_child_env()) with the same lazy import + rationale comment as the manifest probe at :214. cua-driver is a third-party binary, and every other spawn site in this file (manifest probe, MCP spawn, install probe) already uses the sanitized env. The --help probe now matches.
Regression test added: TestDriverSupportsNoOverlay.test_help_probe_passes_sanitized_env in tests/computer_use/test_cua_no_overlay.py — verifies subprocess.run is called with env= and that the env is non-None. File-revert negative test confirms it's load-bearing (fails pre-fix, passes post-fix).
…revent idle CPU cua-driver's cursor overlay rendering loop can consume CPU indefinitely when idle (NousResearch#28152, NousResearch#47032). On Linux/WSL2, the overlay serves no visual purpose and the rendering path is the primary source of idle CPU usage. Add computer_use.no_overlay config option (default: auto-detect) that passes --no-overlay to cua-driver when enabled. Auto-detection disables the overlay on Linux (covers WSL2, headless, containers) where it has no benefit, and keeps it enabled on macOS/Windows where it is visually useful. Refs: NousResearch#28152, NousResearch#47032
…rm-dependent assertions - Add autouse fixture to TestMcpInvocationResolution to disable --no-overlay flag so existing tests assert baseline args - Make test_config_load_failure_fails_safe and test_missing_section_enables platform-aware (Linux auto-detect returns True, macOS/Windows False)
…er 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.
…s + manifest probe
The hermes-sweeper review #4701565902 (2026-07-15) flagged two
consistency issues in `_cua_driver_supports_no_overlay` and one
additive-config concern:
1. `cua-backend.py:260` — the `cua-driver --help` support probe
inherited the full parent environment. cua-driver is a third-party
binary; every other spawn site in this file (manifest probe at
`:214`, MCP spawn at `:697`, install probe at `:997`) uses
`_sanitize_subprocess_env(cua_driver_child_env())`. The `--help`
probe should match. This was a low-impact leak (only help output
exits), but inconsistency is the wrong default for a third-party
subprocess.
2. `cua_backend.py:238` — when the manifest returned a `command`
different from the input `driver_cmd` parameter (e.g. a relocated
executable at `/opt/relocated/cua-driver` while the system binary
is at `/usr/bin/cua-driver`), the support probe ran against
`_CUA_DRIVER_CMD` (the default) instead of the manifest-discovered
`command`. Two failure modes:
- The wrapper binary supports `--no-overlay` but the system binary
doesn't → probe returns False → overlay kept despite capability.
- The system binary supports `--no-overlay` but the wrapper doesn't
→ probe returns True → MCP spawn crashes on the unknown flag.
3. The original commit bumped `_config_version` 31→32 for an additive
default (`computer_use.no_overlay: None`). AGENTS.md specifies that
additive defaults in existing sections are handled by deep merge
and should NOT trigger a version bump. After cherry-picking onto
current `origin/main` (which is already at 33), the bump is
effectively dropped — resolved to main's 33.
Changes:
- Add `env=_sanitize_subprocess_env(cua_driver_child_env())` to the
`--help` subprocess (with the same import + rationale comment as
the manifest probe).
- Pass `driver_cmd=command` (or `driver_cmd=driver_cmd` for the
fallback path) into `_mcp_args_with_overlay_flag`, so the support
probe runs against the binary that will actually be launched.
Tests (3 new):
- `test_help_probe_passes_sanitized_env` — verifies `subprocess.run`
is called with an `env=` kwarg.
- `test_manifest_command_drives_support_probe` — verifies the probe
runs against the manifest command when it differs from the input
driver_cmd.
- `test_fallback_uses_input_driver_cmd_for_support_probe` — verifies
the fallback path (no command in manifest) uses the input
driver_cmd.
- `test_probe_distinguishes_support_between_binaries` — sanity check
that the lru_cache key on `driver_cmd` prevents cross-binary
cache leakage.
File-revert negative test confirmed all three of the new
"manifest/probe" tests are load-bearing: with the pre-fix code, they
fail (probe runs against the default binary instead of the resolved
one); with the fix, they pass. 20/20 tests in
`tests/computer_use/test_cua_no_overlay.py` green.
`TestMcpInvocationResolution` (8/8) still green.
Refs: sweeper review #4701565902
962790d to
176e087
Compare
|
Quick follow-up on the third finding from review #4701565902 (the The original PR commit So no further code change needed for that finding. All three findings from the review are now resolved on this branch (force-pushed as
20/20 in |
|
Superseded by #69903 (or whatever number — will fix). |
|
Superseded by #69903. That salvage keeps your |
fix(computer_use): disable cua-driver overlay by default on macOS/WSL (supersedes #53841)
…841-no-overlay fix(computer_use): disable cua-driver overlay by default on macOS/WSL (supersedes NousResearch#53841)
Bug Description
cua-driver's cursor overlay rendering loop can consume CPU indefinitely when idle. On Linux/WSL2, the overlay has no visual benefit and is the primary source of idle CPU usage (~52% reported). On macOS, the overlay's vImage buffer setup can enter an infinite redraw loop at 105%+ CPU (#28152, #47032).
Fixes #28152
Root Cause
The cursor overlay rendered by cua-driver runs a CoreAnimation redraw loop that, on Linux/WSL2, spins continuously even with no work to do. On macOS, a vImage buffer sizing mismatch causes the same loop to error and retry indefinitely. The
--no-overlayflag (cua-driver ≥ 0.6.x) suppresses the overlay entirely.Hermes never passed this flag, so all users got the overlay regardless of platform or need.
Fix
computer_use.no_overlayconfig option (default:None= auto-detect)true/falsein config overrides auto-detectioncomputer_use.cua_telemetryconfig patternHow to Verify
hermes computer-use— cua-driver should start with--no-overlay, CPU idle near 0%computer_use.no_overlay: truein config.yaml — overlay disabled regardless of platformpython -m pytest tests/computer_use/test_cua_no_overlay.py -v— 10 tests passTest Plan
--no-overlayworks)Risk Assessment
Low — config-only change with safe defaults. Auto-detection only affects Linux where the overlay has no benefit. macOS/Windows behavior unchanged. Older cua-driver versions that don't support
--no-overlaywill reject the flag; callers should fall back gracefully (already handled by existing spawn error handling).