Skip to content

feat(list_windows): UIA-first top-level window enumeration on Windows - #1542

Merged
f-trycua merged 5 commits into
mainfrom
feat/cua-driver-rs-list-windows-uia
May 17, 2026
Merged

feat(list_windows): UIA-first top-level window enumeration on Windows#1542
f-trycua merged 5 commits into
mainfrom
feat/cua-driver-rs-list-windows-uia

Conversation

@f-trycua

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

Copy link
Copy Markdown
Collaborator

Summary

crate::win32::list_windows on Windows now walks UI Automation first and unions with the classic EnumWindows result (deduped by HWND), so the visible top-level windows of modern apps stop disappearing.

  • Why: WebView2-hosted Notepad, packaged-UWP frames, and some Electron apps wrap their real surface inside a host HWND that EnumWindows either misses or returns with a misleading title/bounds. Asking list_windows for such a pid was returning an empty array. UIA's AutomationElement::RootElement.FindAll(TreeScope::Children, TrueCondition) returns the real interactable window — with NativeWindowHandle giving back an honest Win32 HWND so the (pid, window_id) tuple stays compatible with the rest of the pipeline.
  • Why keep EnumWindows: UIA can miss specific console window types and some installer dialogs. Merging both lists (HWND-keyed dedupe) is maximally robust at negligible cost.
  • Filter parity: UIA path filters on CurrentIsOffscreen == false + non-empty GetWindowTextW — same intent as the EnumWindows path's IsWindowVisible && !IsIconic + non-empty title.

Implementation

Two commits:

  1. feat(platform-windows): UIA tree walker helper for top-level windows — adds crates/platform-windows/src/uia/windows_enum.rs with enumerate_top_level_windows() -> Vec<WindowInfo>. Purely additive — no wiring yet. Stashes the IUIAutomation instance in a process-lifetime OnceLock (CoCreateInstance is non-trivial). Initializes COM via CoInitializeEx(COINIT_APARTMENTTHREADED) on each call, swallowing RPC_E_CHANGED_MODE (means COM is already up in another mode — harmless).

  2. feat(list_windows): use UIA-first enumeration on Windowscrate::win32::list_windows now calls the helper, then unions with EnumWindows (dedupe by HWND), then applies the existing filter_pid argument at the end. All callers (list_windows tool, get_window_state, launch_app, etc.) pick the improvement up uniformly because they all go through this single helper. Adds a new "Enumeration source (Windows)" subsection to PARITY.md and updates the existing off-screen-windows limitation note.

Notes

  • No new windows-rs features required — Win32_UI_Accessibility and Win32_System_Com were already enabled in platform-windows/Cargo.toml.
  • Pid is still resolved via GetWindowThreadProcessId (not UIA's ProcessId property) so the (hwnd, pid) tuple stays bit-identical to what the EnumWindows path computes — important for the downstream windows_for_pid.iter().any(|w| w.hwnd == hwnd) invariants.
  • Title is read via GetWindowTextW rather than UIA's CurrentName for the same parity reason (UIA's Name property occasionally returns the AX-friendly label rather than the OS-level window caption).

Test plan

  • Manual: launch UWP Notepad from PowerShell (start ms-launch:Microsoft.WindowsNotepad_8wekyb3d8bbwe!App), grab its pid via list_apps, then cua-driver call list_windows '{"pid":<UWP_pid>}' — pre-change returned empty; should now return a non-empty windows array with the real window record.
  • Regression: launch mspaint and regedit (classic Win32 apps); confirm list_windows still returns their windows with identical bounds + titles to pre-change.
  • Regression: run cargo run -p platform-windows --example list_windows_parity against a running daemon — header format, structured-content field shape, and pid-warning path should all still pass.
  • Build: cargo build --release --target x86_64-pc-windows-msvc -p platform-windows — clean (verified locally; full workspace cross-build from macOS fails on an unrelated ring build-script that needs the Windows native toolchain, but platform-windows itself compiles cleanly).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved top-level window discovery on Windows by combining multiple enumeration sources and deduplicating results to surface windows previously missed.
    • More consistent front-to-back ordering (z-order) of discovered windows.
    • Adjusted filtering so PID-based filtering and off-screen/minimized handling behave consistently after merging sources.
  • Documentation

    • Clarified Windows enumeration behavior, ordering, and current limitations (including off-screen/minimized window handling).

Review Change Stack

