Skip to content

fix(tools): keep switch_desktop from resurrecting a disabled overlay - #46678

Closed
lEWFkRAD wants to merge 1 commit into
NousResearch:salvage/windows-uia-computer-usefrom
lEWFkRAD:fix/switch-desktop-overlay-killswitch
Closed

fix(tools): keep switch_desktop from resurrecting a disabled overlay#46678
lEWFkRAD wants to merge 1 commit into
NousResearch:salvage/windows-uia-computer-usefrom
lEWFkRAD:fix/switch-desktop-overlay-killswitch

Conversation

@lEWFkRAD

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a kill-switch regression in the switch_desktop action on the
salvage/windows-uia-computer-use branch behind #46667.

_switch_desktop_via_keybd force-cleared overlay_client._dead and called
start() unconditionally on every virtual-desktop switch:

time.sleep(0.15)
overlay_client._dead = False   # ← clears the kill switch
overlay_client.start()

So an overlay the user had deliberately turned off — computer_use.overlay: false,
or the HERMES_COMPUTER_USE_OVERLAY=0 escape hatch (both set _dead=True with no
subprocess) — was respawned on screen on every desktop switch, overriding the
off switch. An overlay that had already failed itself off was likewise force-retried
on each switch.

This restores the guard from ea294afb5 (dropped in the salvage): capture liveness
before stop() clears _proc, and only restart an overlay that was actually
running. The _dead force-clear is removed — _OverlayClient.start() already
no-ops if self._dead or self._proc is not None, so the overlay_was_live guard is
the single source of truth for whether the overlay comes back.

Verified in code: start() in tools/computer_use/windows_backend.py returns
early while _dead is set, so clearing _dead immediately before start() is
exactly what defeats the kill switch.

Related Issue

Stacked on #46667 — targets the salvage/windows-uia-computer-use branch so the fix
lands with the salvage rather than after it. Restores behavior introduced in the
original #43927 (ea294afb5). (No Fixes # keyword on purpose — this shouldn't
auto-close #46667.)

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/computer_use/windows_backend.py_switch_desktop_via_keybd: compute
    overlay_was_live = overlay_client._proc is not None and not overlay_client._dead
    before stop(); gate both the success-path and the except-path overlay restart on
    it; remove the two overlay_client._dead = False force-clears. Docstring updated to
    state a deliberately-disabled overlay is never resurrected by a switch.
  • tests/tools/test_computer_use_windows.py — re-add the
    TestSwitchDesktopOverlaySafety suite (with a _FakeOverlay mirroring the real
    start/stop/_proc/_dead semantics): test_live_overlay_is_restarted_after_switch,
    test_disabled_overlay_is_not_resurrected (the regression guard), and
    test_invalid_direction_is_rejected_without_side_effects.

How to Test

Before → after, against the branch's own backend code:

  1. test_disabled_overlay_is_not_resurrected on the unpatched branch → FAILS:
    AssertionError: assert 'start' not in ['stop', 'start'] — the disabled overlay was
    revived on the switch.
  2. With this fix → PASSES: a _FakeOverlay(dead=True, proc=None) is never started,
    _dead stays True, and the desktop still switches.
  3. Full relevant suite, run per-file (matching scripts/run_tests.sh's subprocess-per-file
    isolation), venv python on Windows 11:
tests/tools/test_computer_use.py                  79 passed
tests/tools/test_computer_use_capture_routing.py  14 passed
tests/tools/test_computer_use_windows.py          50 passed   (incl. the 3 new tests)
tests/hermes_cli/test_tools_config.py             99 passed
                                                  --------- 242 passed, 0 failed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(tools): ...)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (windows_backend.py + its test)
  • I've run the tests and all pass — per-file isolation, 242 passed (see note below)
  • I've added tests for my changes
  • I've tested on my platform: Windows 11 (live-install venv python)

Documentation & Housekeeping

  • Updated the _switch_desktop_via_keybd docstring; README/docs/ — N/A (no user-facing surface change)
  • cli-config.yaml.example — N/A (no config keys added/changed)
  • CONTRIBUTING.md / AGENTS.md — N/A (no architecture/workflow change)
  • Cross-platform impact considered — Windows-only file (windows_backend.py, gated to win32); no behavior change on macOS/Linux
  • Tool descriptions/schemas — N/A (switch_desktop tool surface and schema unchanged)

Screenshots / Logs

Regression reproduced, then fixed (same _FakeOverlay, run against each version of the backend):

RUN 1 — unpatched salvage backend:
  test_disabled_overlay_is_not_resurrected  FAILED
    assert 'start' not in ['stop', 'start']

RUN 2 — with this fix:
  test_live_overlay_is_restarted_after_switch          PASSED
  test_disabled_overlay_is_not_resurrected             PASSED
  test_invalid_direction_is_rejected_without_side_effects PASSED
  3 passed

Windows-footgun scan + ruff on the diff: clean.

Note on pytest tests/ -q: I ran the relevant files via the venv python with
--timeout-method=thread (the repo's pinned SIGALRM method crashes pytest on
Windows). The four files pass individually (242 total). In a single shared
process
, plugin/platform registration leaked from earlier files trips
test_tools_config.py::test_get_platform_tools_recovers_non_configurable_toolsets_from_composite;
it passes in isolation (1 passed) and under run_tests.sh's subprocess-per-file
isolation — which is how CI runs it. Unrelated to this diff, which only touches
windows_backend.py and its test.

_switch_desktop_via_keybd force-cleared overlay_client._dead before
start() on every virtual-desktop switch, so an overlay the user had
turned off (computer_use.overlay: false / HERMES_COMPUTER_USE_OVERLAY=0,
or one that had already failed itself off) was respawned on screen on
each switch, overriding the kill switch.

Capture liveness before stop() clears _proc and only restart an overlay
that was actually running; drop the _dead force-clear. start() already
no-ops while _dead is set, so the guard is the single source of truth
for whether the overlay comes back.

Re-adds the dedicated switch_desktop overlay-safety tests, including the
disabled-stays-down regression that fails on the force-clear path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 15, 2026
@lEWFkRAD

Copy link
Copy Markdown
Contributor Author

Makes sense — closing this in step with #46667 now that Windows desktop-control is consolidating on the trycua cua-driver-rs path (#44221).

The semantic this fix protects (don't resurrect a user-disabled overlay on a desktop switch) and its regression test sit above the driver layer, so they're exactly the "safety semantics above the driver layer" you mentioned carrying over — worth keeping if the trycua path grows an equivalent overlay/escape-hatch. Happy to rebase the test onto #44221 whenever that's useful. Thanks again for the credit.

@lEWFkRAD lEWFkRAD closed this Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants