fix(#2305): run_cmd returns Result, nix fmt, collapse nested if

- run_cmd now returns Result<String, String> — callers pattern-match
  instead of comparing against an "ok" sentinel string
- Add cmd_result() helper to format run_cmd results as tool strings
- mouse_click: collapse nested if-let into let-chain (clippy collapsible_if)
- nix fmt: reformat screen.nix package list

hive-screen-mcp/src/main.rs

@@ -26,24 +26,35 @@ use rmcp::{
 use serde::Deserialize;
 use tokio::process::Command;
 
-/// Run a command and return a human-readable result string.
-/// On success returns stdout (trimmed) or `"ok"` when stdout is empty.
-/// On failure returns a `"command failed (exit N): <stderr>"` string.
-async fn run_cmd(program: &str, args: &[&str]) -> String {
+/// Run a command and return its result.
+/// `Ok(stdout_trimmed)` on success (may be empty string when the tool
+/// produces no output). `Err(human_readable_message)` on failure
+/// (non-zero exit or failed spawn).
+async fn run_cmd(program: &str, args: &[&str]) -> Result<String, String> {
     match Command::new(program).args(args).output().await {
         Ok(out) if out.status.success() => {
-            let s = String::from_utf8_lossy(&out.stdout).trim().to_owned();
-            if s.is_empty() { "ok".to_owned() } else { s }
+            Ok(String::from_utf8_lossy(&out.stdout).trim().to_owned())
         }
         Ok(out) => {
             let msg = String::from_utf8_lossy(&out.stderr).trim().to_owned();
-            format!(
+            Err(format!(
                 "command failed (exit {}): {}",
                 out.status.code().unwrap_or(-1),
                 if msg.is_empty() { "(no stderr)" } else { &msg }
-            )
+            ))
         }
-        Err(e) => format!("failed to launch `{program}`: {e}"),
+        Err(e) => Err(format!("failed to launch `{program}`: {e}")),
+    }
+}
+
+/// Format a `run_cmd` result as a tool-response string. `Ok("")` →
+/// `"ok"` (most input tools produce no stdout); `Ok(s)` → `s`;
+/// `Err(e)` → `e`.
+fn cmd_result(r: Result<String, String>) -> String {
+    match r {
+        Ok(s) if s.is_empty() => "ok".to_owned(),
+        Ok(s) => s,
+        Err(e) => e,
     }
 }
 
@@ -65,11 +76,9 @@ impl ScreenMcp {
                 .map_or(0, |d| d.as_millis());
             format!("/tmp/hive-screenshot-{ms}.png")
         });
-        let result = run_cmd("grim", &["-t", "png", &path]).await;
-        if result == "ok" {
-            format!("screenshot saved to `{path}` — use the Read tool to view it")
-        } else {
-            result
+        match run_cmd("grim", &["-t", "png", &path]).await {
+            Ok(_) => format!("screenshot saved to `{path}` — use the Read tool to view it"),
+            Err(e) => e,
         }
     }
 
@@ -80,7 +89,7 @@ impl ScreenMcp {
             `WAYLAND_DISPLAY` (provided automatically when `hyperhive.gui.enable = true`)."
     )]
     async fn type_text(&self, Parameters(args): Parameters<TypeTextArgs>) -> String {
-        run_cmd("wtype", &[&args.text]).await
+        cmd_result(run_cmd("wtype", &[&args.text]).await)
     }
 
     #[tool(
@@ -92,7 +101,7 @@ impl ScreenMcp {
             `hyperhive.gui.screenInput = true` in the agent config to enable the daemon."
     )]
     async fn key_press(&self, Parameters(args): Parameters<KeyPressArgs>) -> String {
-        run_cmd("ydotool", &["key", &args.keys]).await
+        cmd_result(run_cmd("ydotool", &["key", &args.keys]).await)
     }
 
     #[tool(
@@ -102,18 +111,20 @@ impl ScreenMcp {
             in the agent config to enable the daemon."
     )]
     async fn mouse_move(&self, Parameters(args): Parameters<MouseMoveArgs>) -> String {
-        run_cmd(
-            "ydotool",
-            &[
-                "mousemove",
-                "--absolute",
-                "-x",
-                &args.x.to_string(),
-                "-y",
-                &args.y.to_string(),
-            ],
+        cmd_result(
+            run_cmd(
+                "ydotool",
+                &[
+                    "mousemove",
+                    "--absolute",
+                    "-x",
+                    &args.x.to_string(),
+                    "-y",
+                    &args.y.to_string(),
+                ],
+            )
+            .await,
         )
-        .await
     }
 
     #[tool(
@@ -125,8 +136,8 @@ impl ScreenMcp {
     )]
     async fn mouse_click(&self, Parameters(args): Parameters<MouseClickArgs>) -> String {
         // Move first when coordinates are supplied.
-        if let (Some(x), Some(y)) = (args.x, args.y) {
-            let mv = run_cmd(
+        if let (Some(x), Some(y)) = (args.x, args.y)
+            && let Err(e) = run_cmd(
                 "ydotool",
                 &[
                     "mousemove",
@@ -137,10 +148,9 @@ impl ScreenMcp {
                     &y.to_string(),
                 ],
             )
-            .await;
-            if mv != "ok" {
-                return mv;
-            }
+            .await
+        {
+            return e;
         }
         // Map the button name to ydotool's button code.
         let button = match args.button.as_deref().unwrap_or("left") {
@@ -148,7 +158,7 @@ impl ScreenMcp {
             "middle" => "0xC2",
             _ => "0xC0", // left
         };
-        run_cmd("ydotool", &["click", button]).await
+        cmd_result(run_cmd("ydotool", &["click", button]).await)
     }
 }
 

nix/agent-modules/screen.nix

@@ -40,9 +40,11 @@
 
     # grim: Wayland screenshot; wtype: text/key input (no daemon).
     # ydotool: mouse + key injection via uinput (needs screenInput).
-    environment.systemPackages =
-      [ pkgs.grim pkgs.wtype ]
-      ++ lib.optional config.hyperhive.gui.screenInput pkgs.ydotool;
+    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