f-trycua and others added 2 commits May 17, 2026 18:38
Adds `crate::uia::enumerate_top_level_windows()` — a UI Automation-based
enumerator that walks `AutomationElement::RootElement.FindAll(Children, ...)`
and yields one `WindowInfo` per visible top-level window. Each entry's
`hwnd` is the element's `NativeWindowHandle`, so downstream code keyed on
the (pid, HWND) tuple keeps working unchanged.

Modern apps (WebView2 hosts, packaged-UWP frames, Electron apps that wrap
their real surface inside a container HWND) often hide their visible
window from `EnumWindows`; UIA surfaces them with the real title + bounds.

This commit is purely additive — `list_windows` still uses `EnumWindows`.
Wiring happens in the next commit.

Implementation notes:
- COM init: STA via `CoInitializeEx(COINIT_APARTMENTTHREADED)`, with
  `RPC_E_CHANGED_MODE` swallowed (harmless — COM is up either way).
- `IUIAutomation` is cached for the process lifetime in a `OnceLock`
  (CoCreateInstance is mildly expensive and the interface is thread-safe).
- Filters on `CurrentIsOffscreen == false` and non-empty
  `GetWindowTextW` to match the existing `EnumWindows + IsWindowVisible`
  filter intent.
- Pid is resolved via `GetWindowThreadProcessId` (not UIA's ProcessId
  property) so the (hwnd, pid) tuple stays bit-identical to whatever the
  rest of the driver computes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`crate::win32::list_windows` now walks UI Automation first
(`AutomationElement::RootElement.FindAll(TreeScope::Children, ...)`,
filtered to `IsOffscreen == false` + non-empty `GetWindowTextW`), then
takes the union with the classic `EnumWindows` walk, deduped by HWND.
The `filter_pid` argument is applied to the merged list at the end.

Why: modern apps (WebView2-hosted Notepad, packaged-UWP frames, some
Electron containers) hide their visible window inside a host HWND that
`EnumWindows` either misses or surfaces with the wrong title/bounds. UIA
returns the real interactable window with its true title + bounds. The
UWP-Notepad case in particular went from "empty list for that pid" to
"returns the actual window record".

Why the EnumWindows union is kept: UIA can miss specific console window
types and some installer dialogs. Merging both lists is maximally robust
at negligible cost (dedupe is HWND-keyed).

All callers (`list_windows`, `get_window_state`, `launch_app`, etc.) pick
up the improvement uniformly through the shared `crate::win32::list_windows`
helper — no per-tool changes needed. The z_index comment in the
`list_windows` tool is updated to reflect the new ordering source.

PARITY.md gains a new "Enumeration source (Windows)" subsection describing
the UIA-first behavior, and the existing off-screen-windows limitation note
is updated to mention both filters.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview May 17, 2026 5:27pm

Request Review

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto incremental reviews are disabled on this repository.

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

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2ad6715e-6e06-4bbb-810c-4be3f8410bf5

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

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a UI Automation–based top-level window enumerator, integrates its results with EnumWindows in list_windows (union + HWND dedupe), moves PID filtering to after the merge, and updates docs and z-index comments to describe the new ordering and on_screen_only behavior.

Changes

Windows Dual-Source Window Enumeration

Layer / File(s) Summary
UIA Top-Level Window Enumeration Implementation
libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs
Per-thread COM (STA) init and cached IUIAutomation; enumerate_top_level_windows() walks desktop children, converts UIA elements to WindowInfo (skip null/virtual HWNDs, offscreen/minimized, PID 0, empty titles), computes bounds via DWM then GetWindowRect.
list_windows Dual-Source Enumeration and Merge
libs/cua-driver-rs/crates/platform-windows/src/uia/mod.rs, libs/cua-driver-rs/crates/platform-windows/src/win32/windows.rs
list_windows collects Win32 EnumWindows and UIA results, unions them with deduplication by hwnd, and applies filter_pid after merging. EnumWindows callback no longer performs per-callback PID filtering; uia/mod.rs re-exports enumerate_top_level_windows.
Documentation: Strategy, Limitations, and Z-Index Ordering
libs/cua-driver-rs/PARITY.md, libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs
PARITY.md documents UIA-first enumeration, HWND-based dedupe, and that on_screen_only is accepted but currently ineffective; tools/impl_.rs clarifies z_index inversion to match the Swift convention.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant COM as COM / CoInitializeEx
  participant UIA as IUIAutomation
  participant UIATree as UIA Desktop Children
  participant Win32 as Win32 APIs (GetWindowText, GetWindowThreadProcessId, GetWindowRect)
  participant DWM as DWM (DwmGetWindowAttribute)

  Caller->>COM: get_uia() (CoInitializeEx STA)
  COM-->>Caller: cached IUIAutomation
  Caller->>UIA: get desktop root & children
  UIA-->>UIATree: list of elements
  loop for each element
    Caller->>UIA: element->get_NativeWindowHandle
    UIA-->>Caller: HWND
    Caller->>Win32: GetWindowThreadProcessId, GetWindowTextW
    Win32-->>Caller: PID, title
    Caller->>DWM: DwmGetWindowAttribute(bounds)
    DWM-->>Caller: bounds or fallback
    Caller-->>Caller: build WindowInfo (filter offscreen/minimized/title)
  end
  Caller->>Win32: EnumWindows collection
  Win32-->>Caller: Vec WindowInfo
  Caller-->>Caller: merge UIA + EnumWindows, dedupe by HWND, apply filter_pid, return Vec
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I woke the COM thread with a hop and a cheer,
Asked UIA for windows both far and near,
Then Win32 filled holes with a careful sweep,
We deduped handles and sorted the heap,
Now window lists are tidy — hop, tidy, clear!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: introducing UI Automation-first enumeration for top-level windows on Windows in the list_windows function, which is the core feature documented throughout the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cua-driver-rs-list-windows-uia

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs`:
- Around line 65-81: The code currently writes 0 into UIA_SINGLETON on
CoCreateInstance failure which permanently caches a failure; change the
initialization logic so UIA_SINGLETON is only written on success: update
UIA_SINGLETON to a cell that can remain uninitialized on failure (e.g.,
OnceLock/OnceCell holding usize or Option<usize> as appropriate), call
CoCreateInstance(CUIAutomation) and if it Err(e) simply log and return None
without storing 0 into UIA_SINGLETON, and only store the raw_ptr (from
inst.as_raw()) after a successful creation (remember to std::mem::forget(inst))
so future calls will retry activation until a success is observed.
- Around line 51-60: The current ensure_com_initialized() calls CoInitializeEx
on every enumeration and never balances with CoUninitialize, which violates COM
per-thread init semantics; update ensure_com_initialized to perform a one-time,
per-thread initialization using a thread-local flag (e.g. thread_local! static
DID_INIT: Cell<bool> or OnceCell) so CoInitializeEx is invoked only once per
thread and subsequent calls are no-ops, preserving the existing error handling
for RPC_E_CHANGED_MODE; locate ensure_com_initialized (and callers
enumerate_top_level_windows / get_uia) and replace the unconditional
CoInitializeEx call with a thread-local guard that runs CoInitializeEx only on
the first entry for that thread.

In `@libs/cua-driver-rs/crates/platform-windows/src/win32/windows.rs`:
- Around line 58-69: Currently you append all uia_windows first then
EnumWindows-only entries, which pushes fallback (EnumWindows-only) windows
behind UIA ones and skews z_index; instead preserve Win32 (EnumWindows) stacking
order by iterating win32_windows first to build merged and seen, then iterate
uia_windows and for each w: if its hwnd is unseen, append it, but if it's
already present in merged (seen contains w.hwnd) do not change its
position—locate the existing merged entry by hwnd and merge/overwrite UIA
metadata fields into that entry so UIA data is preferred while keeping Win32
stack order; use the existing variables/structures (win32_windows, uia_windows,
merged, seen, w.hwnd) and update fields rather than reordering entries.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd37e0fa-c340-4b82-b610-26ebbfe2c19c

📥 Commits

Reviewing files that changed from the base of the PR and between b4536a9 and 768f4de.

📒 Files selected for processing (5)
  • libs/cua-driver-rs/PARITY.md
  • libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs
  • libs/cua-driver-rs/crates/platform-windows/src/uia/mod.rs
  • libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs
  • libs/cua-driver-rs/crates/platform-windows/src/win32/windows.rs

Comment thread libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs Outdated
Comment thread libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs Outdated
Comment on lines +58 to +69
// UIA first — preserves UIA's preferred ordering for modern apps.
for w in uia_windows {
if seen.insert(w.hwnd) {
merged.push(w);
}
}
// Then any EnumWindows entry whose HWND wasn't already covered.
for w in win32_windows {
if seen.insert(w.hwnd) {
merged.push(w);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Merge order can skew z_index for fallback windows.

Appending EnumWindows-only entries after all UIA entries forces those windows behind UIA windows in list order, so downstream z_index is incorrect for exactly the fallback cases this PR keeps.

🧭 Suggested merge strategy (preserve Win32 stack order, prefer UIA metadata)
-    // UIA first — preserves UIA's preferred ordering for modern apps.
-    for w in uia_windows {
-        if seen.insert(w.hwnd) {
-            merged.push(w);
-        }
-    }
-    // Then any EnumWindows entry whose HWND wasn't already covered.
-    for w in win32_windows {
-        if seen.insert(w.hwnd) {
-            merged.push(w);
-        }
-    }
+    use std::collections::HashMap;
+    let mut uia_by_hwnd: HashMap<u64, WindowInfo> =
+        uia_windows.into_iter().map(|w| (w.hwnd, w)).collect();
+
+    // Keep EnumWindows order as canonical for z-order semantics,
+    // but replace overlapping HWND payloads with UIA-enriched data.
+    for w in win32_windows {
+        let hwnd = w.hwnd;
+        let preferred = uia_by_hwnd.remove(&hwnd).unwrap_or(w);
+        if seen.insert(hwnd) {
+            merged.push(preferred);
+        }
+    }
+
+    // UIA-only HWNDs (not present in EnumWindows) are appended.
+    for (_hwnd, w) in uia_by_hwnd {
+        if seen.insert(w.hwnd) {
+            merged.push(w);
+        }
+    }
🤖 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-rs/crates/platform-windows/src/win32/windows.rs` around lines
58 - 69, Currently you append all uia_windows first then EnumWindows-only
entries, which pushes fallback (EnumWindows-only) windows behind UIA ones and
skews z_index; instead preserve Win32 (EnumWindows) stacking order by iterating
win32_windows first to build merged and seen, then iterate uia_windows and for
each w: if its hwnd is unseen, append it, but if it's already present in merged
(seen contains w.hwnd) do not change its position—locate the existing merged
entry by hwnd and merge/overwrite UIA metadata fields into that entry so UIA
data is preferred while keeping Win32 stack order; use the existing
variables/structures (win32_windows, uia_windows, merged, seen, w.hwnd) and
update fields rather than reordering entries.

f-trycua and others added 2 commits May 17, 2026 19:03
… cache)

Replaces the process-wide OnceLock<usize> singleton with a thread_local
RefCell<Option<IUIAutomation>>. The previous design had two issues:

1. A failed CoCreateInstance wrote a sentinel (0) into the OnceLock,
   permanently caching the failure: every subsequent call short-circuited to
   None and never retried.
2. ensure_com_initialized() called CoInitializeEx unconditionally on every
   enumeration with no balancing CoUninitialize, ignoring the per-thread
   init-once semantics COM expects.

IUIAutomation objects are COM-apartment-scoped, so a thread-local naturally
matches their lifetime: each spawn_blocking worker initializes COM as STA
exactly once, builds its own IUIAutomation, and reuses it. On error the
cell stays None so the next call retries from scratch — no sticky failure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…merge

EnumWindows iterates the Win32 window manager's z-order list top-to-bottom,
so its iteration order IS the actual z-order — making the merged array's
index a meaningful z_index for any HWND the Win32 path saw. UIA's
FindAll(TreeScope::Children, ...) makes no z-order guarantee, so listing
UIA first (as before) skewed the index for windows present in both sources
and demoted Win32-only entries to the tail despite their canonical position.

Reorder the merge: EnumWindows entries first (canonical z-order), then
UIA-only HWNDs appended after (extra coverage for modern containers
EnumWindows misses, no claim on z-order priority).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@f-trycua

Copy link
Copy Markdown
Collaborator Author

Pushed fixup commits addressing the three review findings:

  • c6374fa4 — fix(uia): thread-local IUIAutomation + COM init (no permanent failure cache)
  • 698ae575 — fix(list_windows): preserve EnumWindows z-order in UIA ∪ EnumWindows merge

Finding 1 (UIA_SINGLETON caches failure) + Finding 2 (ensure_com_initialized called every enumeration) — addressed together in c6374fa4. Replaced the global OnceLock<usize> singleton with a thread_local! RefCell<Option<IUIAutomation>>. Each worker thread runs CoInitializeEx(STA) + CoCreateInstance(CUIAutomation) exactly once on first use; on failure the cell stays None, so the next call retries from scratch instead of being stuck on a sentinel. This also matches COM apartment semantics (IUIAutomation is apartment-bound, so per-thread is the correct lifetime — not per-process), and the per-thread cell IS the init flag, so the redundant ensure_com_initialized() call goes away.

Finding 3 (z-order skewed by union order) — addressed in 698ae575. EnumWindows walks the Win32 z-order list top-to-bottom, so its iteration order IS the canonical z-order. UIA's FindAll(TreeScope::Children, ...) makes no ordering guarantee. Reversed the merge: EnumWindows first (canonical), UIA-only HWNDs appended after (extra coverage, no claim on stacking priority). Z-index values for HWNDs already in the Win32 list are now the true ones.

cargo check --release -p platform-windows --target x86_64-pc-windows-msvc is clean on both commits.

@coderabbitai please re-review.

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

I'll pull the latest state of the modified files to verify the fixes before re-reviewing.


🏁 Script executed:

#!/bin/bash
cat libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs
echo "---"
cat libs/cua-driver-rs/crates/platform-windows/src/win32/windows.rs
echo "---"
cat libs/cua-driver-rs/crates/platform-windows/src/uia/mod.rs

Length of output: 27790


@f-trycua All three findings are cleanly resolved. Here's a commit-by-commit confirmation:


c6374fa4 — thread-local IUIAutomation + COM init (Findings 1 & 2) ✅

Finding 1 (failure-caching singleton): The OnceLock<usize> sentinel is gone. UIA_THREAD_LOCAL holds RefCell<Option<IUIAutomation>>; a None cell on the next call will retry CoInitializeEx + CoCreateInstance from scratch. No permanent failure lock-in.

Finding 2 (redundant ensure_com_initialized): The helper is removed entirely; COM init now lives inside get_uia() on the first-use path, making it impossible to call enumeration with a stale or missing COM state.

Additional correctness observations in get_uia():

  • The two RefCell borrows are sequential (borrow() guard drops before borrow_mut()), so no BorrowMutError at runtime.
  • IUIAutomation::clone() in windows-rs is an AddRef, correct for returning a second owning reference while keeping one in the cell.
  • Leaving the apartment alive for the thread's lifetime (no CoUninitialize) is the right call for a cached STA.

698ae575 — preserve EnumWindows z-order (Finding 3) ✅

The merge loop in list_windows is now:

// EnumWindows first — canonical Win32 z-order
for w in win32_windows { if seen.insert(w.hwnd) { merged.push(w); } }
// UIA-only extras appended (no z-order claim)
for w in uia_windows   { if seen.insert(w.hwnd) { merged.push(w); } }

z_index values for HWNDs that EnumWindows already found are exactly the Win32 stacking order. UIA-sourced extras sit at the tail with no false stacking priority. ✅


One minor pre-existing note (non-blocking)

mod.rs's walk_tree_unsafe initializes COM as COINIT_MULTITHREADED, while get_uia() uses COINIT_APARTMENTTHREADED. If both are invoked on the same OS thread the second CoInitializeEx returns RPC_E_CHANGED_MODE, which both paths handle gracefully (log + continue). This is a pre-existing design tension — not introduced by this PR — and the graceful error handling makes it safe in practice.


All three original findings are fully addressed. The implementation is correct. 🐇

✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs`:
- Around line 268-272: Update the explanatory comment for z_index in impl_.rs to
describe the current merge order used by list_windows: EnumWindows order is
preserved first and UIA-only entries are appended afterward, so the inversion
`(len - 1 - i)` is applied so higher z_index corresponds to closer-to-front
windows; reference list_windows, z_index, EnumWindows and UIA in the comment to
make the rationale accurate.

In `@libs/cua-driver-rs/PARITY.md`:
- Around line 365-390: The documentation incorrectly states that
crate::win32::list_windows runs UIA-first; update the PARITY.md text to reflect
the actual behavior: EnumWindows is the canonical ordering and UIA contributes
only missing HWNDs after the EnumWindows walk (i.e., change "UIA-first" wording
to "EnumWindows-first" and rephrase the paragraph that explains ordering,
deduping by HWND, and why UIA is used only to add missing windows so it no
longer implies UIA determines z-order or parity expectations).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a902751a-94d9-4ac7-b5d2-73f02bbc7cc8

📥 Commits

Reviewing files that changed from the base of the PR and between b4536a9 and 698ae57.

📒 Files selected for processing (5)
  • libs/cua-driver-rs/PARITY.md
  • libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs
  • libs/cua-driver-rs/crates/platform-windows/src/uia/mod.rs
  • libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs
  • libs/cua-driver-rs/crates/platform-windows/src/win32/windows.rs

Comment on lines +268 to +272
// z_index: the underlying enumerator returns UIA-first (modern
// top-to-bottom) then any EnumWindows entries UIA missed (also
// roughly top-to-bottom). Higher list index = farther from front.
// Swift convention: higher z_index = closer to front. Invert via
// `(len - 1 - i)` so the front-most window gets the largest z.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Correct z_index comment to match current merge order.

The note still describes UIA-first ordering, but list_windows now preserves EnumWindows order first and appends UIA-only entries. The explanation should reflect that to keep z-index rationale accurate.

Suggested comment update
-        // z_index: the underlying enumerator returns UIA-first (modern
-        // top-to-bottom) then any EnumWindows entries UIA missed (also
-        // roughly top-to-bottom). Higher list index = farther from front.
+        // z_index: the underlying enumerator returns EnumWindows first
+        // (canonical top-to-bottom z-order), then appends UIA-only HWNDs
+        // that EnumWindows missed. Higher list index = farther from front.
🤖 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-rs/crates/platform-windows/src/tools/impl_.rs` around lines
268 - 272, Update the explanatory comment for z_index in impl_.rs to describe
the current merge order used by list_windows: EnumWindows order is preserved
first and UIA-only entries are appended afterward, so the inversion `(len - 1 -
i)` is applied so higher z_index corresponds to closer-to-front windows;
reference list_windows, z_index, EnumWindows and UIA in the comment to make the
rationale accurate.

Comment thread libs/cua-driver-rs/PARITY.md Outdated
Comment on lines +365 to +390
- windows: VERIFIED (UIA-first enumeration; EnumWindows kept as union member)
- macOS: OPEN (audit pending — macOS port already exists)
- linux: OPEN
- Test: `crates/platform-windows/examples/list_windows_parity.rs`

### Enumeration source (Windows)

`crate::win32::list_windows` runs UI Automation first
(`AutomationElement::RootElement.FindAll(TreeScope::Children, TrueCondition)`,
filtered to `IsOffscreen == false` + non-empty `GetWindowTextW`) and then
takes the union with the classic `EnumWindows` walk, deduped by HWND. Each
UIA element contributes its `NativeWindowHandle` as the canonical HWND, so
downstream code keyed on `(pid, window_id)` keeps working unchanged.

Why UIA-first: modern apps (WebView2-hosted Notepad, packaged-UWP frames,
some Electron containers) hide their visible window inside a host HWND that
`EnumWindows` either misses or surfaces with the wrong title/bounds. UIA's
desktop-children walk returns the real interactable window.

Why the EnumWindows union is kept: UIA can miss specific console window
types and some installer dialogs. Merging both lists is maximally robust at
negligible cost.

`filter_pid` is applied to the merged list, so a UWP app's pid that
previously returned empty now returns its real window.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix stale ordering docs (UIA-firstEnumWindows-first).

This section now contradicts the actual merge behavior: EnumWindows order is canonical, and UIA contributes only missing HWNDs after that. Please update this wording to avoid incorrect z-order/parity expectations.

Suggested doc patch
-  - windows: VERIFIED (UIA-first enumeration; EnumWindows kept as union member)
+  - windows: VERIFIED (EnumWindows-first enumeration; UIA kept as union member for missed HWNDs)

