Skip to content

feat(cua-driver-rs)(harness): WebView + Electron + background-modality + capture-mode coverage - #1699

Merged
f-trycua merged 9 commits into
mainfrom
cua-driver-rs-test-harness-phase2
May 26, 2026
Merged

feat(cua-driver-rs)(harness): WebView + Electron + background-modality + capture-mode coverage#1699
f-trycua merged 9 commits into
mainfrom
cua-driver-rs-test-harness-phase2

Conversation

@f-trycua

@f-trycua f-trycua commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Phase 2 of the Windows test harness. Comprehensive coverage of cua-driver's Windows automation surface across 4 host types (WPF, WinUI3, WebView2, Electron) with 36 integration tests, 3 real cua-driver bug fixes landed, and 6 additional gaps documented as regression guards.

What's new

libs/cua-driver/test-harness/
├── shared-web/index.html            # single DOM, loaded by both WebView2 + Electron
├── CuaTestHarness.WebView/          # .NET 8 + Microsoft.Web.WebView2.Wpf
└── CuaTestHarness.Electron/         # Electron 31 unpacked-runtime stage

libs/cua-driver/rust/crates/cua-driver/tests/
├── harness_wpf_test.rs              # 18 tests: slider, checkbox, radio, combo, list, menu, ...
├── harness_winui3_test.rs           #  7 tests: counter, type_text, slider, popup, ...
├── harness_web_test.rs              #  4 tests: WebView/Electron CDP discovery + page tool
└── harness_bg_modality_test.rs      #  8 tests: focus-steal sentinel + ax/vision modalities

cua-driver fixes landed in this branch

  1. click tool: multi-pattern dispatch (commit 75aa5ea8). Tries Invoke → Toggle → SelectionItem → ExpandCollapse before falling through to PostMessage. Closes the WinUI3 CheckBox / RadioButton / ComboBox no-op gaps the harness caught — those controls' UIA patterns are now actually invoked instead of being silently dropped.

  2. set_value tool: RangeValuePattern fallback (commit 14c1db3f). Tries ValuePattern first, falls through to RangeValuePattern.SetValue(double) for Sliders / ProgressBars / numeric ranges. Returns an actionable error pointing at the click tool for Toggle / SelectionItem targets.

  3. CDP /json discovery: header-driven body reads (commit be1581e5). cua-driver's CDP discovery was using read_to_end after sending Connection: close, but Chromium ignores that header and keeps the socket alive — so the discovery hung until the 10 s timeout. Now parses Content-Length / Transfer-Encoding headers and reads exactly the body bytes. Unlocks page tool against Electron and any Chromium-CDP target.

Background-modality findings (the core promise of cua-driver Windows)

The bg-modality suite uses the existing focus-monitor-win sentinel to verify cua-driver actions don't steal foreground from the user.

cua-driver call Path Result
get_window_state (som / ax / vision) UIA + WGC capture ✅ no focus steal
press_key (F5) PostMessage WM_KEYDOWN ✅ no focus steal
scroll (down) PostMessage WM_VSCROLL ✅ no focus steal
click → UIA Invoke (WPF Button) UIA accessibility channel ⚠️ focus steal
set_value → UIA ValuePattern (WPF TextBox) UIA accessibility channel ⚠️ focus steal

The WPF focus-steal gaps remain — root cause is WPF's ButtonBase.OnClick and TextBoxAutomationPeer.SetValue calling UIElement.Focus() synchronously, which routes through SetForegroundWindow. The existing EnableWindow(false) bypass only gates the input-queue path, not SetForegroundWindow. The user-foreground restoration trick also fails because non-UIAccess processes can't bypass the foreground-lock. Real mitigation requires routing UIA activations through cua-driver-uia.exe (UIAccess-manifested worker) — tracked as bg_modality_uia_invoke_click_DOCUMENTED_steals_focus and bg_modality_set_value_DOCUMENTED_steals_focus.

Capture-mode coverage

capture_mode Tested by Asserts
som (default) bg-modality + WPF/WinUI3 suites tree + image both present
ax capture_mode_ax_returns_tree_only tree present, no image content
vision capture_mode_vision_returns_image_only image present, no tree markdown
ax → invoke → vision+ax roundtrip capture_mode_ax_and_vision_invoke_roundtrip full agent flow

Other documented gaps (regression-guarded)

  • Slider AutomationId not in UIA flat tree — both WPF and WinUI3 Slider parents don't get an [N] element_index entry. Tracked by harness_winui3_slider_DOCUMENTED_unreachable.
  • PostMessage drag against WPF Slider Thumb — PostMessage WM_LBUTTONDOWN doesn't update OS mouse-state visible to GetKeyState, so WPF's Slider drag-tracking never sees the button-held state. harness_wpf_slider_drag_tool_returns verifies the tool returns success but doesn't move the value. Companion harness_wpf_slider_increase_large provides positive coverage via UIA Invoke on the IncreaseLarge sub-button.
  • WebView2 --remote-debugging-port filtered when passed via AdditionalBrowserArguments — no CDP listener appears. Harness config TODO, not a cua-driver issue.
  • page.click_element probe-JSON wrapper — the CDP probe runs JS returning a JSON object (vx/vy/sx/sy/dpr) but the response is wrapped as a CDP runtime.evaluate.user_gesture string, causing "probe JSON missing required field vx". Tracked by harness_electron_click_element_DOCUMENTED_wrapper_bug.

Test plan

  • cargo test --test harness_wpf_test -- --ignored --test-threads=118/18
  • cargo test --test harness_winui3_test -- --ignored --test-threads=17/7
  • cargo test --test harness_web_test -- --ignored --test-threads=14/4
  • cargo test --test harness_bg_modality_test -- --ignored --test-threads=18/8
  • Sandbox run — pending merge

🤖 Generated with Claude Code

f-trycua and others added 2 commits May 25, 2026 23:01
…HTML, slider/combo/check/menu coverage

Phase 2 of the Windows test harness adds:

**WebView2 + Electron hosts** loading a single shared HTML page
(`shared-web/index.html`), so cua-driver's `page` tool can be exercised
against two Chromium-based hosts with identical expected behaviour:

  CuaTestHarness.WebView/    .NET 8 + Microsoft.Web.WebView2.Wpf
                             Exposes --remote-debugging-port via
                             AdditionalBrowserArguments (CDP listener
                             needs WebView2-config follow-up — TODO).
  CuaTestHarness.Electron/   Electron 31 unpacked-runtime stage
                             (electron-builder portable mode needs
                             admin for winCodeSign symlinks; flat
                             stage of node_modules/electron/dist
                             renamed to CuaTestHarness.Electron.exe).
                             CDP listener confirmed via
                             --remote-debugging-port flag.
  shared-web/index.html      counter, text_input, slider, click_target,
                             checkable_controls, combo_box,
                             navigation anchor. data-cua-id attrs match
                             the AutomationIds used by the WPF host.

**Slider scenario + drag tool coverage:**
  - WPF Slider doesn't surface its parent AutomationId in the
    flat UIA index list; the SliderAutomationPeer reports its
    DecreaseLarge / IncreaseLarge / Thumb child parts.
  - `harness_wpf_slider_drag_tool_returns` exercises the drag tool
    but doesn't assert value-change — a documented gap caused by
    PostMessage WM_LBUTTONDOWN not updating OS mouse-button state
    visible to GetKeyState (WPF Slider Thumb polls this).
  - `harness_wpf_slider_increase_large` covers the slider via UIA
    Invoke on the IncreaseLarge sub-button — works on a backgrounded
    window.

**Additional WPF control coverage:**
  - checkable_controls: CheckBox toggle + RadioButton group select
  - combo_box: expand + click-item (WPF ComboBox doesn't expose
    ValuePattern at the parent)
  - list_box: UIA SelectionItem.Select on a ListBoxItem
  - menus: top-level Menu expand + invoke; ContextMenu via right-click

**Documented cua-driver gaps** (tests assert error shape so the
gaps are tracked, not silently regressed):
  - CDP `/json` HTTP read uses `stream.read_to_end()` but Chromium
    keeps the socket alive (ignoring `Connection: close`); discovery
    hangs to the 10 s timeout. Confirmed against Electron 31 on
    port 9223 via `harness_electron_page_tool_documented_gap`.
  - WebView2 `--remote-debugging-port` in AdditionalBrowserArguments
    appears to be filtered — no CDP listener on the WebView helper
    processes. Tracked as a harness-config TODO.

Local Win11 verification:
  cargo test --test harness_wpf_test -- --ignored --test-threads=1
    -> 18 passed; 0 failed (was 11, added 7)
  cargo test --test harness_web_test -- --ignored --test-threads=1
    -> 3 passed; 0 failed (smoke + Electron-gap regression guard)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rage

Exercises the core cua-driver promise — background automation must not
steal focus from the user — using the existing focus-monitor-win
sentinel:

  1. Launch CuaTestHarness.Wpf (becomes briefly foreground on activate)
  2. Launch focus-monitor-win (its ShowWindow displaces the harness to
     z+1; sentinel is z+0)
  3. Reset sentinel act/key loss counters to 0
  4. Run a single cua-driver action against the harness
  5. Assert sentinel act_losses delta == 0

8 tests total. 6 are positive assertions, 2 are DOCUMENTED-failure
regression guards (asserting the gap currently exists; once cua-driver
adds foreground-restoration the assertions flip).

Positive (no focus steal):
  get_window_state(som | ax | vision)
  press_key f5     - PostMessage WM_KEYDOWN
  scroll down line - PostMessage WM_VSCROLL
  ax + invoke + vision roundtrip - end-to-end agent flow

Capture modality coverage (per follow-up ask):
  capture_mode=ax     - tree_markdown only, no image content
  capture_mode=vision - image only, no tree markdown

