fix(windows-click): hit-test inside target HWND's UIA subtree for vision (x,y) - #1551
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors UIA click invocation in the Windows platform layer to use window-scoped element hit-testing instead of desktop-wide lookup. A new helper ChangesWindow-scoped UIA click invoke
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ae2aa45 to
55faa3c
Compare
|
Validated a second pass: launched Calculator fresh in the background (not foreground), did 3 + 5 = 8 via pixel clicks against the larger Win11 Calc layout (1024×801, History pane expanded, 62 UIA descendants under the AppFrame subtree). Display shows |
…e-cua parity) The (x, y) click path on Windows now layers UIA Invoke and PostMessage in a single dispatch, mirroring the trope-cua reference implementation (`src/CuaDriver.Win/Tools/ClickTool.cs::InvokePixelClickAsync`): 1. **UIA hit-test in target HWND's subtree** — `ElementFromHandle` resolves the root, `FindAll(TreeScope_Subtree)` enumerates descendants, smallest-area `InvokePattern`-bearing element whose `CurrentBoundingRectangle` contains (sx, sy) wins, `Invoke()` runs. This is the **only** path that lands on UWP / WebView2 / DirectComposition surfaces because those route input through `Windows.UI.Input`, not `WM_LBUTTONDOWN` on the HWND message queue. 2. **PostMessage fallback** — if UIA returned false (no Invokable element under the pixel inside `hwnd`, or `hwnd` has no useful UIA tree at all), fire `PostMessage(WM_LBUTTONDOWN/UP)` against the deepest child HWND at the screen point. Covers plain Win32 native controls without UIA InvokePattern, custom-drawn widgets, native containers, and apps that don't expose UIA. The combined chain delivers what macOS gets for free from `CGEventPostToPSN` (per-pid event routing regardless of z-order): UIA covers UWP-style targets, PostMessage covers plain Win32. Both work occluded; neither activates the receiver, moves the OS cursor, or steals focus. Why not pure PostMessage: research (AutoHotkey, pywinauto, Robot Framework, WinAppDriver, Microsoft docs) confirms `WM_LBUTTONDOWN` does NOT work on UWP — not in foreground, not in background, never. UWP/XAML/WebView2 input arrives via `Windows.UI.Input`, which is a completely separate pipeline from the HWND message queue. Even posting to the inner `Windows.UI.Core.CoreWindow1` child does nothing. Microsoft's blessed input-injection API for UWP (`InjectTouchInput`) requires `inputInjectionBrokered` capability + UWP packaging, so a plain Win32 binary can't use it. UIA Invoke is the only documented, no-focus-steal route to UWP buttons. Why not pure SendInput: SendInput is real OS input, routed by the windowing system to the topmost window at the cursor. Lands on whatever's covering the target; activates the receiver (focus steal). Wrong contract for vision-mode. UIA path runs only for single left/middle clicks. Right-click (UIA has no by-coord `ShowContextMenu`) and multi-click (`Invoke()` is single-fire) skip directly to PostMessage. Validated live on Win11 VM (Calculator UWP, 1024x801 layout, 62 UIA descendants under AppFrame subtree): 6 vision-mode clicks for 17x23 returned "Performed UIA Invoke at (sx,sy)"; display reads 391; History shows "17 x 23 ="; no foreground change; clicks succeeded through Terminal occlusion.
55faa3c to
6d80bfd
Compare
… + deploy at install time (#1553) * docs(skills): port cua-driver Skills/ to cua-driver-rs + add WINDOWS.md / LINUX.md Drop the macOS Skills/cua-driver/ tree into libs/cua-driver-rs/Skills/cua-driver-rs/ verbatim, then add platform-specific carve-outs for the cross-platform port: - `SKILL.md` — top-of-file platform-reading banner pointing Windows / Linux agents at the right carve-out. `name:` updated to `cua-driver-rs`. Body unchanged from the macOS source (still macOS-skewed; full shared-core extraction is a follow-up). - `WINDOWS.md` — new, 686 lines. Windows-specific carve-out covering: - No-foreground contract w/ Windows forbidden-list (`Start-Process`, `SetForegroundWindow`, `SwitchToThisWindow`, `cmd /c start`, `explorer.exe shell:AppsFolder\\…`, `AttachThreadInput` tricks, `SendInput`, Chromium `Ctrl+L` / `Ctrl+T` / `Ctrl+1..9`, Win+key shell shortcuts). - Intent → tool mapping table with Windows analogs. - CLI argument plumbing on Windows (stdin pipe vs argv quoting; PowerShell 5.1 limitations). - Prerequisites including the Session 0 vs Session 1+ distinction (UIA/screenshot fail silently in Session 0; daemon must run via the LogonType=Interactive Scheduled Task, not via SSH-into-Windows). - Click semantics: `element_index` mode = UIA Invoke; `(x,y)` mode = layered UIA hit-test in target HWND subtree → PostMessage fallback. Right-click and count>1 skip UIA Invoke per design. - UWP / packaged apps section: AUMID resolution, `ApplicationFrameHost.exe` cross-process hosting, Win11 Calc/Notepad/Settings AUMIDs. - Web apps on Windows section: Edge/Chrome via `launch_app({urls})`, forbidden browser shortcuts table, tabs-vs-windows pattern, WebView2 in non-browser hosts. - Common failure modes (Session 0, stale HWND, "Posted click" success messages on UWP that didn't actually click, browser tab-switch leak, AUMID resolution, cold-launch HWND race). - Diagnostics via `cua-driver doctor` and `autostart status`. - Recording: not-yet-supported note. - `LINUX.md` — new, 84 lines. Status placeholder. Marks the Linux backend as BETA, documents the X11/Wayland split, lists what works today and what doesn't. - `README.md` — updated header + Files section to reflect cross-platform structure; the WEB_APPS.md note now flags that Windows web-apps content lives in WINDOWS.md instead. `WEB_APPS.md`, `RECORDING.md`, `TESTS.md` — copied verbatim from the macOS skill. WEB_APPS.md remains macOS-only for now (Windows web-apps coverage is in WINDOWS.md's "Web apps on Windows" section). Refactoring `SKILL.md` into a shared core + `MACOS.md` adjunct is planned as a follow-up PR (PR B in the design split discussed in #1551 thread); this PR (PR A) ships the Windows carve-out without disrupting macOS workflows. Install plumbing (drop Skills/ at install time, expose via `cua-driver doctor`, optional `cua-driver skills install` verb) is also a follow-up. * feat(skills): opt-in `cua-driver skills install` verb (no auto-link on install) User feedback on the previous auto-link approach: don't touch ~/.claude/skills/ from the install script. Make it explicit. Match the same shape the user opted into — the verb fetches a versioned release asset from GitHub and symlinks into detected agent dirs. ## Subcommands - `cua-driver skills install` — fetch + place at <HomeDir>/skills/cua-driver-rs + symlink into each detected agent's skills dir. - `cua-driver skills update` — alias for `install --force`; re-fetch and refresh content. - `cua-driver skills uninstall [--all]` — remove agent symlinks; with `--all`, delete the local copy too. - `cua-driver skills status` — read-only probe of local + per-agent state. - `cua-driver skills path` — print the local skill pack path. - `--from main` — bleeding-edge mode: fetch each .md from raw.githubusercontent.com/trycua/cua/main instead of the tagged release. ## Fetch source Default: `https://github.com/trycua/cua/releases/download/cua-driver-rs-v<v>/cua-driver-rs-v<v>-skills.tar.gz` where `<v>` is the binary's own version (`CARGO_PKG_VERSION`). Pins the skill content to the binary release. CD now publishes this tarball as a per-release asset alongside the platform tarballs. ## Agent detection (same 4 the Swift cua-driver wires) - Claude Code: `~/.claude/skills/` - Codex: `~/.agents/skills/` - OpenClaw: `~/.openclaw/skills/` - OpenCode: `~/.config/opencode/skills/` (macOS/Linux), `%APPDATA%\opencode\skills\` (Windows) Only acts when the agent's parent skills dir already exists. Never overwrites an existing `<agent_skills>/cua-driver-rs` link — preserves dev users' hand-rolled symlinks. Windows uses `mklink /J` (no admin/devmode); macOS/Linux use `ln -s`. ## Install scripts revert Previously this branch had auto-link logic in install.ps1 / install.sh / install-local.{ps1,sh}. All four reverted: no skill staging from the release artifact, no agent-symlink loops. Post-install hint now points at the verb: ``` Agent skill pack (optional): cua-driver skills install ``` ## CD workflow - New step in the release job: package `Skills/cua-driver-rs/` into a single platform-agnostic `cua-driver-rs-v<v>-skills.tar.gz` asset and upload alongside the binary tarballs. - Reverted: the per-platform tarball changes (skills inside the macOS .app bundle, alongside the Linux/Windows binaries). The verb's dedicated asset is the only source. ## Module - New: `crates/cua-driver/src/skills.rs` (~390 LOC). HTTP via `ureq` (already a dep), tarball via `tar` + `flate2` (new deps). - `cli.rs`: new `Command::Skills { subcommand, flags }` variant + parsing + telemetry mapping (`cua_driver_skills_<sub>`). - `main.rs`: dispatch in both the macOS and other-OS branches. - Help text updated in cli.rs to document the verb. ## Telemetry `cua_driver_skills_install` / `_update` / `_uninstall` / `_status` / `_path` events, same shape as autostart's per-subcommand split. Respects the opt-out flag. ## Test plan - [x] Code review / structure - [ ] cua-driver-rs release with the skills.tar.gz asset - [ ] `cua-driver skills install` on the Win11 VM: fetches the tarball, places at `%USERPROFILE%\.cua-driver-rs\skills\cua-driver-rs`, junctions into `%USERPROFILE%\.claude\skills\cua-driver-rs` - [ ] `cua-driver skills status` reports correctly - [ ] `cua-driver skills uninstall --all` cleans up - [ ] macOS `ln -s` symlink path Follow-up issues for future work: - `cua-driver doctor` reports skill install state - `cua-driver skills install --local` flag pointing at install-local's staged copy * docs(skills): address CodeRabbit nits — cross-platform README, platform banners - README.md: fix stale `Skills/cua-driver` paths → `Skills/cua-driver-rs`. Split Prerequisites into macOS / Windows / Linux sections (the previous flat list was macOS-only despite the cross-platform claim in the intro). Update install + invocation snippets (`/cua-driver` → `/cua-driver-rs`). Replace yabai/SLEventPostToPid macOS-specific jargon in "What the skill covers" with a per-platform breakdown. - WEB_APPS.md: platform banner at top — doc is macOS-only; point Windows readers at WINDOWS.md's "Web apps on Windows" section and Linux at LINUX.md. - RECORDING.md: platform banner — recording is macOS-only today; documents the Windows "not yet supported" message + Linux unsupported status. - TESTS.md: platform banner — prompts reference NSWorkspace / cmd / Finder / Numbers; explicitly mark macOS-only and point Windows / Linux readers at their respective triage sections. - LINUX.md: expand the X11 vs Windows-SendInput comparison to explain why focus-stealing matters here (Windows avoids it via PostMessage; Linux has no equivalent per-window-message channel). Net effect: a reader landing on any platform-flavoured doc can immediately tell whether they're on the right page; all install / copy / invoke snippets in README work as written; no stale `cua-driver` (Swift) path references remain in the cua-driver-rs Skills tree.
Summary
The (x,y) click path's UIA Invoke fallback shipped in #1549 used desktop-wide
IUIAutomation::ElementFromPoint, which silently failed for vision-mode clicks against UWP / packaged-app frames on the Win11 VM:(sx, sy)(terminal, browser, etc.) madeElementFromPointreturn that window's element. Caller already names the intended HWND; we should respect it.ApplicationFrameHost.exehosts UWP content (CalculatorApp, Notepad-Win11, Settings, …) in a separate process.ElementFromPointwas observed returning the frame's outer Pane (noInvokePattern) instead of descending. The fallback PostMessage(WM_LBUTTONDOWN) silently no-ops on UWP because UWP routes input via Windows.UI.Input, not raw WM_LBUTTONDOWN.Net: vision-mode clicks on Calculator returned
Posted click to pid N(PostMessage path) and the calculator never updated.Fix
New helper
uia::windows_enum::try_invoke_in_window_at_point(hwnd, sx, sy)mirrors macOS vision-mode semantics (CGEvent delivered to a specific pid regardless of z-order):hwndviaElementFromHandleFindAll(TreeScope_Subtree, TrueCondition)CurrentBoundingRectanglecontains the point AND which exposesInvokePatternInvoke()The (x,y) click path now uses this instead of the desktop-wide hit-test. The original
try_invoke_at_pointstays in place (docstring updated to flag its desktop-wide nature) for any future callers without an HWND in scope.Result
Validated live on the Win11 VM (Calculator UWP) under
tsconconsole-attached state:Posted click to pid N(PostMessage fallback); display stayed at0.Performed UIA Invoke at (sx,sy); display reads391; History pane shows17 x 23 =; no foreground change.Test plan
Summary by CodeRabbit