-`crate::win32::list_windows` runs UI Automation first
-(`AutomationElement::RootElement.FindAll(TreeScope::Children, TrueCondition)`,
-filtered to `IsOffscreen == false` + non-empty `GetWindowTextW`) and then
-takes the union with the classic `EnumWindows` walk, deduped by HWND.
+`crate::win32::list_windows` runs `EnumWindows` first (canonical z-order),
+then appends UIA-only HWNDs from
+`AutomationElement::RootElement.FindAll(TreeScope::Children, TrueCondition)`
+(filtered to `IsOffscreen == false` + non-empty `GetWindowTextW`), deduped by HWND.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- windows: VERIFIED (UIA-first enumeration; EnumWindows kept as union member)
- macOS: OPEN (audit pending — macOS port already exists)
- linux: OPEN
- Test: `crates/platform-windows/examples/list_windows_parity.rs`
### Enumeration source (Windows)
`crate::win32::list_windows` runs UI Automation first
(`AutomationElement::RootElement.FindAll(TreeScope::Children, TrueCondition)`,
filtered to `IsOffscreen == false` + non-empty `GetWindowTextW`) and then
takes the union with the classic `EnumWindows` walk, deduped by HWND. Each
UIA element contributes its `NativeWindowHandle` as the canonical HWND, so
downstream code keyed on `(pid, window_id)` keeps working unchanged.
Why UIA-first: modern apps (WebView2-hosted Notepad, packaged-UWP frames,
some Electron containers) hide their visible window inside a host HWND that
`EnumWindows` either misses or surfaces with the wrong title/bounds. UIA's
desktop-children walk returns the real interactable window.
Why the EnumWindows union is kept: UIA can miss specific console window
types and some installer dialogs. Merging both lists is maximally robust at
negligible cost.
`filter_pid` is applied to the merged list, so a UWP app's pid that
previously returned empty now returns its real window.
- windows: VERIFIED (EnumWindows-first enumeration; UIA kept as union member for missed HWNDs)
- macOS: OPEN (audit pending — macOS port already exists)
- linux: OPEN
- Test: `crates/platform-windows/examples/list_windows_parity.rs`
### Enumeration source (Windows)
`crate::win32::list_windows` runs `EnumWindows` first (canonical z-order),
then appends UIA-only HWNDs from
`AutomationElement::RootElement.FindAll(TreeScope::Children, TrueCondition)`
(filtered to `IsOffscreen == false` + non-empty `GetWindowTextW`), deduped by HWND. Each
UIA element contributes its `NativeWindowHandle` as the canonical HWND, so
downstream code keyed on `(pid, window_id)` keeps working unchanged.
Why UIA-first: modern apps (WebView2-hosted Notepad, packaged-UWP frames,
some Electron containers) hide their visible window inside a host HWND that
`EnumWindows` either misses or surfaces with the wrong title/bounds. UIA's
desktop-children walk returns the real interactable window.
Why the EnumWindows union is kept: UIA can miss specific console window
types and some installer dialogs. Merging both lists is maximally robust at
negligible cost.
`filter_pid` is applied to the merged list, so a UWP app's pid that
previously returned empty now returns its real window.
🤖 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-rs/PARITY.md` around lines 365 - 390, The documentation
incorrectly states that crate::win32::list_windows runs UIA-first; update the
PARITY.md text to reflect the actual behavior: EnumWindows is the canonical
ordering and UIA contributes only missing HWNDs after the EnumWindows walk
(i.e., change "UIA-first" wording to "EnumWindows-first" and rephrase the
paragraph that explains ordering, deduping by HWND, and why UIA is used only to
add missing windows so it no longer implies UIA determines z-order or parity
expectations).

@f-trycua

Copy link
Copy Markdown
Collaborator Author

Pushed fixup for the 2 doc-drift nits: z_index comment in impl_.rs + PARITY.md "Enumeration source" section now correctly say EnumWindows-first / UIA-as-union-only. @coderabbitai please re-review.

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs | grep -n -A 20 "z_index"
echo "---PARITY---"
cat libs/cua-driver-rs/PARITY.md

Length of output: 104192


@f-trycua Both doc-drift nits are confirmed fixed:

impl_.rsz_index comment (Lines 268–275) ✅
The comment now accurately reads:

list_windows merges EnumWindows first (canonical top-to-bottom z-order), then appends any UIA-only HWNDs (no canonical Win32 ordering, land at the bottom of the z-stack).

This is consistent with the actual merge logic in windows.rs.

PARITY.md — "Enumeration source" section ✅
The section now leads with EnumWindows-first and frames UIA explicitly as a union-only supplement:

crate::win32::list_windows walks EnumWindows first — that's the Win32 window manager's canonical top-to-bottom z-order... It then asks UI Automation for any top-level windows EnumWindows missed...

The "Why EnumWindows-first" rationale paragraph is also a nice addition — it explains the non-determinism risk of a UIA-primary ordering, which is a useful comment for future maintainers.

All findings across both rounds of review are resolved. The implementation is correct and the documentation now faithfully describes it. 🐇

[approve]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant