fix(verification): bound the KiCad UI health check

crates/konnect-core/src/tools/verification.rs

@@ -80,13 +80,15 @@ pub fn tools() -> Vec<ToolDef> {
         ),
         tool!(
             "check_kicad_ui",
-            "Check whether the KiCAD GUI application is running and responsive.",
+            "Check whether the KiCad GUI application is running and whether IPC responds within a bounded timeout.",
             json!({
                 "type": "object",
                 "properties": {
                     "timeout_seconds": {
                         "type": "integer",
                         "description": "Timeout for the health check in seconds",
+                        "minimum": 1,
+                        "maximum": 300,
                         "default": 5
                     }
                 },
@@ -431,29 +433,51 @@ async fn handle_get_design_rules(
 
 // ─── KiCAD UI management ──────────────────────────────────────────────────────
 
-/// Check if the KiCAD GUI is running by scanning the process list.
+const KICAD_GUI_PROCESS_NAMES: &[&str] = &["kicad", "pcbnew", "eeschema"];
+
+fn is_kicad_process_name(name: &str) -> bool {
+    let file_name = std::path::Path::new(name.trim_matches('"'))
+        .file_name()
+        .and_then(|name| name.to_str())
+        .unwrap_or(name)
+        .to_ascii_lowercase();
+    let stem = file_name.strip_suffix(".exe").unwrap_or(&file_name);
+    KICAD_GUI_PROCESS_NAMES.contains(&stem)
+}
+
+fn process_list_has_kicad(output: &str) -> bool {
+    output.lines().any(|line| {
+        line.split_whitespace()
+            .next()
+            .is_some_and(is_kicad_process_name)
+    })
+}
+
+/// Check if the KiCad project manager or either standalone editor is running.
 fn is_kicad_running() -> bool {
     #[cfg(target_os = "windows")]
     {
-        // On Windows, use `tasklist` to check
         std::process::Command::new("tasklist")
             .output()
             .ok()
-            .map(|o| String::from_utf8_lossy(&o.stdout).contains("kicad.exe"))
+            .map(|output| process_list_has_kicad(&String::from_utf8_lossy(&output.stdout)))
             .unwrap_or(false)
     }
     #[cfg(not(target_os = "windows"))]
     {
-        std::process::Command::new("pgrep")
-            .arg("-x")
-            .arg("kicad")
+        std::process::Command::new("ps")
+            .args(["-A", "-o", "comm="])
             .output()
             .ok()
-            .map(|o| o.status.success())
+            .map(|output| process_list_has_kicad(&String::from_utf8_lossy(&output.stdout)))
             .unwrap_or(false)
     }
 }
 
+fn ui_running(process_detected: bool, ipc_responsive: bool) -> bool {
+    process_detected || ipc_responsive
+}
+
 /// Resolve the KiCAD binary path from config or well-known locations.
 fn find_kicad_binary(config_binary: &str) -> String {
     if !config_binary.is_empty() && std::path::Path::new(config_binary).exists() {
@@ -494,39 +518,84 @@ fn find_kicad_binary(config_binary: &str) -> String {
     }
 }
 
-async fn handle_check_kicad_ui(
-    _args: &serde_json::Value,
-    ctx: &ToolContext,
-) -> anyhow::Result<CallToolResult> {
-    let running = task::spawn_blocking(is_kicad_running).await?;
-
-    if !running {
-        return Ok(CallToolResult::text(
-            serde_json::to_string(&json!({
-                "running": false,
-                "ipc_responsive": false
-            }))
-            .unwrap(),
+fn health_timeout_seconds(args: &serde_json::Value) -> Result<u64, CallToolResult> {
+    let timeout = match args.get("timeout_seconds") {
+        None | Some(serde_json::Value::Null) => 5,
+        Some(value) => value.as_u64().ok_or_else(|| {
+            CallToolResult::error_kind(
+                crate::mcp::error::ToolErrorKind::InvalidArgument {
+                    field: "timeout_seconds".to_string(),
+                    reason: "must be an integer from 1 to 300".to_string(),
+                },
+                "Argument 'timeout_seconds' must be an integer from 1 to 300",
+            )
+        })?,
+    };
+    if !(1..=300).contains(&timeout) {
+        return Err(CallToolResult::error_kind(
+            crate::mcp::error::ToolErrorKind::InvalidArgument {
+                field: "timeout_seconds".to_string(),
+                reason: "must be between 1 and 300 seconds".to_string(),
+            },
+            "Argument 'timeout_seconds' must be between 1 and 300 seconds",
         ));
     }
+    Ok(timeout)
+}
+
+async fn bounded_health_check<F, T>(
+    timeout: std::time::Duration,
+    future: F,
+) -> Result<T, tokio::time::error::Elapsed>
+where
+    F: std::future::Future<Output = T>,
+{
+    tokio::time::timeout(timeout, future).await
+}
 
-    // Try IPC ping
+async fn handle_check_kicad_ui(
+    args: &serde_json::Value,
+    ctx: &ToolContext,
+) -> anyhow::Result<CallToolResult> {
+    let timeout_seconds = match health_timeout_seconds(args) {
+        Ok(timeout) => timeout,
+        Err(error) => return Ok(error),
+    };
     let addr = ctx.config.ipc_address.clone();
-    let ipc_ok = task::spawn_blocking(move || {
-        konnect_ipc::client::KiCadIpcClient::new(&addr)
-            .ping()
-            .unwrap_or(false)
-    })
-    .await
-    .unwrap_or(false);
+    let started = std::time::Instant::now();
+    let check = async move {
+        let process_detected = task::spawn_blocking(is_kicad_running).await?;
+        let ipc_responsive = task::spawn_blocking(move || {
+            konnect_ipc::client::KiCadIpcClient::new(&addr)
+                .ping()
+                .unwrap_or(false)
+        })
+        .await?;
+        Ok::<_, tokio::task::JoinError>((process_detected, ipc_responsive))
+    };
 
-    Ok(CallToolResult::text(
-        serde_json::to_string(&json!({
-            "running": true,
-            "ipc_responsive": ipc_ok
-        }))
-        .unwrap(),
-    ))
+    match bounded_health_check(std::time::Duration::from_secs(timeout_seconds), check).await {
+        Ok(result) => {
+            let (process_detected, ipc_responsive) = result?;
+            Ok(CallToolResult::json(&json!({
+                "running": ui_running(process_detected, ipc_responsive),
+                "process_detected": process_detected,
+                "ipc_responsive": ipc_responsive,
+                "timed_out": false,
+                "timeout_seconds": timeout_seconds,
+                "elapsed_ms": started.elapsed().as_millis() as u64
+            })))
+        }
+        Err(_) => Ok(CallToolResult::json(&json!({
+            "running": null,
+            "process_detected": null,
+            "ipc_responsive": false,
+            "timed_out": true,
+            "timeout_seconds": timeout_seconds,
+            "elapsed_ms": started.elapsed().as_millis() as u64,
+            "note": "KiCad health check exceeded the requested timeout"
+        }))),
+    }
 }
 
 async fn handle_launch_kicad_ui(
@@ -952,6 +1021,54 @@ mod tests {
         )
     }
 
+    #[test]
+    fn health_timeout_is_bounded_and_typed() {
+        assert_eq!(health_timeout_seconds(&json!({})).unwrap(), 5);
+        assert_eq!(
+            health_timeout_seconds(&json!({ "timeout_seconds": 17 })).unwrap(),
+            17
+        );
+        for invalid in [json!(0), json!(301), json!(1.5), json!("5")] {
+            let error = health_timeout_seconds(&json!({ "timeout_seconds": invalid }))
+                .expect_err("out-of-range or non-integer timeout must be refused");
+            assert!(error.is_error);
+        }
+    }
+
+    #[test]
+    fn standalone_editors_count_as_kicad_ui_processes() {
+        for name in ["kicad", "pcbnew", "eeschema", "PCBNEW.EXE"] {
+            assert!(is_kicad_process_name(name), "did not recognize {name}");
+        }
+        assert!(!is_kicad_process_name("kicad-cli"));
+        assert!(!is_kicad_process_name("freerouting"));
+        assert!(process_list_has_kicad(
+            "/usr/bin/Finder\n/Applications/KiCad/pcbnew\n"
+        ));
+    }
+
+    #[test]
+    fn responsive_ipc_is_sufficient_running_evidence() {
+        assert!(ui_running(false, true));
+        assert!(ui_running(true, false));
+        assert!(!ui_running(false, false));
+    }
+
+    #[tokio::test]
+    async fn health_deadline_returns_without_waiting_for_the_inner_future() {
+        let timed_out = bounded_health_check(std::time::Duration::from_millis(1), async {
+            tokio::time::sleep(std::time::Duration::from_millis(100)).await;
+            1
+        })
+        .await;
+        assert!(timed_out.is_err());
+
+        let completed = bounded_health_check(std::time::Duration::from_secs(1), async { 2 })
+            .await
+            .unwrap();
+        assert_eq!(completed, 2);
+    }
+
     fn blank_board() -> &'static str {
         "(kicad_pcb\n  (version 20250610)\n  (generator \"test\")\n  (general (thickness 1.6))\n  (paper \"A4\")\n  (layers\n    (0 \"F.Cu\" signal)\n    (31 \"B.Cu\" signal)\n    (44 \"Edge.Cuts\" user)\n  )\n  (setup (pad_to_mask_clearance 0))\n  (net 0 \"\")\n)\n"
     }

tool-directory.md

@@ -340,7 +340,7 @@ Six tools, grouped into *discovery/routing* and *observability*.
 | `run_drc` | Run the Design Rule Check on the PCB and return structured violation results. |
 | `set_design_rules` | Set board-level design rules (clearance, trace width, via size) in the sibling `.kicad_pro` project file. The board file is not modified. |
 | `get_design_rules` | Return the current design rule constraints from the sibling `.kicad_pro` project file. |
-| `check_kicad_ui` | Check whether the KiCAD GUI application is running and responsive. |
+| `check_kicad_ui` | Check whether the KiCad GUI is running and whether IPC responds within the requested bounded timeout. |
 | `launch_kicad_ui` | Launch the KiCAD GUI application and optionally open a project file. |
 | `copy_routing_pattern` | Copy a routing pattern (traces and vias) from one region of the board to another. |
 | `set_layer_constraints` | Set per-layer design constraints (min trace width, clearance) as named rules in the sibling `.kicad_dru` custom-rules file. |