From 4329489f99b16d5a0f9ee26271aa771d0bef8248 Mon Sep 17 00:00:00 2001 From: Avi Fenesh Date: Sat, 25 Jul 2026 04:06:30 +0300 Subject: [PATCH] fix: route X11 keyboard input through xdotool XTEST (#58) press_key and type_text used ydotool on X11, which injects raw evdev scancodes into a virtual uinput device. X11 then re-interprets those scancodes through the active XKB layout, so named keys and chords (Return, ctrl+a, ctrl+s) arrived as stray characters and literal text mangled symbols and digits (_ -> %, 1 -> +) even on a plain US layout. Prefer xdotool (XTEST) on non-Wayland sessions with DISPLAY set and the binary present; XTEST resolves keysyms against the live layout. The xdotool key grammar is validated through the existing key_chord parser so both backends accept exactly the same keys. ydotool remains the fallback when xdotool is absent or fails, and Wayland is untouched. doctor reports an xdotool input backend on X11. COMPUTER_USE_LINUX_FORCE_YDOTOOL_KEYBOARD=1 opts out; COMPUTER_USE_LINUX_FORCE_XDOTOOL_KEYBOARD=1 forces it on. Verified on a real Xvfb display driving an xterm through the MCP server: type_text "RAW_LINE1 _ 1 __ 11" landed byte-exact and press_key Return produced a real newline. --- CHANGELOG.md | 10 ++ src/diagnostics.rs | 19 ++++ src/server.rs | 250 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d2da22..3ec4cc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- X11 keyboard input now goes through `xdotool` (XTEST) instead of ydotool. + ydotool injects raw evdev scancodes into a virtual uinput device, which X11 + re-interprets through the active XKB layout — so `press_key "Return"` and + chords like `ctrl+a` landed as stray characters, and `type_text` mangled + symbols and digits (`_` → `%`, `1` → `+`) even on a plain US layout. XTEST + resolves keysyms against the live layout. Wayland behaviour is unchanged, and + the ydotool path remains the fallback when `xdotool` is missing or fails. + `doctor` now reports an `xdotool` input backend on X11. Override with + `COMPUTER_USE_LINUX_FORCE_YDOTOOL_KEYBOARD=1` (opt out) or + `COMPUTER_USE_LINUX_FORCE_XDOTOOL_KEYBOARD=1` (force on). (#58) - Portal `scroll` direction on KDE Plasma Wayland: vertical discrete axis steps are inverted for the xdg-desktop-portal-kde discrete path so `direction: "up"|"down"` matches viewport motion. ydotool / REL_WHEEL polarity is diff --git a/src/diagnostics.rs b/src/diagnostics.rs index ac5061b..493e991 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -122,6 +122,9 @@ pub struct InputReport { pub ydotoold: Check, pub ydotool_socket: Check, pub uinput: Check, + /// X11 XTEST keyboard backend. Preferred over ydotool on X11 sessions, + /// where raw evdev scancodes are re-mapped by the active XKB layout. + pub xdotool: Check, } #[derive(Debug, Clone, Serialize, JsonSchema)] @@ -211,6 +214,9 @@ fn capability_map( if input.ydotool_socket.ok { input_backends.push("ydotool".to_string()); } + if input.xdotool.ok && !session_is_wayland_env() { + input_backends.push("xdotool".to_string()); + } let mut screenshot_backends = Vec::new(); if platform.gnome_shell_version.ok { @@ -639,6 +645,7 @@ fn input_report() -> InputReport { ydotoold: process_check("ydotoold"), ydotool_socket: ydotool_socket_check(), uinput: read_write_path_check(Path::new("/dev/uinput")), + xdotool: command_path_check("xdotool"), } } @@ -809,6 +816,17 @@ fn command_path_check(command: &str) -> Check { command_check("sh", &["-c", &format!("command -v {command}")]) } +/// True when this looks like a Wayland session. Used to avoid advertising the +/// X11-only `xdotool` backend on Wayland, where XTEST does not reach clients. +fn session_is_wayland_env() -> bool { + match std::env::var("XDG_SESSION_TYPE") { + Ok(value) => value.eq_ignore_ascii_case("wayland"), + Err(_) => std::env::var("WAYLAND_DISPLAY") + .map(|value| !value.trim().is_empty()) + .unwrap_or(false), + } +} + fn process_check(process_name: &str) -> Check { command_check("pgrep", &["-a", process_name]) } @@ -1065,6 +1083,7 @@ mod tests { ydotoold, ydotool_socket, uinput, + xdotool: Check::fail("missing xdotool"), } } diff --git a/src/server.rs b/src/server.rs index 8bf7078..5bf5ad9 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1231,6 +1231,29 @@ impl ComputerUseLinux { received, }); }; + // X11: prefer xdotool/XTEST. ydotool's raw evdev scancodes get + // re-mapped by the active XKB layout on X11, so named keys and chords + // arrive as stray glyphs instead of real key events (issue #58). + if self.should_prefer_xdotool_keyboard() { + if let Some(spec) = xdotool_key_spec(¶ms.key) { + let args = vec!["key".to_string(), "--clearmodifiers".to_string(), spec]; + // On error, fall through to ydotool rather than failing outright. + if let Ok(output) = run_xdotool(&args).await { + let mut result = action_result_with_focus( + "press_key", + Ok(vec![output]), + received, + focus.clone(), + ); + result.message = "Action sent through xdotool (X11 XTEST).".to_string(); + if result.ok && focus.is_some() { + let notes = self.input_landing_notes(focus.as_ref(), false).await; + result = with_notes(result, notes); + } + return Json(result); + } + } + } let mut args = vec!["key".to_string()]; args.extend(key_events); let result = run_ydotool(&args).await.map(|output| vec![output]); @@ -1336,6 +1359,32 @@ impl ComputerUseLinux { } } } + // X11: xdotool type resolves keysyms against the live XKB layout. + // ydotool's raw scancodes get re-mapped by X11 and mangle symbols and + // digits (`_` → `%`, `1` → `+`) even on a plain US layout (issue #58). + if self.should_prefer_xdotool_keyboard() { + let args = vec![ + "type".to_string(), + "--clearmodifiers".to_string(), + "--".to_string(), + params.text.clone(), + ]; + // On error, fall through to ydotool rather than failing outright. + if let Ok(output) = run_xdotool(&args).await { + let mut result = action_result_with_focus( + "type_text", + Ok(vec![output]), + received, + focus.clone(), + ); + result.message = "Action sent through xdotool (X11 XTEST).".to_string(); + if result.ok && focus.is_some() { + let notes = self.input_landing_notes(focus.as_ref(), true).await; + result = with_notes(result, notes); + } + return Json(result); + } + } let result = run_ydotool_type_text(¶ms.text) .await .map(|output| vec![output]); @@ -2056,6 +2105,26 @@ impl ComputerUseLinux { && self.is_kde_wayland_session() } + /// Keyboard policy for X11 sessions: prefer `xdotool` (XTEST). + /// + /// ydotool writes raw evdev scancodes to a virtual uinput device. On X11 + /// the server then re-interprets them through the active XKB layout, so + /// `press_key "Return"` and chords like `ctrl+a` land as stray characters, + /// and literal text can mangle symbols/digits (issue #58). XTEST resolves + /// keysyms against the live layout instead. + /// + /// `COMPUTER_USE_LINUX_FORCE_YDOTOOL_KEYBOARD=1` opts out; + /// `COMPUTER_USE_LINUX_FORCE_XDOTOOL_KEYBOARD=1` forces it on. + fn should_prefer_xdotool_keyboard(&self) -> bool { + if env_flag_enabled("COMPUTER_USE_LINUX_FORCE_YDOTOOL_KEYBOARD") { + return false; + } + if env_flag_enabled("COMPUTER_USE_LINUX_FORCE_XDOTOOL_KEYBOARD") { + return xdotool_available(); + } + !self.is_wayland_session() && env_var_non_empty("DISPLAY") && xdotool_available() + } + fn is_kde_wayland_session(&self) -> bool { self.is_wayland_session() && (env_contains("XDG_CURRENT_DESKTOP", "kde") @@ -3051,6 +3120,12 @@ fn env_flag_enabled(key: &str) -> bool { env::var(key).ok().as_deref() == Some("1") } +fn env_var_non_empty(key: &str) -> bool { + env::var(key) + .map(|value| !value.trim().is_empty()) + .unwrap_or(false) +} + /// Return the base64 payload of a `data:` URL (or the original string if bare). fn data_url_payload(data_url: &str) -> String { data_url @@ -3671,6 +3746,135 @@ fn ydotool_output_error(output: Output) -> String { command_output_error("ydotool", output) } +/// X11 keyboard input runs through `xdotool` (XTEST) instead of ydotool. +/// +/// ydotool injects raw evdev keycodes into a virtual uinput device. Under X11 +/// the server re-interprets those scancodes through the active XKB layout, so +/// named keys and chords land as unrelated glyphs and literal text can mangle +/// symbols/digits (`_` → `%`, `1` → `+`). XTEST resolves keysyms against the +/// live layout, which is what X11 clients actually expect. See issue #58. +async fn run_xdotool(args: &[String]) -> std::result::Result { + let mut command = TokioCommand::new("xdotool"); + command.args(args); + command.stdout(Stdio::piped()); + command.stderr(Stdio::piped()); + + match command.spawn() { + Ok(child) => match wait_for_ydotool_output(child).await { + Ok(output) if output.status.success() => Ok(output), + Ok(output) => Err(command_output_error("xdotool", output)), + Err(error) => Err(error), + }, + Err(error) => Err(format!("failed to run xdotool: {error}")), + } +} + +/// True when `xdotool` can drive this session: an X11 session with `DISPLAY` +/// set and the binary present. `COMPUTER_USE_LINUX_FORCE_YDOTOOL_KEYBOARD=1` +/// opts out; `COMPUTER_USE_LINUX_FORCE_XDOTOOL_KEYBOARD=1` forces it on. +fn xdotool_available() -> bool { + which_in_path("xdotool") +} + +fn which_in_path(binary: &str) -> bool { + let Ok(path) = env::var("PATH") else { + return false; + }; + env::split_paths(&path).any(|dir| { + let candidate = dir.join(binary); + std::fs::metadata(&candidate) + .map(|meta| meta.is_file()) + .unwrap_or(false) + }) +} + +/// Map our key grammar onto an `xdotool key` spec such as `ctrl+a`, `Return`, +/// or `shift+F5`. Returns `None` for keys the grammar does not accept, so the +/// caller keeps its existing "never silently dropped" error. +fn xdotool_key_spec(key: &str) -> Option { + let parts = key + .split('+') + .map(str::trim) + .filter(|part| !part.is_empty()) + .collect::>(); + let (key_part, modifier_parts) = parts.split_last()?; + + // Validate through the same evdev grammar so both backends accept exactly + // the same input set. + key_chord(key)?; + + let mut spec = Vec::new(); + for part in modifier_parts { + spec.push(xdotool_modifier_name(part)?.to_string()); + } + + if modifier_parts.is_empty() { + if let Some(bare) = xdotool_modifier_keysym(key_part) { + return Some(bare.to_string()); + } + } + spec.push(xdotool_keysym_name(key_part)?); + Some(spec.join("+")) +} + +fn xdotool_modifier_name(key: &str) -> Option<&'static str> { + match normalize_key(key).as_str() { + "ctrl" | "control" => Some("ctrl"), + "alt" | "option" => Some("alt"), + "shift" => Some("shift"), + "meta" | "super" | "cmd" | "command" => Some("super"), + _ => None, + } +} + +/// Standalone keysym for a bare modifier press (`press_key "Super"`). +fn xdotool_modifier_keysym(key: &str) -> Option<&'static str> { + match normalize_key(key).as_str() { + "ctrl" | "control" => Some("ctrl"), + "alt" | "option" => Some("alt"), + "shift" => Some("shift"), + "meta" | "super" | "cmd" | "command" => Some("super"), + _ => None, + } +} + +fn xdotool_keysym_name(key: &str) -> Option { + let normalized = normalize_key(key); + let named = match normalized.as_str() { + "enter" | "return" => "Return", + "escape" | "esc" => "Escape", + "tab" => "Tab", + "backspace" => "BackSpace", + "delete" | "del" => "Delete", + "space" => "space", + "home" => "Home", + "end" => "End", + "pageup" | "page_up" => "Page_Up", + "pagedown" | "page_down" => "Page_Down", + "arrowleft" | "left" => "Left", + "arrowright" | "right" => "Right", + "arrowup" | "up" => "Up", + "arrowdown" | "down" => "Down", + "f1" => "F1", + "f2" => "F2", + "f3" => "F3", + "f4" => "F4", + "f5" => "F5", + "f6" => "F6", + "f7" => "F7", + "f8" => "F8", + "f9" => "F9", + "f10" => "F10", + "f11" => "F11", + "f12" => "F12", + value if value.len() == 1 && value.as_bytes()[0].is_ascii_alphanumeric() => { + return Some(value.to_string()); + } + _ => return None, + }; + Some(named.to_string()) +} + fn command_output_error(command: &str, output: Output) -> String { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); @@ -4817,6 +5021,52 @@ mod tests { ); } + #[test] + fn xdotool_key_spec_maps_named_keys_to_x11_keysyms() { + assert_eq!(xdotool_key_spec("Return"), Some("Return".to_string())); + assert_eq!(xdotool_key_spec("enter"), Some("Return".to_string())); + assert_eq!(xdotool_key_spec("Escape"), Some("Escape".to_string())); + assert_eq!(xdotool_key_spec("backspace"), Some("BackSpace".to_string())); + assert_eq!(xdotool_key_spec("PageUp"), Some("Page_Up".to_string())); + assert_eq!(xdotool_key_spec("ArrowLeft"), Some("Left".to_string())); + assert_eq!(xdotool_key_spec("f5"), Some("F5".to_string())); + assert_eq!(xdotool_key_spec("space"), Some("space".to_string())); + } + + #[test] + fn xdotool_key_spec_maps_chords_with_modifier_prefixes() { + assert_eq!(xdotool_key_spec("ctrl+a"), Some("ctrl+a".to_string())); + assert_eq!(xdotool_key_spec("Ctrl+S"), Some("ctrl+s".to_string())); + assert_eq!( + xdotool_key_spec("Ctrl+Shift+P"), + Some("ctrl+shift+p".to_string()) + ); + assert_eq!( + xdotool_key_spec("Meta+Return"), + Some("super+Return".to_string()) + ); + assert_eq!(xdotool_key_spec("Alt+F4"), Some("alt+F4".to_string())); + } + + #[test] + fn xdotool_key_spec_maps_bare_modifier_to_single_keysym() { + assert_eq!(xdotool_key_spec("Super"), Some("super".to_string())); + assert_eq!(xdotool_key_spec("ctrl"), Some("ctrl".to_string())); + } + + /// The xdotool path must accept exactly the keys the evdev grammar accepts, + /// so switching backends can never silently widen or narrow the surface. + #[test] + fn xdotool_key_spec_rejects_everything_key_chord_rejects() { + for key in ["NotAKey", "", "ctrl+", "ctrl+NotAKey", "f13", "hyper+a"] { + assert_eq!( + xdotool_key_spec(key).is_some(), + key_chord(key).is_some(), + "backend grammars diverged for {key:?}" + ); + } + } + #[test] fn key_sequence_keeps_shortcuts_and_navigation_on_raw_events() { assert_eq!(