fix(computer_use): reset _started in _lifecycle_coro finally block + expose dispatch param - #55048
fix(computer_use): reset _started in _lifecycle_coro finally block + expose dispatch param#55048heidis168 wants to merge 3 commits into
Conversation
When the cua-driver MCP connection drops or the lifecycle coroutine exits abnormally, _started remains True even though _session is None. Subsequent calls hang because the wrapper believes the session is alive. This sets self._started = False in the finally block so callers either get an explicit 'session not started' error or trigger a reconnect via the existing _is_closed_session_error retry path, instead of waiting indefinitely for a session that no longer exists.
Companion to the previous _lifecycle_coro fix: when the lifecycle coroutine dies (sets _started = False in finally), the next list_apps call returned an empty apps list instead of triggering a reconnect, because list_apps treats the empty data path as 'no apps running'. Add _ensure_session_alive() that re-runs _session.start() when _started is False. start() is idempotent (self-loop guarded at the top), so this only fires when the session genuinely died, and callers get a fresh session + session id without needing to know the session ever dropped. E2E verified on Windows 11 + cua-driver 0.6.8: after cua-driver daemon restart, list_apps now returns 11 running apps instead of 0.
…ound SendInput Companion to the session-hang fix (NousResearch#55048). The default background dispatch walks UIA hit-test → Invoke, which Qt clients (WeChat, Telegram) silently drop. Foreground dispatch sends real SendInput events — works on every Windows app — but the wrapper was hiding the override. Wiring `dispatch` through _dispatch() lets the agent pick the path explicitly. Element- and coordinate-based clicks both honour it. This unlocks the whole Windows desktop to the agent — without it, click() on Qt apps was a no-op even though the daemon returned OK. Note for operators: foreground dispatch requires Hermes to be running as Administrator (cua-driver needs UIAccess integrity to bypass the Windows foreground lock). Background dispatch works without elevation but only delivers to apps that implement UIA InvokePattern.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the lifecycle-state inconsistency; current main still clears _session without resetting _started in tools/computer_use/cua_backend.py:658-663.
Problems
- The new recovery is only called by
list_apps()(PRcua_backend.py:1406).capture()still calls the session directly (tools/computer_use/cua_backend.py:1261on main), so after_startedis reset it will fail instead of reconnecting. dispatchis not declared intools/computer_use/schema.py:29-214, so the model cannot discover the new argument.- The explicit-dispatch branch at PR
tools/computer_use/tool.py:385-405bypassesComputerUseBackend.click()via private_action()and dropsmodifiersfrom its payload.
Suggested changes
- Centralize recovery at the session boundary and add forced-lifecycle-exit tests for both
list_apps()andcapture(). - Expose a constrained schema field and thread it through the public backend interface, preserving modifiers.
Automated hermes-sweeper review.
| # If the lifecycle coroutine exited without stop() being called | ||
| # (e.g. MCP connection dropped), mark the session as not started | ||
| # so callers don't hang on a dead session. | ||
| self._started = False |
There was a problem hiding this comment.
Resetting _started fixes the inconsistent state, but only list_apps() invokes the new recovery helper. capture() still calls the session directly, so it will now raise “session not started” after this path instead of reconnecting. Put recovery at the session boundary or cover every direct caller.
| x, y = (coord[0], coord[1]) if coord and coord[0] is not None else (None, None) | ||
| dispatch = args.get("dispatch") | ||
| modifiers = args.get("modifiers") | ||
| if dispatch: |
There was a problem hiding this comment.
dispatch is not declared in tools/computer_use/schema.py, so this branch is not exposed to models. Thread a constrained field through the schema and public backend interface rather than relying on the private Cua-only _action() path.
| "element_index": element, | ||
| "window_id": backend._active_window_id, | ||
| "button": button or "left", | ||
| "dispatch": dispatch, |
There was a problem hiding this comment.
This raw payload omits modifiers, although they are read on line 384 and passed by the normal backend.click() path. A foreground modifier-click will silently lose its modifiers.
|
Both bugs here are addressed:
Will close once #67138 merges. Thanks for the two-part diagnosis and the draft. |
…ging (#67138) Bug 1 of #55048: when the MCP connection dropped (driver crash / restart), _lifecycle_coro exited but left _started=True, so the next list_apps/capture passed _require_started() and then operated on a None session — hanging forever instead of reconnecting. - _lifecycle_coro's finally now resets _started=False on ANY exit, so a dead session is re-enterable (idempotent no-op on the normal stop() path; atomic bool write, safe from the bridge-loop thread without the lock stop() holds). - call_tool() re-enters start() when the session isn't active, rebuilding it before the call. The start_session/end_session handshake (driven by start()/ stop() themselves) is exempted so bootstrap doesn't recurse. Tests: two cases in test_computer_use_delivery_ladder.py — finally resets _started, and call_tool restarts a dead session exactly once. Full computer_use suite green (233). Refs #55048 (Bug 1). Bug 2 (expose foreground dispatch) is covered by the delivery_mode work in #67123.
|
Thanks for this — your two-part diagnosis was spot on, and both fixes have now landed on
Both were part of the broader cua-driver verify→escalate ladder work (#67052). Closing this as superseded by those merges — but the credit for surfacing both bugs, with a clear repro path, is yours. Appreciated. |
…esearch#57623) Add parameter (background|foreground|auto) to the computer_use schema and wire it through _dispatch() for all mutating actions: click, double_click, right_click, middle_click, drag, scroll, type, key, and set_value. Previously the dispatch override was only discussed in PR NousResearch#55048 (click variants) and was never actually declared in the tool schema, so the model could never produce it. This left drag, scroll, type, key, and set_value with no way to request foreground delivery on Windows apps (Explorer SysListView32, Qt clients like WeChat/Telegram) where background dispatch silently fails. The parameter flows through the backend ABC, CuaDriverBackend, and NoopBackend. When omitted or None, behavior is unchanged (background default).
…ging (NousResearch#67138) Bug 1 of NousResearch#55048: when the MCP connection dropped (driver crash / restart), _lifecycle_coro exited but left _started=True, so the next list_apps/capture passed _require_started() and then operated on a None session — hanging forever instead of reconnecting. - _lifecycle_coro's finally now resets _started=False on ANY exit, so a dead session is re-enterable (idempotent no-op on the normal stop() path; atomic bool write, safe from the bridge-loop thread without the lock stop() holds). - call_tool() re-enters start() when the session isn't active, rebuilding it before the call. The start_session/end_session handshake (driven by start()/ stop() themselves) is exempted so bootstrap doesn't recurse. Tests: two cases in test_computer_use_delivery_ladder.py — finally resets _started, and call_tool restarts a dead session exactly once. Full computer_use suite green (233). Refs NousResearch#55048 (Bug 1). Bug 2 (expose foreground dispatch) is covered by the delivery_mode work in NousResearch#67123.
Summary
Two fixes for
computer_useon Windows — one for the session hang thatmade
list_apps/capturereturn empty after the MCP session died, andone for the click being a no-op on Qt apps like WeChat and Telegram.
Bug 1: session hang (the original report)
When the cua-driver MCP connection drops,
_lifecycle_coroexits but_startedstaysTrue. The next call tolist_apps/capturethenhits
_require_started()and hangs forever instead of reconnecting.This made the wrapper look broken from the very first MCP blip.
Fix —
_lifecycle_coro.finallynow sets_started = Falseon exit,so subsequent calls re-enter
start()and rebuild the session.Bug 2: click is a no-op on Qt apps (discovered while testing #1)
Default
backgrounddispatch walks UIA hit-test → UIA Invoke, which Qtclients silently drop. Even when the daemon returned "Posted click to
pid 460592", the WeChat window never opened the chat. Foreground dispatch
sends real
SendInputevents — works on every Windows app — but thewrapper was hiding the override.
Fix — wire
dispatchthrough_dispatch(). Both element- andcoordinate-based clicks now honour it:
Without this fix, the wrapper could observe any Windows desktop but
couldn't actually drive most non-Microsoft apps.
Operator note
Foreground dispatch requires Hermes to be running as Administrator on
Windows. cua-driver needs UIAAccess integrity to bypass the Windows
foreground lock — without it, SendInput events are silently dropped by
the desktop. Background dispatch works without elevation but only delivers
to apps that implement UIA InvokePattern.
The
computer-use-setupskill (heidis168/hermes-tools) documents thefull Windows setup, including the elevation requirement and the
mcp_discovery_timeoutfix.Verification
End-to-end on Windows 11 24H2, Hermes running as Administrator:
list_appscapture(app=Telegram)click(element=196, dispatch=foreground)click(element=81, dispatch=foreground)type_text,hotkey,scrollRepro of original bug — kill cua-driver daemon while a Hermes session is
active, then call
list_apps(). Without this fix the call hangs; withthis fix it transparently reconnects (see PR diff for the exact line).