feat(windows): implement multi platform process kill (#1875)

Author: cwoolum

.cargo/config.toml

@@ -0,0 +1,10 @@
+[target.x86_64-pc-windows-msvc]
+rustflags = [
+	"-C", "link-arg=/STACK:8000000"
+]
+
+# 64 bit Mingw
+[target.x86_64-pc-windows-gnu]
+rustflags = [
+    "-C", "link-arg=-Wl,--stack,8000000"
+]

Cargo.lock

@@ -178,7 +178,10 @@ security-framework = "3.2.0"
 [target.'cfg(windows)'.dependencies]
 windows = { version = "0.61.1", features = [
     "Foundation",
+    "Win32_System_ProcessStatus",
+    "Win32_System_Kernel",
     "Win32_System_Threading",
+    "Wdk_System_Threading",
 ] }
 winreg = "0.55.0"
 

crates/chat-cli/Cargo.toml

@@ -2,6 +2,16 @@
 //! This lib.rs is only here for testing purposes.
 //! `test_mcp_server/test_server.rs` is declared as a separate binary and would need a way to
 //! reference types defined inside of this crate, hence the export.
+pub mod api_client;
+pub mod auth;
+pub mod aws_common;
+pub mod cli;
+pub mod database;
+pub mod logging;
 pub mod mcp_client;
+pub mod platform;
+pub mod request;
+pub mod telemetry;
+pub mod util;
 
 pub use mcp_client::*;

crates/chat-cli/src/lib.rs

@@ -11,8 +11,6 @@ use std::sync::{
 };
 use std::time::Duration;
 
-use nix::sys::signal::Signal;
-use nix::unistd::Pid;
 use serde::{
     Deserialize,
     Serialize,
@@ -46,6 +44,10 @@ use super::{
     ServerCapabilities,
     ToolsListResult,
 };
+use crate::util::process::{
+    Pid,
+    terminate_process,
+};
 
 pub type ClientInfo = serde_json::Value;
 pub type StdioTransport = JsonRpcStdioTransport;
@@ -165,23 +167,22 @@ impl Client<StdioTransport> {
                 .stdin(Stdio::piped())
                 .stdout(Stdio::piped())
                 .stderr(Stdio::piped())
-                .process_group(0)
                 .envs(std::env::vars());
+
+            #[cfg(not(windows))]
+            command.process_group(0);
+
             if let Some(env) = env {
                 for (env_name, env_value) in env {
                     command.env(env_name, env_value);
                 }
             }
             command.args(args).spawn()?
         };
+
         let server_process_id = child.id().ok_or(ClientError::MissingProcessId)?;
-        #[allow(clippy::map_err_ignore)]
-        let server_process_id = Pid::from_raw(
-            server_process_id
-                .try_into()
-                .map_err(|_| ClientError::MissingProcessId)?,
-        );
-        let server_process_id = Some(server_process_id);
+        let server_process_id = Some(Pid::from_u32(server_process_id));
+
         let transport = Arc::new(transport::stdio::JsonRpcStdioTransport::client(child)?);
         Ok(Self {
             server_name,
@@ -205,7 +206,7 @@ where
     // This drop trait is here as a fail safe to ensure we don't leave behind any orphans.
     fn drop(&mut self) {
         if let Some(process_id) = self.server_process_id {
-            let _ = nix::sys::signal::kill(process_id, Signal::SIGTERM);
+            let _ = terminate_process(process_id);
         }
     }
 }

crates/chat-cli/src/mcp_client/client.rs

@@ -1,11 +1,11 @@
 mod cli_context;
+pub mod consts;
 pub mod directories;
 pub mod open;
+pub mod process;
 pub mod spinner;
 pub mod system_info;
 
-pub mod consts;
-
 use std::fmt::Display;
 use std::io::{
     ErrorKind,

crates/chat-cli/src/util/mod.rs

@@ -0,0 +1,11 @@
+pub use sysinfo::Pid;
+
+#[cfg(target_os = "windows")]
+mod windows;
+#[cfg(target_os = "windows")]
+pub use windows::*;
+
+#[cfg(not(windows))]
+mod unix;
+#[cfg(not(windows))]
+pub use unix::*;

crates/chat-cli/src/util/process/mod.rs

@@ -0,0 +1,64 @@
+use nix::sys::signal::Signal;
+use sysinfo::Pid;
+
+pub fn terminate_process(pid: Pid) -> Result<(), String> {
+    let nix_pid = nix::unistd::Pid::from_raw(pid.as_u32() as i32);
+    nix::sys::signal::kill(nix_pid, Signal::SIGTERM).map_err(|e| format!("Failed to terminate process: {}", e))
+}
+
+#[cfg(test)]
+#[cfg(not(windows))]
+mod tests {
+    use std::process::Command;
+    use std::time::Duration;
+
+    use super::*;
+
+    // Helper to create a long-running process for testing
+    fn spawn_test_process() -> std::process::Child {
+        let mut command = Command::new("sleep");
+        command.arg("30");
+        command.spawn().expect("Failed to spawn test process")
+    }
+
+    #[test]
+    fn test_terminate_process() {
+        // Spawn a test process
+        let mut child = spawn_test_process();
+        let pid = Pid::from_u32(child.id());
+
+        // Terminate the process
+        let result = terminate_process(pid);
+
+        // Verify termination was successful
+        assert!(result.is_ok(), "Process termination failed: {:?}", result.err());
+
+        // Give it a moment to terminate
+        std::thread::sleep(Duration::from_millis(100));
+
+        // Verify the process is actually terminated
+        match child.try_wait() {
+            Ok(Some(_)) => {
+                // Process exited, which is what we expect
+            },
+            Ok(None) => {
+                panic!("Process is still running after termination");
+            },
+            Err(e) => {
+                panic!("Error checking process status: {}", e);
+            },
+        }
+    }
+
+    #[test]
+    fn test_terminate_nonexistent_process() {
+        // Use a likely invalid PID
+        let invalid_pid = Pid::from_u32(u32::MAX - 1);
+
+        // Attempt to terminate a non-existent process
+        let result = terminate_process(invalid_pid);
+
+        // Should return an error
+        assert!(result.is_err(), "Terminating non-existent process should fail");
+    }
+}