feat(cu): add exact cua-driver executor service - #893
Conversation
|
@astrohan This Computer Use stack slice is ready for review. Please follow the dependency and merge order stated at the top of the PR; the exact net diff is linked there. |
1430643 to
2514e3a
Compare
2514e3a to
e0e1b5e
Compare
|
@astrohan P0 blocker found in live concurrent use: the current compatibility driver PID-bound CGEvent path does not move the pointer or steal focus, but it can still interfere with the user physical mouse button state. Do not merge the Computer Use stack as fully background-safe yet. I am preparing a fail-closed follow-up that disables every CGEvent mouse/keyboard path and retains only AX/CDP/screenshot operations until the native event backend can prove isolation. |
ceb3ab7 to
f57fb9d
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Requesting changes for one blocker. I rechecked the latest f57fb9d0 fixes and did not keep the earlier claims about background drag or scroll being incompatible with the bundled fork.
P1
- The production selector cannot enable
allowCompatibilityInputDispatch. It is not part ofselectComputerUseBackend()'s input and is never passed tocreateCuaDriverBackend()(select-backend.ts:54-75). As a result, coordinate click, scroll, drag, andpress_keyreturnunsupported_actionon every normal Desktop call (cua-driver-backend.ts:1800-1803, 1938-1945, 2113-2116, 2169-2172).typeis also unreachable because its keyboard target is established only by a successfulleft_click, which the same fence blocks (cua-driver-backend.ts:1990-2002, 2395-2414). This is the executor's advertised core surface, not an edge case. Either expose only the semantic actions that actually work, or wire a safe production implementation for these actions. Simply setting the flag to true would reopen several P2 issues below.
P2
- Semantic refetch treats one matching role and label as the same element and ignores the original frame, depth, and value (
cua-driver-backend.ts:914-929, 1313-1360). A dynamic UI can replace a uniqueContinueorDeletecontrol and the action will be sent to the replacement. physicalInputRecentlyActiveis optional and the selector does not require a host guard. Semantic click and set-value dispatch proceed when no callback is supplied (cua-driver-backend.ts:309-323;select-backend.ts:54-75).- If compatibility dispatch is enabled,
typeuses the last click-owned keyboard target for the session and turn without checking the current bound window or page (cua-driver-backend.ts:1372-1379, 2395-2414). Observing another window can still send text to the old one. - The Electron click script can deliver
element.click()and observe a click event but returnno_observable_effectwhen no synchronous DOM mutation follows (cua-driver-page-target.ts:230-267). The backend converts that delivered action to ordinarycapture_failed, which makes an unsafe retry look reasonable (cua-driver-backend.ts:1704-1715). - A delivered native or Electron text write can also become
capture_failed: thetypecatch rewrites every lifecycle error afterset_valueorinsert_text(cua-driver-backend.ts:2395-2422). It should preserveoutcome_unknownafter delivery. select_textandperform_secondary_actionare called here, but neither tool exists in the pinned bundled source registry (cua-driver-backend.ts:1840-1850). These actions always fail at runtime.- One process-wide
operationQueueserializes unrelated sessions (cua-driver-backend.ts:305-306, 494-529). A slow capture or timeout for one user stalls every other Computer Use session. - Window observations return the raw
get_window_stateimage without the compression and 8 MiB cap used by desktop capture (cua-driver-backend.ts:821-875). A large window frame can exceed provider limits or consume unbounded memory. - Backend observations remain in the
observationsmap until a semantic action consumes that exact ID or the session is cleared (cua-driver-backend.ts:294-305, 855-865, 1786-1793). Repeated coordinate observations retain old AX labels and values for the life of the session. - The native content fingerprint hashes role, frame, and depth but not label or value (
cua-driver-backend.ts:338-350). A control changing fromCanceltoDeletein the same structural slot does not invalidate a bound coordinate action.
The latest CI is green, and the focused package tests pass. The P1 is about the production path being permanently fenced off, not test coverage.
Astro-Han
left a comment
There was a problem hiding this comment.
Approving after correcting my earlier severity call. The disabled compatibility input surface is a deliberate fail-closed limitation. This PR is not wired into Desktop on its own, and native AX/CDP semantic actions remain available. The missing coordinate path is therefore P2, not a P1 production regression. The later physical-input guard should land before compatibility dispatch is enabled or the stack is described as concurrent-user safe.
P2
- Coordinate click, scroll, drag,
press_key, and the click-establishedtypepath are unavailable through the normal selector becauseallowCompatibilityInputDispatchis not exposed there (select-backend.ts:54-75;cua-driver-backend.ts:1800-1803, 1938-1945, 1990-2002, 2113-2116, 2169-2172, 2395-2414). This is a material product limitation, but it fails closed. - Semantic refetch treats one matching role and label as the same element and ignores the original frame, depth, and value (
cua-driver-backend.ts:914-929, 1313-1360). A dynamic UI can replace a unique control and receive the pending action. physicalInputRecentlyActiveis optional, and the selector does not require a host guard. Semantic click and set-value dispatch proceed when no callback is supplied (cua-driver-backend.ts:309-323;select-backend.ts:54-75).- If compatibility dispatch is enabled,
typeuses the last click-owned keyboard target for the session and turn without checking the current bound window or page (cua-driver-backend.ts:1372-1379, 2395-2414). Observing another window can still send text to the old one. - The Electron click script can deliver
element.click()and observe a click event but returnno_observable_effectwhen no synchronous DOM mutation follows (cua-driver-page-target.ts:230-267). The backend converts that delivered action tocapture_failed, which can invite a retry. - A delivered native or Electron text write can also become
capture_failed: thetypecatch rewrites every lifecycle error afterset_valueorinsert_text(cua-driver-backend.ts:2395-2422). It should preserveoutcome_unknownafter delivery. select_textandperform_secondary_actionare called here, but neither tool exists in the pinned bundled source registry (cua-driver-backend.ts:1840-1850). These actions fail at runtime.- One process-wide
operationQueueserializes unrelated sessions (cua-driver-backend.ts:305-306, 494-529). A slow capture or timeout for one user stalls every other Computer Use session. - Window observations return the raw
get_window_stateimage without the compression and 8 MiB cap used by desktop capture (cua-driver-backend.ts:821-875). A large frame can exceed provider limits or consume excessive memory. - Observations remain in the
observationsmap until a semantic action consumes that exact ID or the session is cleared (cua-driver-backend.ts:294-305, 855-865, 1786-1793). Repeated coordinate observations retain old AX data for the life of the session. - The native content fingerprint hashes role, frame, and depth but not label or value (
cua-driver-backend.ts:338-350). A control changing meaning in the same structural slot does not invalidate a bound coordinate action.
No P0 or P1. CI is green, and the focused package tests pass. This approval supersedes my earlier request for changes.
|
Post-merge follow-up #910 now tracks and fixes the actionable executor findings from the final review. It covers semantic replacement/ambiguity, label/value fingerprints, window screenshot limits, bounded observation retention, keyboard target rebinding, delivered-action |
Upstream stack notice
This is stack PR C. It depends on #892 and must not merge before it.
The Files tab is cumulative until preceding fork branches are rebased after merge.
Review the exact 21-file executor net diff now in fork-local PR hqhq1025#2.
Current rebase verification: full repository build; @maka/computer-use 103/103; full repository typecheck.
Stack position
PR C, stacked on fork PR #1 (
codex/cu-runtime-foundation), which is stacked on upstream draft #857.This fork-local PR preserves the executor-only diff. After the preceding slices merge upstream, it will be retargeted to
maka-agent/main.Summary
Adds the sole native Computer Use executor behind the Runtime contract.
Reverse-engineering alignment
outcome_unknown, never automatically retried;Review hardening
Adversarial review findings fixed before publication:
set_configtool errors are attested;Verification
@maka/computer-use: 97/97;git diff --checkpassed.Non-goals