Documented cua-driver focus-steal gaps:
  click(...) -> UIA Invoke on a WPF Button transfers Win32 focus
  to the WPF window (WPF's ButtonBase.OnClick calls Focus()). Overlay
  is NOT the cause (verified with set_agent_cursor_enabled:false).

  set_value(...) -> UIA ValuePattern.SetValue on a WPF TextBox
  similarly transfers focus (TextBoxAutomationPeer.SetValue path).

Both gaps were caught by sentinel act_losses delta=1 after each action.
The mitigation is to wrap the UIA call with GetForegroundWindow() +
post-action SetForegroundWindow(prev_fg) (the AttachThreadInput
restoration pattern already used by `bring_to_front`).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fad52a61-0e73-4d13-b9b0-9961ca4276bb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR introduces WebView and Electron test harness applications alongside comprehensive Windows integration tests for the cua-driver tool. A shared web UI (HTML/JavaScript) is used by both browser-based harnesses. The existing WPF harness is extended with new interactive controls (slider, checkbox, radio, combo box, list box, menu). Integration tests verify background-safe driver behavior, window discovery via Chrome DevTools Protocol, and UI interactions across all three harness platforms.

Changes

CUA Driver Test Harness Expansion

Layer / File(s) Summary
WebView Harness Implementation
CuaTestHarness.WebView/App.xaml, App.xaml.cs, MainWindow.xaml, MainWindow.xaml.cs, CuaTestHarness.WebView.csproj, app.manifest, .gitignore
New WPF application hosting a WebView2 control; reads CUA_WEBVIEW_CDP_PORT (default 9222), configures remote debugging port, and loads web/index.html from build output.
Electron Harness Implementation
CuaTestHarness.Electron/main.js, package.json, build.ps1, .gitignore
New Electron entry point and build staging script; sets CDP port from CUA_ELECTRON_CDP_PORT (default 9223), enforces deterministic window title, and stages Electron runtime with bundled web assets into CuaTestHarness.Electron.exe.
Shared Web Content and Scenarios Configuration
shared-web/index.html, scenarios.json
HTML/JavaScript UI controls (counter, text input, slider, click target, checkable controls, combo box, list box, navigation) used by both WebView and Electron harnesses. Scenarios JSON extended with new WPF UI test targets (slider, checkbox/radio, combo, list, menu) and new harness configurations for WebView and Electron with CDP port metadata.
Build Infrastructure and Solution Configuration
CuaTestHarness.sln, build.ps1, rust/test-apps/.gitignore
Visual Studio solution added WebView project entry and build configurations; PowerShell build script expanded with webview and electron targets, including conditional publishing and Electron build orchestration; .gitignore entries for built harness artifacts.
WPF Harness UI Extensions
CuaTestHarness.Wpf/MainWindow.xaml, MainWindow.xaml.cs
Expanded WPF main window with new GroupBox sections: slider with value display, checkbox/radio group with agreed/priority state aggregation, combo box with color selection, list box with item selection, and menu/context-menu for action tracking. Corresponding event handlers update snapshot-accessible labels.
Background Modality and Capture Mode Tests
tests/harness_bg_modality_test.rs
Windows-only integration tests verifying background-safe driver actions don't trigger focus/activation loss counters via a sentinel process. Asserts get_window_state, press_key, scroll, and capture-mode switching (ax, vision) preserve focus. Documents two known UIA focus-stealing gaps (Invoke button click, SetValue textbox) via inverted assertions.
Web Harness Integration Tests
tests/harness_web_test.rs
Windows-only integration tests for WebView and Electron harness discovery via JSON-RPC list_windows polling. Documents a known CDP /json discovery gap in Electron via an error-path assertion test.
Extended WPF UI Interaction Tests
tests/harness_wpf_test.rs
Windows-only integration tests for new WPF controls: slider drag tool and Invoke increase actions, checkbox toggle, radio button selection, combo box item selection via dropdown expansion, list box item selection, and menu/context-menu invocation. Tests validate state changes via UIA tree re-snapshots.

Sequence Diagram(s)

sequenceDiagram
  participant Rust as Rust Test
  participant CuaDriver as cua-driver JSON-RPC
  participant WpfHarness as WPF Harness
  participant Sentinel as focus-monitor-win Sentinel
  Rust->>Sentinel: launch & wait for pid/hwnd
  Rust->>CuaDriver: launch with CUA_DRIVER_CDP_PORT
  Rust->>CuaDriver: init() JSON-RPC
  Rust->>CuaDriver: list_windows to get harness window id
  Rust->>Sentinel: snapshot activation/key loss counts
  Rust->>CuaDriver: call tools/call with action (get_window_state, press_key, etc.)
  CuaDriver->>WpfHarness: perform action
  Rust->>Sentinel: re-snapshot counts
  Rust->>Rust: assert deltas remain zero (focus-preserving)
Loading
sequenceDiagram
  participant Rust as Rust Test
  participant CuaDriver as cua-driver JSON-RPC
  participant WebViewHost as WebView2 Host (or Electron)
  Rust->>WebViewHost: launch with CUA_WEBVIEW_CDP_PORT / CUA_ELECTRON_CDP_PORT
  Rust->>CuaDriver: launch with CUA_DRIVER_CDP_PORT
  Rust->>CuaDriver: init() JSON-RPC
  Rust->>CuaDriver: list_windows polling loop
  CuaDriver->>WebViewHost: discover window via CDP
  Rust->>Rust: assert window title found in list
Loading
sequenceDiagram
  participant Rust as Rust Test
  participant CuaDriver as cua-driver JSON-RPC
  participant WpfHarness as WPF Harness UI
  Rust->>WpfHarness: focus window
  Rust->>CuaDriver: get_window_state with capture_mode=ax
  CuaDriver->>WpfHarness: query UIA tree
  Rust->>Rust: locate element by AutomationId in snapshot
  Rust->>CuaDriver: click with dispatch=foreground
  CuaDriver->>WpfHarness: execute click action
  Rust->>CuaDriver: get_window_state again
  Rust->>Rust: re-snapshot and assert state changed (e.g., slider_value, selected item)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • trycua/cua#1692: Background modality tests in this PR exercise the same background-vs-foreground dispatch semantics and z-order/get_window_state changes introduced there.
  • trycua/cua#1666: Web harness tests explicitly call the driver's page tool with execute_javascript commands, depending on the tool unification and cross-platform registration change introduced there.
  • trycua/cua#1698: Extends the same harness_wpf_test.rs test file and builds on the harness-launch and snapshot/assert patterns established in that PR.

Poem

🐰 WebViews and Electrons dance in tests so bright,
Background modes and snapshots held just right,
Sliders click and menus flow with cheer,
Focus-safe with each interaction clear—
New harnesses bloom, the driver's got its pair! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main changes: adding WebView and Electron harness hosts, background-modality testing, and capture-mode coverage for the cua-driver test suite.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cua-driver-rs-test-harness-phase2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 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/cua-driver/tests/harness_bg_modality_test.rs`:
- Around line 48-50: read_count currently swallows read/parse errors and returns
0, which can mask failures; change read_count to surface errors instead of
treating them as zero — either (preferred) change its signature to return
Result<u32, Box<dyn std::error::Error>> and propagate read_to_string/parse
errors to the caller, or (acceptable for tests) replace the
ok().and_then(...).unwrap_or(0) chain with explicit error handling (e.g., use
read_to_string(p)? and s.trim().parse()? or .expect with a clear message
including the path) so failures cause the test to fail rather than returning 0.
Ensure references to the function name read_count are updated where called to
handle the Result if you choose the Result-returning approach.
- Around line 159-162: When setup() times out waiting for the sentinel files it
currently returns None but leaves the spawned child processes running; update
the timeout branch to cleanly terminate those children: capture the
std::process::Child handles created in setup(), and on the deadline timeout call
child.kill().ok() and then child.wait().ok() (or send a graceful shutdown if
available) for each child before returning None so no stray processes remain.

In `@libs/cua-driver/rust/crates/cua-driver/tests/harness_web_test.rs`:
- Line 197: The test currently indexes
resp["result"]["content"][0]["text"].as_str().unwrap_or("") which can panic if
"content" is missing, not an array, or empty; change the extraction to safely
traverse the JSON (e.g., use resp.get("result").and_then(|r|
r.get("content")).and_then(|c| c.as_array()).and_then(|arr|
arr.get(0)).and_then(|item| item.get("text")).and_then(|t|
t.as_str()).unwrap_or("")) so the code checks for existence and array-ness
before accessing index 0 and avoids unwraps that can panic; update the usage in
harness_web_test.rs where resp is parsed to use this safe path.

In `@libs/cua-driver/test-harness/build.ps1`:
- Around line 74-83: The Electron build invocation (& $elecBuild) can throw a
terminating error and bypass the subsequent $LASTEXITCODE warning handling; wrap
the call to $elecBuild in a try/catch around the & $elecBuild invocation inside
the if (Test-Path $elecBuild) block so any thrown terminating errors are caught,
set or inspect $LASTEXITCODE as before, and emit the same "[WARN] Electron build
failed (exit $LASTEXITCODE)" message in the catch path to ensure failures
degrade to warnings rather than aborting the harness build.

In `@libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js`:
- Around line 9-10: The code forwards process.env.CUA_ELECTRON_CDP_PORT directly
into CDP_PORT and app.commandLine.appendSwitch('remote-debugging-port',
CDP_PORT) without validation; update the handling so you parse and validate
process.env.CUA_ELECTRON_CDP_PORT (trim, parseInt base 10), ensure it is an
integer within 1–65535, and only use it when valid; otherwise log a warning and
fall back to the default '9223' before calling app.commandLine.appendSwitch;
refer to the CDP_PORT constant, process.env.CUA_ELECTRON_CDP_PORT, and
app.commandLine.appendSwitch to locate and change the code.
- Around line 34-36: The loadFile promise on mainWindow is not handled for
failures; update the mainWindow.loadFile(...) usage (the call that currently
chains .then(() => mainWindow.setTitle(fixedTitle))) to handle errors
explicitly: either await it inside an async init function wrapped in try/catch
or append a .catch handler that logs the error (or shows an error dialog) and
takes a deterministic failure path (e.g., app.exit/non-zero or
mainWindow.destroy()); ensure mainWindow.setTitle(fixedTitle) only runs on
success.

In `@libs/cua-driver/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs`:
- Around line 37-39: Check for existence of the built HTML before creating a
URI: verify File.Exists(htmlPath) after computing htmlPath (from
Path.Combine(AppContext.BaseDirectory, "web", "index.html")) and fail fast if
missing by throwing a clear exception (e.g., FileNotFoundException with
htmlPath) or logging and exiting the app instead of constructing a Uri; update
the MainWindow.xaml.cs initialization that sets Wv.Source so it first performs
this existence check and only constructs new Uri(fileUri) when the file is
present.
- Around line 29-33: MainWindow.xaml.cs currently injects the raw
CUA_WEBVIEW_CDP_PORT value into AdditionalBrowserArguments (portStr) without
validation; parse the environment value returned by
Environment.GetEnvironmentVariable("CUA_WEBVIEW_CDP_PORT") into an integer,
ensure it's within a safe port range (e.g. 1024–65535) and not zero, and if
parsing fails or the value is out of range fall back to the default "9222" and
emit a warning/log; update the code paths that set portStr and the
CoreWebView2EnvironmentOptions.AdditionalBrowserArguments so they use the
validated/sanitized port and not the raw env string (refer to the portStr
variable and the CoreWebView2EnvironmentOptions creation in MainWindow.xaml.cs).
🪄 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: 68279bcb-2fdf-4264-982f-f23ce17b80a8

📥 Commits

Reviewing files that changed from the base of the PR and between f51ce80 and 1c3b522.

📒 Files selected for processing (21)
  • libs/cua-driver/rust/crates/cua-driver/tests/harness_bg_modality_test.rs
  • libs/cua-driver/rust/crates/cua-driver/tests/harness_web_test.rs
  • libs/cua-driver/rust/crates/cua-driver/tests/harness_wpf_test.rs
  • libs/cua-driver/rust/test-apps/.gitignore
  • libs/cua-driver/test-harness/CuaTestHarness.Electron/.gitignore
  • libs/cua-driver/test-harness/CuaTestHarness.Electron/build.ps1
  • libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js
  • libs/cua-driver/test-harness/CuaTestHarness.Electron/package.json
  • libs/cua-driver/test-harness/CuaTestHarness.WebView/.gitignore
  • libs/cua-driver/test-harness/CuaTestHarness.WebView/App.xaml
  • libs/cua-driver/test-harness/CuaTestHarness.WebView/App.xaml.cs
  • libs/cua-driver/test-harness/CuaTestHarness.WebView/CuaTestHarness.WebView.csproj
  • libs/cua-driver/test-harness/CuaTestHarness.WebView/MainWindow.xaml
  • libs/cua-driver/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs
  • libs/cua-driver/test-harness/CuaTestHarness.WebView/app.manifest
  • libs/cua-driver/test-harness/CuaTestHarness.Wpf/MainWindow.xaml
  • libs/cua-driver/test-harness/CuaTestHarness.Wpf/MainWindow.xaml.cs
  • libs/cua-driver/test-harness/CuaTestHarness.sln
  • libs/cua-driver/test-harness/build.ps1
  • libs/cua-driver/test-harness/scenarios/scenarios.json
  • libs/cua-driver/test-harness/shared-web/index.html

Comment on lines +48 to +50
fn read_count(p: &std::path::Path) -> u32 {
std::fs::read_to_string(p).ok().and_then(|s| s.trim().parse().ok()).unwrap_or(0)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t treat unreadable counter files as zero.

read_count returning 0 on read/parse failure can hide sentinel failures and produce false “no focus steal” passes.

Proposed fix
-fn read_count(p: &std::path::Path) -> u32 {
-    std::fs::read_to_string(p).ok().and_then(|s| s.trim().parse().ok()).unwrap_or(0)
+fn read_count(p: &std::path::Path) -> u32 {
+    let raw = std::fs::read_to_string(p)
+        .unwrap_or_else(|e| panic!("failed reading {:?}: {e}", p));
+    raw.trim()
+        .parse::<u32>()
+        .unwrap_or_else(|e| panic!("failed parsing {:?} as u32: {e}", p))
 }
📝 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
fn read_count(p: &std::path::Path) -> u32 {
std::fs::read_to_string(p).ok().and_then(|s| s.trim().parse().ok()).unwrap_or(0)
}
fn read_count(p: &std::path::Path) -> u32 {
let raw = std::fs::read_to_string(p)
.unwrap_or_else(|e| panic!("failed reading {:?}: {e}", p));
raw.trim()
.parse::<u32>()
.unwrap_or_else(|e| panic!("failed parsing {:?} as u32: {e}", p))
}
🤖 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/cua-driver/tests/harness_bg_modality_test.rs`
around lines 48 - 50, read_count currently swallows read/parse errors and
returns 0, which can mask failures; change read_count to surface errors instead
of treating them as zero — either (preferred) change its signature to return
Result<u32, Box<dyn std::error::Error>> and propagate read_to_string/parse
errors to the caller, or (acceptable for tests) replace the
ok().and_then(...).unwrap_or(0) chain with explicit error handling (e.g., use
read_to_string(p)? and s.trim().parse()? or .expect with a clear message
including the path) so failures cause the test to fail rather than returning 0.
Ensure references to the function name read_count are updated where called to
handle the Result if you choose the Result-returning approach.

Comment on lines +159 to +162
if std::time::Instant::now() > deadline {
eprintln!("focus-monitor sentinel never published pid/hwnd files");
return None;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Ensure spawned processes are cleaned up on setup timeout.

If the sentinel pid/hwnd files never appear, setup() returns None after both child processes were spawned; they are left running and can poison subsequent tests.

Proposed fix
         if std::time::Instant::now() > deadline {
             eprintln!("focus-monitor sentinel never published pid/hwnd files");
+            let mut h = harness;
+            let mut f = fm;
+            let _ = h.kill();
+            let _ = h.wait();
+            let _ = f.kill();
+            let _ = f.wait();
             return None;
         }
📝 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
if std::time::Instant::now() > deadline {
eprintln!("focus-monitor sentinel never published pid/hwnd files");
return None;
}
if std::time::Instant::now() > deadline {
eprintln!("focus-monitor sentinel never published pid/hwnd files");
let mut h = harness;
let mut f = fm;
let _ = h.kill();
let _ = h.wait();
let _ = f.kill();
let _ = f.wait();
return None;
}
🤖 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/cua-driver/tests/harness_bg_modality_test.rs`
around lines 159 - 162, When setup() times out waiting for the sentinel files it
currently returns None but leaves the spawned child processes running; update
the timeout branch to cleanly terminate those children: capture the
std::process::Child handles created in setup(), and on the deadline timeout call
child.kill().ok() and then child.wait().ok() (or send a graceful shutdown if
available) for each child before returning None so no stray processes remain.

"pid": pid as i64, "window_id": wid, "action": "execute_javascript",
"javascript": "1+1"
}));
let text = resp["result"]["content"][0]["text"].as_str().unwrap_or("");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Potential panic when accessing content array.

Line 197 directly indexes resp["result"]["content"][0] which will panic if content is missing, not an array, or empty. Since this test expects error responses, the structure may vary. The CDP response structure can differ based on error type (see context from mcp-server/src/cdp.rs).

🛡️ Proposed fix to safely access content
-        let text = resp["result"]["content"][0]["text"].as_str().unwrap_or("");
+        let text = resp.get("result")
+            .and_then(|r| r.get("content"))
+            .and_then(|c| c.as_array())
+            .and_then(|arr| arr.get(0))
+            .and_then(|item| item.get("text"))
+            .and_then(|t| t.as_str())
+            .unwrap_or("");
📝 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 text = resp["result"]["content"][0]["text"].as_str().unwrap_or("");
let text = resp.get("result")
.and_then(|r| r.get("content"))
.and_then(|c| c.as_array())
.and_then(|arr| arr.get(0))
.and_then(|item| item.get("text"))
.and_then(|t| t.as_str())
.unwrap_or("");
🤖 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/cua-driver/tests/harness_web_test.rs` at line
197, The test currently indexes
resp["result"]["content"][0]["text"].as_str().unwrap_or("") which can panic if
"content" is missing, not an array, or empty; change the extraction to safely
traverse the JSON (e.g., use resp.get("result").and_then(|r|
r.get("content")).and_then(|c| c.as_array()).and_then(|arr|
arr.get(0)).and_then(|item| item.get("text")).and_then(|t|
t.as_str()).unwrap_or("")) so the code checks for existence and array-ness
before accessing index 0 and avoids unwraps that can panic; update the usage in
harness_web_test.rs where resp is parsed to use this safe path.

Comment on lines +74 to +83
if ($Skip -ne "electron") {
$elecBuild = Join-Path $harnessDir "CuaTestHarness.Electron\build.ps1"
if (Test-Path $elecBuild) {
Write-Host ""
Write-Host "[BUILD] CuaTestHarness.Electron -> $testAppsDir\harness-electron\" -ForegroundColor Cyan
& $elecBuild
if ($LASTEXITCODE -ne 0) {
Write-Host "[WARN] Electron build failed (exit $LASTEXITCODE)" -ForegroundColor Yellow
}
} else {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Catch terminating errors from Electron build invocation.

At Line 79, & $elecBuild can throw (terminating error), which skips the Line 80-82 warning path and aborts the whole harness build. Wrap this call in try/catch so Electron failures degrade to warning as intended.

Proposed fix
 if ($Skip -ne "electron") {
     $elecBuild = Join-Path $harnessDir "CuaTestHarness.Electron\build.ps1"
     if (Test-Path $elecBuild) {
         Write-Host ""
         Write-Host "[BUILD] CuaTestHarness.Electron -> $testAppsDir\harness-electron\" -ForegroundColor Cyan
-        & $elecBuild
-        if ($LASTEXITCODE -ne 0) {
-            Write-Host "[WARN] Electron build failed (exit $LASTEXITCODE)" -ForegroundColor Yellow
-        }
+        try {
+            & $elecBuild
+            if ($LASTEXITCODE -ne 0) {
+                Write-Host "[WARN] Electron build failed (exit $LASTEXITCODE)" -ForegroundColor Yellow
+            }
+        } catch {
+            Write-Host "[WARN] Electron build errored: $($_.Exception.Message)" -ForegroundColor Yellow
+        }
     } else {
         Write-Host "[SKIP] Electron project not present yet - skipping." -ForegroundColor Yellow
     }
 }
📝 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
if ($Skip -ne "electron") {
$elecBuild = Join-Path $harnessDir "CuaTestHarness.Electron\build.ps1"
if (Test-Path $elecBuild) {
Write-Host ""
Write-Host "[BUILD] CuaTestHarness.Electron -> $testAppsDir\harness-electron\" -ForegroundColor Cyan
& $elecBuild
if ($LASTEXITCODE -ne 0) {
Write-Host "[WARN] Electron build failed (exit $LASTEXITCODE)" -ForegroundColor Yellow
}
} else {
if ($Skip -ne "electron") {
$elecBuild = Join-Path $harnessDir "CuaTestHarness.Electron\build.ps1"
if (Test-Path $elecBuild) {
Write-Host ""
Write-Host "[BUILD] CuaTestHarness.Electron -> $testAppsDir\harness-electron\" -ForegroundColor Cyan
try {
& $elecBuild
if ($LASTEXITCODE -ne 0) {
Write-Host "[WARN] Electron build failed (exit $LASTEXITCODE)" -ForegroundColor Yellow
}
} catch {
Write-Host "[WARN] Electron build errored: $($_.Exception.Message)" -ForegroundColor Yellow
}
} else {
Write-Host "[SKIP] Electron project not present yet - skipping." -ForegroundColor Yellow
}
}
🤖 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/test-harness/build.ps1` around lines 74 - 83, The Electron
build invocation (& $elecBuild) can throw a terminating error and bypass the
subsequent $LASTEXITCODE warning handling; wrap the call to $elecBuild in a
try/catch around the & $elecBuild invocation inside the if (Test-Path
$elecBuild) block so any thrown terminating errors are caught, set or inspect
$LASTEXITCODE as before, and emit the same "[WARN] Electron build failed (exit
$LASTEXITCODE)" message in the catch path to ensure failures degrade to warnings
rather than aborting the harness build.

Comment on lines +9 to +10
const CDP_PORT = process.env.CUA_ELECTRON_CDP_PORT || '9223';
app.commandLine.appendSwitch('remote-debugging-port', CDP_PORT);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate CUA_ELECTRON_CDP_PORT before appending the Electron remote-debugging-port switch.

libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js forwards process.env.CUA_ELECTRON_CDP_PORT to app.commandLine.appendSwitch(...) without checking that it’s a valid port number/range; a mis-set env value can break CDP and cause misleading test failures.

💡 Suggested fix
-const CDP_PORT = process.env.CUA_ELECTRON_CDP_PORT || '9223';
+const rawPort = process.env.CUA_ELECTRON_CDP_PORT ?? '9223';
+const portNum = Number(rawPort);
+if (!Number.isInteger(portNum) || portNum &lt; 1 || portNum &gt; 65535) {
+  throw new Error(`Invalid CUA_ELECTRON_CDP_PORT: "${rawPort}". Expected 1-65535.`);
+}
+const CDP_PORT = String(portNum);
 app.commandLine.appendSwitch('remote-debugging-port', CDP_PORT);
📝 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
const CDP_PORT = process.env.CUA_ELECTRON_CDP_PORT || '9223';
app.commandLine.appendSwitch('remote-debugging-port', CDP_PORT);
const rawPort = process.env.CUA_ELECTRON_CDP_PORT ?? '9223';
const portNum = Number(rawPort);
if (!Number.isInteger(portNum) || portNum < 1 || portNum > 65535) {
throw new Error(`Invalid CUA_ELECTRON_CDP_PORT: "${rawPort}". Expected 1-65535.`);
}
const CDP_PORT = String(portNum);
app.commandLine.appendSwitch('remote-debugging-port', CDP_PORT);
🤖 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/test-harness/CuaTestHarness.Electron/main.js` around lines 9
- 10, The code forwards process.env.CUA_ELECTRON_CDP_PORT directly into CDP_PORT
and app.commandLine.appendSwitch('remote-debugging-port', CDP_PORT) without
validation; update the handling so you parse and validate
process.env.CUA_ELECTRON_CDP_PORT (trim, parseInt base 10), ensure it is an
integer within 1–65535, and only use it when valid; otherwise log a warning and
fall back to the default '9223' before calling app.commandLine.appendSwitch;
refer to the CDP_PORT constant, process.env.CUA_ELECTRON_CDP_PORT, and
app.commandLine.appendSwitch to locate and change the code.

Comment on lines +34 to +36
mainWindow.loadFile(path.join(__dirname, 'web', 'index.html')).then(() => {
mainWindow.setTitle(fixedTitle);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js"

echo "=== File exists? ==="
ls -l "$FILE"

echo "=== Lines around 34-36 ==="
nl -ba "$FILE" | sed -n '1,120p' | sed -n '25,70p'

echo "=== Find loadFile usage in this repo ==="
rg -n "loadFile\\(" -S .

echo "=== Check if there's any global unhandledRejection handler ==="
rg -n "unhandledRejection|process\\.on\\(['\"]unhandledRejection['\"]" -S .

echo "=== Check electron BrowserWindow creation / events in this file ==="
nl -ba "$FILE" | sed -n '1,220p'

Repository: trycua/cua

Length of output: 274


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js"

echo "=== Lines around 34-36 ==="
# Prefer cat -n over nl (nl may not be available)
cat -n "$FILE" | sed -n '25,70p'

echo "=== Find loadFile usage in this repo ==="
rg -n "loadFile\\(" -S .

echo "=== Check if there's any global unhandledRejection handler ==="
rg -n "unhandledRejection|process\\.on\\(['\"]unhandledRejection['\"]" -S .

echo "=== Check if this file has app.exit / promise catch patterns around mainWindow.loadFile ==="
rg -n "loadFile\\(|catch\\(|then\\(|unhandledRejection|app\\.exit" -S "$FILE"

Repository: trycua/cua

Length of output: 1570


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js"

echo "=== Search within file for did-fail-load / webContents error handlers ==="
rg -n "did-fail-load|did-finish-load|did-fail|webContents\\.on|did-start-loading" "$FILE" || true

echo "=== Search repo for unhandledRejection handler again (full repo) ==="
rg -n "process\\.on\\(['\"]unhandledRejection['\"]|unhandledRejection" -S . || true

echo "=== Search for electron load failure patterns ==="
rg -n "did-fail-load|webContents\\.loadURL\\(|loadFile\\(|did-fail" -S libs/cua-driver/test-harness/CuaTestHarness.Electron || true

Repository: trycua/cua

Length of output: 389


🌐 Web query:

Electron BrowserWindow loadFile returns Promise reject on failure

💡 Result:

Electron BrowserWindow.loadFile(filePath[, options]) returns a Promise that resolves when the page finishes loading and rejects if the page fails to load. Specifically, the docs state the promise resolves on did-finish-load and rejects on did-fail-load [1][2]. So if you see “Promise reject on failure”, that is expected behavior: you should handle it with.catch/try-catch on await. Example (main process): const win = new BrowserWindow(/*... */); try { await win.loadFile('index.html'); // page loaded (did-finish-load) } catch (err) { // page failed to load (did-fail-load) console.error('loadFile failed:', err); } If your code isn’t observing the rejection, common causes are: 1) Not awaiting the Promise / not attaching a rejection handler (so the error appears as an unhandled rejection). 2) The path is wrong or the file can’t be found; Electron will reject with errors like ERR_FILE_NOT_FOUND (example reported when loadFile can’t find the target file) [3][4]. 3) The load is triggered too early in app lifecycle (BrowserWindows should be created after app.whenReady) [5]. Note: Electron also attaches a noop rejection handler internally to avoid unhandled-rejection warnings for the loadURL/loadFile promise [1][2]. Still, your own.catch is the right way to respond to load failures.

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js"

echo "=== File header (first 1-80 lines) ==="
sed -n '1,90p' "$FILE"

echo "=== Search for fixedTitle definition ==="
rg -n "fixedTitle" "$FILE"

echo "=== Search for mainWindow variable declaration ==="
rg -n "let|const|var mainWindow|mainWindow\\s*=" "$FILE"

echo "=== Search for app.exit / app.quit usage ==="
rg -n "app\\.exit\\(|app\\.quit\\(|app\\." "$FILE"

Repository: trycua/cua

Length of output: 2427


Handle loadFile failures explicitly.

mainWindow.loadFile(...).then(...) is used without any .catch/await, so if web/index.html fails to load (e.g., missing/unreadable), the harness won’t take a deterministic failure path.

💡 Suggested fix
-  mainWindow.loadFile(path.join(__dirname, 'web', 'index.html')).then(() => {
-    mainWindow.setTitle(fixedTitle);
-  });
+  mainWindow
+    .loadFile(path.join(__dirname, 'web', 'index.html'))
+    .then(() => {
+      if (mainWindow && !mainWindow.isDestroyed()) {
+        mainWindow.setTitle(fixedTitle);
+      }
+    })
+    .catch((err) => {
+      console.error('Failed to load harness web page', err);
+      app.exit(1);
+    });
📝 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
mainWindow.loadFile(path.join(__dirname, 'web', 'index.html')).then(() => {
mainWindow.setTitle(fixedTitle);
});
mainWindow
.loadFile(path.join(__dirname, 'web', 'index.html'))
.then(() => {
if (mainWindow && !mainWindow.isDestroyed()) {
mainWindow.setTitle(fixedTitle);
}
})
.catch((err) => {
console.error('Failed to load harness web page', err);
app.exit(1);
});
🤖 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/test-harness/CuaTestHarness.Electron/main.js` around lines 34
- 36, The loadFile promise on mainWindow is not handled for failures; update the
mainWindow.loadFile(...) usage (the call that currently chains .then(() =>
mainWindow.setTitle(fixedTitle))) to handle errors explicitly: either await it
inside an async init function wrapped in try/catch or append a .catch handler
that logs the error (or shows an error dialog) and takes a deterministic failure
path (e.g., app.exit/non-zero or mainWindow.destroy()); ensure
mainWindow.setTitle(fixedTitle) only runs on success.

Comment on lines +29 to +33
var portStr = Environment.GetEnvironmentVariable("CUA_WEBVIEW_CDP_PORT") ?? "9222";
var opts = new CoreWebView2EnvironmentOptions
{
AdditionalBrowserArguments = $"--remote-debugging-port={portStr}",
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ls -la libs/cua-driver/test-harness/CuaTestHarness.WebView || true
sed -n '1,120p' libs/cua-driver/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs
nl -ba libs/cua-driver/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs | sed -n '1,120p'
rg -n "CUA_WEBVIEW_CDP_PORT|remote-debugging-port" -S libs/cua-driver/test-harness/CuaTestHarness.WebView
rg -n "CUA_WEBVIEW_CDP_PORT" -S libs/cua-driver/test-harness || true

Repository: trycua/cua

Length of output: 2626


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="libs/cua-driver/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs"

# Show with line numbers via awk (nl isn't available here)
awk '{print FNR "\t" $0}' "$FILE" | sed -n '1,120p'

echo "---- references in harness ----"
rg -n "CUA_WEBVIEW_CDP_PORT" -S libs/cua-driver/test-harness || true
rg -n "remote-debugging-port" -S libs/cua-driver/test-harness || true

Repository: trycua/cua

Length of output: 3433


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "CDP_PORT|remote-debugging-port|CUA_WEBVIEW_CDP_PORT" -S libs/cua-driver/test-harness/CuaTestHarness.Electron libs/cua-driver/test-harness/scenarios/scenarios.json

echo "---- main.js (top 120 lines) ----"
sed -n '1,120p' libs/cua-driver/test-harness/CuaTestHarness.Electron/main.js

echo "---- scenarios.json (around description) ----"
sed -n '160,260p' libs/cua-driver/test-harness/scenarios/scenarios.json

Repository: trycua/cua

Length of output: 5978


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "CUA_WEBVIEW_CDP_PORT" -S .

Repository: trycua/cua

Length of output: 549


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="libs/cua-driver/rust/crates/cua-driver/tests/harness_web_test.rs"
sed -n '1,220p' "$FILE"

Repository: trycua/cua

Length of output: 9294


🌐 Web query:

Chromium --remote-debugging-port=0 allowed?

💡 Result:

Yes—Chromium allows using --remote-debugging-port=0, and it treats it as “ephemeral port” selection. Evidence: 1) In Chromium’s remote debugging server code, when the --remote-debugging-port value parses to 0, Chromium treats this as “client requested an ephemeral port” and then bootstraps the connection by writing the selected port to the profile’s well-known DevToolsActivePort file. [1] 2) The Chrome DevTools Protocol documentation also states that if Chrome/Chromium is launched with --remote-debugging-port=0 and chooses an open port, the browser endpoint is written to stderr and to DevToolsActivePort in the browser profile folder. [2] 3) Remote debugging may still be rejected by policy/restrictions (independent of 0), since Chromium checks whether remote debugging is allowed before starting the server. [1] Practical implication: --remote-debugging-port=0 should start DevTools on a random available local port, and you should read it from stderr or from <user-data-dir>/DevToolsActivePort (or the profile directory in use). [2][1] Example usage: - /path/to/chromium --remote-debugging-port=0 --user-data-dir=/tmp/chrome-debug then read /tmp/chrome-debug/DevToolsActivePort for the chosen port. [2][1]

Citations:


Validate CUA_WEBVIEW_CDP_PORT before using it in --remote-debugging-port

libs/cua-driver/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs injects CUA_WEBVIEW_CDP_PORT verbatim into AdditionalBrowserArguments; there’s no range/numeric validation. Since the harness expects a fixed port (it doesn’t read DevToolsActivePort), invalid values (including 0) will break CDP exposure and make failures harder to diagnose.

💡 Suggested fix
-            var portStr = Environment.GetEnvironmentVariable("CUA_WEBVIEW_CDP_PORT") ?? "9222";
+            var portStr = Environment.GetEnvironmentVariable("CUA_WEBVIEW_CDP_PORT") ?? "9222";
+            if (!ushort.TryParse(portStr, out var cdpPort) || cdpPort == 0)
+            {
+                throw new InvalidOperationException($"Invalid CUA_WEBVIEW_CDP_PORT: '{portStr}'. Expected 1-65535.");
+            }
             var opts = new CoreWebView2EnvironmentOptions
             {
-                AdditionalBrowserArguments = $"--remote-debugging-port={portStr}",
+                AdditionalBrowserArguments = $"--remote-debugging-port={cdpPort}",
             };
📝 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
var portStr = Environment.GetEnvironmentVariable("CUA_WEBVIEW_CDP_PORT") ?? "9222";
var opts = new CoreWebView2EnvironmentOptions
{
AdditionalBrowserArguments = $"--remote-debugging-port={portStr}",
};
var portStr = Environment.GetEnvironmentVariable("CUA_WEBVIEW_CDP_PORT") ?? "9222";
if (!ushort.TryParse(portStr, out var cdpPort) || cdpPort == 0)
{
throw new InvalidOperationException($"Invalid CUA_WEBVIEW_CDP_PORT: '{portStr}'. Expected 1-65535.");
}
var opts = new CoreWebView2EnvironmentOptions
{
AdditionalBrowserArguments = $"--remote-debugging-port={cdpPort}",
};
🤖 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/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs`
around lines 29 - 33, MainWindow.xaml.cs currently injects the raw
CUA_WEBVIEW_CDP_PORT value into AdditionalBrowserArguments (portStr) without
validation; parse the environment value returned by
Environment.GetEnvironmentVariable("CUA_WEBVIEW_CDP_PORT") into an integer,
ensure it's within a safe port range (e.g. 1024–65535) and not zero, and if
parsing fails or the value is out of range fall back to the default "9222" and
emit a warning/log; update the code paths that set portStr and the
CoreWebView2EnvironmentOptions.AdditionalBrowserArguments so they use the
validated/sanitized port and not the raw env string (refer to the portStr
variable and the CoreWebView2EnvironmentOptions creation in MainWindow.xaml.cs).

Comment on lines +37 to +39
var htmlPath = Path.Combine(AppContext.BaseDirectory, "web", "index.html");
var fileUri = new Uri(htmlPath).AbsoluteUri;
Wv.Source = new Uri(fileUri);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fail fast if web/index.html is missing.

Line 37 builds a file URI even when the file is absent, which can lead to confusing downstream test failures.

💡 Suggested fix
             var htmlPath = Path.Combine(AppContext.BaseDirectory, "web", "index.html");
+            if (!File.Exists(htmlPath))
+            {
+                throw new FileNotFoundException("Harness web entry point not found.", htmlPath);
+            }
             var fileUri  = new Uri(htmlPath).AbsoluteUri;
📝 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
var htmlPath = Path.Combine(AppContext.BaseDirectory, "web", "index.html");
var fileUri = new Uri(htmlPath).AbsoluteUri;
Wv.Source = new Uri(fileUri);
var htmlPath = Path.Combine(AppContext.BaseDirectory, "web", "index.html");
if (!File.Exists(htmlPath))
{
throw new FileNotFoundException("Harness web entry point not found.", htmlPath);
}
var fileUri = new Uri(htmlPath).AbsoluteUri;
Wv.Source = new Uri(fileUri);
🤖 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/test-harness/CuaTestHarness.WebView/MainWindow.xaml.cs`
around lines 37 - 39, Check for existence of the built HTML before creating a
URI: verify File.Exists(htmlPath) after computing htmlPath (from
Path.Combine(AppContext.BaseDirectory, "web", "index.html")) and fail fast if
missing by throwing a clear exception (e.g., FileNotFoundException with
htmlPath) or logging and exiting the app instead of constructing a Uri; update
the MainWindow.xaml.cs initialization that sets Wv.Source so it first performs
this existence check and only constructs new Uri(fileUri) when the file is
present.

…entinel with WA_ACTIVE/WM_SETFOCUS gains

Investigates the UIA Invoke / ValuePattern.SetValue focus-steal observed
by the bg-modality test suite. Findings:

1. **Root cause** is in WPF's automation peers (not cua-driver alone).
   WPF's ButtonBase.OnClick handler and TextBoxAutomationPeer.SetValue
   both call `UIElement.Focus()` synchronously, which routes through
   SetForegroundWindow. That happens IN the target process during the
   UIA pattern call, before cua-driver gets control back.

2. **EnableWindow(false) bypass doesn't help for WPF.** The existing
   UWP/XAML bypass works by gating the host's input queue, but
   SetForegroundWindow doesn't go through the input queue. Tried
   extending the bypass to WPF — confirmed no improvement.

3. **Foreground restoration after the call also doesn't help** when
   cua-driver is not at UIAccess integrity: the foreground-lock blocks
   non-UIAccess SetForegroundWindow even with AttachThreadInput. Tried
   adding a snapshot-and-restore path — confirmed the restore call
   doesn't take effect, GetForegroundWindow stays at the harness HWND
   after the action.

4. **Real mitigation** is to route UIA activations through
   `cua-driver-uia.exe` (UIAccess-manifested worker). The worker has
   the privilege needed both to suppress self-foreground in the target
   (via input-queue gating) and to restore the user's foreground if it
   leaked through. Not yet implemented.

This change leaves cua-driver code unchanged from main (the experiments
described above were reverted) and updates the bg-modality tests to:

- Document the gap explicitly with `_DOCUMENTED_steals_focus` test
  names that read as TODOs. Assertion is delta >= 1 so the test fails
  loud if cua-driver later fixes it (and we can flip the assertion).
- Add a `windows` dev-dep so the test can call GetForegroundWindow
  directly (for the future restoration-assertion path).
- Extend focus-monitor-win to also track WM_ACTIVATE(WA_ACTIVE) gains
  and WM_SETFOCUS gains. The gains/losses delta lets future tests
  distinguish "transient blip with restored foreground" from
  "permanent focus steal" — useful when cua-driver-uia is wired up.

8 bg-modality + capture-mode tests all pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel

vercel Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview May 26, 2026 7:50am

Request Review

f-trycua and others added 6 commits May 25, 2026 23:41
…wiring + cua-driver UIA pattern-dispatch gaps documented

WinUI3 harness now mirrors the WPF scenarios (Slider, CheckBox,
RadioButton, ComboBox). Driving these via cua-driver's current
`click` / `set_value` tools surfaces 4 additional UIA-pattern-dispatch
gaps that were previously hidden because no harness exercised them:

  CheckBox      - exposes TogglePattern only, no Invoke.
                  `click` tries Invoke -> falls to PostMessage which
                  doesn't reach WinUI3's CoreInput dispatcher
                  (same constraint as type_text on XAML hosts).
                  Fix: try TogglePatternId.Toggle() on XAML hosts.

  RadioButton   - exposes SelectionItemPattern.Select, no Invoke.
                  Same PostMessage-doesn't-reach gap.
                  Fix: try SelectionItemPatternId.Select() on XAML hosts.

  ComboBox      - parent exposes ExpandCollapsePattern, items expose
                  SelectionItemPattern. Current `click` flow can't
                  open the dropdown.
                  Fix: try ExpandCollapsePatternId.Expand() on parents
                  with that pattern + items via SelectionItem.

  Slider        - AutomationId doesn't surface in the flat UIA element
                  list (SliderAutomationPeer doesn't get a [N] entry),
                  AND `set_value` only tries ValuePatternId — Slider
                  uses RangeValuePatternId.
                  Fix: enumerate slider sub-parts + fall through to
                  RangeValuePattern.SetValue in `set_value`.

These four gaps are documented as `_DOCUMENTED_no_op` tests that
assert the current no-op behaviour, so the harness fails loudly if
cua-driver fixes them (assertion is INVERTED — expect no change).

WinUI3 suite: 7/7 tests green.

Other wiring:
  - run-tests-in-sandbox.ps1 + sandbox-runner.ps1: stage and run the
    new harness_web_test and harness_bg_modality_test binaries inside
    Windows Sandbox; export HARNESS_WEBVIEW_EXE / HARNESS_ELECTRON_EXE
    env vars for those tests.
  - fg_bypass.rs / impl_.rs: reverted the broader bypass experiment
    (see prior commit's investigation notes). EnableWindow(false)
    bypass remains XAML-host-gated; WPF UIA focus-steal stays a
    documented gap.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…on in fg_bypass

Adds a paragraph to the run_with_uwp_bypass docstring documenting why
the bypass doesn't help for WPF Buttons / TextBoxes:

  WPF's automation peers (ButtonBase / TextBoxAutomationPeer) call
  UIElement.Focus() synchronously inside the UIA Invoke / SetValue
  handler. Focus() routes through SetForegroundWindow, which is NOT
  gated by EnableWindow — the existing bypass only blocks the input-
  queue path used by UWP/XAML hosts. The user-foreground restoration
  pattern (snapshot + AttachThreadInput + SetForegroundWindow) also
  fails from a non-UIAccess process because the foreground-lock
  rejects the restore call.

Real mitigation: route UIA activations through cua-driver-uia.exe
(UIAccess-manifested worker). See harness_bg_modality_test.rs for
the regression guards documenting this gap.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ctionItem / ExpandCollapse before PostMessage fallback

Closes three of the four WinUI3 pattern-dispatch gaps documented in the
phase 2 harness (PR #1699):

  CheckBox    - TogglePattern.Toggle()
  RadioButton - SelectionItemPattern.Select()
  ComboBox    - ExpandCollapsePattern.Expand() on parent, then
                SelectionItemPattern.Select() on the selected item

The Slider RangeValuePattern dispatch + Slider element enumeration in
the UIA flat tree are still open (tracked by
harness_winui3_slider_DOCUMENTED_unreachable).

Each new pattern is wrapped in the same `run_with_uwp_bypass` guard as
UIA Invoke, so the EnableWindow(false) UWP foreground-steal mitigation
applies to all four UIA activation paths uniformly.

Dispatch order: Invoke > Toggle > SelectionItem > ExpandCollapse.
This ordering matches the click-semantics specificity an agent
typically expects:
  - Buttons / hyperlinks      -> Invoke
  - Checkboxes                -> Toggle
  - Radio buttons / ListItems -> SelectionItem.Select
  - ComboBox parents          -> ExpandCollapse.Expand
  - Anything else             -> PostMessage WM_LBUTTONDOWN/UP

The bg-modality WPF gaps (Invoke + SetValue on WPF Button/TextBox
trigger Focus() inside the target) are NOT addressed by this change —
those still require the UIAccess worker (cua-driver-uia.exe) to
reliably restore foreground.

Verified:
  - cargo test --test harness_winui3_test  -> 7/7 (was 7/7 with 4 gap
    docs; now 7/7 with 3 fixes + 1 remaining gap)
  - cargo test --test harness_wpf_test     -> 18/18 (no regression)
  - cargo test --test harness_bg_modality_test -> 8/8 (no regression)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…geValuePattern for sliders / progress bars

Closes the last of the four WinUI3 pattern-dispatch gaps the phase 2
harness documented (PR #1699). cua-driver's `set_value` previously
queried only `UIA_ValuePatternId`, which Slider / ProgressBar / other
numeric-range controls don't implement — they expose
`UIA_RangeValuePatternId` instead, with `SetValue(double)`.

Behaviour:
  1. Try ValuePattern.SetValue(string)        — text inputs, editable combos.
  2. Fall through to RangeValuePattern.SetValue(double) — Sliders, ranges.
     Parses `value` as f64; returns an actionable error if it isn't.
  3. Otherwise: actionable error pointing at the `click` tool for
     CheckBox (Toggle) / RadioButton+ListItem (SelectionItem) targets.

The set_value response now reports which pattern was used:
   ✅ Set AXValue on [N] (UIA ValuePattern).
   ✅ Set AXValue on [N] (UIA RangeValuePattern).

The remaining open item is the Slider element enumeration in
get_window_state's flat UIA index — SliderAutomationPeer's parent
doesn't appear with an [N] index, so the test harness can't address
it by AutomationId directly. Tracked by
harness_winui3_slider_DOCUMENTED_unreachable.

Verified:
  - cargo test --test harness_wpf_test harness_wpf_set_value -> ok
  - cargo test --test harness_winui3_test                    -> 7/7
  - cargo test --test harness_bg_modality_test               -> 8/8

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…/json discovery instead of read_to_end

Closes the cua-driver CDP gap PR #1699's harness_electron_page_tool
caught: cua-driver's `mcp_server::cdp::cdp_list_pages` opened a TCP
socket, sent `GET /json HTTP/1.1\r\nConnection: close\r\n`, then called
`stream.read_to_end(&mut buf).await`. Chromium's CDP HTTP server
ignores `Connection: close` and keeps the socket alive — so the read
hung until the 10 s discovery timeout, making the entire `page` tool
unusable against Electron.

Fix:
  1. Read response headers line-by-line via BufReader::read_line.
  2. Parse Content-Length / Transfer-Encoding from those headers.
  3. Read exactly that many bytes for the body (Content-Length) OR
     parse the chunked transfer-encoding framing.
  4. Drop the socket when done (server keeps the alive-state on its
     side; we just stop talking to it).
  5. Fallback to read_to_end if neither header is present — preserves
     behaviour for legacy CDP servers that honour Connection: close.

The earlier write-half-shutdown experiment (to signal "done writing"
on the half-duplex tcp) made Chromium close the whole connection
immediately and return EOF before any headers — keep the stream
full-duplex.

Verified end-to-end:
  cargo test --test harness_web_test harness_electron_page_tool
  -> ok (execute_javascript via CDP works against Electron 31)

A separate gap remains in `page.click_element`'s probe-JSON parsing
(the CDP result wraps the inner JSON, causing 'probe JSON missing
required field vx'). Documented as harness_electron_click_element_
DOCUMENTED_wrapper_bug.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Eight actionable comments from CodeRabbit:

1. harness_bg_modality_test.rs `read_count` — was swallowing read/parse
   errors as 0, which would mask sentinel-file failures as "no focus
   steal" passes. Panic with a clear path-bearing message instead.

2. harness_bg_modality_test.rs `setup()` — child processes leaked on
   sentinel-pid/hwnd-file timeout. Spawn failures of focus-monitor or
   cua-driver also left the prior child running. Reap all spawned
   children before returning None.

3. harness_web_test.rs DOCUMENTED_wrapper_bug — direct
   `resp["result"]["content"][0]["text"]` indexing would panic on
   shape variation (e.g. when the error path returns `error.message`
   instead). Safe traversal via `.get(...).and_then(...)` chain with
   a fallback to `error.message`.

4. build.ps1 Electron invocation — same try/catch pattern we applied
   to the WPF/WinUI3 builds last PR. `& $elecBuild` can throw under
   $ErrorActionPreference=Stop, aborting the whole harness build
   before the [WARN] log line runs.

5. Electron/main.js CDP_PORT — validate before forwarding to
   `app.commandLine.appendSwitch('remote-debugging-port', ...)`. Reject
   port 0 (Chromium reads it as "ephemeral", which would break the
   harness's fixed-port discovery), non-numeric input, and out-of-
   range values. Throw at startup so failures are diagnosed clearly.

6. Electron/main.js loadFile — `.then()` had no `.catch`, so a missing
   web/index.html would silently leave the harness window blank.
   Added a `.catch` that logs + `app.exit(1)`, and an isDestroyed
   guard before setTitle.

7. WebView2 MainWindow.xaml.cs CDP port — same validation pattern as
   Electron, with the same port=0 rejection rationale.

8. WebView2 MainWindow.xaml.cs html-existence — File.Exists check
   before constructing the file:// URI. Without it, a stale build that
   skipped the <None Include="..\shared-web\index.html"/> copy rule
   would silently render an empty page rather than failing fast.

All 36 harness tests still green:
  cargo test --test harness_bg_modality_test  -> 8/8
  cargo test --test harness_web_test          -> 4/4

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@f-trycua
f-trycua merged commit d7981d1 into main May 26, 2026
6 of 7 checks passed
@f-trycua
f-trycua deleted the cua-driver-rs-test-harness-phase2 branch May 26, 2026 09:50
f-trycua added a commit that referenced this pull request May 26, 2026
…1699 harness (#1705)

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant