fix(cu): fence compatibility input dispatch - #905
Conversation
|
@astrohan P0 live-user safety fix. The current PID-bound CGEvent compatibility path can interfere with the user physical mouse button state even without pointer movement or focus steal. This PR fails closed and disables that path by default. |
4834e75 to
e006f04
Compare
|
@astrohan P0 safety update is ready for review. The latest commit removes compatibility input from the real-machine E2E itself, fixes the CI targetless-observe failures, and adds a 5-round AX-only restart soak: stale target_missing 5/5, fresh AX readback 5/5, zero compatibility dispatch, stable driver generations. The user confirmed mouse/keyboard input stayed normal during the AX-only run; the visible fixture launch can still be noticeable, which is documented separately. |
65bf7b9 to
9a4c7db
Compare
|
@astrohan Rebased onto current main after #893 merged. The duplicate executor/coordinate/semantic-token patches were dropped because #893 squash already contains them; the remaining stack starts at provenance and preserves overlay, host invalidation, and physical-input guard wiring. Post-rebase local build, typecheck, Computer Use 119/119, and script contracts 10/10 pass. New CI is running on 9a4c7db. |
Astro-Han
left a comment
There was a problem hiding this comment.
Approved. I found no P0 or P1 on the current restack. Compatibility CGEvent input is disabled by default, Desktop supplies the physical-input guard, and an unreadable guard fails closed. These remaining findings are non-blocking.
P2
- The PR's retained Electron CDP claim is still too broad. The default-off compatibility check exits the whole coordinate-click branch before
runElectronSemanticPointer()can handle an Electron target (packages/computer-use/src/cua-driver-backend.ts:1943, before the CDP path near 1981). That click is also the only path that establishes the Electron text target, soInput.insertTextis unreachable in normal Desktop use. This is the safe loss of a capability, not a safety regression. Move the compatibility check to the pixel fallback boundary if CDP click and text are meant to remain enabled. - The AX restart soak's "zero stale dispatch" oracle cannot observe native semantic dispatch. It searches for a
dispatchtrace (scripts/cu-process-restart-e2e.mjs:289), but nativeset_valueinrunSemantic()emits no such trace. The fallbackstate.jsonoracle also does not change for AXValue writes, as the new documentation notes. The run provestarget_missingwas returned, but not that stale native dispatch count was zero. Add a native semantic dispatch trace or verify the field through a fresh AX readback after the stale attempt.
P3
scripts/cu-physical-input-probe.swiftremains after the final test stopped compiling or invoking it. The monitor's parsedinitialPhysicalInputAgeis also unused. Remove these leftovers so they are not mistaken for active safety coverage.
CI is green, including 119 Computer Use tests, 55 Runtime Computer Use tests, and 29 script tests. This PR should merge only after #903 so the reviewed stack layers remain separate.
|
@astrohan This consolidated stack is approved, CI is green, and GitHub reports it mergeable. I audited it against current main and the #893 hardening follow-up: #905 has no file overlap with #910. Please squash-merge #905 when ready; my account cannot execute the upstream merge. After it lands I will close the superseded conflicting PRs and rebase any remaining independent increment. |
|
@Astro-Han Status update after #895 and #896 merged: the production safety boundary from this cumulative PR is now on main. Compatibility CGEvent input remains disabled by default in the merged executor, and Desktop now supplies the physical-input guard. The two actionable P2 executor findings from this review have been moved into the independent #910 follow-up: Electron CDP semantic click/text stays reachable behind the compatibility fence, and native semantic AX dispatch now emits a privacy-safe trace for restart oracles. I do not recommend merging this cumulative branch. Please review #910 and #911; after those land I will close #905 as superseded. |
9a4c7db to
ae21f70
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Approved on ae21f70e. I found no P0 or P1 after restacking onto current main. The conflict resolution keeps the #905 fail-closed behavior: compatibility input remains disabled in both real E2E modes. Current CI is green for typecheck, test, and e2e. The remaining findings are non-blocking.
P2
- The default compatibility fence also makes safe Electron CDP click and text unreachable.
run()returnsunsupported_actionatpackages/computer-use/src/cua-driver-backend.ts:1943, beforerunElectronSemanticPointer()near line 1981. Without a successful click, normal Desktop use cannot establish the page target required byInput.insertText. This is a capability loss, not a safety regression. If CDP operations are meant to remain available, move the gate to the pixel fallback boundary. - The process-restart soak does not independently prove zero stale native dispatch. It looks for a
dispatchtrace atscripts/cu-process-restart-e2e.mjs:289, but AXset_valueemits no such trace. Itsstate.jsoncheck also cannot see AXValue writes, as the added documentation notes. The run provestarget_missing, but a stale native write would not be observed by this oracle. Add a native semantic dispatch trace or read the field through a fresh AX observation immediately after the stale attempt.
P3
scripts/cu-physical-input-probe.swiftis not compiled or invoked by any harness. The monitor also computes and the launchers parseinitialPhysicalInputAge, but no consumer uses it. Remove these leftovers or wire them into an asserted check.
Stack position
P0 safety follow-up after #903. This PR should merge before the Computer Use stack is described as safe for concurrent user operation.
Live user finding
During real no-focus testing, the pointer did not move and the synthetic target never became frontmost. The user nevertheless confirmed that physical mouse clicks became unreliable while the agent operated another display.
This is not a focus or cursor-warp bug. The compatibility executor posts mouse down/up events to a target PID through the CGEvent/SLEvent backend. Those events can still interfere with global physical mouse button state.
Root cause
Runtime already has a
physicalUserIntervenedstate machine, but Desktop had no real macOS producer or late dispatch guard. More importantly, a host-side quiet window cannot prove that the compatibility CGEvent backend is atomically isolated from physical HID delivery.Final fail-closed policy
Desktop provides an approximately one-second physical-input quiet window through:
Active input or an unreadable signal returns typed
user_intervened, emits no dispatch trace, calls no cua-driver input tool, and requires a fresh observation.The compatibility event backend is disabled by default:
press_key: blocked;supported:falsefallback to pixel: blocked.Retained paths:
set_value;insertText.Controlled unit tests can explicitly opt into compatibility dispatch. Desktop production and real-machine E2E do not.
Real-machine evidence
The replacement process-restart soak now uses only native AX
set_valueplus fresh AX readback. It sends no compatibility click, scroll, drag, key event, or HID pulse.Five-round real macOS result:
target_missing: 5/5;set_valuewith exact readback: 5/5;The visible AppKit fixture can still be noticeable when launched or ordered on another display. This proves input-path isolation, not zero user-perceived WindowServer disturbance.
Verification
Passed locally:
npm test;npm run typecheck;npm run build;@maka/computer-use: 119/119;npx knipwas also run. It reports the repository's existing baseline findings (unused Harbor/scripts files, unlisted dependencies/binaries, and existing unused exports); none point to a new unused symbol introduced by this follow-up.Remaining boundary
Re-enabling coordinate input requires a native executor that can prove its event backend does not affect global physical button state. A timing-only quiet window is mitigation, not sufficient evidence for safe concurrent CGEvent delivery. PID reuse also remains unproven.