test-harness: cross-toolkit control parity + macOS/Windows driver fixes - #2064
test-harness: cross-toolkit control parity + macOS/Windows driver fixes#2064f-trycua wants to merge 56 commits into
Conversation
…fier (WPF + GTK3) Adds the single-modality recording tooling used to capture the no-foreground contract across the action matrix on Windows (WPF) and Linux (GTK3), plus a per-action EFFECT verifier (does the action change the app, not just whether it steals focus). FINDINGS.md documents the driver behavior the verifier surfaced: right_click landing on the taskbar, scroll/type no-ops, and the GTK XSendEvent limitation — to be triaged and fixed in the driver, then re-tested. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
When an element_index resolves to a cached center outside its own window (scrolled out of a ScrollViewer, or pushed off-screen by a tall form on a small display), the click/double_click/right_click tools previously injected the synthetic tap at the raw coordinate — landing on whatever was there (the taskbar, the desktop), e.g. opening the Windows taskbar context menu instead of the target's. Add point_in_window_bounds() and return a clear error in that case instead of clicking the wrong target. Surfaced by the modality recordings: a right-click on an off-screen harness button opened the taskbar menu. Recorder: focus the text box before type_text (type targets the focused control; set_value doesn't focus, so type was a no-op without this). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
… shipped + verifier results Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…Electron) Parameterize the recorder by -Toolkit and run an ax-bg pass on each Windows toolkit harness. Key finding: the no-foreground contract holds on WPF/WinUI3/ WebView2 but breaks on Electron (7/8 actions stole focus — Chromium self- foregrounds). Effect coverage varies by harness (WinUI3 thinnest at 2/7). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…elivery paths only The first off-screen guard sat before the click dispatch branch, so it also blocked the background UIA Invoke path — which fires an element's handler with no coordinates and works fine for off-screen elements. That regressed opening (and then listing via list_windows) the WPF legacy modals from their off-screen buttons. Move the guard to the foreground SendInput tap and the background coordinate-injection fallback only; UIA Invoke runs unguarded. Confirmed: bg click → UIA Invoke opens Harness MessageBox / Owned Popup / Layered Popup from off-screen buttons, and list_windows enumerates them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…a dead session fail loudly end_session tombstoned a session id permanently: every later tool call carrying that id hit the daemon's resurrection guard and got a benign "session ended; tool call ignored" (isError:false) — a silent trap that looked like success while doing nothing. Re-declaring the session did not help, forcing callers (e.g. the modality recorder) to mint a fresh unique id per run. Two changes make the lifecycle behave as a caller expects: - start_session with a previously-ended id now REVIVES it (session::revive_session clears the ENDED_SESSIONS tombstone), so explicitly re-declaring a recycled id works. The lifecycle tools (start_session/end_session) are exempt from the resurrection guard so start_session can run to revive and end_session stays idempotent. - A non-lifecycle action on an ended-and-not-revived id is now rejected LOUDLY (isError) with a message telling the caller to start_session to revive — instead of the silent no-op. The original safety (a stray late action can't silently resurrect a dead session) is preserved: reviving requires an explicit start_session re-declare. Adds unit tests for revive_session and updates the daemon gate_tests integration test to assert loud rejection + revive-then-run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ss launches The AppKit harness window opened at slightly different heights between launches (observed 832 vs 858 pt), shifting screenshot dimensions and breaking tests with hardcoded pixel coords. Root cause is the harness app, not the driver: the window was .resizable with macOS state restoration ON (NSWindow.isRestorable defaults to true) and no pinned content size, so the OS persisted and restored a drifted frame on the next launch. Fix in the harness: drop .resizable, set isRestorable = false, clear the frame autosave name, and re-pin a fixed content size (720x800) on every launch so each run is byte-identical. Rebuild via test-harness/build/macos.sh. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dalities Bring the cross-platform Electron test harness to fruition on Linux (Chromium via AT-SPI) and record its 5 modality modes — the Linux analogue of the Windows Electron lane. Build/launch glue: - electron/build.sh: Linux analogue of build.ps1 (stage shared web/, npm install the prebuilt linux-x64 electron runtime, optional flat-stage with a deterministic CuaTestHarness.Electron binary name). - electron/package.json: add start:linux (electron . --no-sandbox --disable-gpu --force-renderer-accessibility — the flags Chromium needs under Xvfb for the web-AX tree to register on AT-SPI) and a linux electron-builder dir target. Recorder (modality-recordings/linux/): - lin-rec-electron.py: launches the Electron app, resolves the harness window by the "CuaTestHarness Electron [cdp=NNNN]" title, and reads BOTH control geometry and harness state from the web-AX tree (get_window_state capture_mode=ax) — same per-action verifier as Windows, AT-SPI text instead of UIA. - lin-run-electron.sh / lin-dashboard-electron.html / lin-all-electron.sh. Headline (FINDINGS.md): Linux Electron breaks the no-foreground contract LESS than Windows Electron. Windows stole 7/8 in ax-bg; Linux steals 3/8 in ax-bg (only set_value/type/press-key; AX pointer actions all held) and 6/7 in vision-bg (pixel coordinate-injection foregrounds Chromium). All 5 modes frame-verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
Add the controls the 8-action matrix needs and that GTK3 lacked: click-target (btn-clicktarget, left/right/double → last_action=/clicks=), slider (sld-value), checkbox (chk-agree → agreed=), context-menu (btn-context → menu_action=), and a tall scroll-target (scroll-tall → scroll_offset=), matching the WPF harness's AutomationIds + status-label format. Keeps all existing controls the Rust harness_gtk3_test asserts (btn-increment/reset, txt-input, btn-open-popover, btn-exit, HARNESS_TEXT_MARKER_v1, counter=0, POPOVER_MARKER_v1). Verified on the Linux VM: btn-clicktarget/chk-agree/btn-context resolve via AT-SPI; slider + scroll are present but (as documented) not surfaced by Linux get_window_state. Updated scenarios.json gtk3 section to the unified set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…n matrix parity Brings the WinUI3 test harness to control parity with the WPF harness so the cua-driver action matrix (click/double/right/drag/scroll/set_value/type/press) can be exercised on WinUI3. Adds, mirroring the WPF harness AutomationIds + label formats so the shared modality recorder resolves them identically: - click-target: Button (AutomationId border-click-target, "Click target (left / right / double)") with lbl-last-action (last_action=left_click|double_click|right_click) and lbl-click-count (clicks=N). Click fires per tap (timing promotes the 2nd to double_click); RightTapped/DoubleTapped cover secondary + double gestures. - scroll-target: ScrollViewer (AutomationId scroll-tall) with tall SCROLL_TOP_MARKER_v1..SCROLL_BOTTOM_MARKER_v1 content and lbl-scroll-offset (scroll_offset=N) bound to VerticalOffset. The window subclasses its HWND to translate the driver's posted WM_VSCROLL into ScrollViewer.ChangeView (WinUI3 routes input through composition and does not consume WM_VSCROLL natively) — same rationale as the WPF harness HwndSource hook. scenarios.json: adds click_target + scroll_target entries to the winui3 section so the source-of-truth reflects parity. Verified on the Windows VM: rebuilt harness, get_window_state shows border-click-target + scroll-tall (+ labels/markers) in the tree; ax-bg modality recorder effects-landed rose 2/7 -> 3/7 (scroll now lands). The click-target handlers are confirmed correct via foreground SendInput (double_click -> last_action=double_click clicks=2; right_click -> last_action=right_click). double/right do NOT land under ax-bg background dispatch because the driver does not yet detect WinUI3 in would_be_silently_dropped, so clicks fall through to PostMessage (dropped by WinUI3 composition input) instead of inject_click_screen — driver-side follow-up, tracked separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
Add CuaTestHarness.WKWebView — the macOS analogue of the Windows WebView2 harness. A minimal AppKit NSWindow hosting a WKWebView (Apple WebKit, not Chromium) that loads the SAME shared web DOM (test-harness/shared/web/ index.html) the WebView2 and Electron harnesses use. Fixed content size + isRestorable=false so the window geometry doesn't drift between launches. build/macos.sh: add a `wkwebview` target (links -framework WebKit and copies shared/web/index.html into the app bundle's Resources so the harness is self-contained and reuses the canonical content rather than duplicating source). The .app bundle stays gitignored under rust/test-apps/. electron/package.json: add `start:macos` (electron . --force-renderer- accessibility — the flag needed to expose Chromium web-AX to cua-driver on macOS) and a `build:macos` electron-builder target, so the existing cross-platform Electron harness has a documented macOS launch/build path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…t not a simple WPF-style arm) The WinUI3 harness parity work surfaced that background double-click/right-click don't land on WinUI3. Tried mirroring the WPF arm in would_be_silently_dropped (route mouse clicks through the coordinate injector) — it did NOT work: WinUI3's composition input-site doesn't consume the synthetic pen the way WPF's stylus stack does, so double/right still no-op AND the no-foreground contract regressed (ax-bg 0/8 -> 8/8 stolen). Reverted; left an explanatory comment so the next attempt knows it needs a WinUI3-specific input path, not the WPF treatment. Single left-click already works via UIA Invoke. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…rix parity The shared web DOM (loaded by WebView2 + Electron on Win/Linux/macOS + the new macOS WKWebView) had click-target/slider/checkbox/text but no scrollable region, so the scroll action couldn't be exercised on any web surface. Add a scroll_target fieldset: a tall overflow:auto div (scroll-tall) between SCROLL_TOP/BOTTOM markers with a scroll_offset= label that updates on scroll — matching the WPF harness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…WPF/GTK3 Bring the macOS harnesses to control parity so the 8-action matrix can be exercised on macOS via the same controls as every other platform. AppKit (apps/macos/appkit/main.swift): - click_target: replace the non-AX NSView with a real NSButton id=btn-clicktarget (AX-addressable via AXPress) → last_action=click|double_click|right_click, clicks=N - slider id=sld-value (NSSlider 0..100) → slider_value=N - checkable_controls id=chk-agree (NSButton checkbox) → agreed=true|false - context_menu id=btn-context (NSButton + NSMenu Cut/Copy/Paste) → menu_action=... - scroll_target: add SCROLL_TOP_MARKER_v1; scroll-tall id moved to the document AXTextArea (the surfaced scrollable node); offset label → scroll_offset=N - all existing controls (counter, text_body, text_input, ns_menubar, exit) kept SwiftUI (apps/macos/swiftui/main.swift): - click_target id=btn-clicktarget (Button + count:2 gesture) → last_action/clicks - slider id=sld-value (Slider) → slider_value=N - checkable_controls id=chk-agree (Toggle .checkbox) → agreed=true|false - context_menu id=btn-context (Button + .contextMenu) → menu_action=... - scroll_target id=scroll-tall (ScrollView, GeometryReader offset) → scroll_offset=N - existing controls (counter, text_body, text_input, popover, exit) kept scenarios.json: add slider/checkable_controls/context_menu (+ updated click_target and scroll_target) to the appkit section; add click_target/slider/checkable_controls/ context_menu/scroll_target to the swiftui section. Surgical edits only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…e controls NSSlider/AXStepper expose a numeric AXValue and reject a CFString write with -25201 (illegal argument) — so set_value on a slider failed. Add set_number_attr and, in the default AXValue path, try a CFNumber when the value parses as a number, falling back to a CFString (text fields still take a string). Surfaced by the macOS harness control-parity work. Compile-verified; runtime check on a live NSSlider deferred (Mac in use). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…order Wire the Linux Electron modality recorder to the new shared web scroll-target: - resolve `scr` to the scroll-tall clipped viewport (section ~260x120) in the Chromium web-AX tree instead of leaving it None - aim pixel/desktop scroll at the resolved region (AX path uses element_index) - verify scroll via the now-exposed `scroll_offset=` web-AX label (ok/fail instead of na) Measured on Linux Electron: scroll-tall resolves in all 5 modes, but the scroll action is a no-op everywhere (scroll_offset stays 0) — same AT-SPI XSendEvent limitation as the other Linux Electron pointer actions. ax-bg focus contract unchanged at 3/8 stole. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…surfaces Linux Electron scroll-target re-record (resolves all 5 modes, no-op via AT-SPI — contract still 3/8 stole in ax-bg), WebView2 harness restore + deferred full-parity recording, macOS 4-surface set + the numeric-set_value CFNumber driver fix and the two macOS session/geometry gotchas. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…roll + set_value SwiftUI harness exposes all 6 WPF-parity controls (AX-actionable; checkbox press flips agreed=); set_value on the SwiftUI AXSlider reproduces the -25200 CFString-write failure the CFNumber fix targets (broaden the set_number_attr comment to note -25200 as well as -25201); WKWebView page-scroll works via keystroke PageDown but a nested overflow div without tabindex is a no-op. Live "after" verification of the fix needs a re-signed reinstall + TCC re-grant (Developer-ID/cdhash-pinned), recorded as such. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…t NSSlider Post install-local reinstall (post-fix daemon, TCC re-granted): set_value on the AppKit NSSlider now succeeds, slider_value 0->50 (frame-verified). The SwiftUI slider failure is separate — its AXSlider exposes only increment/decrement and rejects any AXValue write (-25200), so the value-type fix cannot help there; needs AXIncrement/AXDecrement (future work). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…AXValue is unsettable SwiftUI's AXSlider exposes a readable-but-unsettable AXValue plus increment/decrement actions, rejecting every direct AXValue write with -25200. Add a bounded AXIncrement/AXDecrement stepping fallback that runs only after both the CFNumber and CFString writes fail for a numeric target. - bindings.rs: add copy_number_attr() to read the control's current AXValue as f64 (mirrors copy_string_attr, downcasts to CFNumber). - set_value.rs: step_to_value() reads current value, steps toward target, stops within half the last observed step, breaks if a step doesn't move the value, hard-capped at 500 iterations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
Replace the live Azure blob URL with a generic reference before the branch goes up as a PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
|
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 adds session revival ( ChangesSession Revival and Ended-Session Enforcement
macOS Numeric AX
Windows Off-Screen Click Guards
Cross-Platform Test Harness Parity and Modality Recordings
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (21)
libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs-2248-2254 (1)
2248-2254: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard the pixel-addressed branches too.
These checks only protect cached
element_indexcenters. The(x, y)paths later in this file still passsx_i/sy_iintosend_click_synthesized/inject_click_screenwithoutpoint_in_window_bounds, so stale or out-of-range screenshot coordinates can still click the taskbar/desktop — the same wrong-target failure this PR is fixing.Suggested fix
let (sx_i, sy_i) = bitmap_to_screen(hwnd, px as i32, py as i32); +if !crate::input::point_in_window_bounds(hwnd, sx_i, sy_i) { + return ToolResult::error(format!( + "Point ({sx_i},{sy_i}) falls outside its window — adjust the coordinates or scroll the target into view before clicking." + )); +}Apply the same guard in the pixel-addressed
click,double_click, andright_clickforeground/background-injection branches.Also applies to: 2379-2384
🤖 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/tools/impl_.rs` around lines 2248 - 2254, The pixel-addressed click paths in the window tool still bypass the same bounds check used for cached element centers, so stale or out-of-range screenshot coordinates can be injected into the wrong target. Update the foreground/background branches in the click handling code (including the click, double_click, and right_click paths) to validate sx_i/sy_i with point_in_window_bounds before calling send_click_synthesized or inject_click_screen, and return a ToolResult::error with the same style of outside-window message when the point is not within hwnd.libs/cua-driver/rust/crates/platform-macos/src/tools/set_value.rs-242-249 (1)
242-249: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon’t accept a one-unit miss before observing the control’s step size.
Line 244 starts with
step_radius = 1.0, so Line 248 can return success without moving the control when the current value is merely within1.0of the target. A slider at49asked to set50would report success and remain unchanged.Proposed fix
- // Half of the last observed step; before any step is observed, treat 1.0 as - // the "close enough" radius (covers integer-stepped controls). - let mut step_radius = 1.0_f64; + // Half of the last observed step. Before observing a step, only an exact + // current-value match should count as success; otherwise perform at least + // one increment/decrement so the actual control granularity is known. + let mut step_radius = f64::EPSILON;🤖 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-macos/src/tools/set_value.rs` around lines 242 - 249, The early-exit check in `set_value.rs` is too permissive because `step_radius` starts at 1.0 before any step is observed, allowing `set_value` to return success on a one-unit miss without actually moving the control. Update the convergence logic in the loop that uses `step_radius` so the initial tolerance is stricter until a real step size has been measured, and only allow the wider radius after observing movement. Keep the fix localized around the `step_radius` initialization and the `if (current - target).abs() <= step_radius` success path.libs/cua-driver/test-harness/modality-recordings/linux/lin-harness.py-1-1 (1)
1-1: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd the standard SPDX header to the new modality-recording files.
CI is already red on the SPDX check, and this file is part of the new headerless cohort. Please add the repo’s required SPDX header here and to the other newly added modality-recording assets before merge.
🤖 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/modality-recordings/linux/lin-harness.py` at line 1, This new modality-recording file is missing the repo’s required SPDX header and is failing the SPDX check; add the standard SPDX license header at the top of lin-harness.py and apply the same header to the other newly added modality-recording assets in this cohort so they match the existing repository convention.Source: Pipeline failures
libs/cua-driver/test-harness/modality-recordings/README.md-27-33 (1)
27-33: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winThe WPF metrics here don’t match the rest of the findings bundle.
This table says
wpf-ax-bgstole 1/8 andwpf-vision-bgstole **2/7, butindex.htmlpublishes **2/8** and **4/7**, andFINDINGS.md` also uses 2/8 for WPF ax-bg. These docs need to agree before the recordings are trustworthy.🤖 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/modality-recordings/README.md` around lines 27 - 33, The WPF recording metrics in the README are inconsistent with the rest of the findings bundle. Update the summary table in the README to match the authoritative values used by index.html and FINDINGS.md for the same recording entries, especially the wpf-ax-bg and wpf-vision-bg counts. Verify the numbers across the related documentation artifacts and make them all agree before publishing.libs/cua-driver/test-harness/modality-recordings/linux/lin-rec.py-159-170 (1)
159-170: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winWait for the whole plan to resolve, not just
chk/txt.Line 169 exits the resolve loop as soon as either the checkbox or text field appears. The later actions still depend on
btn,ctx,sld, andscr, so a slow AT-SPI publish will turn those steps into recorder-created no-ops and skew the landed-action counts.🤖 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/modality-recordings/linux/lin-rec.py` around lines 159 - 170, The resolve loop in lin-rec.py is breaking too early because it exits as soon as either the checkbox or text field is found, while later steps still require btn, ctx, sld, and scr. Update the loop condition in the recording setup block so it waits until all required controls for the plan are resolved before continuing, using the existing resolve dict keys in the els()/find() lookup logic rather than only chk/txt.libs/cua-driver/test-harness/modality-recordings/linux/lin-rec.py-93-98 (1)
93-98: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClear stale artifacts before each run.
os.makedirs(..., exist_ok=True)preserves old recordings, and Line 244 then picks the first*.mp4it finds underREC. If a rerun fails before writing a new video, the script can silently report yesterday’s MP4 and metrics as if they came from this run.Also applies to: 244-248
🤖 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/modality-recordings/linux/lin-rec.py` around lines 93 - 98, Clear out stale recording artifacts before each run so the harness cannot reuse yesterday’s outputs. In lin-rec.py, update the setup around os.makedirs(REC, exist_ok=True) to remove old files in REC (and any run-specific status/metrics artifacts in WORK) before starting, while keeping the wrapper-owned daemon behavior unchanged. Also update the selection logic near the MP4 lookup to only accept artifacts produced by the current run, rather than the first *.mp4 found under REC.libs/cua-driver/test-harness/modality-recordings/linux/lin-rec.py-217-218 (1)
217-218: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
typenever targets the text box selected in the plan.For the
"type"step,sel == "txt"is ignored and the code only sendstype_textto the window pid. If focus is still on the previous control, the keystrokes go somewhere else, so this is not actually measuring “type into the text box”.Suggested fix
elif t == "type": - D("type_text", {"pid":WP,"text":"typed-by-cua","session":"d1"}) + if use_ax: + D("click", {"pid":WP,"window_id":WD,"element_index":eidx,"session":"d1"}) + elif c: + D("click", { + "pid": WP, + "window_id": WD, + "x": wl[0], + "y": wl[1], + "dispatch": ("foreground" if m["fg"] else "background"), + "session": "d1", + }) + time.sleep(0.3) + D("type_text", {"pid":WP,"text":"typed-by-cua","session":"d1"})🤖 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/modality-recordings/linux/lin-rec.py` around lines 217 - 218, The type step in lin-rec.py ignores the selected text-box target and only emits type_text to the window pid, so the keystrokes may go to whatever control still has focus. Update the type-handling branch in the step dispatcher to honor sel == "txt" by targeting the text box explicitly before typing, using the existing plan/selection flow around D and the type case so the action truly types into the selected textbox.libs/cua-driver/test-harness/modality-recordings/linux/lin-all.sh-2-5 (1)
2-5: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDon’t mark a mode as
DONEwhen the runner failed.Lines 4-5 ignore the exit status from
lin-run.sh, suppress all diagnostics, and still appendDONE ...even whenmetric.logwas never produced. That makes the batch summary unreliable.Suggested fix
#!/bin/bash +set -euo pipefail rm -f /tmp/lin-allmodes.log for MODE in ax-fg ax-bg vision-fg vision-bg vision-desktop; do - bash /tmp/lin-run.sh "$MODE" >/dev/null 2>&1 - echo "DONE $MODE $(cat /tmp/cua-lin-$MODE/metric.log 2>/dev/null)" | tee -a /tmp/lin-allmodes.log + if bash /tmp/lin-run.sh "$MODE" >/dev/null 2>&1; then + echo "DONE $MODE $(cat /tmp/cua-lin-$MODE/metric.log)" | tee -a /tmp/lin-allmodes.log + else + echo "FAILED $MODE" | tee -a /tmp/lin-allmodes.log + exit 1 + fi done🤖 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/modality-recordings/linux/lin-all.sh` around lines 2 - 5, The batch summary in lin-all.sh is marking every MODE as DONE even when bash /tmp/lin-run.sh fails and metric.log is missing. Update the loop around the lin-run.sh invocation and the DONE echo so it only records success when the runner exits successfully; if it fails, emit a failure message or skip appending DONE for that MODE. Use the existing MODE loop and the lin-run.sh call as the key points to gate the tee -a /tmp/lin-allmodes.log output.libs/cua-driver/test-harness/modality-recordings/linux/lin-harness.py-132-139 (1)
132-139: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve
double_clickinstead of overwriting it withclick.Line 133 unconditionally resets
_last_actionto"click"after Line 137 records a double-click. On GTK, theclickedsignal still fires on the release that completes the double-click, so the verifier can report a landed double-click as a plain click.Suggested fix
class Harness(Gtk.Window): def __init__(self): super().__init__(title="CuaTestHarness GTK3") @@ self.clicks = 0 self._last_action = "none" self._ctx = "none" + self._saw_double_click = False @@ def _on_click(self, *_): self.clicks += 1; self._last_action = "click" + if self._saw_double_click: + self._saw_double_click = False + self._last_action = "double_click" self.btn_status.set_text(f"last_action=click clicks={self.clicks}"); self._save_state() def _on_btn_press(self, _w, ev): if ev.type == Gdk.EventType.DOUBLE_BUTTON_PRESS: + self._saw_double_click = True self._last_action = "double_click" self.btn_status.set_text(f"last_action=double_click clicks={self.clicks}"); self._save_state()🤖 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/modality-recordings/linux/lin-harness.py` around lines 132 - 139, The click handler in _on_click is overwriting a previously recorded double_click from _on_btn_press, so update the click path to preserve an existing double_click state instead of always setting _last_action to click. Use the _on_click and _on_btn_press methods in lin-harness.py to gate the click update so a GTK double-click final release does not get downgraded, while still keeping the click count and state save behavior intact.libs/cua-driver/test-harness/modality-recordings/linux/lin-rec-electron.py-19-20 (1)
19-20: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResolve dashboard assets from the repo, not
/tmp.These paths ignore the files added in this PR and require
lin-dash.py/lin-dashboard-electron.htmlto be pre-copied into/tmp. On a clean checkout, Line 113 will raiseFileNotFoundErrorbefore the recorder ever launches.Proposed fix
-import json, os, subprocess, sys, time, glob, re +import json, os, subprocess, sys, time, glob, re +from pathlib import Path @@ -DASH = "/tmp/lin-dash.py" -DASH_HTML = "/tmp/lin-dashboard-electron.html" +HERE = Path(__file__).resolve().parent +DASH = HERE / "lin-dash.py" +DASH_HTML = HERE / "lin-dashboard-electron.html" @@ -import shutil; shutil.copy(DASH_HTML, f"{WORK}/dashboard.html") +import shutil; shutil.copy(DASH_HTML, f"{WORK}/dashboard.html") @@ -subprocess.Popen(["python3", DASH, f"http://127.0.0.1:{PORT}/dashboard.html", str(PANX),"0",str(PANW),str(PANH)], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL); time.sleep(4) +subprocess.Popen(["python3", str(DASH), f"http://127.0.0.1:{PORT}/dashboard.html", str(PANX),"0",str(PANW),str(PANH)], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL); time.sleep(4)Also applies to: 113-113, 142-142
🤖 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/modality-recordings/linux/lin-rec-electron.py` around lines 19 - 20, The dashboard asset paths are hardcoded to /tmp, which bypasses the files added to the repo and breaks clean checkouts. Update the constants in lin-rec-electron.py for DASH and DASH_HTML so they resolve from the repository location instead of /tmp, and make sure the path usage in the recording flow that loads these assets before launch uses those repo-resolved paths consistently.libs/cua-driver/test-harness/modality-recordings/linux/lin-run.sh-10-18 (1)
10-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLaunch the checked-in GTK recorder directly.
Line 17 hard-codes
/tmp/lin-rec.py, so this script does not actually run the recorder from the repository. Unless another step copied the file into/tmp, the wrapper cannot work from a fresh checkout.🤖 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/modality-recordings/linux/lin-run.sh` around lines 10 - 18, The launcher in lin-run.sh is hard-coded to use a temporary /tmp copy of the recorder, so it may not run the checked-in tool from a fresh checkout. Update the python3 invocation to reference the repository’s tracked GTK recorder directly, using the same recorder path symbol currently represented by /tmp/lin-rec.py, so the script works without any prior copy step.libs/cua-driver/test-harness/modality-recordings/windows/wpf-recorder.ps1-1-8 (1)
1-8: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTear down the selected toolkit, not just WPF.
This recorder advertises four toolkits, but cleanup only kills
CuaTestHarness.Wpf. After a WinUI3/WebView2/Electron run, the old harness can stay alive and Line 153 can resolve that stale window on the next invocation.Proposed fix
$tk=$TK[$Toolkit]; if(-not $tk){ Write-Output "unknown toolkit $Toolkit"; exit 3 } +$toolkitProcess = [System.IO.Path]::GetFileNameWithoutExtension($tk.exe) $ErrorActionPreference="Continue" @@ +Get-Process cua-driver,$toolkitProcess,chrome -EA SilentlyContinue | Stop-Process -Force -EA SilentlyContinue +Start-Sleep 2 Start-Process $drv -ArgumentList "serve" -WindowStyle Hidden; Start-Sleep 4 @@ -Get-Process cua-driver,CuaTestHarness.Wpf,chrome -EA SilentlyContinue | Stop-Process -Force -EA SilentlyContinue +Get-Process cua-driver,$toolkitProcess,chrome -EA SilentlyContinue | Stop-Process -Force -EA SilentlyContinueAlso applies to: 149-154, 228-228
🤖 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/modality-recordings/windows/wpf-recorder.ps1` around lines 1 - 8, The cleanup in wpf-recorder.ps1 is hardcoded to the WPF harness, so selected toolkits like WinUI3, WebView2, or Electron are not torn down correctly. Update the teardown logic to use the selected toolkit entry from $TK/$tk, and ensure the process/window cleanup path references the current toolkit’s exe/title instead of only CuaTestHarness.Wpf. Apply the same toolkit-aware fix anywhere the recorder resolves or closes the harness window so stale instances do not get reused on the next run.libs/cua-driver/test-harness/modality-recordings/linux/lin-run-electron.sh-12-25 (1)
12-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis wrapper launches a temp copy instead of the recorder in the repo.
Line 24 bypasses the checked-in
lin-rec-electron.pyand assumes it was copied into/tmpfirst. That makes the wrapper non-reproducible from the repository contents and easy to desync from future edits.🤖 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/modality-recordings/linux/lin-run-electron.sh` around lines 12 - 25, The Electron recording wrapper is invoking a temporary script copy instead of the checked-in recorder, which makes it drift from the repo. Update the `lin-run-electron.sh` flow so it executes the repository’s `lin-rec-electron.py` directly (the script referenced near the `python3` invocation) and remove the dependency on `/tmp`-copied content. Keep the rest of the launch sequence intact, but make sure the path resolution is tied to the repo location so future edits to `lin-rec-electron.py` are always picked up.libs/cua-driver/test-harness/modality-recordings/linux/lin-all-electron.sh-6-8 (1)
6-8: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInvoke the checked-in runner, not a
/tmpcopy.This loop never calls the
lin-run-electron.shadded in this PR; it only works if some external staging step has already copied that file into/tmp. From a normal checkout this batch runner will just fail to start.Proposed fix
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" + for MODE in ax-fg ax-bg vision-fg vision-bg vision-desktop; do - bash /tmp/lin-run-electron.sh "$MODE" >/dev/null 2>&1 + bash "$SCRIPT_DIR/lin-run-electron.sh" "$MODE" >/dev/null 2>&1 echo "DONE $MODE $(cat /tmp/cua-lin-electron-$MODE/metric.log 2>/dev/null)" | tee -a /tmp/lin-electron-allmodes.log🤖 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/modality-recordings/linux/lin-all-electron.sh` around lines 6 - 8, The batch loop in lin-all-electron.sh is still invoking a /tmp copy instead of the checked-in lin-run-electron.sh script, so it won’t work from a normal checkout. Update the runner invocation in the MODE loop to call the repository-backed script directly, keeping the rest of the logging and metric collection logic unchanged. Reference the existing lin-all-electron.sh loop and the lin-run-electron.sh script name so the correct call site is easy to find.libs/cua-driver/test-harness/modality-recordings/windows/run-one.ps1-1-4 (1)
1-4: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRun the recorder from this directory, not
C:\Users\Public.This wrapper never calls the
wpf-recorder.ps1introduced by the PR. It only works if someone has separately staged and renamed the script toC:\Users\Public\wpf-v7.ps1, which makes the checked-in wrapper unreliable.Proposed fix
param([string]$Mode="ax-bg") Get-Process cua-driver,CuaTestHarness.Wpf,chrome -EA SilentlyContinue | Stop-Process -Force -EA SilentlyContinue Start-Sleep 2 -& powershell -NoProfile -ExecutionPolicy Bypass -File C:\Users\Public\wpf-v7.ps1 -Mode $Mode *> "C:\Users\Public\cua-$Mode-run.log" +$scriptPath = Join-Path $PSScriptRoot 'wpf-recorder.ps1' +& powershell -NoProfile -ExecutionPolicy Bypass -File $scriptPath -Mode $Mode *> "C:\Users\Public\cua-$Mode-run.log"🤖 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/modality-recordings/windows/run-one.ps1` around lines 1 - 4, The run-one.ps1 wrapper is hardcoded to launch a script from C:\Users\Public instead of the checked-in recorder in this directory, so it bypasses the new wpf-recorder.ps1 flow. Update the wrapper to invoke the local recorder script relative to the current location, and keep the existing process cleanup and log redirection behavior intact. Use the run-one.ps1 entrypoint and the wpf-recorder.ps1 script name as the key references when making the change.libs/cua-driver/test-harness/apps/macos/swiftui/main.swift-132-180 (1)
132-180: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
click_targetstill cannot reportright_clickon SwiftUI.This target only ever writes
clickordouble_click; a secondary click is redirected to the separate context-menu control. That breaks parity with the shared click-target scenario, which is supposed to be the dedicated right/double-click surface across toolkits.🤖 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/apps/macos/swiftui/main.swift` around lines 132 - 180, The click_target control in main.swift only records left and double clicks, so it cannot surface a right_click action as expected by the shared scenario. Update the Button("Click target (left / right / double)") and its surrounding click handling so the target itself recognizes a secondary click and calls the same recording path with right_click, instead of relying on the separate context_menu section. Keep the existing double-click gesture for double_click, and make sure the click_target remains the dedicated surface for left/right/double interactions while leaving the context menu example separate.libs/cua-driver/test-harness/apps/macos/wkwebview/main.swift-43-49 (1)
43-49: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTreat a missing bundled
index.htmlas startup failure.Rendering an in-page error keeps the harness discoverable, so packaging regressions turn into noisy scenario failures later. This should abort immediately instead.
Suggested fix
if let url = Bundle.main.url(forResource: "index", withExtension: "html", subdirectory: "web") { webView.loadFileURL(url, allowingReadAccessTo: url.deletingLastPathComponent()) } else { - webView.loadHTMLString( - "<h2>FATAL: bundled web/index.html missing</h2>", baseURL: nil) + fatalError("Bundled web/index.html missing from Resources/web/") }🤖 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/apps/macos/wkwebview/main.swift` around lines 43 - 49, The WKWebView startup path in main.swift currently falls back to rendering an in-page fatal message when Bundle.main cannot find web/index.html, but this should be treated as an immediate startup failure instead. Update the load logic around webView.loadFileURL so that the app aborts or exits right away when the bundled HTML is missing, and remove the loadHTMLString fallback path entirely.libs/cua-driver/test-harness/apps/windows/winui3/MainWindow.xaml.cs-152-157 (1)
152-157: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFail fast when the scroll hook cannot be installed.
If
SetWindowSubclassfails here,scroll_targetsilently stops working and the recorder reports a misleading scroll regression instead of a startup error. Please check the return value and abort with the Win32 error.Suggested fix
private void InstallScrollMessageHook() { var hwnd = WinRT.Interop.WindowNative.GetWindowHandle(this); _subclassProc = SubclassWndProc; - SetWindowSubclass(hwnd, _subclassProc, 1, IntPtr.Zero); + if (!SetWindowSubclass(hwnd, _subclassProc, 1, IntPtr.Zero)) + { + throw new InvalidOperationException( + $"SetWindowSubclass failed: {Marshal.GetLastWin32Error()}"); + } }🤖 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/apps/windows/winui3/MainWindow.xaml.cs` around lines 152 - 157, InstallScrollMessageHook currently ignores the SetWindowSubclass result, so a hook failure is hidden and later looks like a scroll regression; update this method to check the return value after calling SetWindowSubclass on the hwnd and _subclassProc, and if it fails, fail fast by throwing or aborting with the Win32 error details so startup clearly reports the installation problem.libs/cua-driver/test-harness/shared/scenarios.json-209-216 (1)
209-216: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftThe shared Electron executable path is still Windows-only.
This PR adds non-Windows Electron launch/staging, but the shared config still hardcodes
test-apps/harness-electron/CuaTestHarness.Electron.exe. The new Linux staging script producesrust/test-apps/harness-electron/CuaTestHarness.Electroninstead, so any launcher that consumesexe_relative_pathcross-platform will look for a file that does not exist.🤖 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/shared/scenarios.json` around lines 209 - 216, The Electron harness config still hardcodes a Windows-only executable name, so cross-platform launchers will fail to find the staged binary. Update the `electron` entry in `scenarios.json` so `exe_relative_path` is platform-aware and can resolve the Linux-staged `CuaTestHarness.Electron` as well as the Windows `.exe`, keeping the existing `process_name` and other Electron scenario fields unchanged.libs/cua-driver/test-harness/shared/scenarios.json-186-207 (1)
186-207: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude the new shared web scroll IDs in
shared_dom_aids.
shared/web/index.htmlnow exportsscroll-tall,scroll-top,scroll-bottom, andlbl-scroll-offset, but this allowlist still stops at the pre-existing IDs. Any WebView/Electron parity check that usesshared_dom_aidsas the canonical shared-DOM contract will miss the newscroll_targetsurface entirely.💡 Suggested fix
"shared_dom_aids": [ "btn-increment", "btn-reset", "lbl-counter", "txt-input", "lbl-input-mirror", "sld-value", "lbl-slider-value", "border-click-target", "lbl-last-action", "lbl-click-count", "chk-agreed", "rdo-low", "rdo-med", "rdo-high", "lbl-chk-state", "cbo-color", "lbl-combo-value", "lnk-anchor", "lbl-nav-state", - "section-target" + "section-target", + "scroll-tall", + "scroll-top", + "scroll-bottom", + "lbl-scroll-offset" ]🤖 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/shared/scenarios.json` around lines 186 - 207, The shared DOM allowlist in shared_dom_aids is missing the new scroll-related web IDs, so update the array to include scroll-tall, scroll-top, scroll-bottom, and lbl-scroll-offset alongside the existing shared entries. Use shared_dom_aids in scenarios.json as the canonical contract and keep the new scroll_target surface aligned with the IDs exported by shared/web/index.html.libs/cua-driver/test-harness/apps/linux/gtk3/main.py-162-173 (1)
162-173: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve
double_clickthrough the secondclickedcallback.
on_click_target_press()marks the second press asdouble_click, buton_click_target()still runs afterward and immediately rewrites the visible state back toclick. That makes the harness report the wrong final action for a real double-click.💡 Suggested fix
class HarnessWindow(Gtk.Window): def __init__(self): super().__init__(title="CuaTestHarness GTK3") self.set_default_size(480, 760) self.counter = 0 self.clicks = 0 self._last_action = "none" + self._pending_click_action = None self._menu_action = "none" @@ def on_click_target(self, *_): self.clicks += 1 - self._last_action = "click" - self.click_status.set_text(f"last_action=click clicks={self.clicks}") + action = self._pending_click_action or "click" + self._pending_click_action = None + self._last_action = action + self.click_status.set_text(f"last_action={action} clicks={self.clicks}") def on_click_target_press(self, _w, ev): - if ev.type == Gdk.EventType.DOUBLE_BUTTON_PRESS: - self._last_action = "double_click" - self.click_status.set_text(f"last_action=double_click clicks={self.clicks}") + if ev.type == Gdk.EventType.DOUBLE_BUTTON_PRESS and ev.button == 1: + self._pending_click_action = "double_click" elif ev.button == 3: self._last_action = "right_click" self.click_status.set_text(f"last_action=right_click clicks={self.clicks}")🤖 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/apps/linux/gtk3/main.py` around lines 162 - 173, The double-click state is being overwritten because on_click_target() always sets the action back to click after on_click_target_press() has already marked a DOUBLE_BUTTON_PRESS as double_click. Update the click-handling flow in on_click_target and on_click_target_press so the later clicked callback preserves an existing double_click state instead of replacing it, keeping the final visible status correct for real double-clicks.
🟡 Minor comments (7)
libs/cua-driver/test-harness/modality-recordings/README.md-63-69 (1)
63-69: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe macOS scope note is no longer true.
Line 69 says macOS was out of scope because there was no recordable host session, but
FINDINGS.mdnow documents recorded AppKit, SwiftUI, Electron, and WKWebView runs. This README should either be updated or explicitly say it only covers the published WPF/GTK3 subset.🤖 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/modality-recordings/README.md` around lines 63 - 69, The macOS scope note in the README is now outdated because recorded AppKit, SwiftUI, Electron, and WKWebView runs are documented elsewhere. Update the Notes/known limitations section to either remove the claim that macOS was out of scope or replace it with a precise statement that this README only describes the published WPF/GTK3 subset, using the existing Notes section and the “macOS (AppKit/SwiftUI)” bullet as the anchor.libs/cua-driver/test-harness/modality-recordings/linux/lin-dashboard.html-17-29 (1)
17-29: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winStop feeding window titles into
innerHTML.
foregroundcomes fromxdotool getwindownameinlinux/lin-rec.py, so any active window title is treated as HTML here. That is enough to break the panel on ordinary titles containing</&, and a hostile title can inject script into the dashboard WebView.🤖 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/modality-recordings/linux/lin-dashboard.html` around lines 17 - 29, Stop inserting the `foreground` value from `status.json` into `innerHTML` in `tick()`, since window titles from `linux/lin-rec.py` can contain HTML and break or inject into the dashboard. Update the `lin-dashboard.html` rendering paths that build the foreground/status messages (`fg.innerHTML` and related subtext) to use text-safe DOM updates or escaping so the title is treated as plain text. Keep the existing `tick()` flow and `fg`/`now`/`rows` updates, but ensure all user-controlled title content is rendered safely.Source: Linters/SAST tools
libs/cua-driver/test-harness/modality-recordings/FINDINGS.md-184-205 (1)
184-205: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis findings section is stale against fixes already in this branch.
Lines 187-188 still describe AXIncrement/AXDecrement stepping as “future driver work”, and Lines 204-205 still list ended-session reuse poisoning as an open gotcha. The stack for this PR already adds both the numeric stepping fallback and session revival/loud rejection, so this section now misstates what shipped.
🤖 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/modality-recordings/FINDINGS.md` around lines 184 - 205, The findings document is stale and still describes issues that are already fixed in this branch. Update the SwiftUI slider note in FINDINGS.md to reflect the current `set_number_attr` behavior and remove the “future driver work” wording about AXIncrement/AXDecrement, and also revise the macOS gotchas section to stop listing ended-session reuse poisoning as an open problem since session revival/loud rejection is already implemented. Keep the wording aligned with the actual behavior of `set_number_attr`, `end_session`, and the related session-handling logic.libs/cua-driver/test-harness/modality-recordings/linux/lin-dashboard-electron.html-17-25 (1)
17-25: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winStop feeding
foregroundintoinnerHTML.
status.json.foregroundcomes from the active window title, so Lines 19-21 turn external desktop state into live HTML. A titled window containing markup is enough to script or corrupt the dashboard. Build the status DOM withtextContent/created nodes instead.🤖 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/modality-recordings/linux/lin-dashboard-electron.html` around lines 17 - 25, The tick() rendering logic is using status.json.foreground inside innerHTML for the fg message, which allows untrusted desktop window titles to be interpreted as HTML. Update the tick() path to build the status DOM with textContent or explicit created nodes instead of concatenating s.foreground into innerHTML, and keep the existing fg/now/rows updates but ensure any dynamic text from s.foreground is inserted as plain text.Source: Linters/SAST tools
libs/cua-driver/test-harness/modality-recordings/windows/wpf-recorder.ps1-47-58 (1)
47-58: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAvoid
innerHTMLfor the foreground title.
Flush()publishes the live foreground window title, and the dashboard template injects it withinnerHTML. A window title containing markup can therefore alter or script the panel instead of just being displayed.Also applies to: 139-140
🤖 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/modality-recordings/windows/wpf-recorder.ps1` around lines 47 - 58, The foreground status rendering in tick() is unsafe because it injects the live window title into fg.innerHTML, which can allow markup from Flush()’s foreground title to alter the dashboard. Update the foreground/title display logic to insert the title as plain text instead of HTML, and keep the surrounding status markup separate; apply the same safe rendering approach to the related foreground-mode path noted in the comment.Source: Linters/SAST tools
libs/cua-driver/test-harness/apps/windows/winui3/MainWindow.xaml.cs-100-105 (1)
100-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the platform double-click interval instead of
600ms.This hardcoded window can promote two unrelated clicks to
double_click, and it can also miss genuine doubles on hosts with a different system setting. For a parity harness, this should track the native double-click threshold instead of baking in its own.🤖 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/apps/windows/winui3/MainWindow.xaml.cs` around lines 100 - 105, The double-click detection in OnTargetClick is using a hardcoded 600ms threshold, which should instead follow the platform’s native setting. Update the double-click check to use the system double-click interval rather than comparing against a fixed value, and keep the logic localized to MainWindow so the harness matches host behavior.libs/cua-driver/test-harness/apps/cross-platform/electron/build.sh-1-13 (1)
1-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the repo-standard SPDX header to this new script.
CI is already flagging missing SPDX headers under
libs/cua-driver, and this file currently ships without one.🤖 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/apps/cross-platform/electron/build.sh` around lines 1 - 13, Add the repo-standard SPDX header to this new build.sh script. Update the script header near the existing shebang and introductory comments so it matches the licensing pattern used elsewhere under libs/cua-driver, keeping the change aligned with build.sh’s top-of-file metadata.Source: Pipeline failures
🧹 Nitpick comments (1)
libs/cua-driver/rust/crates/cua-driver/src/serve.rs (1)
1631-1635: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the new
revivedresponse contract.The follow-up probe proves behavioral revival, but a regression returning
revived: falsefromStartSessionToolwould still pass. Please assert thestart_sessionresponse includesstructuredContent.revived == truefor this ended id.🤖 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/src/serve.rs` around lines 1631 - 1635, The start_session test in serve.rs only checks resp.ok, so it can miss regressions where StartSessionTool no longer reports revival. Update the assertion around the send_request/start_session flow to verify the response’s structuredContent includes revived == true for the ended id, using the existing resp from the spawn_blocking call and the start_session response contract.
🤖 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.
Major comments:
In `@libs/cua-driver/rust/crates/platform-macos/src/tools/set_value.rs`:
- Around line 242-249: The early-exit check in `set_value.rs` is too permissive
because `step_radius` starts at 1.0 before any step is observed, allowing
`set_value` to return success on a one-unit miss without actually moving the
control. Update the convergence logic in the loop that uses `step_radius` so the
initial tolerance is stricter until a real step size has been measured, and only
allow the wider radius after observing movement. Keep the fix localized around
the `step_radius` initialization and the `if (current - target).abs() <=
step_radius` success path.
In `@libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs`:
- Around line 2248-2254: The pixel-addressed click paths in the window tool
still bypass the same bounds check used for cached element centers, so stale or
out-of-range screenshot coordinates can be injected into the wrong target.
Update the foreground/background branches in the click handling code (including
the click, double_click, and right_click paths) to validate sx_i/sy_i with
point_in_window_bounds before calling send_click_synthesized or
inject_click_screen, and return a ToolResult::error with the same style of
outside-window message when the point is not within hwnd.
In `@libs/cua-driver/test-harness/apps/linux/gtk3/main.py`:
- Around line 162-173: The double-click state is being overwritten because
on_click_target() always sets the action back to click after
on_click_target_press() has already marked a DOUBLE_BUTTON_PRESS as
double_click. Update the click-handling flow in on_click_target and
on_click_target_press so the later clicked callback preserves an existing
double_click state instead of replacing it, keeping the final visible status
correct for real double-clicks.
In `@libs/cua-driver/test-harness/apps/macos/swiftui/main.swift`:
- Around line 132-180: The click_target control in main.swift only records left
and double clicks, so it cannot surface a right_click action as expected by the
shared scenario. Update the Button("Click target (left / right / double)") and
its surrounding click handling so the target itself recognizes a secondary click
and calls the same recording path with right_click, instead of relying on the
separate context_menu section. Keep the existing double-click gesture for
double_click, and make sure the click_target remains the dedicated surface for
left/right/double interactions while leaving the context menu example separate.
In `@libs/cua-driver/test-harness/apps/macos/wkwebview/main.swift`:
- Around line 43-49: The WKWebView startup path in main.swift currently falls
back to rendering an in-page fatal message when Bundle.main cannot find
web/index.html, but this should be treated as an immediate startup failure
instead. Update the load logic around webView.loadFileURL so that the app aborts
or exits right away when the bundled HTML is missing, and remove the
loadHTMLString fallback path entirely.
In `@libs/cua-driver/test-harness/apps/windows/winui3/MainWindow.xaml.cs`:
- Around line 152-157: InstallScrollMessageHook currently ignores the
SetWindowSubclass result, so a hook failure is hidden and later looks like a
scroll regression; update this method to check the return value after calling
SetWindowSubclass on the hwnd and _subclassProc, and if it fails, fail fast by
throwing or aborting with the Win32 error details so startup clearly reports the
installation problem.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-all-electron.sh`:
- Around line 6-8: The batch loop in lin-all-electron.sh is still invoking a
/tmp copy instead of the checked-in lin-run-electron.sh script, so it won’t work
from a normal checkout. Update the runner invocation in the MODE loop to call
the repository-backed script directly, keeping the rest of the logging and
metric collection logic unchanged. Reference the existing lin-all-electron.sh
loop and the lin-run-electron.sh script name so the correct call site is easy to
find.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-all.sh`:
- Around line 2-5: The batch summary in lin-all.sh is marking every MODE as DONE
even when bash /tmp/lin-run.sh fails and metric.log is missing. Update the loop
around the lin-run.sh invocation and the DONE echo so it only records success
when the runner exits successfully; if it fails, emit a failure message or skip
appending DONE for that MODE. Use the existing MODE loop and the lin-run.sh call
as the key points to gate the tee -a /tmp/lin-allmodes.log output.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-harness.py`:
- Line 1: This new modality-recording file is missing the repo’s required SPDX
header and is failing the SPDX check; add the standard SPDX license header at
the top of lin-harness.py and apply the same header to the other newly added
modality-recording assets in this cohort so they match the existing repository
convention.
- Around line 132-139: The click handler in _on_click is overwriting a
previously recorded double_click from _on_btn_press, so update the click path to
preserve an existing double_click state instead of always setting _last_action
to click. Use the _on_click and _on_btn_press methods in lin-harness.py to gate
the click update so a GTK double-click final release does not get downgraded,
while still keeping the click count and state save behavior intact.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-rec-electron.py`:
- Around line 19-20: The dashboard asset paths are hardcoded to /tmp, which
bypasses the files added to the repo and breaks clean checkouts. Update the
constants in lin-rec-electron.py for DASH and DASH_HTML so they resolve from the
repository location instead of /tmp, and make sure the path usage in the
recording flow that loads these assets before launch uses those repo-resolved
paths consistently.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-rec.py`:
- Around line 159-170: The resolve loop in lin-rec.py is breaking too early
because it exits as soon as either the checkbox or text field is found, while
later steps still require btn, ctx, sld, and scr. Update the loop condition in
the recording setup block so it waits until all required controls for the plan
are resolved before continuing, using the existing resolve dict keys in the
els()/find() lookup logic rather than only chk/txt.
- Around line 93-98: Clear out stale recording artifacts before each run so the
harness cannot reuse yesterday’s outputs. In lin-rec.py, update the setup around
os.makedirs(REC, exist_ok=True) to remove old files in REC (and any run-specific
status/metrics artifacts in WORK) before starting, while keeping the
wrapper-owned daemon behavior unchanged. Also update the selection logic near
the MP4 lookup to only accept artifacts produced by the current run, rather than
the first *.mp4 found under REC.
- Around line 217-218: The type step in lin-rec.py ignores the selected text-box
target and only emits type_text to the window pid, so the keystrokes may go to
whatever control still has focus. Update the type-handling branch in the step
dispatcher to honor sel == "txt" by targeting the text box explicitly before
typing, using the existing plan/selection flow around D and the type case so the
action truly types into the selected textbox.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-run-electron.sh`:
- Around line 12-25: The Electron recording wrapper is invoking a temporary
script copy instead of the checked-in recorder, which makes it drift from the
repo. Update the `lin-run-electron.sh` flow so it executes the repository’s
`lin-rec-electron.py` directly (the script referenced near the `python3`
invocation) and remove the dependency on `/tmp`-copied content. Keep the rest of
the launch sequence intact, but make sure the path resolution is tied to the
repo location so future edits to `lin-rec-electron.py` are always picked up.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-run.sh`:
- Around line 10-18: The launcher in lin-run.sh is hard-coded to use a temporary
/tmp copy of the recorder, so it may not run the checked-in tool from a fresh
checkout. Update the python3 invocation to reference the repository’s tracked
GTK recorder directly, using the same recorder path symbol currently represented
by /tmp/lin-rec.py, so the script works without any prior copy step.
In `@libs/cua-driver/test-harness/modality-recordings/README.md`:
- Around line 27-33: The WPF recording metrics in the README are inconsistent
with the rest of the findings bundle. Update the summary table in the README to
match the authoritative values used by index.html and FINDINGS.md for the same
recording entries, especially the wpf-ax-bg and wpf-vision-bg counts. Verify the
numbers across the related documentation artifacts and make them all agree
before publishing.
In `@libs/cua-driver/test-harness/modality-recordings/windows/run-one.ps1`:
- Around line 1-4: The run-one.ps1 wrapper is hardcoded to launch a script from
C:\Users\Public instead of the checked-in recorder in this directory, so it
bypasses the new wpf-recorder.ps1 flow. Update the wrapper to invoke the local
recorder script relative to the current location, and keep the existing process
cleanup and log redirection behavior intact. Use the run-one.ps1 entrypoint and
the wpf-recorder.ps1 script name as the key references when making the change.
In `@libs/cua-driver/test-harness/modality-recordings/windows/wpf-recorder.ps1`:
- Around line 1-8: The cleanup in wpf-recorder.ps1 is hardcoded to the WPF
harness, so selected toolkits like WinUI3, WebView2, or Electron are not torn
down correctly. Update the teardown logic to use the selected toolkit entry from
$TK/$tk, and ensure the process/window cleanup path references the current
toolkit’s exe/title instead of only CuaTestHarness.Wpf. Apply the same
toolkit-aware fix anywhere the recorder resolves or closes the harness window so
stale instances do not get reused on the next run.
In `@libs/cua-driver/test-harness/shared/scenarios.json`:
- Around line 209-216: The Electron harness config still hardcodes a
Windows-only executable name, so cross-platform launchers will fail to find the
staged binary. Update the `electron` entry in `scenarios.json` so
`exe_relative_path` is platform-aware and can resolve the Linux-staged
`CuaTestHarness.Electron` as well as the Windows `.exe`, keeping the existing
`process_name` and other Electron scenario fields unchanged.
- Around line 186-207: The shared DOM allowlist in shared_dom_aids is missing
the new scroll-related web IDs, so update the array to include scroll-tall,
scroll-top, scroll-bottom, and lbl-scroll-offset alongside the existing shared
entries. Use shared_dom_aids in scenarios.json as the canonical contract and
keep the new scroll_target surface aligned with the IDs exported by
shared/web/index.html.
---
Minor comments:
In `@libs/cua-driver/test-harness/apps/cross-platform/electron/build.sh`:
- Around line 1-13: Add the repo-standard SPDX header to this new build.sh
script. Update the script header near the existing shebang and introductory
comments so it matches the licensing pattern used elsewhere under
libs/cua-driver, keeping the change aligned with build.sh’s top-of-file
metadata.
In `@libs/cua-driver/test-harness/apps/windows/winui3/MainWindow.xaml.cs`:
- Around line 100-105: The double-click detection in OnTargetClick is using a
hardcoded 600ms threshold, which should instead follow the platform’s native
setting. Update the double-click check to use the system double-click interval
rather than comparing against a fixed value, and keep the logic localized to
MainWindow so the harness matches host behavior.
In `@libs/cua-driver/test-harness/modality-recordings/FINDINGS.md`:
- Around line 184-205: The findings document is stale and still describes issues
that are already fixed in this branch. Update the SwiftUI slider note in
FINDINGS.md to reflect the current `set_number_attr` behavior and remove the
“future driver work” wording about AXIncrement/AXDecrement, and also revise the
macOS gotchas section to stop listing ended-session reuse poisoning as an open
problem since session revival/loud rejection is already implemented. Keep the
wording aligned with the actual behavior of `set_number_attr`, `end_session`,
and the related session-handling logic.
In
`@libs/cua-driver/test-harness/modality-recordings/linux/lin-dashboard-electron.html`:
- Around line 17-25: The tick() rendering logic is using status.json.foreground
inside innerHTML for the fg message, which allows untrusted desktop window
titles to be interpreted as HTML. Update the tick() path to build the status DOM
with textContent or explicit created nodes instead of concatenating s.foreground
into innerHTML, and keep the existing fg/now/rows updates but ensure any dynamic
text from s.foreground is inserted as plain text.
In `@libs/cua-driver/test-harness/modality-recordings/linux/lin-dashboard.html`:
- Around line 17-29: Stop inserting the `foreground` value from `status.json`
into `innerHTML` in `tick()`, since window titles from `linux/lin-rec.py` can
contain HTML and break or inject into the dashboard. Update the
`lin-dashboard.html` rendering paths that build the foreground/status messages
(`fg.innerHTML` and related subtext) to use text-safe DOM updates or escaping so
the title is treated as plain text. Keep the existing `tick()` flow and
`fg`/`now`/`rows` updates, but ensure all user-controlled title content is
rendered safely.
In `@libs/cua-driver/test-harness/modality-recordings/README.md`:
- Around line 63-69: The macOS scope note in the README is now outdated because
recorded AppKit, SwiftUI, Electron, and WKWebView runs are documented elsewhere.
Update the Notes/known limitations section to either remove the claim that macOS
was out of scope or replace it with a precise statement that this README only
describes the published WPF/GTK3 subset, using the existing Notes section and
the “macOS (AppKit/SwiftUI)” bullet as the anchor.
In `@libs/cua-driver/test-harness/modality-recordings/windows/wpf-recorder.ps1`:
- Around line 47-58: The foreground status rendering in tick() is unsafe because
it injects the live window title into fg.innerHTML, which can allow markup from
Flush()’s foreground title to alter the dashboard. Update the foreground/title
display logic to insert the title as plain text instead of HTML, and keep the
surrounding status markup separate; apply the same safe rendering approach to
the related foreground-mode path noted in the comment.
---
Nitpick comments:
In `@libs/cua-driver/rust/crates/cua-driver/src/serve.rs`:
- Around line 1631-1635: The start_session test in serve.rs only checks resp.ok,
so it can miss regressions where StartSessionTool no longer reports revival.
Update the assertion around the send_request/start_session flow to verify the
response’s structuredContent includes revived == true for the ended id, using
the existing resp from the spawn_blocking call and the start_session response
contract.
Port the macOS #1796 retain-under-lock fix to platform-windows. The element cache handed out a bare COM vtable pointer; under concurrent sessions a get_window_state snapshot-replace could Release the element to zero while an in-flight click/type/set_value still dereferenced it (use-after-free → daemon crash), the Windows analogue of the macOS AXUIElementCopyActionNames SIGTRAP. New RetainedElement guard AddRefs the element inside with_snapshot (under the same mutex insert takes), so the refcount can't reach zero between lookup and AddRef; the guard's +1 keeps the object alive across a concurrent replace and Releases on drop. ClickTool / TypeTextTool / SetValueTool hold the guard for the whole blocking action. Compile-verified on the Windows VM (cargo build -p platform-windows, clean); the concurrency race was not runtime-reproduced (timing-dependent dual-session). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…-only widgets Two AT-SPI gaps surfaced by the modality verifier: 1. Pixel clicks were silent no-ops on GTK. GTK3/4 take input via XInput2, so the background XSendEvent path (synthetic, send_event=True) is dropped, and XTEST core events don't reach an XI2-only client (measured on the Xvfb stack: xinput sees the event, the GTK client sees zero). Route a plain left single-click through AT-SPI instead: hit-test the window-local pixel to the smallest-area actionable element (GetExtents(CoordType::Window)) and doAction it — lands without activating/raising the window (no-foreground contract preserved, verified). Scoped to left single-clicks; right/double/ middle and points with no actionable element fall back to XSendEvent. 2. Sliders / scroll bars never appeared in get_window_state. The walk indexed only nodes advertising Actions; GtkScale and scroll bars expose the Value interface but no Action, so they were dropped from `elements` (though they showed in tree_markdown). New is_indexable = !actions.is_empty() || has_value, applied identically in render and every action_nodes filter to keep the index space consistent. element_count 8->13 on the GTK3 harness; set_value on the slider (0->50) and scroll bar (scroll_offset 0->300) now work. Compile-verified on the Linux VM (cargo build -p platform-linux clean, 32 platform-linux unit tests pass). Honest scope: right/double-click pixel actions on GTK still can't land on this Xvfb (no AT-SPI doAction equivalent) and were left unchanged rather than shipping a focus-stealing XTEST hack. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…+ fix MP4 pipeline The cross-toolkit recorder under-recorded WinUI3/WebView2 and produced empty MP4s. Fixes: - WebView2: set WEBVIEW2_ADDITIONAL_BROWSER_ARGUMENTS=--force-renderer- accessibility so the web DOM surfaces in get_window_state (without it only the chrome frame appeared → every web action resolved to nothing → SIZE=0). - ffmpeg: prepend the Gyan.FFmpeg bin to PATH before serve. The daemon probes cuademo's WinGet LOCALAPPDATA but the task runs as fbonacci, so the probe missed ffmpeg → video present:false. - Resolver now finds the new parity controls across toolkits: scroll-tall by role Pane|Group, click-target by label or name, checkbox by role or "agree", context-menu falls back to the click-target (WinUI3/web record last_action=right_click); verifier accepts last_action=right_click. - Honest: WebView2's checkbox sits below the fold and web content can't be scrolled by the driver in ax mode (AX-scroll/WM_MOUSEWHEEL/PageDown all no-ops — the host HWND doesn't route scroll to the Chromium renderer), so its left-click step reports a true no-op rather than being faked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
… fixes WinUI3 (3/7) + WebView2 (4/7) re-recorded with parity controls (frame-verified, 0 focus steals); WebView2 needed --force-renderer-accessibility + ffmpeg-on-PATH + an attached Session 2. WinUI3 double/right-click confirmed not WPF-path-fixable (needs a composition-input-site path). Plus the Windows UAF, Linux GTK left-click/value-widget, and macOS numeric-set_value fixes from this round. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
How the modality suite works (harness-left/dashboard-right recorder, 5 modality modes, dual ✓worked/STOLE scoring), the per-OS/toolkit harnesses + their 6 parity controls + instrumented labels, capture modes (ax/vision/som) × scopes (window/desktop), the 8×6 action×control matrix, and the core results matrix (OS × harness × mode × action → success/failure/limit with commit SHAs). Plus the vision-agent coordinate-invariant test and the two overfitting caveats. GTK real-Xorg path and the Electron 0/8 re-baseline are recorded as runtime-verified (the doc's briefing predated those results). Named MODALITY_TEST_SUITE.md to avoid clobbering the existing Rust-testkit TEST_SUITE.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…genuine baseline Same flaw the Windows recorder had (49bdb41): lin-rec.py + lin-rec-electron.py only re-asserted the dashboard panel with `wmctrl -b add,above` (z-order / _NET_WM_STATE_ABOVE, NO activation), so they never held a real foreground baseline — once the harness was activated, xdotool getactivewindow stuck on it and later steps false-positived as steals, inflating the counts. Fix: launch a real xterm anchor (-class cua-anchor) once in background modes, parked under the above-panel rect (invisible in the recording but a valid activatable non-harness foreground window). Before each action, anchor_front() does a genuine `xdotool windowactivate --sync` on it, confirms getactivewindow == anchor, logs anchor_held, then re-asserts the panel ABOVE (z-order only). steal = active window moves OFF the anchor onto the harness. Re-measured: GTK3 ax-bg 3/8 -> 1/8, Linux-Electron ax-bg 3/8 -> 1/8 (only set_value genuinely steals; the rest were baseline artifacts). Anchor held before all 8 steps in both runs (baseline.log + independent metric.log). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…aseline fix GTK3 + Linux-Electron ax-bg 3/8 -> 1/8 (only set_value genuinely steals; the rest were recorder-baseline artifacts, fixed in 5c0a1d3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
Add reference/cua-driver/modality-test-suite.mdx (from the test-harness MODALITY_TEST_SUITE.md) + nav entry: how the suite works, the harnesses per OS/toolkit + their instrumented read-back labels, capture modes x scopes, the action matrix, the per-platform results matrix, the vision-agent test, and the overfitting caveats. Dropped the repo-internal preamble (branch ref, naming note) for the public page. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…s leaking sessions (a) New cua-driver-core integration test (tests/session_lifecycle.rs, 3 tests, cross-platform): start_session → touch → end_session DISPOSES + a reused id starts fresh (not poisoned); idle-TTL evicts an untouched session and disposes it the SAME way end_session does; a just-touched session survives a short sweep. The concrete cursor/config registries live in platform/daemon layers, so the test asserts the disposal HOOK CONTRACT both end_session and evict_idle fan out to (fire_session_end → each hook fires exactly once on both paths); registry post-state is deferred-to-daemon (documented in the test). Uses a short Duration passed straight to evict_idle — the production 300s default is unchanged. (b) Recorder session-disposal audit: 3 of 4 recorders LEAKED their agent-cursor session (no end_session, relying on the TTL). Fixed: lin-rec.py / lin-rec-electron.py / wpf-recorder.ps1 now end_session in a try/finally BEFORE they kill the daemon; mac-rec.py already ended it on the happy path — added an atexit net so it disposes on error too (it doesn't kill the daemon). Verified: cargo test -p cua-driver-core --test session_lifecycle (3 passed); python + PowerShell recorders parse clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…uite reference Add a "Session lifecycle & disposal" subsection (both the harness MD and the fumadocs MDX, kept in sync): sessions own the agent cursor; end_session + the idle-TTL sweep (300s default, env-tunable) both dispose via the same fire_session_end hook fan-out; the new session_lifecycle.rs test; and that the recorders now end_session in try/finally so they don't leak. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…ection Add §9 "Edge cases — real closed-source apps" to the modality test-suite doc (both the test-harness MD and the fumadocs MDX copy), consolidating the overnight findings from driving live closed-source apps: - macOS (Finder, System Settings, Calculator, Safari): pixel-click hits right pixel/wrong window in overlapping same-pid windows; set_value + type_text both falsely succeed on a background search field; Finder filenames lack AXPress; Calculator result is AX-invisible; AppKit AX-tree duplication bloat. - Windows (UWP Calculator/Notepad, Edge, Explorer): UWP window-identity split (real HWND owned by ApplicationFrameHost); real UWP drivable via the element path without the uiAccess worker; real Edge/Chromium holds the foreground contract (synthetic-Electron shield generalizes); element_index requires pid (fail-fast under-documented); get_screen_size under-reports desktop width. Also correct the Linux-Electron ax-bg row from 1/8 to 2/8 stole: drag lands a value-change but its synthetic window-coord activates Chromium without reaching the slider thumb (measured in the overnight re-record with the genuine-anchor recorder). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…hotos/Snipping Tool) Append findings F–I + a methodology note to §9 of the modality test-suite doc (both copies), from a second real-app hunt: - F (headline, DoS-class): an unhandled-protocol launch_app (e.g. bingmaps: with Maps uninstalled) spawns a Windows "you'll need a new app" modal on the daemon's session desktop and deadlocks the worker thread inside the shell-launch call — the whole daemon wedges and stop reports "not running". Any bad app name/protocol is a full-daemon DoS. Needs a non-blocking launch + timeout / handler validation. - G: one ApplicationFrameHost pid multiplexes N unrelated UWP apps → pid is not an app identity, only window_id is. - H: launch_app's return contract is per-app-architecture (brokered pid:0, real-pid /empty-windows race, clean pid+window; Store is a 3-way launch/AFH/WinStore.App split). - I: the AFH UIA root is a caption-only ~188x32 strip while content frames are full (dual-provider window). - Methodology note: a disconnected-RDP console session returns GetForegroundWindow=0, so focus-steal isn't measurable by that probe there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…locks the daemon
An unregistered protocol or file association (e.g. `bingmaps:` with Maps not
installed) made the ShellExecuteExW("open") launch path block on a modal shell
dialog ("you'll need a new app to open this …") raised on the *session* desktop.
A Session-0/headless daemon can neither see nor dismiss it, so the launch — and
that request — wedged indefinitely; a follow-up `stop` then reported "daemon is
not running" because the wedged worker couldn't service the pipe. Any bad app
name/protocol was a full-daemon DoS.
Two layered fixes on the legacy ShellExecuteExW path:
- Add SEE_MASK_FLAG_NO_UI to the primary and secondary (extra-URL) launches so
the no-association case fails fast with an error code instead of raising the
"open with" / store-suggestion UI.
- Wrap the spawn_blocking launch in a 15s tokio timeout as a backstop for any
*other* blocking broker dialog (SmartScreen, an elevation/consent surface):
on timeout the request returns a clear error and the daemon stays responsive
instead of hanging. (A spawn_blocking thread can't be cancelled, so the
abandoned thread unblocks if/when the modal is dismissed — but NO_UI keeps the
common no-handler case from ever raising one.)
The UWP/AUMID branch is unaffected (it has its own Session-0 short-circuit guards
in launch_uwp). Found via the overnight real-app edge-case hunt (bingmaps:).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…ct UWP uiAccess worker comment 1) element_index schema docs: state that element_index REQUIRES pid (and window_id where the platform needs it) across click/double_click/right_click/ set_value/type_text on macOS, Windows, and Linux. An agent that omits pid and filters stderr was reading the "Missing required integer field: pid" fail-fast as a silent no-op; the descriptions now spell out that contract. Validation logic is unchanged. Mirrored into docs mcp-tools.mdx. 2) serve.rs / #1602 comments: correct the "UWP needs the uiAccess worker" assumption. The element_index UIA Invoke/ValuePattern path drives real UWP as-is from the Medium-IL daemon (verified: Calculator num5Button 0->5, no worker). Only the pixel/SendInput path needs the worker, and only against AppContainer (UWP) targets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…meHost host window launch_app of a UWP app (Calculator, Settings, Store, Photos) returned the real packaged-app pid from IApplicationActivationManager::ActivateApplication (e.g. CalculatorApp.exe), but a UWP app's top-level window — the HWND a caller must actually drive — is owned by ApplicationFrameHost.exe, not by that process. The app process only owns a Windows.UI.Core.CoreWindow reparented as a child of the AFH frame. So list_windows(Some(app_pid)) was empty and the returned pid+window_id resolved to no window: get_window_state errored "No window with window_id … exists" and element actions never landed. One AFH pid also hosts many unrelated UWP apps, so pid alone is not an app identity — only the specific frame HWND disambiguates. Add win32::resolve_uwp_host_window(app_pid): walk every visible top-level ApplicationFrameWindow, scan its child windows, and return the frame whose child CoreWindow is owned by app_pid. The child stays owned by the app process even while reparented under the AFH frame, so child-window process ownership is a stable identity link that survives the HWND churn UWP activations exhibit right after launch. launch_app's UWP branch now reports (frame_hwnd, afh_pid) with a 10×200ms retry budget (the frame lags the process), so the returned handles validate in get_window_state and element actions land. Non-UWP launches are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…lick/drag/press-key fidelity From video feedback on the wpf ax-fg recording: - Panel mojibake (âœ" / · for ✓ ·): PowerShell 5.1 reads the no-BOM .ps1 as Windows-1252, mangling the literal glyphs before they're written. Keep the script pure-ASCII — emit the panel glyphs as HTML entities (✓ ✗ ·) and JS \u escapes (✓ ✗ ▶ ·); also set explicit charset=utf-8 on the loopback server's Content-Type. - Right-click scored a false no-op: the recorder opened the WPF context menu then ESC-dismissed it, so menu_action stayed "none". In WPF foreground mode, dwell so the menu is visible then pick the first item (Down+Enter) -> menu_action=ctx_*, scored as landed. Background modes keep ESC (preserves the known steal measure). - Drag / textbox were off-screen below the 556px reflow (drag uses pixel coords -> landed off-screen -> no-op; textbox invisible). Add a UIA ScrollItemPattern ScrollIntoView before the slider/textbox/context-button steps and re-resolve the element so its post-scroll frame drives the cursor glide + coordinate dispatch. - press-key reported neither worked nor no-op: no 'key' Verify case (-> 'na', shown as blank), and the harness exposes no key-event signal. Change the step to focus the textbox + Backspace (edits the field -> mirror changes) and add a 'key' Verify on mirror-change. Also parse counter= in ReadState for future use. Re-record of wpf ax-fg to follow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…und in fg modes Re-record showed drag-slider still a no-op even with the slider scrolled into view. Root cause (confirmed in platform-windows/tools/impl_.rs:4741-4746): a WPF Slider thumb only tracks a real SendInput drag — the default PostMessage drag path doesn't update per-thread input state, so the thumb "sees a button-up world during the drag and never starts tracking." The recorder's drag step passed no dispatch field (unlike the click step, which already switches by fg/bg), so it fell to the PostMessage path. Pass dispatch:foreground in foreground modes (ax-fg / vision-fg) so the thumb follows a real drag; background modes keep dispatch:background (honest: the thumb won't move, matching the documented limitation). Driver already supports this — recorder-only fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
… in fg modes (fixes drag) Two clean re-records still showed drag-slider failing even with dispatch:foreground. Root cause: the drag's SendInput path (send_drag_synthesized) calls SetForegroundWindow(target) and BAILS if the swap is rejected — and in fg mode the recorder never actually foregrounded the harness (the foreground-assert loop was background-only). The panel even showed "FOREGROUND MODE" with sub "--" (not "app is foreground"), confirming the harness wasn't foreground. Element-path steps (click/double/right/set_value/type) don't care, so only the coordinate drag no-oped. Add an fg-mode branch mirroring the bg anchor: [W]::Front(hHar) (ALT-tap + SetForegroundWindow unlock) with an 8x retry before each step, so the harness is genuinely foreground when the drag's SetForegroundWindow runs. Also makes the panel correctly show "app is foreground". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…nate steps only The fg-mode foreground-assert (added to fix drag) regressed double-click and right-click: [W]::Front taps ALT to unlock SetForegroundWindow, which drops the WPF window into menu-accelerator mode, so the element-path double/right then mis-fire. Element-path steps don't need foreground at all — only coordinate dispatch does (the drag's SendInput). Gate the assert to vision/desktop modes + the ax-mode drag step. Restores double/right (element path, no ALT taint) while keeping drag landing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…ames, not stale coords WKWebView modality clips showed every action failing. Root cause: the WEBISH dispatch used a hardcoded per-surface COORDS_PT map that had drifted ~185px off WKWebView's actual web layout (click_target hardcoded y=459 vs actual 644), so every pixel action missed the target and no-oped — and the assumption that web spans have "no AX press" was wrong. Verified live on the macOS daemon: element_index AXPress lands click/double/right on the web spans, type_text works, and a pixel click/drag lands when given coords derived from the real element frame. Rewrite do_electron: - AX mode dispatches by element_index (the actual ax path) for click/double/right/type. - Vision mode derives pixel coords from the live element frame (web_el reads the smallest-index copy — WebKit duplicates the web subtree, lower index is hittable). - drag uses a real synthetic pixel drag at the live slider frame (fires the range input event; AXValue-set does not). - set_value stays element_index but is an honest no-op on web inputs (sets AXValue without firing the DOM input event — mirror doesn't update); type_text is the working path. Result (ax-fg, frame-verified): 5/6 landed — click/double/right/drag/type all ok, only set_value the honest web no-op (was ~0/6 before). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…SC clears menu mode Drag-only gating still let the slider drag no-op (foreground drifted before the daemon's own SetForegroundWindow ran), while every-step assert broke double/right via ALT menu-mode. Do both: assert harness-foreground on every fg step (keeps the state primed for the drag), then press ESC to exit the ALT-induced menu-accelerator mode so element-path double/right-click aren't tainted. Targets 7/8 (scroll stays the off-screen no-op). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…onest web set_value Update the WKWebView results row (both doc copies): the web dispatch now derives coords from the live element frame (was a stale hardcoded map ~185px off, so every WKWebView pixel action missed and the clips read as all-failing). Re-measured ax-fg 5/6 — click/double/right/drag/type land; set_value is the honest web no-op (AXValue set doesn't fire the DOM input event, so the mirror doesn't update; type_text works). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…set_value (resolves drag-vs-clicks) The WPF slider coordinate-drag and the element-path double/right-click are mutually exclusive: a SendInput thumb-drag only tracks under forced harness-foreground, but forcing foreground re-activates the window and eats the element double/right clicks (confirmed across 6 runs — every config traded one for the other). Resolve it the same way the WKWebView fix did: in AX mode use the element path. Drive the slider through its RangeValue pattern (set_value) — the ax-path way to move a slider, reliable and foreground-independent — so no ax step needs a forced foreground and double/right keep working. Vision mode keeps the real pixel drag (+ its foreground-assert, now gated to vision/desktop only, where there are no element clicks to taint). Targets ax-fg 7/8 (scroll the lone off-screen no-op). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
…SOrderWindow (#1800) The macOS agent cursor floated frontmost instead of sitting just above the window it acts on. The overlay was correctly at NSNormalWindowLevel and the tools sent PinAbove(target_wid), but the z-order enforcer used AppKit's `orderWindow:relativeTo:`, which does NOT reliably order a window above ANOTHER process's window — so the overlay stayed wherever it was last fronted. Bridge the private SkyLight `SLSOrderWindow(cid, wid, order, relativeTo)` (the daemon already links SkyLight for input synthesis) and use it in the overlay's reorder callback: order the overlay's own CGWindowID (via `[win windowNumber]`) one step above the target window through the WindowServer connection. This is the cross-process-reliable sandwich. AppKit's call remains as a fallback when the SPI can't be resolved. Now the cursor correctly disappears behind a window that occludes its target instead of always being on top. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
Summary
Brings the cross-platform test harnesses to control parity with the WPF baseline so the same 8-action matrix (click, double-click, right-click, drag, scroll, set_value, type, press-key) over 6 controls is exercisable on every surface, plus a set of driver fixes the per-action verifier surfaced.
shared/scenarios.json+shared/web/index.htmlare the single source of truth; each toolkit's harness was extended to match.Driver fixes
set_valuewrites a CFNumber for numericAXValuecontrols (NSSlider/NSStepper reject a CFString write —-25200/-25201); falls back to CFString for text fields. Verified live on an AppKit NSSlider:slider_value0 → 50.set_valuestepsAXIncrement/AXDecrementwhenAXValueis unsettable (SwiftUI'sAXSliderexposes a readable-but-unsettable value plus increment/decrement actions). Bounded loop (cap 500), stops within half the observed step, breaks if a step doesn't move the value. Compile-verified; runtime verification pending a daemon reinstall.point_in_window_bounds): refuses a coordinate click that resolves outside its window (previously misfired onto the taskbar at the 556px reflow), applied to the coordinate-delivery paths only so background UIA Invoke still opens off-screen modals.dispatch.rsrather than shipped.Harness parity
Findings (per-action verifier)
modality-recordings/FINDINGS.mddocuments what actually lands per surface and whether the no-foreground contract holds — including the headline that Windows Electron/Chromium self-foregrounds (contract violated) while macOS WKWebView and Linux Electron hold it, and the synthetic-input limits (GTK ignores XSendEvent; nested web overflow-divs don't scroll via keystrokes).Verification status (honest)
/docsupdate is required.🤖 Generated with Claude Code
https://claude.ai/code/session_01CdJawS4PRvgn3egsa56KyR
Summary by CodeRabbit
New Features
Bug Fixes
Documentation