feat(cua-driver-rs)(linux): gate native-Wayland backend behind opt-in flag - #1935
Conversation
… opt-in flag The native-Wayland backend (#1910) was default-on: `is_wayland()` engaged it for any pure-Wayland session (WAYLAND_DISPLAY set, DISPLAY unset). But the backend is incomplete (toplevel enumeration + virtual-pointer/keyboard input; capture and full AT-SPI parity still landing), so silently driving it on every Wayland session is the wrong default. Make it opt-in: `is_wayland()` now also requires `CUA_DRIVER_RS_ENABLE_WAYLAND` (any value other than empty/0/false). Off by default, a pure-Wayland session is treated as unsupported instead of quietly using a half-complete path; set the flag to try the experimental backend. XWayland sessions are unaffected (they already have DISPLAY set, so is_wayland() was already false for them). `doctor` reflects the state (native Wayland session — backend ENABLED vs OFF with the flag to set), and the structured output gains `wayland_enabled`. Docs: linux.mdx gains an "Experimental native Wayland" section. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughThe native Wayland backend in ChangesExperimental Wayland Opt-in Gate
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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 |
…sion wrapper The native-Wayland backend is now opt-in (CUA_DRIVER_RS_ENABLE_WAYLAND). Every Wayland NixOS test (xfce-labwc/xfce-sway/kde/gnome x all scenarios) drives the shared session.nix driverWrapper, so set the flag once in appBackendEnv — these tests exist specifically to exercise the backend, which would otherwise be off. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![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
🤖 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/src/tools/impl_.rs`:
- Around line 2802-2810: The code at the Some(s) if
crate::wayland::wayland_enabled() match arm only checks for WAYLAND_DISPLAY but
fails to distinguish between native Wayland and XWayland sessions. Add logic to
check whether the DISPLAY environment variable is also set to differentiate
these cases. If both WAYLAND_DISPLAY and DISPLAY are present, it indicates an
XWayland session which should be reported separately from a native Wayland
session where only WAYLAND_DISPLAY is set. Update the branching logic to create
distinct reporting paths for native Wayland versus XWayland based on whether
DISPLAY is additionally present.
🪄 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: f710dc08-300e-47c5-9776-723d94149094
📒 Files selected for processing (3)
docs/content/docs/cua-driver/guide/getting-started/linux.mdxlibs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rslibs/cua-driver/rust/crates/platform-linux/src/wayland/mod.rs
| Some(s) if crate::wayland::wayland_enabled() => | ||
| format!("✅ native Wayland session (WAYLAND_DISPLAY={s}) — experimental backend ENABLED"), | ||
| Some(s) => format!( | ||
| "⚠️ native Wayland session (WAYLAND_DISPLAY={s}) — experimental backend OFF; \ | ||
| set {}=1 to enable it", | ||
| crate::wayland::ENABLE_WAYLAND_ENV | ||
| ), | ||
| None => "❌ not a Wayland session".to_string(), | ||
| }, |
There was a problem hiding this comment.
Incorrectly classifies XWayland sessions as native Wayland.
At Line 2802, matching only on WAYLAND_DISPLAY causes sessions with both WAYLAND_DISPLAY and DISPLAY to be reported as “native Wayland session,” which is inaccurate for XWayland. Add a DISPLAY check in this branching logic so XWayland is reported separately.
Suggested fix
- let wayland_display = std::env::var("WAYLAND_DISPLAY").ok();
+ let wayland_display = std::env::var("WAYLAND_DISPLAY").ok();
+ let x_display = std::env::var("DISPLAY").ok();
@@
- match &wayland_display {
- Some(s) if crate::wayland::wayland_enabled() =>
+ match (&wayland_display, &x_display) {
+ (Some(s), Some(x)) => format!(
+ "✅ Wayland session with XWayland (WAYLAND_DISPLAY={s}, DISPLAY={x}) — using X11/XWayland path"
+ ),
+ (Some(s), None) if crate::wayland::wayland_enabled() =>
format!("✅ native Wayland session (WAYLAND_DISPLAY={s}) — experimental backend ENABLED"),
- Some(s) => format!(
+ (Some(s), None) => format!(
"⚠️ native Wayland session (WAYLAND_DISPLAY={s}) — experimental backend OFF; \
set {}=1 to enable it",
crate::wayland::ENABLE_WAYLAND_ENV
),
- None => "❌ not a Wayland session".to_string(),
+ (None, _) => "❌ not a Wayland session".to_string(),
},🤖 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/tools/impl_.rs` around lines
2802 - 2810, The code at the Some(s) if crate::wayland::wayland_enabled() match
arm only checks for WAYLAND_DISPLAY but fails to distinguish between native
Wayland and XWayland sessions. Add logic to check whether the DISPLAY
environment variable is also set to differentiate these cases. If both
WAYLAND_DISPLAY and DISPLAY are present, it indicates an XWayland session which
should be reported separately from a native Wayland session where only
WAYLAND_DISPLAY is set. Update the branching logic to create distinct reporting
paths for native Wayland versus XWayland based on whether DISPLAY is
additionally present.
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
1 participant
Why
The native-Wayland backend (#1910) was default-on:
is_wayland()engaged it for any pure-Wayland session (WAYLAND_DISPLAYset,DISPLAYunset). But it's incomplete (toplevel enumeration + virtual-pointer/keyboard input; screen capture and full AT-SPI parity are still landing), so silently driving it on every Wayland session is the wrong default for a release.What
Make it opt-in.
is_wayland()now also requiresCUA_DRIVER_RS_ENABLE_WAYLAND(any value other than empty /0/false):DISPLAYset, sois_wayland()was already false for them.doctorreflects the state (native Wayland session — experimental backend ENABLEDvsOFF; set CUA_DRIVER_RS_ENABLE_WAYLAND=1 to enable), and the structured output gainswayland_enabled.Docs:
linux.mdxgains an "Experimental native Wayland" section + updates the display-server probe table.Release context
Landing this before cutting 0.5.7 so the experimental Wayland backend ships safely behind a flag, and we can validate it on a real sway VM using the exact released artifact.
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features