fix(cua-driver-rs): close 4 functional gaps exposed by PR #1699 harness - #1705
Merged
Conversation
…1699 harness Four real cua-driver bugs the phase 2 harness exposed and documented as inverted-assertion regression guards — all now actually fixed end-to-end. 1) page.click_element probe double-decode (page.rs) The CDP runtime.evaluate response wraps the probe's stringified JSON in another JSON-string. The previous parser tried `serde_json::from_str(&probe_json).or_else(|_| ...)` — but `from_str` happily parses a quoted JSON-string into a `Value::String`, so the inner-decode branch never ran and parsed.get("vx") returned None. Match on Value::String and re-decode explicitly. 2) drag dispatch:foreground via SendInput (mouse.rs + impl_.rs) New `send_drag_synthesized` helper modelled on `send_click_synthesized`. PostMessage drag doesn't update the per-thread keyboard state that GetKeyState(VK_LBUTTON) reads, so frameworks polling Mouse.LeftButton during their drag handler (WPF Thumb.IsDragging) never see the button as held and the drag no-ops. SendInput goes through the system input queue and DOES update GetKeyState — WPF Slider thumbs now track. Same UIAccess foreground-lock caveat as send_click_synthesized. 3) Slider parent AID in UIA flat tree (uia/mod.rs) Added UIA_RangeValuePatternId to the cache pre-fetch list and to `detect_cached_actions`. Without it, Slider/ProgressBar parents reported `actions=[]` -> marked non-actionable -> no `[N]` index in the rendered tree -> unaddressable by AutomationId. Now they surface with `actions=[set_value]` like ValuePattern targets, and the set_value tool already falls through to RangeValuePattern. 4) WebView2 CDP listener actually works (test fix only) No cua-driver change — earlier "WebView2 filters --remote-debugging-port" hypothesis was wrong. The real reason the page-tool test failed against WebView2 was the same `/json` read_to_end bug fixed in PR #1699's commit be1581e. The flag IS honoured; CDP listens on the configured port; the earlier discovery hang was the shared underlying bug. Upgraded harness_webview_window_discoverable -> harness_webview_page_tool with full execute_javascript + click_element coverage. Verification: cargo test --test harness_wpf_test -> 18/18 cargo test --test harness_winui3_test -> 7/7 cargo test --test harness_web_test -> 5/5 (+1 from upgrade) cargo test --test harness_bg_modality_test -> 8/8 The remaining open ship-blockers documented by PR #1699 are the two WPF UIA focus-steal cases (Invoke + SetValue trigger UIElement.Focus() in the target process before cua-driver gets control back). Those require the cua-driver-uia.exe UIAccess worker to fix and remain inverted-assertion regression guards. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
📝 WalkthroughWalkthroughThis PR implements SendInput-based drag synthesis for Windows UI automation, extends UIA pattern detection to recognize slider set_value actions, fixes probe JSON double-encoding in click operations, and adds regression tests for drag and click across WebView2, Electron, WinUI3, and WPF platforms. ChangesDrag synthesis, action detection, and cross-platform testing
Sequence Diagram(s)sequenceDiagram
participant DragTool
participant send_drag_synthesized
participant SendInput
participant SetCursorPos
participant SetForegroundWindow
DragTool->>send_drag_synthesized: target, coords, duration, steps
send_drag_synthesized->>SetForegroundWindow: swap foreground
send_drag_synthesized->>SendInput: inject MOUSEEVENTF_MOVE + down
loop interpolate drag path
send_drag_synthesized->>SetCursorPos: position cursor
send_drag_synthesized->>SendInput: MOUSEEVENTF_MOVE
end
send_drag_synthesized->>SendInput: inject mouse up
send_drag_synthesized->>SetForegroundWindow: restore foreground
send_drag_synthesized-->>DragTool: Ok(())
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs`:
- Around line 445-447: The send_drag_synthesized implementation currently sets
step_delay_ms to 0 when steps == 1, causing the single MOVE to have no delay;
change the calculation so step_delay_ms is duration_ms when steps == 1 (i.e.,
use step_delay_ms = if steps > 1 { duration_ms / steps as u64 } else {
duration_ms }) while keeping the existing steps = steps.max(1) logic; update the
send_drag_synthesized function and any related comments so the Windows SendInput
path honors duration_ms for the single-step drag like the PostMessage and
macOS/Linux paths.
- Around line 484-501: In send_drag_synthesized, the SendInput calls for the
interpolated move (mv) and final release (release) ignore return values and
therefore can leave the button logically held; change both SendInput invocations
to check the returned sent count (e.g., compare sent as usize == mv.len() and ==
release.len()) and handle partial/zero insertions the same way the drag prelude
does (restore state or return an error), and also ensure duration_ms is honored
when steps == 1 by making step_delay_ms use duration_ms in that case (or
special-case a single-step delay to wait duration_ms before sending the release,
consistent with post_drag). Ensure you reference send_drag_synthesized, mv,
release, SendInput, steps, step_delay_ms, duration_ms and post_drag when
applying the fixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09852dde-deff-425d-9140-e1fb372dac75
📒 Files selected for processing (7)
libs/cua-driver/rust/crates/cua-driver/tests/harness_web_test.rslibs/cua-driver/rust/crates/cua-driver/tests/harness_winui3_test.rslibs/cua-driver/rust/crates/cua-driver/tests/harness_wpf_test.rslibs/cua-driver/rust/crates/platform-windows/src/input/mouse.rslibs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rslibs/cua-driver/rust/crates/platform-windows/src/tools/page.rslibs/cua-driver/rust/crates/platform-windows/src/uia/mod.rs
Comment on lines
+445
to
+447
| let steps = steps.max(1); | ||
| let step_delay_ms = if steps > 1 { duration_ms / steps as u64 } else { 0 }; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
# Show the exact section around the reported lines
echo "=== Context around $FILE:430-470 ==="
nl -ba "$FILE" | sed -n '420,480p'
# Search for step_delay_ms usage within the file
echo "=== step_delay_ms usages in $FILE ==="
rg -n "step_delay_ms" "$FILE" -n
# Find any other mention of duration_ms/steps in the file
echo "=== duration_ms and steps in $FILE ==="
rg -n "duration_ms|steps\.max" "$FILE" -nRepository: trycua/cua
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
echo "=== Context around $FILE:430-470 ==="
cat -n "$FILE" | sed -n '420,480p'
echo "=== step_delay_ms usages in $FILE ==="
rg -n "step_delay_ms" "$FILE"
echo "=== duration_ms and steps in $FILE ==="
rg -n "duration_ms|steps\.max" "$FILE"
echo "=== Search for duration/steps/drag contract clues in repo ==="
rg -n "drag.*duration|duration.*drag|steps.*drag|step_delay_ms" libs/cua-driver/rust/crates/platform-windows/src/input libs/cua-driver/rust/crates -S || trueRepository: trycua/cua
Length of output: 7138
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
echo "=== mouse.rs: lines 90-200 ==="
cat -n "$FILE" | sed -n '80,210p'
echo "=== mouse.rs: lines 360-540 ==="
cat -n "$FILE" | sed -n '360,540p'
echo "=== macOS mouse.rs: step_delay_ms logic ==="
MAC="libs/cua-driver/rust/crates/platform-macos/src/input/mouse.rs"
cat -n "$MAC" | sed -n '220,340p'
echo "=== Linux mod.rs: step_delay_ms logic ==="
LIN="libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs"
cat -n "$LIN" | sed -n '60,170p'Repository: trycua/cua
Length of output: 23555
Honor duration_ms when steps == 1 in Windows SendInput drag.
send_drag_synthesized sets step_delay_ms to 0 when steps == 1, so the only MOVE step isn’t followed by any delay (unlike the Windows PostMessage path and macOS/Linux, which use else { duration_ms }).
🛠️ Proposed fix
- let step_delay_ms = if steps > 1 { duration_ms / steps as u64 } else { 0 };
+ let step_delay_ms = if steps > 1 { duration_ms / steps as u64 } else { duration_ms };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let steps = steps.max(1); | |
| let step_delay_ms = if steps > 1 { duration_ms / steps as u64 } else { 0 }; | |
| let steps = steps.max(1); | |
| let step_delay_ms = if steps > 1 { duration_ms / steps as u64 } else { duration_ms }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs` around lines
445 - 447, The send_drag_synthesized implementation currently sets step_delay_ms
to 0 when steps == 1, causing the single MOVE to have no delay; change the
calculation so step_delay_ms is duration_ms when steps == 1 (i.e., use
step_delay_ms = if steps > 1 { duration_ms / steps as u64 } else { duration_ms
}) while keeping the existing steps = steps.max(1) logic; update the
send_drag_synthesized function and any related comments so the Windows SendInput
path honors duration_ms for the single-step drag like the PostMessage and
macOS/Linux paths.
Comment on lines
+484
to
+501
| for i in 1..=steps { | ||
| let t = i as f64 / steps as f64; | ||
| let x = sx_from + ((sx_to - sx_from) as f64 * t).round() as i32; | ||
| let y = sy_from + ((sy_to - sy_from) as f64 * t).round() as i32; | ||
| let (nx, ny) = norm(x, y); | ||
| let _ = SetCursorPos(x, y); | ||
| let mv = [make_input(nx, ny, MOUSEEVENTF_MOVE)]; | ||
| let _ = SendInput(&mv, std::mem::size_of::<INPUT>() as i32); | ||
| if step_delay_ms > 0 { | ||
| sleep(Duration::from_millis(step_delay_ms)); | ||
| } | ||
| } | ||
|
|
||
| // 3. Release at the end. | ||
| let (ntx, nty) = norm(sx_to, sy_to); | ||
| let release = [make_input(ntx, nty, up_flag)]; | ||
| let _ = SendInput(&release, std::mem::size_of::<INPUT>() as i32); | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
# Show relevant function/region with line numbers around 430-520
nl -ba "$FILE" | sed -n '420,525p'
# Also search for SendInput usage in this file to compare patterns
rg -n "SendInput\\(" "$FILE"
# Show definitions of any variables referenced in proposed fix (prev_cursor, prev_fg, up_flag) if present
rg -n "prev_cursor|prev_fg|up_flag" "$FILE"Repository: trycua/cua
Length of output: 97
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
# Show relevant function/region with line numbers around 420-525
nl -ba "$FILE" | sed -n '420,525p'
# Search for SendInput usage patterns in this file
rg -n "SendInput\\(" "$FILE"
# Search for variables mentioned in the proposed diff
rg -n "prev_cursor|prev_fg|up_flag" "$FILE"Repository: trycua/cua
Length of output: 97
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
nl -ba "$FILE" | sed -n '420,525p'
rg -n "SendInput\\(" "$FILE"
rg -n "prev_cursor|prev_fg|up_flag" "$FILE"Repository: trycua/cua
Length of output: 97
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
# Show relevant function/region with line numbers around 420-525 (use cat -n since nl missing)
cat -n "$FILE" | sed -n '420,525p'
# Search for SendInput usage patterns in this file
rg -n "SendInput\\(" "$FILE"
# Search for variables mentioned in the proposed diff
rg -n "prev_cursor|prev_fg|up_flag" "$FILE"Repository: trycua/cua
Length of output: 5577
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
rg -n "post_drag|duration_ms|step_delay_ms|steps > 1|drag.*duration" "$FILE"Repository: trycua/cua
Length of output: 506
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
cat -n "$FILE" | sed -n '90,210p'Repository: trycua/cua
Length of output: 5747
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
# Find the function definition line numbers
rg -n "send_drag_synthesized" "$FILE"
# Show a broader window around its signature and doc (to understand duration_ms contract)
cat -n "$FILE" | sed -n '320,430p'Repository: trycua/cua
Length of output: 5794
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs"
# Find call sites of send_drag_synthesized
rg -n "send_drag_synthesized\\(" -S .
# Search for mentions of duration_ms contract in code/docs near drag usage
rg -n "duration_ms" libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs
rg -n "send_drag_synthesized|post_drag" -S libs/cua-driver/rust/crates/platform-windows/srcRepository: trycua/cua
Length of output: 941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs"
# Show around the send_drag_synthesized call and any computation of duration_ms/steps
cat -n "$FILE" | sed -n '3380,3520p'Repository: trycua/cua
Length of output: 7918
Check SendInput return values for synthesized drag move + release
In send_drag_synthesized, the SendInput results are ignored for the interpolated move (mv, line 491) and final release (release, line 500). The drag prelude already checks for partial insertion and bails/restores state; the same sent as usize != ...len() handling should be added here to avoid cases where the release isn’t actually inserted (leaving the button logically held).
duration_ms is also not honored when steps == 1 in send_drag_synthesized (step_delay_ms becomes 0 at line 446), unlike post_drag which uses duration_ms in that case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/cua-driver/rust/crates/platform-windows/src/input/mouse.rs` around lines
484 - 501, In send_drag_synthesized, the SendInput calls for the interpolated
move (mv) and final release (release) ignore return values and therefore can
leave the button logically held; change both SendInput invocations to check the
returned sent count (e.g., compare sent as usize == mv.len() and ==
release.len()) and handle partial/zero insertions the same way the drag prelude
does (restore state or return an error), and also ensure duration_ms is honored
when steps == 1 by making step_delay_ms use duration_ms in that case (or
special-case a single-step delay to wait duration_ms before sending the release,
consistent with post_drag). Ensure you reference send_drag_synthesized, mv,
release, SendInput, steps, step_delay_ms, duration_ms and post_drag when
applying the fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #1699 landed the test harness with 6 documented cua-driver gaps. Two were architectural ship-blockers (UIA focus-steal on WPF — needs UIAccess worker). The other 4 were code-level bugs each fixable in this branch, and all 4 are now closed end-to-end.
Fixes
1.
page.click_elementprobe double-decodecrates/platform-windows/src/tools/page.rs. The CDPruntime.evaluateresponse for the probe JS wraps the stringified JSON in another JSON-string. The parser usedserde_json::from_str(&probe_json).or_else(|_| ...)— butfrom_strparses a quoted JSON-string into aValue::String(not an error), so the inner-decode branch never ran andparsed.get("vx")returned None. Match onValue::Stringand re-decode explicitly.Test:
harness_electron_click_element(was_DOCUMENTED_wrapper_bug).2.
dragtool:dispatch:"foreground"via SendInputcrates/platform-windows/src/input/mouse.rs(new helper) +tools/impl_.rs(wiring). PostMessage drag emitsWM_LBUTTONDOWN/MOUSEMOVE/LBUTTONUPbut doesn't update the per-thread input state thatGetKeyState(VK_LBUTTON)reads. Frameworks that pollMouse.LeftButtonduring their drag handler (WPFThumb.IsDragging) never see the button as held, and the drag no-ops. Newsend_drag_synthesizedmirrorssend_click_synthesized:SetForegroundWindow, then interpolatedSetCursorPos+MOUSEEVENTF_MOVEfrom start to end, then release. SendInput goes through the system input queue and DOES update GetKeyState.Test:
harness_wpf_slider_drag— WPF Slider thumb now tracks.3. Slider parent AutomationId not in UIA flat tree
crates/platform-windows/src/uia/mod.rs. The UIA cache pre-fetch anddetect_cached_actionsdidn't check forRangeValuePattern. Sliders/ProgressBars implement that pattern (not Value/Invoke/Toggle/etc.), so the parent reportedactions=[]→ marked non-actionable → no[N]index in the tree → unaddressable by AutomationId. AddedUIA_RangeValuePatternIdto both the cache schema and the action detector.Test:
harness_winui3_slider_set_value— Slider parent now indexed andset_valuereachesRangeValuePattern.SetValue.4. WebView2 CDP listener (test fix only)
The earlier "WebView2 filters
--remote-debugging-port" hypothesis was wrong. The real reason the page-tool test failed was the same/jsonread_to_endbug fixed in PR #1699 (commitbe1581e5). WebView2 does honour the flag; CDP listens on the configured port (verified manually + via the now-passing test). Upgradedharness_webview_window_discoverableto a fullharness_webview_page_tooltest withexecute_javascript+click_elementagainst the shared HTML.Test plan
cargo test --test harness_wpf_test -- --ignored --test-threads=1→ 18/18cargo test --test harness_winui3_test -- --ignored --test-threads=1→ 7/7cargo test --test harness_web_test -- --ignored --test-threads=1→ 5/5 (was 4)cargo test --test harness_bg_modality_test -- --ignored --test-threads=1→ 8/8Total: 38 tests passing, 0 failures, 0 regressions.
Still open
The two WPF UIA focus-steal cases from PR #1699:
bg_modality_uia_invoke_click_DOCUMENTED_steals_focusbg_modality_set_value_DOCUMENTED_steals_focusRoot cause is WPF's automation peers calling
UIElement.Focus()→SetForegroundWindowsynchronously inside the UIA pattern handler, in the target process, before cua-driver gets control back. Mitigation requires routing UIA activations throughcua-driver-uia.exe(UIAccess-manifested worker). Tracked separately.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests