From 7fa7e2bdfd5fea83370554122b0b55507891987c Mon Sep 17 00:00:00 2001 From: iris Date: Mon, 20 Jul 2026 20:41:58 +0200 Subject: [PATCH] refactor(#2305): drop mouse tools + ydotool, switch key_press to wtype - remove mouse_move and mouse_click (no Wayland-native alternative on Weston without /dev/uinput; follow-up filed for future investigation) - replace key_press from 'ydotool key' to 'wtype -k': parses mod1+mod2+key into -M mod1 ... -k key ... -m mod1 sequence via virtual-keyboard protocol - remove dest_path parameter from screenshot: always writes to /tmp/ (fixes arbitrary write-path concern from security review) - simplify screen.nix: drop screenInput option, ydotoold systemd unit, ydotool package; only grim + wtype remain (both compositor-mediated, no /dev/uinput) - update module header comment to reflect three-tool surface Addresses mara's /dev/uinput veto (PR #2617 comment #40524). --- hive-screen-mcp/src/main.rs | 159 ++++++++++------------------------- nix/agent-modules/screen.nix | 50 ++--------- 2 files changed, 55 insertions(+), 154 deletions(-) diff --git a/hive-screen-mcp/src/main.rs b/hive-screen-mcp/src/main.rs index 8de2acbe..9cf3d53f 100644 --- a/hive-screen-mcp/src/main.rs +++ b/hive-screen-mcp/src/main.rs @@ -1,19 +1,17 @@ //! `hive-screen-mcp` — stdio MCP bridge for GUI agents. //! -//! Provides screenshot, text input, and mouse/keyboard tools for agents -//! running a Weston Wayland compositor (`hyperhive.gui.enable = true`). -//! Each tool shells out to the appropriate utility: +//! Provides screenshot and keyboard-input tools for agents running a Weston +//! Wayland compositor (`hyperhive.gui.enable = true`). Each tool shells out +//! to the appropriate utility: //! //! - `screenshot` → `grim` (always available with gui enabled) //! - `type_text` → `wtype` (Wayland-native text input, no daemon) -//! - `key_press` → `ydotool key` (needs ydotoold + `/dev/uinput`) -//! - `mouse_move` → `ydotool mousemove` (same requirement) -//! - `mouse_click` → `ydotool click` (same requirement) +//! - `key_press` → `wtype -k` (Wayland virtual-keyboard protocol, no daemon) //! +//! All three tools use Wayland protocols mediated by the compositor — +//! no `/dev/uinput` or kernel-level injection. //! `WAYLAND_DISPLAY` and `XDG_RUNTIME_DIR` are injected globally by the //! `weston-vnc` module so all tools find the compositor automatically. -//! `ydotool`-based tools require the ydotoold daemon; enable it by -//! setting `hyperhive.gui.screenInput = true` in the agent config. use anyhow::Result; use rmcp::{ @@ -64,18 +62,15 @@ struct ScreenMcp; impl ScreenMcp { #[tool( description = "Take a screenshot of the agent's Wayland display and save it as a PNG \ - file. Returns the path to the saved file — pass that path to the `Read` tool to \ - view the image visually. `dest_path` is optional; when omitted a timestamped temp \ - file under `/tmp/` is used. Requires `grim` and a live `WAYLAND_DISPLAY` \ - (provided automatically when `hyperhive.gui.enable = true`)." + file under `/tmp/`. Returns the path to the saved file — pass that path to the \ + `Read` tool to view the image visually. Requires `grim` and a live \ + `WAYLAND_DISPLAY` (provided automatically when `hyperhive.gui.enable = true`)." )] - async fn screenshot(&self, Parameters(args): Parameters) -> String { - let path = args.dest_path.unwrap_or_else(|| { - let ms = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map_or(0, |d| d.as_millis()); - format!("/tmp/hive-screenshot-{ms}.png") - }); + async fn screenshot(&self) -> String { + let ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_millis()); + let path = format!("/tmp/hive-screenshot-{ms}.png"); match run_cmd("grim", &["-t", "png", &path]).await { Ok(_) => format!("screenshot saved to `{path}` — use the Read tool to view it"), Err(e) => e, @@ -83,92 +78,48 @@ impl ScreenMcp { } #[tool( - description = "Type text into the currently focused Wayland window. Uses `wtype`, \ - which supports arbitrary Unicode. For special keys (Enter, Tab, Escape, function \ - keys, arrow keys) or modifier combos, use `key_press` instead. Requires a live \ - `WAYLAND_DISPLAY` (provided automatically when `hyperhive.gui.enable = true`)." + description = "Type text into the currently focused Wayland window. Uses `wtype` \ + via the Wayland virtual-keyboard protocol (compositor-mediated, no kernel \ + injection). Supports arbitrary Unicode. For special keys (Enter, Tab, Escape, \ + function keys, arrow keys) or modifier combos, use `key_press` instead. \ + Requires a live `WAYLAND_DISPLAY` (provided automatically when \ + `hyperhive.gui.enable = true`)." )] async fn type_text(&self, Parameters(args): Parameters) -> String { cmd_result(run_cmd("wtype", &[&args.text]).await) } #[tool( - description = "Press a key or key combination. `keys` uses ydotool syntax: a single \ - key name (`Return`, `Tab`, `Escape`, `BackSpace`, `Delete`, `space`, `F1`–`F12`, \ - `Left`, `Right`, `Up`, `Down`) or a modifier+key combo (`ctrl+c`, \ - `ctrl+shift+t`, `super+l`, `alt+F4`). Requires the ydotoold daemon and \ - `/dev/uinput` device access in the container — set \ - `hyperhive.gui.screenInput = true` in the agent config to enable the daemon." + description = "Press a key or key combination via the Wayland virtual-keyboard \ + protocol (compositor-mediated, no kernel injection). `keys` uses XKB keysym \ + syntax: a single key name (`Return`, `Tab`, `Escape`, `BackSpace`, `Delete`, \ + `space`, `F1`–`F12`, `Left`, `Right`, `Up`, `Down`) or a modifier+key combo \ + with `+`-separated parts where all but the last are modifiers (`ctrl+c`, \ + `ctrl+shift+t`, `super+l`, `alt+F4`). Modifier names: `ctrl`, `shift`, `alt`, \ + `super`. Requires a live `WAYLAND_DISPLAY` (provided automatically when \ + `hyperhive.gui.enable = true`)." )] async fn key_press(&self, Parameters(args): Parameters) -> String { - cmd_result(run_cmd("ydotool", &["key", &args.keys]).await) - } - - #[tool( - description = "Move the mouse cursor to an absolute pixel position on the display. \ - `x` is measured from the left edge, `y` from the top edge. Requires the ydotoold \ - daemon and `/dev/uinput` device access — set `hyperhive.gui.screenInput = true` \ - in the agent config to enable the daemon." - )] - async fn mouse_move(&self, Parameters(args): Parameters) -> String { - cmd_result( - run_cmd( - "ydotool", - &[ - "mousemove", - "--absolute", - "-x", - &args.x.to_string(), - "-y", - &args.y.to_string(), - ], - ) - .await, - ) - } - - #[tool( - description = "Click a mouse button, optionally moving to a position first. `button` \ - is `\"left\"` (default), `\"right\"`, or `\"middle\"`. When `x` and `y` are \ - provided the cursor is moved to that absolute pixel position before clicking. \ - Requires the ydotoold daemon and `/dev/uinput` device access — set \ - `hyperhive.gui.screenInput = true` in the agent config to enable the daemon." - )] - async fn mouse_click(&self, Parameters(args): Parameters) -> String { - // Move first when coordinates are supplied. - if let (Some(x), Some(y)) = (args.x, args.y) - && let Err(e) = run_cmd( - "ydotool", - &[ - "mousemove", - "--absolute", - "-x", - &x.to_string(), - "-y", - &y.to_string(), - ], - ) - .await - { - return e; + // Parse "mod1+mod2+key" into: -M mod1 -M mod2 -k key -m mod2 -m mod1 + // split('+') always yields at least one element, so split_last() is safe. + let parts: Vec<&str> = args.keys.split('+').collect(); + let (key, mods) = parts.split_last().expect("split yields ≥1 element"); + let mut wtype_args: Vec = Vec::new(); + for &m in mods { + wtype_args.push("-M".to_owned()); + wtype_args.push(m.to_owned()); } - // Map the button name to ydotool's button code. - let button = match args.button.as_deref().unwrap_or("left") { - "right" => "0xC1", - "middle" => "0xC2", - _ => "0xC0", // left - }; - cmd_result(run_cmd("ydotool", &["click", button]).await) + wtype_args.push("-k".to_owned()); + wtype_args.push((*key).to_owned()); + for &m in mods.iter().rev() { + wtype_args.push("-m".to_owned()); + wtype_args.push(m.to_owned()); + } + let arg_refs: Vec<&str> = wtype_args.iter().map(String::as_str).collect(); + cmd_result(run_cmd("wtype", &arg_refs).await) } } -#[derive(Debug, Deserialize, JsonSchema)] -struct ScreenshotArgs { - /// Optional destination path for the PNG file. When omitted a - /// timestamped path under `/tmp/` is generated automatically. - dest_path: Option, -} - #[derive(Debug, Deserialize, JsonSchema)] struct TypeTextArgs { /// Text to type into the focused window. Supports arbitrary Unicode. @@ -177,29 +128,11 @@ struct TypeTextArgs { #[derive(Debug, Deserialize, JsonSchema)] struct KeyPressArgs { - /// Key or key combination in ydotool syntax — e.g. `"Return"`, - /// `"Tab"`, `"ctrl+c"`, `"ctrl+shift+t"`, `"super+l"`, `"alt+F4"`. + /// Key or key combination in XKB keysym syntax — e.g. `"Return"`, `"Tab"`, + /// `"ctrl+c"`, `"ctrl+shift+t"`, `"super+l"`, `"alt+F4"`. keys: String, } -#[derive(Debug, Deserialize, JsonSchema)] -struct MouseMoveArgs { - /// Horizontal pixel coordinate from the left edge of the display. - x: i32, - /// Vertical pixel coordinate from the top edge of the display. - y: i32, -} - -#[derive(Debug, Deserialize, JsonSchema)] -struct MouseClickArgs { - /// Button to click: `"left"` (default), `"right"`, or `"middle"`. - button: Option, - /// Optional X coordinate to move to before clicking (absolute pixels). - x: Option, - /// Optional Y coordinate to move to before clicking (absolute pixels). - y: Option, -} - #[tool_handler] impl ServerHandler for ScreenMcp {} diff --git a/nix/agent-modules/screen.nix b/nix/agent-modules/screen.nix index 620bac4c..0ba0f16f 100644 --- a/nix/agent-modules/screen.nix +++ b/nix/agent-modules/screen.nix @@ -1,15 +1,13 @@ -# Screen MCP — screenshot + input injection for GUI agents. +# Screen MCP — screenshot + keyboard input for GUI agents. # # Auto-activated when `hyperhive.gui.enable = true`. Wires the # `hive-screen-mcp` stdio bridge as `extraMcpServers.screen` so claude -# gets five tools: `screenshot`, `type_text`, `key_press`, `mouse_move`, -# and `mouse_click`. +# gets three tools: `screenshot`, `type_text`, and `key_press`. # -# `screenshot` and `type_text` work out of the box (grim + wtype, both -# pure Wayland clients). `key_press`, `mouse_move`, and `mouse_click` -# require the ydotoold daemon, which injects events via `/dev/uinput` -# at the kernel level — enable it by setting -# `hyperhive.gui.screenInput = true`. +# All tools use Wayland protocols mediated by the Weston compositor — +# no `/dev/uinput` or kernel-level injection needed. `grim` takes +# screenshots; `wtype` handles both text input and key/modifier combos +# via the `zwp-virtual-keyboard-unstable-v1` protocol. { pkgs, lib, @@ -17,20 +15,6 @@ ... }: { - options.hyperhive.gui.screenInput = lib.mkOption { - type = lib.types.bool; - default = false; - description = '' - Enable mouse and keyboard injection via ydotool + the ydotoold - daemon. Requires `/dev/uinput` device access inside the container - (the host must bind it in via `extraSystemdProperties` or - `systemd.nspawn..filesConfig.Bind`). When false, - `screenshot` and `type_text` still work; `key_press`, - `mouse_move`, and `mouse_click` return an error from ydotool - until ydotoold is running and `/dev/uinput` is accessible. - ''; - }; - config = lib.mkIf config.hyperhive.gui.enable { # Register the screen MCP bridge so claude gets the screen tools. hyperhive.extraMcpServers.screen = { @@ -38,27 +22,11 @@ args = [ ]; }; - # grim: Wayland screenshot; wtype: text/key input (no daemon). - # ydotool: mouse + key injection via uinput (needs screenInput). + # grim: Wayland screenshot; wtype: text + key input via virtual-keyboard + # protocol (compositor-mediated, no /dev/uinput required). environment.systemPackages = [ pkgs.grim pkgs.wtype - ] - ++ lib.optional config.hyperhive.gui.screenInput pkgs.ydotool; - - # ydotoold — uinput event injection daemon. The socket lands at - # /tmp/.ydotool_socket by default; ydotool picks it up - # automatically. Only started when screenInput is enabled. - systemd.services.ydotoold = lib.mkIf config.hyperhive.gui.screenInput { - description = "ydotool input injection daemon"; - wantedBy = [ "multi-user.target" ]; - after = [ "local-fs.target" ]; - serviceConfig = { - ExecStart = "${pkgs.ydotool}/bin/ydotoold"; - Restart = "on-failure"; - RestartSec = "2s"; - SyslogIdentifier = "ydotoold"; - }; - }; + ]; }; }