feat(computer_use): add switch_desktop with overlay-safe restart - #1
Conversation
Stop the overlay subprocess before switching virtual desktops, then restart it on the new desktop — avoids the tkinter display context teardown that kills the overlay during SendInput-based Ctrl+Win+Left/Right. - _switch_desktop_via_keybd(): stop overlay → two-phase SendInput → restart overlay - switch_desktop() method on WindowsUIABackend - Schema and dispatch updated with 'switch_desktop' action and 'direction' parameter Co-authored-by: lEWFkRAD
|
Status update after several rounds of debugging — the root cause is clearer now, and it's not what I initially thought. What we tested
Root cause: process architecture, not keyboard simulationHermes has two gateway modes, and the crash only happens in one:
When the agent and the Dashboard share the same process, any virtual-desktop transition causes the (Node.js) to receive a SIGHUP and perform a graceful shutdown of the gateway. The exact trigger mechanism (Console event? Electron window visibility? PTY disconnect?) is still under investigation, but it's definitely NOT caused by the keyboard-injection method — changing SendInput to COM made zero difference. What this means for the PRThe code itself (overlay-safe restart pattern) is correct and tested working. The crash is a pre-existing Hermes architectural limitation: the embedded gateway (Chat tab) cannot survive a virtual-desktop switch regardless of how the switch is triggered. The path has the same problem — we just got lucky that the model fell back to it during a session where the full gateway happened to be running. I've added a docstring note about this limitation. The PR is still ready for review — the implementation is sound, but I wanted to be transparent about where the boundary lies between this feature and Hermes' runtime architecture. |
|
Final conclusion after a full day of bisecting. Root cause identified and fixed. Root cause: SendInput batch timingThe embedded gateway (Dashboard Chat tab) runs the agent in-process with the Dashboard's uvicorn server. It has a single-channel event loop that cannot handle multi-batch SendInput without disruption. The full system gateway (spawned via the System page) is multi-channel by design (handles simultaneous WebSocket + WeChat clients) and is unaffected.
The embedded gateway's single-channel design predates computer-use on Windows. When SendInput delivers two batches with a sleep interval, the PTY pipeline gets disrupted and the Node.js TUI parent triggers a graceful shutdown (SIGHUP → kill gateway). The full gateway's multi-client dispatch loop handles this without issue. Fix appliedChanged Bisect log (for posterity)
Takeaway for future Windows contributorsAny tool using SendInput should use single-batch delivery. Multi-batch SendInput with sleep intervals between batches will work under the full system gateway but crash the embedded (Chat tab) gateway — which is the gateway most users encounter during first-time testing. |
Two-batch SendInput (press → sleep → release) crashes the embedded gateway (Dashboard Chat tab) because its single-channel event loop cannot handle multi-batch keyboard injection without disrupting the PTY pipeline. The full system gateway is unaffected because its multi-client dispatch loop handles concurrent channels. Switch to single-batch SendInput matching _press_combo semantics (hold modifiers → tap arrow → release). This works in both embedded and full gateway modes.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new switch_desktop action to the computer-use tool to switch Windows virtual desktops via a Ctrl+Win+Arrow SendInput sequence while restarting the overlay.
Changes:
- Implemented Windows backend virtual-desktop switching with overlay stop/restart.
- Added tool dispatcher handling for
switch_desktop. - Extended the action schema with
switch_desktopand adirectionparameter.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tools/computer_use/windows_backend.py | Adds SendInput-based virtual desktop switching and a new backend action method. |
| tools/computer_use/tool.py | Routes switch_desktop requests to the backend. |
| tools/computer_use/schema.py | Adds switch_desktop to allowed actions and defines direction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Validate direction before key dispatch (unknown values return False) - Log exceptions instead of silently swallowing them - Add 150ms delay before overlay restart to avoid DWM race - Route switch_desktop through _maybe_follow_capture for consistency - Add JSON Schema if/then to require direction for switch_desktop
|
Final conclusion after a full day of bisecting. The issue is fully understood now. Root cause: SendInput batch timingThe embedded gateway (Dashboard Chat tab) runs the agent in-process with the Dashboard's uvicorn server. It has a single-channel event loop that cannot handle multi-batch SendInput without disruption. The full system gateway (spawned via the System page) is multi-channel by design (supporting simultaneous WebSocket + WeChat clients) and is unaffected.
WhyThe embedded gateway's single-channel design predates computer-use on Windows. When SendInput delivers two batches with a sleep interval, something in the event loop / PTY pipeline gets disrupted and the Node.js TUI parent triggers a graceful shutdown (SIGHUP → kill gateway). The full gateway handles this because its multi-client dispatch loop is designed for concurrent WebSocket + WeChat sessions. Fix appliedChanged to use the same single-batch SendInput sequence as (the existing helper): Takeaway for future Windows computer-use contributorsAny tool that needs SendInput should use single-batch delivery. Multi-batch SendInput with sleep intervals will work under the full gateway but crash the embedded (Chat tab) gateway — which is the first gateway most users encounter during testing. |
|
Merged — thank you @Icather 🙏 Your One follow-up commit on top (ea294af): the overlay restart force-cleared Folded in the dependency-free test you'd offered to include — live-restart respawns, disabled-stays-down, and invalid-direction. The disabled-stays-down case fails on the old force-clear, so it pins the regression. Full The single-batch SendInput rework and the embedded-vs-full-gateway testing you did is exactly the real-use coverage I can't reach from one box — much appreciated. |
Phase 1 of the pluggable cron-scheduler refactor (Axis B — the trigger).
No call-site changes; this phase only makes the abstraction exist + tested
in isolation.
Task 1.1: cron/scheduler_provider.py — the EXPERIMENTAL CronScheduler ABC.
Required surface is name + start; is_available()/stop() carry safe defaults.
is_available has a no-network invariant. Docstring marks it experimental
until the Chronos provider (Phase 4) validates the shape.
Task 1.2: InProcessCronScheduler wraps the historical 60s ticker loop, calling
cron.scheduler.tick(sync=False) exactly as the raw ticker does. Uses
stop_event.wait(interval) for responsive stop (both raw tickers already do).
Tests: ABC-is-abstract, default-is_available, the InProcess loop drives tick
and stops, stop() no-op, and test_abc_growth_stays_additive (the forward-compat
guard: required abstractmethods must stay exactly {name, start}, so the three
Phase-4 hooks land as NON-abstract additions).
tick() internals in cron/scheduler.py are byte-unchanged (only new file added).
Phase 0 characterization tests still green. Full tests/cron/: 445 passed.
…s (blocker #1) A window title is attacker-influenceable (browser tab/doc titles), and the @window: expansion spliced it raw into a bash command the model is told to run. Now: - resolve the title to an exact native handle via the sidecar and emit commands that target `--hwnd <int>` (an integer — injection-proof); - ambiguous matches are listed for the model to disambiguate by handle, never auto-driven (also addresses wrong-window control); - the title only ever appears shlex.quote()'d in the no-handle fallback. Tests: parse + actionable-block + malicious-title-quoting (3 pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…onnect ladder can't freeze silently (NousResearch#66377) The Telegram gateway could go silently deaf for hours: the reconnect ladder stalled mid-way (e.g. "attempt 4/10, reconnecting in 40s" then nothing) while the process stayed active(running), so Restart=always never fired. Root class: every recovery path — the ladder's re-entry (_schedule_polling_recovery), the pending-update probe (_probe_pending_updates), and PTB's error callback — gates new recovery on _polling_error_task.done(). If that single task wedges on any hung await, all recovery returns early forever and nothing retries. The heartbeat loop is a separate task, so make it an independent, cause-agnostic watchdog: if the same recovery task stays in-flight past _POLLING_ERROR_TASK_STUCK_TIMEOUT (300s — well beyond a healthy ladder attempt's bounded stop+drain+start+backoff), force a retryable-fatal so the background reconnector rebuilds the adapter instead of relying on the frozen ladder. This guarantees progress regardless of *where* the stall is (issue direction #1), tracked locally so no task-assignment site needs to change. Also salvages @koduri-mahesh-bhushan-chowdary's NousResearch#66492 (drain-await timeout), which closes the one concrete wedge vector documented in the incident (_drain_polling_connections' unbounded shutdown()/initialize() on a wedged CLOSE-WAIT pool). The watchdog covers the rest of the class. Co-authored-by: Koduri Mahesh Bhushan Chowdary <mkoduri73@gmail.com>
…reaming Two real render-cost wins found by inspection (no behavior change): 1. Sidebar re-rendered on every stream token. $sessionStates is republished on every message delta (tens/sec during a turn), and the derived ID computeds ($workingSessionIds, $attentionSessionIds, $backgroundRunningSessionIds) allocated a fresh array each time. nanostores notifies on !==, so the whole ChatSidebar + every mounted row re-rendered per token even when the working/ attention/background set was unchanged. Return the previous array reference when the contents match → nanostores skips the notify unless the set actually changes. Turns streaming from O(visible rows)/token into O(0) for the sidebar. 2. Tool rows normalized the FULL uncapped detail every render. `looksRedundant` (lowercase + whitespace-collapse over the entire read_file/terminal payload) ran twice in the ToolEntry render body, so every completed tool re-normalized its whole output on every stream tick of the running message. Memoize on the view fields so it recomputes only when the tool's content changes. Both are correctness-preserving (stable refs + memoization). The CI stream scenario drives $messages directly, not the publishSessionState path, so it won't reflect #1 — verified by inspection.
Blocking #1 — gateway-connecting-overlay.tsx reduced-motion regression: the top `if (reduce) setPhase('gone')` fired unconditionally on mount whenever reduce-motion was on, so every OS reduced-motion user lost the CONNECTING overlay during cold boot entirely (jumped to 'gone' before the gateway was even open). The intent was to skip the exit *choreography*, not to skip showing the overlay. Removed the unconditional top block and the redundant nested preview block; kept only the third branch (`gatewayState === 'open' && shownRef.current` → `reduce ? 'gone' : 'text-out'`) which correctly gates the short-circuit on connect. Also fixed `if(reduce)` missing-space, 6-space misindent, and the same 3-line comment pasted three times. Nit #1 — tsconfig excludes e2e, so specs were never typechecked in CI. Added tsconfig.e2e.json (extends base, includes e2e/ + playwright.config.ts, adds @playwright/test types) and wired it into the typecheck script. This surfaced three latent type errors that are fixed in the same commit: - fix-electron-tracing.ts: `app._context` and `electron._playwright` are private APIs — added `as any` on the access before the existing cast. - playwright.config.ts: `reducedMotion: 'reduce'` directly under `use:` is not a valid UseOptions property in playwright 1.58; it's a BrowserContextOption accessed via `contextOptions: { reducedMotion: 'reduce' }`. The old form was silently ignored at runtime, so reduced-motion emulation wasn't actually active — screenshots could catch overlays mid-fade (exactly what the comment warned about). Nit #2 — fix-electron-tracing.ts reaches into Playwright internals (_playwright, _allContexts, _context) with no public contract. Added a header comment calling out the `@playwright/test` exact pin (=1.58.2) so a future bump knows to re-verify the private symbols still exist. Nit NousResearch#3 — main.ts TEST_WORKER_INDEX block had stray 6-space indentation. Verified: tsc -p . && tsconfig.electron && tsconfig.e2e → 0 errors; vitest boot-failure-overlay (3/3) + boot-failure-reauth (21/21) pass; npm run build clean; playwright e2e/boot-failure.spec.ts 2/2 pass.
…native extension) unicode61 indexes a CJK run as ONE token, so 2-char Korean terms (일본, 구글, 우리, ...) can never match it and the trigram tokenizer needs >=3 chars per term — any query containing a 1-2 char CJK token falls through to a LIKE full-table scan (measured 3-6.4s CPU per query on a 6.8GB production state.db; the #1 base cost behind a 12.4s session_search average on CJK workloads). This ships a ~250-line loadable FTS5 tokenizer (no deps) that wraps unicode61: maximal CJK runs inside its tokens are re-emitted as overlapping character bigrams (Lucene CJKAnalyzer semantics), everything else passes through unchanged. FTS5 phrase semantics turn consecutive sub-tokens into exact substring matching down to 2-char terms at index speed. Build: native/fts5_cjk/build.sh -> ~/.hermes/lib/libfts5_cjk.so (override: HERMES_FTS5_CJK_SO). Salvaged from PR NousResearch#65544; the schema integration lands separately on the v23 external-content layout.
…add same-pid self-reclaim guard Hardening on top of the salvaged dead-PID lease reclamation from PR NousResearch#65775 (@the3asic): - Probe via psutil.pid_exists (hard dependency; CONTRIBUTING.md critical rule #1) with the contributor's os.kill(pid, 0) POSIX probe retained only as a scaffold-phase fallback when psutil is missing. - Same-process holders (pid == os.getpid()) are never probed and never self-reclaimed — another thread's live lease is owned by the lease refresher/release path. - Any probe doubt (exceptions, permission errors) conservatively keeps the lease until normal TTL expiry; Windows stays TTL-only. - Tests: psutil-first dead-pid reclaim (probe call pinned), os.kill fallback path, probe-doubt keeps lease, same-pid no self-reclaim, legacy holder + Windows paths assert NO probe via either API.
…own (NousResearch#74136) Fix-up for the cherry-picked cooldown persistence: the PR's tests mocked the DB (SimpleNamespace(_db=MagicMock())), which cannot prove the cooldown survives a restart. Replace with the production shape — a real SessionDB on disk behind the real AsyncSessionDB facade — and add a restart regression: fail a hygiene compression on runner #1, tear it down, build a fresh GatewayRunner on the SAME database, and assert the cooldown is still honored (no compression agent instantiated). Also updates the timeout test to assert the DB-backed record_compression_failure_cooldown write instead of the removed in-memory dict. Sabotage-verified: reverting gateway/run.py to the in-memory dict makes the restart test fail.
Users following abbreviated links guess /docs/quickstart and /docs/installation and hit raw GitHub-Pages 404s — the real pages live under /docs/getting-started/. Add client redirects for both. Consumer-onboarding audit finding #1, Aug 2026.
The #1 patch failure class in production (state.db mining, 250k-window) is a re-send of an edit that already landed: 'old_string and new_string are identical' (299 occurrences) plus a share of hunk-not-found errors where the new text is already in the file. These errored, sending models into re-read/re-patch loops. New tools/fuzzy_match.is_already_applied(content, old, new) — a conservative check requiring (1) non-trivial new_string (>=8 chars), (2) EXACT presence of new_string, (3) old_string gone (unless identical). Wired into three sites: - patch_replace (replace mode): returns success + no_change: true + an explicit note instead of the identical-strings / no-match error. - V4A validation phase: an already-applied hunk validates as a no-op so multi-hunk patches no longer fail wholesale when one hunk landed in a prior call. - V4A apply phase: mirrors the same skip so the two phases agree. Genuine no-matches (new text absent) and half-applied renames (old text still present) keep their error behavior — covered by tests.
process(action='wait') hitting its window returned status='timeout' with a terse note — models read it as an error and re-issued identical waits (process is the #1 exact-duplicate tool call in production: 511 dupes in a 400k-msg window; wait is 57% of all process actions). The timeout result now carries: - process_running: true — machine-readable 'this is a status, not a failure' - an explicit note: 'Wait window of Ns elapsed — the process is still running. This is not an error. Uptime: Ms.' plus the right next step: when notify_on_complete is set, 'you will be notified on exit — do more work instead of waiting again'; otherwise a pointer to notify_on_complete for next time. - the clamp note (requested > max) now composes with the status note instead of replacing it. Exited/interrupted results are unchanged.
…e-review #1) revoke_commit_admission() used to invoke the holder-qualified lease release unconditionally — including while an admitted commit was still mutating SessionDB — letting a second compressor acquire the durable lock mid-commit and interleave with the first commit's writes. The admission_revoked flag store stays lock-free, but the lease-release decision now coordinates with the fence lock: - revoke acquires the fence lock non-blocking; on success no commit can be in flight (an admitted commit retains the lock until finish_commit) and the release runs immediately, still under the lock so a racing begin_commit cannot slip between the check and the release. - on failure the release is deferred: finish_commit() re-checks _admission_revoked and performs it AFTER the commit completes (prompt even if the worker thread is later parked), and the begin_commit refusal path does the same for a revoke that lost the race to a transient lock-setup/cancel boundary. All paths are idempotent with the worker's own outer cleanup (DB release is holder-qualified). Invariant encoded + tested: no second compressor can acquire the durable lock while an admitted commit is still mutating; after a post-revoke commit finishes the lease is released promptly. Both regressions (revoke-during-commit deferral, revoke-before-commit immediate release + refused begin_commit) are sabotage-verified.
Summary
Implements
switch_desktopaction that safely switches virtual desktops without killing the overlay subprocess.Why
ctrl+win+left/rightviakey()action successfully switches desktop but destroys the full-screen tkinter overlay child process, crashing Hermes Dashboard/Gateway. This is because any SendInput-based virtual-desktop transition tears down the overlay's display context.Solution
Three-phase approach to keep the overlay alive:
_dead = False(prevent crash-recovery) and start on new desktopChanges
windows_backend.py_switch_desktop_via_keybd()helper +switch_desktop()methodschema.py"switch_desktop"action withdirection(left/right) parametertool.pyhasattrguard for non-Windows backendsTesting
directionvalidation tested (invalid values return error)"not supported"viahasattrguard