diff --git a/CHANGELOG.md b/CHANGELOG.md index 0205945..713b722 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Absolute uinput pointer axes now end at the final logical desktop pixel, so + edge coordinates are advertised and clamped consistently. +- Capability maps now advertise AT-SPI only when its bus is reachable and a + toolkit accessibility status is actually enabled. +- Buttons outside the absolute uinput device's left, middle, and right set now + fall through to a backend that can synthesize them instead of becoming left clicks. +- Temporary KWin script callbacks now accept one matching response from the + current `org.kde.KWin` bus owner, reject spoofed or replayed responses, and + time out the complete script transaction before cleaning up owned temporary + state without disturbing a colliding callback registration. +- KWin window listings now classify Plasma 6 native Wayland and Xwayland + clients when the legacy client flags are unavailable. +- GNOME extension setup now reports when changed files require an already-active + Shell extension to reload before its newly installed DBus methods are served, + and requires that reload when the previous extension state cannot be read. + ## [0.4.9] - 2026-08-12 ### Fixed diff --git a/src/abs_pointer.rs b/src/abs_pointer.rs index c0d2920..d53df43 100644 --- a/src/abs_pointer.rs +++ b/src/abs_pointer.rs @@ -21,10 +21,46 @@ use evdev::{ PropType, UinputAbsSetup, }; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[must_use = "pointer input may be clamped; inspect requested and emitted coordinates"] +pub(crate) struct PointerLanding { + pub(crate) requested: (i32, i32), + pub(crate) emitted: (i32, i32), +} + +#[derive(Clone, Copy)] +struct AbsPointerGeometry { + max_x: i32, + max_y: i32, +} + +impl AbsPointerGeometry { + fn from_dimensions(width: i32, height: i32) -> Self { + Self { + max_x: width.max(1).saturating_sub(1), + max_y: height.max(1).saturating_sub(1), + } + } + + fn axis_maxima(self) -> (i32, i32) { + (self.max_x, self.max_y) + } + + fn clamp_coordinates(self, x: i32, y: i32) -> (i32, i32) { + (x.clamp(0, self.max_x), y.clamp(0, self.max_y)) + } + + fn landing_for(self, x: i32, y: i32) -> PointerLanding { + PointerLanding { + requested: (x, y), + emitted: self.clamp_coordinates(x, y), + } + } +} + pub struct AbsPointer { device: VirtualDevice, - width: i32, - height: i32, + geometry: AbsPointerGeometry, } impl AbsPointer { @@ -32,13 +68,13 @@ impl AbsPointer { /// (the portal screenshot dimensions). Blocks ~`settle` ms so libinput picks /// the device up before the first event. pub fn create(width: i32, height: i32) -> Result { - let width = width.max(1); - let height = height.max(1); + let geometry = AbsPointerGeometry::from_dimensions(width, height); + let (max_x, max_y) = geometry.axis_maxima(); // value, min, max, fuzz, flat, resolution. resolution=1 unit/px. let abs_x = - UinputAbsSetup::new(AbsoluteAxisCode::ABS_X, AbsInfo::new(0, 0, width, 0, 0, 1)); + UinputAbsSetup::new(AbsoluteAxisCode::ABS_X, AbsInfo::new(0, 0, max_x, 0, 0, 1)); let abs_y = - UinputAbsSetup::new(AbsoluteAxisCode::ABS_Y, AbsInfo::new(0, 0, height, 0, 0, 1)); + UinputAbsSetup::new(AbsoluteAxisCode::ABS_Y, AbsInfo::new(0, 0, max_y, 0, 0, 1)); let keys = AttributeSet::from_iter([KeyCode::BTN_LEFT, KeyCode::BTN_RIGHT, KeyCode::BTN_MIDDLE]); // INPUT_PROP_DIRECT marks the device as a direct (absolute) pointer so @@ -59,29 +95,32 @@ impl AbsPointer { // Give udev/libinput time to enumerate the new device. sleep(Duration::from_millis(500)); - Ok(Self { - device, - width, - height, - }) + Ok(Self { device, geometry }) } - /// Move the pointer to absolute logical coordinates `(x, y)`. - pub fn move_to(&mut self, x: i32, y: i32) -> Result<()> { - let x = x.clamp(0, self.width); - let y = y.clamp(0, self.height); + /// Move the pointer to absolute logical coordinates `(x, y)` and report + /// both the requested point and the values emitted after edge clamping. + pub fn move_to(&mut self, x: i32, y: i32) -> Result { + let landing = self.geometry.landing_for(x, y); + let (emitted_x, emitted_y) = landing.emitted; self.device .emit(&[ - InputEvent::new_now(EventType::ABSOLUTE.0, AbsoluteAxisCode::ABS_X.0, x), - InputEvent::new_now(EventType::ABSOLUTE.0, AbsoluteAxisCode::ABS_Y.0, y), + InputEvent::new_now(EventType::ABSOLUTE.0, AbsoluteAxisCode::ABS_X.0, emitted_x), + InputEvent::new_now(EventType::ABSOLUTE.0, AbsoluteAxisCode::ABS_Y.0, emitted_y), ]) .context("failed to emit absolute motion")?; - Ok(()) + Ok(landing) } /// Move to `(x, y)` then press+release `button` `count` times. - pub fn click(&mut self, x: i32, y: i32, button: PointerButton, count: u32) -> Result<()> { - self.move_to(x, y)?; + pub fn click( + &mut self, + x: i32, + y: i32, + button: PointerButton, + count: u32, + ) -> Result { + let landing = self.move_to(x, y)?; sleep(Duration::from_millis(30)); let code = button.key_code(); for _ in 0..count.max(1) { @@ -92,7 +131,7 @@ impl AbsPointer { .emit(&[InputEvent::new_now(EventType::KEY.0, code, 0)])?; sleep(Duration::from_millis(40)); } - Ok(()) + Ok(landing) } /// Press at `(start)`, move to `(end)`, release — a drag with `button`. @@ -103,12 +142,14 @@ impl AbsPointer { button: PointerButton, ) -> Result<()> { let code = button.key_code(); - self.move_to(start.0, start.1)?; + // Drag currently reports backend success only; retain the landing + // values explicitly so their intentional omission stays visible. + let _start_landing = self.move_to(start.0, start.1)?; sleep(Duration::from_millis(30)); self.device .emit(&[InputEvent::new_now(EventType::KEY.0, code, 1)])?; sleep(Duration::from_millis(40)); - self.move_to(end.0, end.1)?; + let _end_landing = self.move_to(end.0, end.1)?; sleep(Duration::from_millis(40)); self.device .emit(&[InputEvent::new_now(EventType::KEY.0, code, 0)])?; @@ -125,11 +166,12 @@ pub enum PointerButton { } impl PointerButton { - pub fn from_name(name: Option<&str>) -> Self { + pub fn from_name(name: Option<&str>) -> Option { match name.unwrap_or("left").to_ascii_lowercase().as_str() { - "right" => Self::Right, - "middle" => Self::Middle, - _ => Self::Left, + "left" => Some(Self::Left), + "right" => Some(Self::Right), + "middle" => Some(Self::Middle), + _ => None, } } @@ -141,3 +183,54 @@ impl PointerButton { } } } + +#[cfg(test)] +mod tests { + use super::{AbsPointerGeometry, PointerButton}; + + #[test] + fn axis_range_ends_at_last_desktop_pixel() { + let geometry = AbsPointerGeometry::from_dimensions(1920, 1080); + + assert_eq!(geometry.axis_maxima(), (1919, 1079)); + } + + #[test] + fn pointer_landing_preserves_the_request_and_emitted_coordinates() { + let geometry = AbsPointerGeometry::from_dimensions(1920, 1080); + + for (requested, emitted) in [ + ((640, 480), (640, 480)), + ((1920, 1080), (1919, 1079)), + ((-1, -1), (0, 0)), + ((i32::MAX, i32::MAX), (1919, 1079)), + ] { + let landing = geometry.landing_for(requested.0, requested.1); + assert_eq!(landing.requested, requested); + assert_eq!(landing.emitted, emitted); + } + } + + #[test] + fn unsupported_buttons_fall_through_to_other_backends() { + assert!(matches!( + PointerButton::from_name(None), + Some(PointerButton::Left) + )); + assert!(matches!( + PointerButton::from_name(Some("right")), + Some(PointerButton::Right) + )); + assert!(matches!( + PointerButton::from_name(Some("middle")), + Some(PointerButton::Middle) + )); + + for button in ["side", "extra", "forward", "back"] { + assert!( + PointerButton::from_name(Some(button)).is_none(), + "{button} must fall through instead of becoming a left click" + ); + } + } +} diff --git a/src/cli.rs b/src/cli.rs index 0333761..40cc68c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -62,11 +62,8 @@ pub(crate) async fn run_from_env() -> Result<()> { let cap = screenshot::capture_screenshot_raw().await?; eprintln!("desktop logical size: {}x{}", cap.width, cap.height); let mut p = abs_pointer::AbsPointer::create(cap.width as i32, cap.height as i32)?; - p.click(x, y, abs_pointer::PointerButton::Left, 1)?; - println!( - "{}", - serde_json::json!({"ok": true, "x": x, "y": y, "w": cap.width, "h": cap.height}) - ); + let landing = p.click(x, y, abs_pointer::PointerButton::Left, 1)?; + println!("{}", abs_test_report(landing, (cap.width, cap.height))); Ok(()) } Some("screenshot") => { @@ -145,8 +142,50 @@ pub(crate) async fn run_from_env() -> Result<()> { } } +fn abs_test_report( + landing: abs_pointer::PointerLanding, + dimensions: (u32, u32), +) -> serde_json::Value { + serde_json::json!({ + "ok": true, + "requested_x": landing.requested.0, + "requested_y": landing.requested.1, + "x": landing.emitted.0, + "y": landing.emitted.1, + "w": dimensions.0, + "h": dimensions.1 + }) +} + fn print_help() { println!( "computer-use-linux\n\nUsage:\n computer-use-linux mcp\n computer-use-linux doctor\n computer-use-linux setup\n computer-use-linux setup-window-targeting\n computer-use-linux apps\n computer-use-linux state [APP_NAME]\n computer-use-linux screenshot\n computer-use-linux windows" ); } + +#[cfg(test)] +mod tests { + use super::{abs_pointer, abs_test_report}; + + #[test] + fn abs_test_report_distinguishes_requested_and_emitted_coordinates() { + assert_eq!( + abs_test_report( + abs_pointer::PointerLanding { + requested: (1920, 1080), + emitted: (1919, 1079), + }, + (1920, 1080) + ), + serde_json::json!({ + "ok": true, + "requested_x": 1920, + "requested_y": 1080, + "x": 1919, + "y": 1079, + "w": 1920, + "h": 1080 + }) + ); + } +} diff --git a/src/command_runner.rs b/src/command_runner.rs index a6a7567..e2fd302 100644 --- a/src/command_runner.rs +++ b/src/command_runner.rs @@ -42,6 +42,10 @@ pub(crate) async fn output_with_stdin( output_with_input(command, action, timeout, Some(input)).await } +pub(crate) fn output_blocking(command: &mut StdCommand, action: &str) -> Result { + output_blocking_with_timeout(command, action, COMMAND_TIMEOUT) +} + pub(crate) fn output_blocking_with_timeout( command: &mut StdCommand, action: &str, @@ -600,6 +604,41 @@ mod tests { assert!(output.stderr.len() >= 200_000); } + #[tokio::test] + async fn default_blocking_timeout_kills_the_process_group() { + let leader_path = temporary_pid_path("blocking-default-leader"); + let descendant_path = temporary_pid_path("blocking-default-descendant"); + let mut command = StdCommand::new("sh"); + command.args([ + "-c", + &format!( + "printf %s $$ > '{}'; sleep 60 & printf %s $! > '{}'; wait", + leader_path.display(), + descendant_path.display() + ), + ]); + let started = Instant::now(); + + let error = output_blocking(&mut command, "run blocking process tree").unwrap_err(); + + assert!(error + .to_string() + .contains("timed out after 2000 ms while trying to run blocking process tree")); + assert!(started.elapsed() < Duration::from_secs(4)); + let leader = fs::read_to_string(&leader_path) + .expect("leader should record its pid") + .parse() + .expect("leader pid should be numeric"); + let descendant = fs::read_to_string(&descendant_path) + .expect("descendant should record its pid") + .parse() + .expect("descendant pid should be numeric"); + wait_for_process_exit(leader).await; + wait_for_process_exit(descendant).await; + let _ = fs::remove_file(leader_path); + let _ = fs::remove_file(descendant_path); + } + fn temporary_pid_path(label: &str) -> PathBuf { std::env::temp_dir().join(format!( "computer-use-linux-command-runner-{label}-{}-{}.pid", diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 8748a1e..8ea9e2e 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -327,9 +327,9 @@ fn capability_map_with_portal_keyboard( if windowing.hyprland.ok { window_backends.push("hyprland".to_string()); } - // i3 and the generic X11/EWMH backend have no dedicated WindowingReport - // field; read them from the probe map (tried last) so the capability list - // matches the backends the registry will actually use. + // i3 and the generic X11/EWMH backend have no dedicated + // WindowingReport field; read them from the probe map so the capability + // list matches the registry order. if windowing .backends .get(I3_BACKEND) @@ -342,7 +342,7 @@ fn capability_map_with_portal_keyboard( } let mut accessibility_backends = Vec::new(); - if accessibility.at_spi_enabled.ok || accessibility.toolkit_accessibility.ok { + if can_build_accessibility_tree(accessibility) { accessibility_backends.push("at_spi".to_string()); } @@ -1460,6 +1460,26 @@ mod tests { assert!(can_build_accessibility_tree(&report)); } + #[test] + fn capability_map_advertises_only_a_buildable_accessibility_tree() { + let platform = platform_report(); + let portals = portal_report(Check::fail("missing")); + let windowing = windowing_report(false, false); + let input = input_report(false); + + for accessibility in [ + accessibility_report(Check::fail("permission denied"), Check::ok("true")), + accessibility_report( + Check::ok("('unix:path=/run/user/1000/at-spi/bus',)"), + Check::ok("false"), + ), + ] { + let capabilities = + capability_map(&platform, &portals, &accessibility, &windowing, &input); + assert!(capabilities.accessibility.is_empty()); + } + } + #[test] fn parses_parent_pid_from_proc_status() { let status = "Name:\ttest\nPid:\t42\nPPid:\t7\n"; diff --git a/src/gnome_extension.rs b/src/gnome_extension.rs index 99bbde6..fc24222 100644 --- a/src/gnome_extension.rs +++ b/src/gnome_extension.rs @@ -1,3 +1,4 @@ +use crate::command_runner; use crate::diagnostics::hydrate_session_bus_env; use crate::identity; use crate::windowing::backends::gnome::list_extension_windows; @@ -38,10 +39,15 @@ pub async fn setup_window_targeting_report() -> WindowTargetingSetupReport { hydrate_session_bus_env(); let extension_dir = extension_dir(); + let extension_was_enabled = gnome_extension_enabled(); let mut wrote_files = false; + let mut changed_files = false; let mut write_error = None; match write_extension_files(&extension_dir) { - Ok(()) => wrote_files = true, + Ok(report) => { + wrote_files = report.wrote_files; + changed_files = report.changed_files; + } Err(error) => write_error = Some(error), } @@ -63,12 +69,15 @@ pub async fn setup_window_targeting_report() -> WindowTargetingSetupReport { } }; - let requires_shell_reload = windows_error.is_some(); + let requires_shell_reload = + setup_requires_shell_reload(windows_error.as_ref(), extension_was_enabled, changed_files); let message = if !wrote_files { "Could not install the computer-use-linux GNOME Shell extension files.".to_string() } else if !enable_command.ok { "computer-use-linux GNOME Shell extension files were installed, but enabling the extension failed. Enable it with gnome-extensions after GNOME Shell sees the new extension." .to_string() + } else if windows_error.is_none() && requires_shell_reload { + shell_reload_message(extension_was_enabled).to_string() } else if windows_error.is_none() { "computer-use-linux GNOME Shell extension is active and window targeting is available." .to_string() @@ -89,11 +98,18 @@ pub async fn setup_window_targeting_report() -> WindowTargetingSetupReport { } } -fn write_extension_files(extension_dir: &Path) -> Result<(), String> { +struct ExtensionWriteReport { + wrote_files: bool, + changed_files: bool, +} + +fn write_extension_files(extension_dir: &Path) -> Result { fs::create_dir_all(extension_dir) .map_err(|error| format!("failed to create {}: {error}", extension_dir.display()))?; let metadata_json = render_extension_asset(METADATA_JSON); let extension_js = render_extension_asset(EXTENSION_JS); + let changed_files = file_content_changed(&extension_dir.join("metadata.json"), &metadata_json) + || file_content_changed(&extension_dir.join("extension.js"), &extension_js); fs::write(extension_dir.join("metadata.json"), metadata_json).map_err(|error| { format!( @@ -107,7 +123,33 @@ fn write_extension_files(extension_dir: &Path) -> Result<(), String> { extension_dir.join("extension.js").display() ) })?; - Ok(()) + Ok(ExtensionWriteReport { + wrote_files: true, + changed_files, + }) +} + +fn file_content_changed(path: &Path, expected: &str) -> bool { + match fs::read_to_string(path) { + Ok(current) => current != expected, + Err(_) => true, + } +} + +fn setup_requires_shell_reload( + windows_error: Option<&String>, + extension_was_enabled: Option, + changed_files: bool, +) -> bool { + windows_error.is_some() || extension_was_enabled != Some(false) && changed_files +} + +fn shell_reload_message(extension_was_enabled: Option) -> &'static str { + if extension_was_enabled == Some(true) { + "computer-use-linux GNOME Shell extension files changed while the extension was already active. Window targeting is available, but GNOME Shell must reload before newly installed DBus methods are served." + } else { + "computer-use-linux GNOME Shell extension files changed, but the previous extension state could not be determined. GNOME Shell must reload before newly installed DBus methods can be relied on." + } } fn render_extension_asset(asset: &str) -> String { @@ -126,25 +168,13 @@ fn render_extension_asset(asset: &str) -> String { fn run_gnome_extensions_enable() -> SetupCommandReport { let mut command = Command::new("gnome-extensions"); command.args(["enable", UUID]); - add_session_env(&mut command); - let primary = match command.output() { - Ok(output) if output.status.success() => SetupCommandReport { + let primary = match run_session_command(&mut command, "enable the GNOME Shell extension") { + Ok(output) => SetupCommandReport { ok: true, detail: output_detail(&output.stdout, &output.stderr, "gnome-extensions enable ok"), }, - Ok(output) => SetupCommandReport { - ok: false, - detail: output_detail( - &output.stdout, - &output.stderr, - &format!("gnome-extensions exited with {}", output.status), - ), - }, - Err(error) => SetupCommandReport { - ok: false, - detail: format!("failed to run gnome-extensions: {error}"), - }, + Err(detail) => SetupCommandReport { ok: false, detail }, }; if primary.ok { return primary; @@ -172,25 +202,9 @@ fn run_gnome_extensions_enable() -> SetupCommandReport { } fn run_gsettings_enable_fallback() -> SetupCommandReport { - let mut get_command = Command::new("gsettings"); - get_command.args(["get", "org.gnome.shell", "enabled-extensions"]); - add_session_env(&mut get_command); - let current = match get_command.output() { - Ok(output) if output.status.success() => { - String::from_utf8_lossy(&output.stdout).trim().to_string() - } - Ok(output) => { - return SetupCommandReport { - ok: false, - detail: output_detail(&output.stdout, &output.stderr, "gsettings get failed"), - } - } - Err(error) => { - return SetupCommandReport { - ok: false, - detail: format!("failed to run gsettings get: {error}"), - } - } + let current = match enabled_extensions_value() { + Ok(current) => current, + Err(detail) => return SetupCommandReport { ok: false, detail }, }; let Some(updated) = enabled_extensions_literal(¤t) else { @@ -208,32 +222,45 @@ fn run_gsettings_enable_fallback() -> SetupCommandReport { let mut set_command = Command::new("gsettings"); set_command.args(["set", "org.gnome.shell", "enabled-extensions", &updated]); - add_session_env(&mut set_command); - match set_command.output() { - Ok(output) if output.status.success() => SetupCommandReport { + match run_session_command( + &mut set_command, + "update org.gnome.shell enabled-extensions", + ) { + Ok(_) => SetupCommandReport { ok: true, detail: format!( "added {UUID} to org.gnome.shell enabled-extensions for the next GNOME Shell load" ), }, - Ok(output) => SetupCommandReport { - ok: false, - detail: output_detail(&output.stdout, &output.stderr, "gsettings set failed"), - }, - Err(error) => SetupCommandReport { - ok: false, - detail: format!("failed to run gsettings set: {error}"), - }, + Err(detail) => SetupCommandReport { ok: false, detail }, } } +fn gnome_extension_enabled() -> Option { + enabled_extensions_value() + .ok() + .map(|current| enabled_extensions_contains_uuid(¤t)) +} + +fn enabled_extensions_value() -> Result { + let mut command = Command::new("gsettings"); + command.args(["get", "org.gnome.shell", "enabled-extensions"]); + let output = run_session_command(&mut command, "read org.gnome.shell enabled-extensions")?; + Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) +} + +fn enabled_extensions_contains_uuid(current: &str) -> bool { + let quoted = format!("'{UUID}'"); + current.trim().contains("ed) +} + fn enabled_extensions_literal(current: &str) -> Option { let trimmed = current.trim(); - let quoted = format!("'{UUID}'"); - if trimmed.contains("ed) { + if enabled_extensions_contains_uuid(trimmed) { return Some(trimmed.to_string()); } + let quoted = format!("'{UUID}'"); let list = if trimmed == "@as []" { "[]" } else { trimmed }; if list == "[]" { return Some(format!("[{quoted}]")); @@ -258,6 +285,24 @@ fn add_session_env(command: &mut Command) { } } +fn run_session_command( + command: &mut Command, + action: &str, +) -> Result { + add_session_env(command); + let output = + command_runner::output_blocking(command, action).map_err(|error| format!("{error:#}"))?; + if output.status.success() { + Ok(output) + } else { + Err(output_detail( + &output.stdout, + &output.stderr, + &format!("{action} failed with {}", output.status), + )) + } +} + fn output_detail(stdout: &[u8], stderr: &[u8], fallback: &str) -> String { let stderr = String::from_utf8_lossy(stderr).trim().to_string(); if !stderr.is_empty() { @@ -281,6 +326,25 @@ fn extension_dir() -> PathBuf { mod tests { use super::*; + struct TestExtensionDirectory(PathBuf); + + impl TestExtensionDirectory { + fn new() -> Self { + let path = env::temp_dir().join(format!( + "computer-use-linux-extension-test-{}-{}", + std::process::id(), + getrandom::u64().unwrap() + )); + Self(path) + } + } + + impl Drop for TestExtensionDirectory { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + #[test] fn enabled_extensions_literal_adds_uuid_to_existing_list() { assert_eq!( @@ -304,6 +368,55 @@ mod tests { assert_eq!(enabled_extensions_literal(&value).unwrap(), value); } + #[test] + fn session_command_failure_reports_command_stderr() { + let mut command = Command::new("sh"); + command.args(["-c", "printf 'settings rejected' >&2; exit 23"]); + + let error = run_session_command(&mut command, "update GNOME settings").unwrap_err(); + + assert_eq!(error, "settings rejected"); + } + + #[test] + fn extension_file_write_reports_content_changes() { + let extension_dir = TestExtensionDirectory::new(); + + let first = write_extension_files(&extension_dir.0).unwrap(); + assert!(first.wrote_files); + assert!(first.changed_files); + + let second = write_extension_files(&extension_dir.0).unwrap(); + assert!(second.wrote_files); + assert!(!second.changed_files); + + fs::write(extension_dir.0.join("extension.js"), "// stale extension").unwrap(); + let third = write_extension_files(&extension_dir.0).unwrap(); + assert!(third.wrote_files); + assert!(third.changed_files); + } + + #[test] + fn enabled_stale_extension_requires_shell_reload() { + assert!(setup_requires_shell_reload(None, Some(true), true)); + assert!(setup_requires_shell_reload( + Some(&"window API unavailable".to_string()), + Some(false), + false + )); + assert!(!setup_requires_shell_reload(None, Some(true), false)); + assert!(!setup_requires_shell_reload(None, Some(false), true)); + assert!(setup_requires_shell_reload(None, None, true)); + } + + #[test] + fn unknown_extension_state_uses_neutral_reload_guidance() { + assert_eq!( + shell_reload_message(None), + "computer-use-linux GNOME Shell extension files changed, but the previous extension state could not be determined. GNOME Shell must reload before newly installed DBus methods can be relied on." + ); + } + #[test] fn rendered_metadata_uses_build_identity() { let rendered = render_extension_asset(METADATA_JSON); diff --git a/src/server.rs b/src/server.rs index ebc70f1..031c7ca 100644 --- a/src/server.rs +++ b/src/server.rs @@ -570,24 +570,25 @@ impl ComputerUseLinux { } } - /// Try a coordinate click through the absolute uinput pointer. `Some(ok)` if - /// the backend was used; `None` to fall through to portal / ydotool. + /// Try a coordinate click through the absolute uinput pointer. Returns the + /// requested and emitted coordinates from that backend, or `None` to fall + /// through. async fn try_abs_click( &self, x: i32, y: i32, button: Option<&str>, count: u32, - ) -> Option { + ) -> Option { + let btn = crate::abs_pointer::PointerButton::from_name(button)?; if !self.ensure_abs_pointer().await { return None; } - let btn = crate::abs_pointer::PointerButton::from_name(button); let abs_pointer = Arc::clone(&self.abs_pointer); tokio::task::spawn_blocking(move || { let mut guard = abs_pointer.lock().ok()?; let pointer = guard.as_mut()?; - Some(pointer.click(x, y, btn, count).is_ok()) + pointer.click(x, y, btn, count).ok() }) .await .ok() @@ -740,11 +741,9 @@ impl ComputerUseLinux { // relative-only device (faked `--absolute` via pin-to-corner + relative // move, which acceleration + fractional scaling distort) and unlike the // portal (per-monitor coordinate scaling + an approval dialog), the - // absolute pointer lands exactly at the screenshot pixel. - // Off-screen coordinates "succeed" at the uinput layer while landing on - // no visible pixel — surface that instead of a silent no-op. - let off_screen_note = self.off_screen_note_for_point(x, y).await; - if self + // absolute pointer uses screenshot-pixel coordinates directly and + // reports the point it emitted after desktop-edge clamping. + if let Some(landing) = self .try_abs_click( x, y, @@ -752,7 +751,6 @@ impl ComputerUseLinux { params.click_count.unwrap_or(1).clamp(1, 10), ) .await - == Some(true) { return Json(with_notes( ActionOutput { @@ -762,9 +760,10 @@ impl ComputerUseLinux { message: "Action sent through the uinput absolute pointer.".to_string(), received, }, - off_screen_note.clone(), + abs_pointer_clamp_note(landing), )); } + let off_screen_note = self.off_screen_note_for_point(x, y).await; if let Some(session) = self.cached_portal_pointer_session() { let Some((portal_x, portal_y)) = portal_target_point.or_else(|| self.logical_portal_point(&session, x, y)) @@ -4035,6 +4034,15 @@ fn with_notes(mut output: ActionOutput, notes: impl IntoIterator) output } +fn abs_pointer_clamp_note(landing: crate::abs_pointer::PointerLanding) -> Option { + (landing.requested != landing.emitted).then(|| { + format!( + "Requested coordinate {},{} was clamped to {},{} by the uinput absolute pointer.", + landing.requested.0, landing.requested.1, landing.emitted.0, landing.emitted.1 + ) + }) +} + fn focus_satisfies_target(focus: &WindowFocusResult, target: &WindowTarget) -> bool { if target.requires_exact_focus() { focus.exact_window_focused @@ -5291,6 +5299,27 @@ mod tests { } } + #[test] + fn absolute_pointer_note_reports_the_emitted_coordinate() { + assert_eq!( + abs_pointer_clamp_note(crate::abs_pointer::PointerLanding { + requested: (1920, 1080), + emitted: (1919, 1079), + }), + Some( + "Requested coordinate 1920,1080 was clamped to 1919,1079 by the uinput absolute pointer." + .to_string() + ) + ); + assert_eq!( + abs_pointer_clamp_note(crate::abs_pointer::PointerLanding { + requested: (640, 480), + emitted: (640, 480), + }), + None + ); + } + #[test] fn accessibility_filter_candidates_prefer_title_and_skip_synthetic_app_id() { let window = window_info( diff --git a/src/windowing/backends/kwin.rs b/src/windowing/backends/kwin.rs index 388f054..b5faf01 100644 --- a/src/windowing/backends/kwin.rs +++ b/src/windowing/backends/kwin.rs @@ -7,11 +7,16 @@ use serde::Deserialize; use std::{ fs::{self, OpenOptions}, io::Write, + sync::atomic::{AtomicBool, AtomicU64, Ordering}, sync::mpsc, - time::{Duration, SystemTime, UNIX_EPOCH}, + time::Duration, }; use tokio::time::{sleep, timeout}; -use zbus::Proxy; +use zbus::{ + message::Header, + names::{BusName, OwnedUniqueName}, + Proxy, +}; pub const KWIN_BACKEND: &str = "kwin"; const KWIN_SCRIPT_TIMEOUT: Duration = Duration::from_secs(2); @@ -20,6 +25,7 @@ const KWIN_SCRIPTING_OBJECT_PATH: &str = "/Scripting"; const KWIN_SCRIPTING_INTERFACE: &str = "org.kde.kwin.Scripting"; const KWIN_CALLBACK_OBJECT_PATH_PREFIX: &str = "/dev/avifenesh/ComputerUseLinux/KWinWindowQuery"; const KWIN_CALLBACK_INTERFACE: &str = "dev.avifenesh.ComputerUseLinux.KWinWindowQuery"; +static KWIN_PLUGIN_SEQUENCE: AtomicU64 = AtomicU64::new(0); pub fn probe() -> BackendProbe { let check = gdbus_introspect_contains( @@ -79,9 +85,12 @@ struct KwinScriptResult { async fn call_kwin_activate_script(uuid: &str) -> Result<()> { let uuid = uuid.to_string(); - let json = call_kwin_script(move |service_name, callback_object_path, plugin_name| { - write_kwin_activate_script(service_name, callback_object_path, plugin_name, &uuid) - }) + let json = call_kwin_script( + KwinCallbackKind::Result, + move |service_name, callback_object_path, plugin_name| { + write_kwin_activate_script(service_name, callback_object_path, plugin_name, &uuid) + }, + ) .await?; let result: KwinScriptResult = serde_json::from_str(&json).context("failed to parse KWin activation script output")?; @@ -97,37 +106,92 @@ async fn call_kwin_activate_script(uuid: &str) -> Result<()> { } async fn call_kwin_window_script() -> Result { - call_kwin_script(write_kwin_window_script).await + call_kwin_script(KwinCallbackKind::Windows, write_kwin_window_script).await } -async fn call_kwin_script(write_script: F) -> Result +async fn call_kwin_script(expected_kind: KwinCallbackKind, write_script: F) -> Result where F: FnOnce(&str, &str, &str) -> Result, { hydrate_session_bus_env(); - let connection = zbus::Connection::session() + let connection = timeout(KWIN_SCRIPT_TIMEOUT, zbus::Connection::session()) .await + .context("timed out while connecting to the session bus for KWin scripting")? .context("failed to connect to session bus")?; + + call_kwin_script_on_connection(connection, expected_kind, write_script, KWIN_SCRIPT_TIMEOUT) + .await +} + +async fn call_kwin_script_on_connection( + connection: zbus::Connection, + expected_kind: KwinCallbackKind, + write_script: F, + transaction_timeout: Duration, +) -> Result +where + F: FnOnce(&str, &str, &str) -> Result, +{ + call_kwin_script_on_connection_with_plugin_name( + connection, + expected_kind, + write_script, + transaction_timeout, + temporary_kwin_plugin_name()?, + ) + .await +} + +async fn call_kwin_script_on_connection_with_plugin_name( + connection: zbus::Connection, + expected_kind: KwinCallbackKind, + write_script: F, + transaction_timeout: Duration, + plugin_name: String, +) -> Result +where + F: FnOnce(&str, &str, &str) -> Result, +{ let unique_name = connection .unique_name() .context("session bus did not assign a unique name")? .to_string(); - let plugin_name = temporary_kwin_plugin_name(); let callback_object_path = format!("{KWIN_CALLBACK_OBJECT_PATH_PREFIX}/{plugin_name}"); - let (sender, receiver) = mpsc::channel(); let mut cleanup = KwinScriptCleanup::new( connection.clone(), plugin_name.clone(), callback_object_path.clone(), ); - connection - .object_server() - .at(callback_object_path.as_str(), KwinWindowCallback { sender }) - .await - .context("failed to register temporary KWin callback object")?; - let result = async { + let transaction = async { + let dbus_proxy = zbus::fdo::DBusProxy::new(&connection) + .await + .context("failed to create session-bus identity proxy")?; + let expected_sender = dbus_proxy + .get_name_owner(BusName::try_from(KWIN_SCRIPTING_SERVICE)?) + .await + .context("failed to resolve the KWin session-bus owner")?; + let (sender, receiver) = mpsc::channel(); + let callback_registered = connection + .object_server() + .at( + callback_object_path.as_str(), + KwinWindowCallback { + sender, + expected_sender, + expected_kind, + plugin_name: plugin_name.clone(), + delivered: AtomicBool::new(false), + }, + ) + .await + .context("failed to register temporary KWin callback object")?; + if !callback_registered { + bail!("temporary KWin callback object path was already registered"); + } + cleanup.owns_callback = true; + let path = write_script(&unique_name, &callback_object_path, &plugin_name)?; cleanup.script_path = Some(path.clone()); let scripting_proxy = Proxy::new( @@ -154,21 +218,22 @@ where .await .context("KWin start failed after loading the temporary script")?; - timeout(KWIN_SCRIPT_TIMEOUT, async move { - loop { - match receiver.try_recv() { - Ok(json) => return Ok(json), - Err(mpsc::TryRecvError::Disconnected) => { - bail!("KWin temporary script callback disconnected before returning data"); - } - Err(mpsc::TryRecvError::Empty) => sleep(Duration::from_millis(20)).await, + loop { + match receiver.try_recv() { + Ok(json) => return Ok(json), + Err(mpsc::TryRecvError::Disconnected) => { + bail!("KWin temporary script callback disconnected before returning data"); } + Err(mpsc::TryRecvError::Empty) => sleep(Duration::from_millis(20)).await, } - }) - .await - .context("KWin temporary script did not return data before timeout")? - } - .await; + } + }; + let result = match timeout(transaction_timeout, transaction).await { + Ok(result) => result, + Err(_) => Err(anyhow::anyhow!( + "KWin temporary script transaction timed out" + )), + }; cleanup.run().await; result } @@ -178,6 +243,7 @@ struct KwinScriptCleanup { plugin_name: String, callback_object_path: String, script_path: Option, + owns_callback: bool, armed: bool, } @@ -192,6 +258,7 @@ impl KwinScriptCleanup { plugin_name, callback_object_path, script_path: None, + owns_callback: false, armed: true, } } @@ -202,9 +269,11 @@ impl KwinScriptCleanup { self.plugin_name.clone(), self.callback_object_path.clone(), self.script_path.clone(), + self.owns_callback, ) .await; self.script_path = None; + self.owns_callback = false; self.armed = false; } } @@ -218,12 +287,14 @@ impl Drop for KwinScriptCleanup { let plugin_name = self.plugin_name.clone(); let callback_object_path = self.callback_object_path.clone(); let script_path = self.script_path.take(); + let owns_callback = self.owns_callback; if let Ok(runtime) = tokio::runtime::Handle::try_current() { runtime.spawn(cleanup_kwin_script( connection, plugin_name, callback_object_path, script_path, + owns_callback, )); } } @@ -234,57 +305,140 @@ async fn cleanup_kwin_script( plugin_name: String, callback_object_path: String, script_path: Option, + owns_callback: bool, ) { - let _ = timeout(Duration::from_secs(1), async { - if let Ok(scripting_proxy) = Proxy::new( - &connection, - KWIN_SCRIPTING_SERVICE, - KWIN_SCRIPTING_OBJECT_PATH, - KWIN_SCRIPTING_INTERFACE, - ) - .await - { - let _: Result = scripting_proxy - .call("unloadScript", &(plugin_name.as_str())) - .await; - } - }) - .await; - let _: Result = connection - .object_server() - .remove::(callback_object_path.as_str()) + if owns_callback { + let _ = timeout(Duration::from_secs(1), async { + if let Ok(scripting_proxy) = Proxy::new( + &connection, + KWIN_SCRIPTING_SERVICE, + KWIN_SCRIPTING_OBJECT_PATH, + KWIN_SCRIPTING_INTERFACE, + ) + .await + { + let _: Result = scripting_proxy + .call("unloadScript", &(plugin_name.as_str())) + .await; + } + }) .await; + let _: Result = connection + .object_server() + .remove::(callback_object_path.as_str()) + .await; + } if let Some(script_path) = script_path { let _ = fs::remove_file(script_path); } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum KwinCallbackKind { + Windows, + Result, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct KwinCallbackEnvelope { + backend: String, + plugin_name: String, +} + struct KwinWindowCallback { sender: mpsc::Sender, + expected_sender: OwnedUniqueName, + expected_kind: KwinCallbackKind, + plugin_name: String, + delivered: AtomicBool, } -#[zbus::interface(name = "dev.avifenesh.ComputerUseLinux.KWinWindowQuery")] impl KwinWindowCallback { - fn receive_windows(&self, json: &str) -> zbus::fdo::Result<()> { + fn accept( + &self, + actual_sender: Option<&str>, + kind: KwinCallbackKind, + json: &str, + ) -> zbus::fdo::Result<()> { + if actual_sender != Some(self.expected_sender.as_str()) { + return Err(zbus::fdo::Error::AccessDenied( + "KWin callback sender did not own org.kde.KWin".to_string(), + )); + } + if kind != self.expected_kind { + return Err(zbus::fdo::Error::AccessDenied( + "KWin callback method did not match the requested operation".to_string(), + )); + } + let envelope: KwinCallbackEnvelope = serde_json::from_str(json).map_err(|error| { + zbus::fdo::Error::InvalidArgs(format!("invalid KWin callback payload: {error}")) + })?; + if envelope.backend != KWIN_BACKEND || envelope.plugin_name != self.plugin_name { + return Err(zbus::fdo::Error::AccessDenied( + "KWin callback payload did not match the active script".to_string(), + )); + } + if self + .delivered + .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) + .is_err() + { + return Err(zbus::fdo::Error::AccessDenied( + "KWin callback response was already delivered".to_string(), + )); + } self.sender .send(json.to_string()) .map_err(|error| zbus::fdo::Error::Failed(error.to_string())) } +} - fn receive_result(&self, json: &str) -> zbus::fdo::Result<()> { - self.sender - .send(json.to_string()) - .map_err(|error| zbus::fdo::Error::Failed(error.to_string())) +#[zbus::interface(name = "dev.avifenesh.ComputerUseLinux.KWinWindowQuery")] +impl KwinWindowCallback { + fn receive_windows( + &self, + #[zbus(header)] header: Header<'_>, + json: &str, + ) -> zbus::fdo::Result<()> { + self.accept( + header.sender().map(|sender| sender.as_str()), + KwinCallbackKind::Windows, + json, + ) + } + + fn receive_result( + &self, + #[zbus(header)] header: Header<'_>, + json: &str, + ) -> zbus::fdo::Result<()> { + self.accept( + header.sender().map(|sender| sender.as_str()), + KwinCallbackKind::Result, + json, + ) } } -fn temporary_kwin_plugin_name() -> String { +fn temporary_kwin_plugin_name() -> Result { let pid = std::process::id(); - let nanos = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_nanos()) - .unwrap_or_default(); - format!("computer_use_linux_kwin_window_query_{pid}_{nanos}") + let sequence = KWIN_PLUGIN_SEQUENCE + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |value| { + value.checked_add(1) + }) + .map_err(|_| anyhow::anyhow!("temporary KWin plugin sequence exhausted"))?; + let mut nonce = [0_u8; 16]; + getrandom::fill(&mut nonce).map_err(|error| { + anyhow::anyhow!("failed to generate temporary KWin plugin nonce: {error}") + })?; + let nonce = nonce + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + Ok(format!( + "computer_use_linux_kwin_window_query_{pid}_{sequence}_{nonce}" + )) } fn write_kwin_window_script( @@ -296,7 +450,7 @@ fn write_kwin_window_script( write_kwin_script_file(plugin_name, &script) } -pub(crate) fn kwin_window_script_source( +fn kwin_window_script_source( service_name: &str, callback_object_path: &str, plugin_name: &str, @@ -395,6 +549,19 @@ pub(crate) fn kwin_window_script_source( if (read(window, "x11Client")) {{ return "x11"; }} + var objectDescription = serialize(window); + if (typeof objectDescription === "string") {{ + var separator = objectDescription.indexOf("("); + var objectClass = (separator >= 0 + ? objectDescription.slice(0, separator) + : objectDescription).trim(); + if (objectClass === "KWin::XdgToplevelWindow") {{ + return "wayland"; + }} + if (objectClass === "KWin::X11Window") {{ + return "x11"; + }} + }} return null; }} @@ -469,7 +636,7 @@ fn write_kwin_activate_script( write_kwin_script_file(plugin_name, &script) } -pub(crate) fn kwin_activate_script_source( +fn kwin_activate_script_source( service_name: &str, callback_object_path: &str, plugin_name: &str, @@ -674,7 +841,7 @@ fn write_kwin_script_file(plugin_name: &str, script: &str) -> Result Result> { +fn parse_kwin_windows(json: &str) -> Result> { let snapshot = parse_kwin_snapshot(json)?; let mut windows = snapshot .windows @@ -689,7 +856,7 @@ pub(crate) fn parse_kwin_windows(json: &str) -> Result> { Ok(windows) } -pub(crate) fn parse_kwin_logical_desktop_rect(json: &str) -> Result<(i32, i32, i32, i32)> { +fn parse_kwin_logical_desktop_rect(json: &str) -> Result<(i32, i32, i32, i32)> { parse_kwin_snapshot(json)?.logical_desktop_rect() } @@ -805,7 +972,7 @@ impl TryFrom for WindowInfo { } } -pub(crate) fn kwin_window_id_from_uuid(uuid: &str) -> u64 { +fn kwin_window_id_from_uuid(uuid: &str) -> u64 { let normalized = normalize_kwin_uuid(uuid).unwrap_or_else(|| uuid.trim().to_ascii_lowercase()); let mut hash = 0xcbf29ce484222325_u64; for byte in normalized.as_bytes() { @@ -917,3 +1084,427 @@ fn gdbus_introspect_contains( }, } } + +#[cfg(test)] +mod adapter_tests { + use super::*; + + #[test] + fn parses_kwin_windows_as_window_info() { + let uuid = "b4dfacf8-a559-43c9-8b1f-ecd5cfd78359"; + let windows_json = r#"{ + "backend": "kwin", + "desktopGeometry": {"x": 100, "y": -50, "width": 3840, "height": "2160"}, + "windows": [ + { + "uuid": "{b4dfacf8-a559-43c9-8b1f-ecd5cfd78359}", + "caption": "Codex", + "desktopFile": "codex-desktop", + "resourceClass": "codex-desktop", + "resourceName": "codex", + "pid": 68986, + "x": 10, + "y": 48, + "width": 1200, + "height": 800, + "workspace": 1, + "minimized": false, + "active": true, + "clientType": "wayland", + "normalWindow": true, + "desktopWindow": false, + "dock": false + }, + { + "uuid": "{11111111-2222-3333-4444-555555555555}", + "caption": "Desktop", + "desktopWindow": true + } + ] + }"#; + + let windows = parse_kwin_windows(windows_json).unwrap(); + + assert_eq!(windows.len(), 1); + assert_eq!(windows[0].window_id, kwin_window_id_from_uuid(uuid)); + assert_eq!(windows[0].title.as_deref(), Some("Codex")); + assert_eq!(windows[0].app_id.as_deref(), Some("codex-desktop")); + assert_eq!(windows[0].wm_class.as_deref(), Some("codex-desktop")); + assert_eq!(windows[0].pid, Some(68986)); + assert_eq!(windows[0].bounds.as_ref().unwrap().x, Some(10)); + assert_eq!(windows[0].bounds.as_ref().unwrap().height, 800); + assert_eq!(windows[0].workspace, Some(1)); + assert!(windows[0].focused); + assert!(!windows[0].hidden); + assert_eq!(windows[0].client_type.as_deref(), Some("wayland")); + assert_eq!(windows[0].backend, KWIN_BACKEND); + assert_eq!( + parse_kwin_logical_desktop_rect(windows_json).unwrap(), + (100, -50, 3840, 2160) + ); + } + + #[test] + fn kwin_window_ids_are_stable_across_uuid_formats() { + let bare = "b4dfacf8-a559-43c9-8b1f-ecd5cfd78359"; + let braced_upper = "{B4DFACF8-A559-43C9-8B1F-ECD5CFD78359}"; + + assert_eq!( + kwin_window_id_from_uuid(bare), + kwin_window_id_from_uuid(braced_upper) + ); + } + + #[test] + fn kwin_window_script_supports_plasma5_and_plasma6_window_apis() { + let script = kwin_window_script_source( + ":1.234", + "/dev/avifenesh/ComputerUseLinux/KWinWindowQuery/test", + "computer_use_linux_kwin_window_query_test", + ) + .unwrap(); + + assert!(script.contains(r#"typeof workspace.windowList === "function""#)); + assert!(script.contains("workspace.windowList()")); + assert!(script.contains(r#"typeof workspace.clientList === "function""#)); + assert!(script.contains("workspace.clientList()")); + assert!(script.contains( + r#"activeWindow = "activeWindow" in workspace ? workspace.activeWindow : workspace.activeClient;"# + )); + assert!(script.contains("workspace.virtualScreenGeometry")); + assert!(script.contains("desktopGeometry: workspaceGeometry()")); + assert!(script.contains("var objectDescription = serialize(window)")); + assert!(script.contains(r#"objectClass === "KWin::XdgToplevelWindow""#)); + assert!(script.contains(r#"objectClass === "KWin::X11Window""#)); + assert!(!script.contains("objectClass: objectClass(window)")); + } + + #[test] + fn kwin_activation_script_focuses_window_directly() { + let script = kwin_activate_script_source( + ":1.234", + "/dev/avifenesh/ComputerUseLinux/KWinWindowQuery/test", + "computer_use_linux_kwin_window_query_test", + "{B4DFACF8-A559-43C9-8B1F-ECD5CFD78359}", + ) + .unwrap(); + + assert!(script.contains(r#"var targetUuid = "b4dfacf8-a559-43c9-8b1f-ecd5cfd78359";"#)); + assert!(script.contains("targetWindow.minimized = false;")); + assert!(script.contains("workspace.activeWindow = targetWindow;")); + assert!(script.contains(r#"typeof workspace.clientList === "function""#)); + assert!(script.contains("workspace.clientList()")); + assert!(script.contains(r#""activeWindow" in workspace"#)); + assert!(script.contains("workspace.activeClient = targetWindow;")); + assert!(script.contains(r#""ReceiveResult""#)); + assert!(!script.contains("WindowsRunner")); + } +} + +#[cfg(test)] +mod callback_tests { + use super::*; + + fn callback() -> (KwinWindowCallback, mpsc::Receiver) { + let (sender, receiver) = mpsc::channel(); + ( + KwinWindowCallback { + sender, + expected_sender: OwnedUniqueName::try_from(":1.42").unwrap(), + expected_kind: KwinCallbackKind::Windows, + plugin_name: "computer_use_linux_kwin_window_query_test".to_string(), + delivered: AtomicBool::new(false), + }, + receiver, + ) + } + + #[test] + fn callback_accepts_only_the_kwin_owner_requested_method_nonce_and_first_response() { + let (callback, receiver) = callback(); + let valid = r#"{"backend":"kwin","pluginName":"computer_use_linux_kwin_window_query_test","windows":[]}"#; + + assert!(callback + .accept(Some(":1.99"), KwinCallbackKind::Windows, valid) + .is_err()); + assert!(receiver.try_recv().is_err()); + + assert!(callback + .accept(Some(":1.42"), KwinCallbackKind::Result, valid) + .is_err()); + assert!(receiver.try_recv().is_err()); + + for payload in [ + "not-json", + r#"{"backend":"other","pluginName":"computer_use_linux_kwin_window_query_test","windows":[]}"#, + r#"{"backend":"kwin","pluginName":"wrong","windows":[]}"#, + ] { + assert!(callback + .accept(Some(":1.42"), KwinCallbackKind::Windows, payload) + .is_err()); + assert!(receiver.try_recv().is_err()); + } + + callback + .accept(Some(":1.42"), KwinCallbackKind::Windows, valid) + .unwrap(); + assert_eq!(receiver.try_recv().unwrap(), valid); + + assert!(callback + .accept(Some(":1.42"), KwinCallbackKind::Windows, valid) + .is_err()); + assert!(receiver.try_recv().is_err()); + } +} + +#[cfg(test)] +mod transaction_tests { + use super::*; + use std::{ + future::pending, + io::{BufRead, BufReader}, + process::{Child, Command, Stdio}, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, Mutex, + }, + time::Instant, + }; + + #[derive(Clone, Copy)] + enum FakeKwinBehavior { + HangLoad, + HangStart, + NoCallback, + } + + struct FakeKwinScripting { + behavior: FakeKwinBehavior, + load_calls: Arc, + start_calls: Arc, + unload_calls: Arc, + } + + #[zbus::interface(name = "org.kde.kwin.Scripting")] + impl FakeKwinScripting { + #[zbus(name = "loadScript")] + async fn load_script(&self, _path: &str, _plugin_name: &str) -> i32 { + self.load_calls.fetch_add(1, Ordering::SeqCst); + if matches!(self.behavior, FakeKwinBehavior::HangLoad) { + pending::<()>().await; + } + 1 + } + + #[zbus(name = "start")] + async fn start(&self) { + self.start_calls.fetch_add(1, Ordering::SeqCst); + if matches!(self.behavior, FakeKwinBehavior::HangStart) { + pending::<()>().await; + } + } + + #[zbus(name = "unloadScript")] + fn unload_script(&self, _plugin_name: &str) -> bool { + self.unload_calls.fetch_add(1, Ordering::SeqCst); + true + } + } + + struct TestSessionBus { + child: Child, + address: String, + } + + impl TestSessionBus { + fn start() -> Self { + let mut child = Command::new("dbus-daemon") + .args(["--session", "--nofork", "--nopidfile", "--print-address=1"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("start private dbus-daemon"); + let mut address = String::new(); + BufReader::new(child.stdout.take().expect("private bus stdout")) + .read_line(&mut address) + .expect("read private bus address"); + assert!(!address.trim().is_empty(), "private bus emitted no address"); + Self { + child, + address: address.trim().to_string(), + } + } + } + + impl Drop for TestSessionBus { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } + } + + async fn assert_transaction_timeout_cleans_up(behavior: FakeKwinBehavior) { + let bus = TestSessionBus::start(); + let load_calls = Arc::new(AtomicUsize::new(0)); + let start_calls = Arc::new(AtomicUsize::new(0)); + let unload_calls = Arc::new(AtomicUsize::new(0)); + let service_connection = zbus::connection::Builder::address(bus.address.as_str()) + .unwrap() + .name(KWIN_SCRIPTING_SERVICE) + .unwrap() + .serve_at( + KWIN_SCRIPTING_OBJECT_PATH, + FakeKwinScripting { + behavior, + load_calls: Arc::clone(&load_calls), + start_calls: Arc::clone(&start_calls), + unload_calls: Arc::clone(&unload_calls), + }, + ) + .unwrap() + .build() + .await + .unwrap(); + let client_connection = zbus::connection::Builder::address(bus.address.as_str()) + .unwrap() + .build() + .await + .unwrap(); + let callback_path = Arc::new(Mutex::new(None::)); + let script_path = Arc::new(Mutex::new(None::)); + let callback_path_for_writer = Arc::clone(&callback_path); + let script_path_for_writer = Arc::clone(&script_path); + let started = Instant::now(); + + let error = call_kwin_script_on_connection( + client_connection.clone(), + KwinCallbackKind::Windows, + move |service_name, object_path, plugin_name| { + let path = write_kwin_window_script(service_name, object_path, plugin_name)?; + *callback_path_for_writer.lock().unwrap() = Some(object_path.to_string()); + *script_path_for_writer.lock().unwrap() = Some(path.clone()); + Ok(path) + }, + Duration::from_millis(100), + ) + .await + .unwrap_err(); + + assert!(error.to_string().contains("timed out")); + assert!(started.elapsed() < Duration::from_secs(2)); + assert!(unload_calls.load(Ordering::SeqCst) >= 1); + + let path = script_path.lock().unwrap().clone().unwrap(); + assert!(!path.exists(), "temporary KWin script was not removed"); + let object_path = callback_path.lock().unwrap().clone().unwrap(); + assert!(client_connection + .object_server() + .interface::<_, KwinWindowCallback>(object_path.as_str()) + .await + .is_err()); + + drop(service_connection); + drop(client_connection); + drop(bus); + } + + #[tokio::test] + async fn transaction_times_out_and_cleans_up_when_load_script_never_replies() { + assert_transaction_timeout_cleans_up(FakeKwinBehavior::HangLoad).await; + } + + #[tokio::test] + async fn transaction_times_out_and_cleans_up_when_start_never_replies() { + assert_transaction_timeout_cleans_up(FakeKwinBehavior::HangStart).await; + } + + #[tokio::test] + async fn transaction_times_out_and_cleans_up_when_callback_never_arrives() { + assert_transaction_timeout_cleans_up(FakeKwinBehavior::NoCallback).await; + } + + #[tokio::test] + async fn duplicate_callback_path_fails_without_disturbing_its_owner() { + let bus = TestSessionBus::start(); + let load_calls = Arc::new(AtomicUsize::new(0)); + let start_calls = Arc::new(AtomicUsize::new(0)); + let unload_calls = Arc::new(AtomicUsize::new(0)); + let service_connection = zbus::connection::Builder::address(bus.address.as_str()) + .unwrap() + .name(KWIN_SCRIPTING_SERVICE) + .unwrap() + .serve_at( + KWIN_SCRIPTING_OBJECT_PATH, + FakeKwinScripting { + behavior: FakeKwinBehavior::NoCallback, + load_calls: Arc::clone(&load_calls), + start_calls: Arc::clone(&start_calls), + unload_calls: Arc::clone(&unload_calls), + }, + ) + .unwrap() + .build() + .await + .unwrap(); + let client_connection = zbus::connection::Builder::address(bus.address.as_str()) + .unwrap() + .build() + .await + .unwrap(); + let plugin_name = "computer_use_linux_kwin_window_query_duplicate"; + let callback_path = format!("{KWIN_CALLBACK_OBJECT_PATH_PREFIX}/{plugin_name}"); + let expected_sender = service_connection.unique_name().unwrap().to_owned(); + let expected_sender_name = expected_sender.to_string(); + let (sender, receiver) = mpsc::channel(); + assert!(client_connection + .object_server() + .at( + callback_path.as_str(), + KwinWindowCallback { + sender, + expected_sender, + expected_kind: KwinCallbackKind::Windows, + plugin_name: plugin_name.to_string(), + delivered: AtomicBool::new(false), + }, + ) + .await + .unwrap()); + + let error = call_kwin_script_on_connection_with_plugin_name( + client_connection.clone(), + KwinCallbackKind::Windows, + |_, _, _| bail!("script writer must not run after a callback collision"), + Duration::from_millis(100), + plugin_name.to_string(), + ) + .await + .unwrap_err(); + + assert!(error.to_string().contains("already registered")); + assert_eq!(load_calls.load(Ordering::SeqCst), 0); + assert_eq!(start_calls.load(Ordering::SeqCst), 0); + assert_eq!(unload_calls.load(Ordering::SeqCst), 0); + + let callback = client_connection + .object_server() + .interface::<_, KwinWindowCallback>(callback_path.as_str()) + .await + .expect("the original callback must remain registered"); + let payload = format!(r#"{{"backend":"kwin","pluginName":"{plugin_name}","windows":[]}}"#); + callback + .get() + .await + .accept( + Some(expected_sender_name.as_str()), + KwinCallbackKind::Windows, + &payload, + ) + .unwrap(); + assert_eq!(receiver.try_recv().unwrap(), payload); + + drop(service_connection); + drop(client_connection); + drop(bus); + } +} diff --git a/src/windowing/mod.rs b/src/windowing/mod.rs index e7e7456..ef38ad3 100644 --- a/src/windowing/mod.rs +++ b/src/windowing/mod.rs @@ -21,10 +21,6 @@ mod tests { use super::backends::gnome::window_from_properties; use super::backends::hyprland::{parse_hyprland_clients, HYPRLAND_BACKEND}; use super::backends::i3::{parse_i3_tree, parse_xprop_pid, I3_BACKEND}; - use super::backends::kwin::{ - kwin_activate_script_source, kwin_window_id_from_uuid, kwin_window_script_source, - parse_kwin_logical_desktop_rect, parse_kwin_windows, KWIN_BACKEND, - }; use super::registry::{ descriptors, list_note, COSMIC_WAYLAND_BACKEND, GNOME_SHELL_EXTENSION_BACKEND, GNOME_SHELL_INTROSPECT_BACKEND, @@ -639,113 +635,6 @@ mod tests { assert_eq!(parse_xprop_pid("_NET_WM_PID: not found.\n"), None); } - #[test] - fn parses_kwin_windows_as_window_info() { - let uuid = "b4dfacf8-a559-43c9-8b1f-ecd5cfd78359"; - let windows_json = r#"{ - "backend": "kwin", - "desktopGeometry": {"x": 100, "y": -50, "width": 3840, "height": "2160"}, - "windows": [ - { - "uuid": "{b4dfacf8-a559-43c9-8b1f-ecd5cfd78359}", - "caption": "Codex", - "desktopFile": "codex-desktop", - "resourceClass": "codex-desktop", - "resourceName": "codex", - "pid": 68986, - "x": 10, - "y": 48, - "width": 1200, - "height": 800, - "workspace": 1, - "minimized": false, - "active": true, - "clientType": "wayland", - "normalWindow": true, - "desktopWindow": false, - "dock": false - }, - { - "uuid": "{11111111-2222-3333-4444-555555555555}", - "caption": "Desktop", - "desktopWindow": true - } - ] - }"#; - - let windows = parse_kwin_windows(windows_json).unwrap(); - - assert_eq!(windows.len(), 1); - assert_eq!(windows[0].window_id, kwin_window_id_from_uuid(uuid)); - assert_eq!(windows[0].title.as_deref(), Some("Codex")); - assert_eq!(windows[0].app_id.as_deref(), Some("codex-desktop")); - assert_eq!(windows[0].wm_class.as_deref(), Some("codex-desktop")); - assert_eq!(windows[0].pid, Some(68986)); - assert_eq!(windows[0].bounds.as_ref().unwrap().x, Some(10)); - assert_eq!(windows[0].bounds.as_ref().unwrap().height, 800); - assert_eq!(windows[0].workspace, Some(1)); - assert!(windows[0].focused); - assert!(!windows[0].hidden); - assert_eq!(windows[0].client_type.as_deref(), Some("wayland")); - assert_eq!(windows[0].backend, KWIN_BACKEND); - assert_eq!( - parse_kwin_logical_desktop_rect(windows_json).unwrap(), - (100, -50, 3840, 2160) - ); - } - - #[test] - fn kwin_window_ids_are_stable_across_uuid_formats() { - let bare = "b4dfacf8-a559-43c9-8b1f-ecd5cfd78359"; - let braced_upper = "{B4DFACF8-A559-43C9-8B1F-ECD5CFD78359}"; - - assert_eq!( - kwin_window_id_from_uuid(bare), - kwin_window_id_from_uuid(braced_upper) - ); - } - - #[test] - fn kwin_window_script_supports_plasma5_and_plasma6_window_apis() { - let script = kwin_window_script_source( - ":1.234", - "/dev/avifenesh/ComputerUseLinux/KWinWindowQuery/test", - "computer_use_linux_kwin_window_query_test", - ) - .unwrap(); - - assert!(script.contains(r#"typeof workspace.windowList === "function""#)); - assert!(script.contains("workspace.windowList()")); - assert!(script.contains(r#"typeof workspace.clientList === "function""#)); - assert!(script.contains("workspace.clientList()")); - assert!(script.contains( - r#"activeWindow = "activeWindow" in workspace ? workspace.activeWindow : workspace.activeClient;"# - )); - assert!(script.contains("workspace.virtualScreenGeometry")); - assert!(script.contains("desktopGeometry: workspaceGeometry()")); - } - - #[test] - fn kwin_activation_script_focuses_window_directly() { - let script = kwin_activate_script_source( - ":1.234", - "/dev/avifenesh/ComputerUseLinux/KWinWindowQuery/test", - "computer_use_linux_kwin_window_query_test", - "{B4DFACF8-A559-43C9-8B1F-ECD5CFD78359}", - ) - .unwrap(); - - assert!(script.contains(r#"var targetUuid = "b4dfacf8-a559-43c9-8b1f-ecd5cfd78359";"#)); - assert!(script.contains("targetWindow.minimized = false;")); - assert!(script.contains("workspace.activeWindow = targetWindow;")); - assert!(script.contains(r#"typeof workspace.clientList === "function""#)); - assert!(script.contains("workspace.clientList()")); - assert!(script.contains(r#""activeWindow" in workspace"#)); - assert!(script.contains("workspace.activeClient = targetWindow;")); - assert!(script.contains(r#""ReceiveResult""#)); - assert!(!script.contains("WindowsRunner")); - } - #[test] fn hyprland_backend_can_exact_focus_targets() { let mut window = window(2, "Codex", "codex-desktop", "codex-desktop");