fix(cua-driver/linux): libxcb fallback for X11 connect so list_windows works where xdotool does (#1978)#1991
fix(cua-driver/linux): libxcb fallback for X11 connect so list_windows works where xdotool does (#1978)#1991f-trycua wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
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:
📝 WalkthroughWalkthroughAdds ChangesX11 libxcb fallback for window enumeration and health probe
Sequence Diagram(s)sequenceDiagram
participant Caller
participant list_windows
participant RustConnection
participant XCBConnection
participant enumerate_windows
Caller->>list_windows: list_windows()
list_windows->>RustConnection: connect(None)
alt pure-Rust connection succeeds
RustConnection-->>list_windows: Ok(conn)
list_windows->>enumerate_windows: enumerate_windows(conn)
else pure-Rust fails, try libxcb
RustConnection-->>list_windows: Err(rust_err)
list_windows->>XCBConnection: connect(None)
alt libxcb succeeds
XCBConnection-->>list_windows: Ok(conn)
list_windows->>enumerate_windows: enumerate_windows(conn)
else both fail
XCBConnection-->>list_windows: Err(xcb_err)
list_windows-->>Caller: Err(x11_connect_error(DISPLAY, XAUTHORITY, rust_err, xcb_err))
end
end
enumerate_windows-->>Caller: Vec<WindowInfo>
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/cua-driver/rust/crates/platform-linux/src/x11/mod.rs (1)
160-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: factor out the duplicated connect-then-fallback logic.
wm_class_for_windowrepeats the same RustConnection→XCBConnection strategy aslist_windows_inner, but in a different shape (sequentialif letvsmatch). A small generic helper that runs a closure against whichever connection succeeds would keep the two call sites from drifting (e.g., if a third connect strategy or a timeout is added later).🤖 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-linux/src/x11/mod.rs` around lines 160 - 169, The wm_class_for_window function duplicates the RustConnection→XCBConnection fallback pattern already present in list_windows_inner. Extract this common connection-retry logic into a generic helper function that accepts a closure parameterized over the connection type, then refactor both wm_class_for_window and list_windows_inner to use this shared helper instead of repeating the sequential if-let connection attempts. This prevents the two implementations from drifting apart if future changes (like adding a third strategy or timeout handling) are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/cua-driver/rust/crates/platform-linux/Cargo.toml`:
- Line 24: The x11rb dependency in Cargo.toml with the `allow-unsafe-code`
feature enabled requires the system libxcb library to be available at build time
for FFI bindings, but the build and CI environments do not currently provision
libxcb-dev. To fix this, update your Dockerfile to install libxcb-dev as part of
the build stage, and update all CI workflow manifests to install libxcb-dev
before running build and test steps. This ensures the system has the required
development headers available when linking against x11rb with
`allow-unsafe-code` enabled.
---
Nitpick comments:
In `@libs/cua-driver/rust/crates/platform-linux/src/x11/mod.rs`:
- Around line 160-169: The wm_class_for_window function duplicates the
RustConnection→XCBConnection fallback pattern already present in
list_windows_inner. Extract this common connection-retry logic into a generic
helper function that accepts a closure parameterized over the connection type,
then refactor both wm_class_for_window and list_windows_inner to use this shared
helper instead of repeating the sequential if-let connection attempts. This
prevents the two implementations from drifting apart if future changes (like
adding a third strategy or timeout handling) are needed.
🪄 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: afb8f180-3c67-45d8-98df-ea7d14c850f0
📒 Files selected for processing (3)
libs/cua-driver/rust/crates/platform-linux/Cargo.tomllibs/cua-driver/rust/crates/platform-linux/src/health_report.rslibs/cua-driver/rust/crates/platform-linux/src/x11/mod.rs
| # X11 background input + window enumeration. | ||
| # `allow-unsafe-code` enables the libxcb-backed `XCBConnection`, used as a | ||
| # fallback when the pure-Rust connect fails strict auth-cookie matching (#1978). | ||
| x11rb = { version = "0.13", features = ["xinput", "randr", "xfixes", "composite", "shape", "xtest", "allow-unsafe-code"] } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find build/CI/container manifests and check for libxcb provisioning.
fd -t f -i -e dockerfile -e yml -e yaml -e toml -e sh -e nix . \
| xargs rg -nI -i 'libxcb|xcb-dev|xorg' 2>/dev/null
echo "---- CI workflow files ----"
fd -t f . .github 2>/dev/null | xargs rg -nI -i 'libxcb|apt|install' 2>/dev/nullRepository: trycua/cua
Length of output: 22534
🌐 Web query:
In x11rb 0.13, which Cargo feature enables the xcb_ffi module / XCBConnection, and does it require linking system libxcb?
💡 Result:
In x11rb 0.13, the xcb_ffi module and the XCBConnection struct are enabled by the allow-unsafe-code Cargo feature [1][2][3]. Without this feature, these components are unavailable because the crate disables all unsafe code by default [4][2]. Regarding system linking: - By default, enabling allow-unsafe-code will link the resulting executable against the system's libxcb [2][3]. - If you wish to avoid linking against the system libxcb at compile time, you can enable the dl-libxcb feature instead [2][3]. This feature dynamically loads libxcb at runtime and also enables allow-unsafe-code [5][6]. The dl-libxcb feature provides the load_libxcb function, which allows your application to handle cases where libxcb is not present on the system [1][2].
Citations:
- 1: https://docs.rs/x11rb/latest/x11rb/xcb_ffi/index.html
- 2: https://docs.rs/x11rb/0.13.2/x11rb/
- 3: https://docs.rs/x11rb/latest/src/x11rb/lib.rs.html
- 4: https://docs.rs/crate/x11rb/0.13.2/source/Cargo.toml.orig
- 5: https://crates.io/crates/x11rb/0.13.2
- 6: https://docs.rs/crate/x11rb/0.13.2/features
Add libxcb-dev to build/CI environments before linking x11rb with allow-unsafe-code.
Enabling allow-unsafe-code activates x11rb's xcb_ffi::XCBConnection, which requires linking against the system libxcb library at build time. The Docker builds and CI workflows do not currently provision libxcb or libxcb-dev, meaning builds will fail on any system lacking it—despite the prior pure-Rust path requiring no such dependency. Update your Dockerfile and CI manifests to install libxcb-dev (or libxcb for runtime) in all build/test environments.
🤖 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-linux/Cargo.toml` at line 24, The x11rb
dependency in Cargo.toml with the `allow-unsafe-code` feature enabled requires
the system libxcb library to be available at build time for FFI bindings, but
the build and CI environments do not currently provision libxcb-dev. To fix
this, update your Dockerfile to install libxcb-dev as part of the build stage,
and update all CI workflow manifests to install libxcb-dev before running build
and test steps. This ensures the system has the required development headers
available when linking against x11rb with `allow-unsafe-code` enabled.
…s works where xdotool does (#1978) x11rb's pure-Rust RustConnection::connect does strict Xauthority family/address cookie matching and returns Err in some setups (XFCE/lightdm, hostname-mismatched auth entries) where libxcb-based tools (xdotool, pyatspi) connect fine. list_windows then silently returned 0 windows and the health report said "X11 is not reachable", despite a working X session. - Try RustConnection first, fall back to the libxcb XCBConnection (enabled via x11rb allow-unsafe-code) — same client lib xdotool/pyatspi use. - Make the window-enumeration helpers generic over the connection type. - When BOTH connects fail, surface a descriptive error (DISPLAY/XAUTHORITY + both underlying errors) instead of collapsing to an empty list. - Mirror the fallback in the doctor's probe_x11_connect so the verdict matches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KMXCW4M5uK1HRGjjH4wueZ
f-trycua
force-pushed
the
fix/x11-libxcb-fallback-1978
branch
from
4fb723d to
28c55e6
Compare
… x11rb xcb-ffi (#1978) Enabling the x11rb `allow-unsafe-code` feature pulls in the `xcb_ffi` module, which depends on `as-raw-xcb-connection` (+ libc) — crates that were not in the committed Cargo.lock. The earlier VM build that validated this change ran online, so cargo fetched the new crate transparently; CI's Nix build runs `cargo build --offline` against a vendored dir and failed with "no matching package named `as-raw-xcb-connection` found", breaking every Linux integration job (build-time, before any test ran). Add the package + the two new x11rb dependency edges to Cargo.lock so the offline/vendored build resolves. Minimal in-place update (cargo metadata), no unrelated version churn. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KMXCW4M5uK1HRGjjH4wueZ
…fi (#1978) The x11rb `allow-unsafe-code` feature compiles its `xcb_ffi` module (the libxcb-backed XCBConnection fallback for strict-Xauthority setups, #1978), which emits `cargo:rustc-link-lib=xcb`. The cua-driver binary therefore now hard-links libxcb, but none of the build manifests provided it: - nix/cua-driver/package.nix: link failed with `-lxcb` and no matching `-L` ("collect2: ld returned 1 exit status") in every nix VM test. Add `libxcb` to buildInputs; update the stale "x11rb -> no libxcb C binding" comment. - cd-rust-cua-driver.yml: the debian:11 release build would fail the same way. Add `libxcb1-dev`. - ci-distro-compat-cua-driver.yml: the released binary now needs the libxcb runtime lib at load time; add `libxcb1` (deb) / `libxcb` (rpm) to every distro's runtime install so --version doesn't exit 127 on an ABI error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KMXCW4M5uK1HRGjjH4wueZ
|
CI now green (minus the known Root cause of the broad CI failure (real regression, not flake)Enabling the x11rb
Fix
Status67 pass / 3 fail. The only red checks are |
1 participant
Problem
On some Linux/X11 setups (reported on Ubuntu 22.04 XFCE/lightdm),
list_windows/get_window_statereturn 0 windows andcua-driver doctorreports "X11 is not reachable" — even though apps are running andxdotool/pyatspienumerate windows fine on the same display. Fixes #1978.Root cause
x11rb's pure-Rust
RustConnection::connect(None)does strictXauthorityfamily/address cookie matching and returnsErrin environments where libxcb-based clients (xdotool, pyatspi, Xlib) connect fine — e.g. an auth entry stored under a hostname/family that doesn't match how x11rb canonicalizes it.list_windows_innerpropagated thatErr, andlist_windowsswallowed it into an emptyVec, so the failure surfaced only as "0 windows".Fix
RustConnectionfirst; on connect failure, fall back to the libxcb-backedXCBConnection(enabled via the x11rballow-unsafe-codefeature) — the same client library xdotool/pyatspi use, so we connect in exactly the environments those tools work in.enumerate_windows,get_window_list,get_atom,get_window_pid,get_window_title,wm_class_inner) are now generic overC: Connectionso both connection types share one code path.DISPLAY/XAUTHORITY+ both underlying errors) instead of an empty list — so the next report like this is diagnosable from logs.probe_x11_connectmirrors the fallback so thedoctorverdict matches whatlist_windowscan actually do.Verification
cargo build -p platform-linuxis green with theallow-unsafe-code/XCBConnectionpath compiled in. Addedlibxcb-*-devare standard X11 build deps.list_windowsnow returns the live windows. Calling this out honestly rather than claiming a runtime pass.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements