diff --git a/Cargo.lock b/Cargo.lock index 43b5b23..caf8da6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -539,9 +539,9 @@ checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] name = "rand" -version = "0.10.0" +version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc266eb313df6c5c09c1c7b1fbe2510961e5bcd3add930c1e31f7ed9da0feff8" +checksum = "d2e8e8bcc7961af1fdac401278c6a831614941f6164ee3bf4ce61b7edb162207" dependencies = [ "chacha20", "getrandom", diff --git a/apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj b/apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj index 0c5d1d0..76efd04 100644 --- a/apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj +++ b/apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj @@ -360,7 +360,7 @@ ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/bash; - shellScript = "set -euo pipefail\n\n# Ensure cargo is available (Xcode strips PATH)\nexport PATH=\"$HOME/.cargo/bin:$PATH:/opt/homebrew/bin:/usr/local/bin\"\n\nRUST_ROOT=\"$SRCROOT/../../crates\"\nENGINE_BIN=\"pu-engine\"\nCLI_BIN=\"pu\"\nOUT_DIR=\"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/MacOS\"\nPU_BIN_DIR=\"$HOME/.pu/bin\"\n\n# Build for current architecture only in Debug, universal in Release\nif [ \"$CONFIGURATION\" = \"Release\" ]; then\n rustup target add aarch64-apple-darwin x86_64-apple-darwin 2>/dev/null || true\n cargo build --release --manifest-path \"$RUST_ROOT/../Cargo.toml\" -p pu-engine -p pu-cli \\\n --target aarch64-apple-darwin\n cargo build --release --manifest-path \"$RUST_ROOT/../Cargo.toml\" -p pu-engine -p pu-cli \\\n --target x86_64-apple-darwin\n lipo -create -output \"$OUT_DIR/$ENGINE_BIN\" \\\n \"$RUST_ROOT/../target/aarch64-apple-darwin/release/$ENGINE_BIN\" \\\n \"$RUST_ROOT/../target/x86_64-apple-darwin/release/$ENGINE_BIN\"\n lipo -create -output \"$OUT_DIR/$CLI_BIN\" \\\n \"$RUST_ROOT/../target/aarch64-apple-darwin/release/$CLI_BIN\" \\\n \"$RUST_ROOT/../target/x86_64-apple-darwin/release/$CLI_BIN\"\nelse\n cargo build --manifest-path \"$RUST_ROOT/../Cargo.toml\" -p pu-engine -p pu-cli\n cp \"$RUST_ROOT/../target/debug/$ENGINE_BIN\" \"$OUT_DIR/$ENGINE_BIN\"\n cp \"$RUST_ROOT/../target/debug/$CLI_BIN\" \"$OUT_DIR/$CLI_BIN\"\n # Also update ~/.cargo/bin and ~/.pu/bin so CLI usage stays in sync\n cp \"$RUST_ROOT/../target/debug/$ENGINE_BIN\" \"$HOME/.cargo/bin/$ENGINE_BIN\" 2>/dev/null || true\n mkdir -p \"$PU_BIN_DIR\"\n cp \"$RUST_ROOT/../target/debug/$CLI_BIN\" \"$PU_BIN_DIR/$CLI_BIN\" 2>/dev/null || true\nfi\n\n# Copy plugin directory into app bundle for CLIInstaller\nmkdir -p \"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/Resources\"\nrm -rf \"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/Resources/pu-plugin\"\ncp -R \"$RUST_ROOT/pu-cli/assets/plugin\" \"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/Resources/pu-plugin\"\n\n# Code sign the binaries with the app's identity\ncodesign --force --sign \"${EXPANDED_CODE_SIGN_IDENTITY:--}\" --options runtime \"$OUT_DIR/$ENGINE_BIN\"\ncodesign --force --sign \"${EXPANDED_CODE_SIGN_IDENTITY:--}\" --options runtime \"$OUT_DIR/$CLI_BIN\"\n"; + shellScript = "set -euo pipefail\n\n# Ensure cargo is available (Xcode strips PATH)\nexport PATH=\"$HOME/.cargo/bin:$PATH:/opt/homebrew/bin:/usr/local/bin\"\n\nRUST_ROOT=\"$SRCROOT/../../crates\"\nENGINE_BIN=\"pu-engine\"\nCLI_BIN=\"pu\"\nOUT_DIR=\"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/MacOS\"\nPU_BIN_DIR=\"$HOME/.pu/bin\"\n\n# Build for current architecture only in Debug, universal in Release\nif [ \"$CONFIGURATION\" = \"Release\" ]; then\n rustup target add aarch64-apple-darwin x86_64-apple-darwin 2>/dev/null || true\n cargo build --release --manifest-path \"$RUST_ROOT/../Cargo.toml\" -p pu-engine -p pu-cli \\\n --target aarch64-apple-darwin\n cargo build --release --manifest-path \"$RUST_ROOT/../Cargo.toml\" -p pu-engine -p pu-cli \\\n --target x86_64-apple-darwin\n lipo -create -output \"$OUT_DIR/$ENGINE_BIN\" \\\n \"$RUST_ROOT/../target/aarch64-apple-darwin/release/$ENGINE_BIN\" \\\n \"$RUST_ROOT/../target/x86_64-apple-darwin/release/$ENGINE_BIN\"\n lipo -create -output \"$OUT_DIR/$CLI_BIN\" \\\n \"$RUST_ROOT/../target/aarch64-apple-darwin/release/$CLI_BIN\" \\\n \"$RUST_ROOT/../target/x86_64-apple-darwin/release/$CLI_BIN\"\nelse\n cargo build --manifest-path \"$RUST_ROOT/../Cargo.toml\" -p pu-engine -p pu-cli\n cp \"$RUST_ROOT/../target/debug/$ENGINE_BIN\" \"$OUT_DIR/$ENGINE_BIN\"\n cp \"$RUST_ROOT/../target/debug/$CLI_BIN\" \"$OUT_DIR/$CLI_BIN\"\n # Also update ~/.cargo/bin and ~/.pu/bin so CLI usage stays in sync\n cp \"$RUST_ROOT/../target/debug/$ENGINE_BIN\" \"$HOME/.cargo/bin/$ENGINE_BIN\" 2>/dev/null || true\n mkdir -p \"$PU_BIN_DIR\"\n cp \"$RUST_ROOT/../target/debug/$ENGINE_BIN\" \"$PU_BIN_DIR/$ENGINE_BIN\" 2>/dev/null || true\n cp \"$RUST_ROOT/../target/debug/$CLI_BIN\" \"$PU_BIN_DIR/$CLI_BIN\" 2>/dev/null || true\nfi\n\n# Copy plugin directory into app bundle for CLIInstaller\nmkdir -p \"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/Resources\"\nrm -rf \"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/Resources/pu-plugin\"\ncp -R \"$RUST_ROOT/pu-cli/assets/plugin\" \"$BUILT_PRODUCTS_DIR/$CONTENTS_FOLDER_PATH/Resources/pu-plugin\"\n\n# Code sign the binaries with the app's identity\ncodesign --force --sign \"${EXPANDED_CODE_SIGN_IDENTITY:--}\" --options runtime \"$OUT_DIR/$ENGINE_BIN\"\ncodesign --force --sign \"${EXPANDED_CODE_SIGN_IDENTITY:--}\" --options runtime \"$OUT_DIR/$CLI_BIN\"\n"; }; /* End PBXShellScriptBuildPhase section */ diff --git a/apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 18fb59e..2a95721 100644 --- a/apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -7,7 +7,7 @@ "location" : "https://github.com/ChimeHQ/Neon", "state" : { "branch" : "main", - "revision" : "56bd2b1febeab0e10e72e2491746eda6787165b6" + "revision" : "484d6fb9e0c4fb679a1d5f5ddaf2cac2ecf21165" } }, { diff --git a/crates/pu-engine/src/engine/mod.rs b/crates/pu-engine/src/engine/mod.rs index fefeb62..d08578b 100644 --- a/crates/pu-engine/src/engine/mod.rs +++ b/crates/pu-engine/src/engine/mod.rs @@ -27,7 +27,9 @@ use pu_core::protocol::{AgentConfigInfo, GridCommand, PROTOCOL_VERSION, Request, use pu_core::types::Manifest; use tokio::sync::OnceCell; -use crate::pty_manager::{AgentHandle, NativePtyHost}; +use crate::pty_manager::{ + AgentHandle, NativePtyHost, descendants_from_tree, snapshot_process_tree, +}; /// Parameters for spawning an agent, extracted to avoid too many positional args. pub(super) struct SpawnParams { @@ -211,6 +213,68 @@ impl Engine { .unwrap_or_default() } + /// Drain every active session and kill its process group (SIGTERM, brief wait, SIGKILL). + /// Used by the managed-mode parent-died path so a force-quit of the macOS app reaps + /// agents and their grandchildren (vitest/node workers, dev servers) before the + /// daemon exits — `process::exit(0)` skips `Drop`, so this must run explicitly. + pub async fn kill_all_sessions(&self, grace: Duration) { + let handles: Vec = { + let mut sessions = self.sessions.lock().await; + sessions.drain().map(|(_, h)| h).collect() + }; + if handles.is_empty() { + return; + } + + // One `ps` snapshot for all handles — cheaper than one per agent. + let tree = snapshot_process_tree().await; + let all_descendants: Vec = handles + .iter() + .filter_map(|h| i32::try_from(h.pid).ok()) + .flat_map(|pid| descendants_from_tree(&tree, pid)) + .collect(); + + for handle in &handles { + if let Ok(pid) = i32::try_from(handle.pid) { + unsafe { + libc::killpg(pid, libc::SIGTERM); + } + } + } + for &desc in &all_descendants { + unsafe { + libc::kill(desc, libc::SIGTERM); + } + } + + let deadline = tokio::time::Instant::now() + grace; + loop { + let all_done = handles.iter().all(|h| h.exit_rx.borrow().is_some()); + if all_done || tokio::time::Instant::now() >= deadline { + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + + for handle in &handles { + if handle.exit_rx.borrow().is_some() { + continue; + } + if let Ok(pid) = i32::try_from(handle.pid) { + unsafe { + libc::killpg(pid, libc::SIGKILL); + } + } + } + for &desc in &all_descendants { + unsafe { + if libc::kill(desc, 0) == 0 { + libc::kill(desc, libc::SIGKILL); + } + } + } + } + pub async fn handle_request(&self, request: Request) -> Response { // Register project for any project-scoped request match &request { @@ -701,11 +765,12 @@ impl Engine { impl Drop for Engine { fn drop(&mut self) { - // Kill all child processes so spawn_blocking reader/waitpid tasks can finish. + // Kill the whole process group of each child (PID == PGID via setsid at spawn) + // so descendants like vitest/node workers don't orphan to launchd. if let Ok(sessions) = self.sessions.try_lock() { for handle in sessions.values() { unsafe { - libc::kill(handle.pid as i32, libc::SIGKILL); + libc::killpg(handle.pid as i32, libc::SIGKILL); } } } diff --git a/crates/pu-engine/src/main.rs b/crates/pu-engine/src/main.rs index 7dd75fe..281231d 100644 --- a/crates/pu-engine/src/main.rs +++ b/crates/pu-engine/src/main.rs @@ -45,37 +45,45 @@ async fn main() { tracing::info!(pid = std::process::id(), socket = %socket.display(), managed, "starting pu-engine"); + let engine = Engine::new(); + let server = match IpcServer::bind(&socket, engine) { + Ok(s) => s, + Err(e) => { + eprintln!("failed to bind socket: {e}"); + std::process::exit(1); + } + }; + + // Start background reaper for dead sessions and orphaned channels + server.engine().start_session_reaper(); + + // Start background scheduler for recurring tasks + server.engine().start_scheduler(); + // In managed mode, exit when the parent process (macOS app) dies. // Without this, the daemon outlives app restarts and stale binaries persist. + // process::exit(0) skips Drop, so explicitly kill agent process groups first + // — otherwise vitest/node workers and other grandchildren orphan to launchd. if managed { let parent_pid = std::os::unix::process::parent_id(); + let engine_for_cleanup = server.engine().clone(); tokio::spawn(async move { loop { tokio::time::sleep(std::time::Duration::from_secs(2)).await; // On macOS, orphaned processes get reparented to PID 1 (launchd) if std::os::unix::process::parent_id() != parent_pid { tracing::info!("parent process died, shutting down managed daemon"); + let _ = tokio::time::timeout( + std::time::Duration::from_secs(1), + engine_for_cleanup.kill_all_sessions(std::time::Duration::from_millis(500)), + ) + .await; std::process::exit(0); } } }); } - let engine = Engine::new(); - let server = match IpcServer::bind(&socket, engine) { - Ok(s) => s, - Err(e) => { - eprintln!("failed to bind socket: {e}"); - std::process::exit(1); - } - }; - - // Start background reaper for dead sessions and orphaned channels - server.engine().start_session_reaper(); - - // Start background scheduler for recurring tasks - server.engine().start_scheduler(); - if let Err(e) = server.run().await { tracing::error!("server error: {e}"); } diff --git a/crates/pu-engine/src/pty_manager.rs b/crates/pu-engine/src/pty_manager.rs index 4b3128e..cfbd3a8 100644 --- a/crates/pu-engine/src/pty_manager.rs +++ b/crates/pu-engine/src/pty_manager.rs @@ -1,3 +1,4 @@ +use std::collections::{HashMap, VecDeque}; use std::ffi::CString; use std::os::fd::{AsRawFd, OwnedFd}; use std::sync::Arc; @@ -11,6 +12,65 @@ use tokio::sync::watch; use crate::output_buffer::OutputBuffer; +/// Snapshot the system process tree as a parent→children map by running +/// `ps -ax -o pid,ppid`. Returns an empty map on failure (graceful +/// degradation — callers fall back to killpg-only behaviour). +/// +/// Separating the snapshot from the walk allows multiple callers (e.g. +/// kill_all_sessions with N handles) to share a single `ps` invocation. +pub(crate) async fn snapshot_process_tree() -> HashMap> { + let output = tokio::process::Command::new("ps") + .args(["-ax", "-o", "pid,ppid"]) + .output() + .await; + let Ok(output) = output else { + return HashMap::new(); + }; + let Ok(stdout) = std::str::from_utf8(&output.stdout) else { + return HashMap::new(); + }; + let mut children: HashMap> = HashMap::new(); + for line in stdout.lines().skip(1) { + let mut parts = line.split_whitespace(); + let pid = parts.next().and_then(|p| p.parse::().ok()); + let ppid = parts.next().and_then(|p| p.parse::().ok()); + if let (Some(pid), Some(ppid)) = (pid, ppid) { + if pid > 0 { + children.entry(ppid).or_default().push(pid); + } + } + } + children +} + +/// Walk `tree` (parent→children map) from `root_pid` and return all +/// descendant PIDs, excluding `root_pid` itself. +/// +/// `killpg` only reaches processes whose PGID matches the session leader — +/// any process that called `setsid()` or `setpgid()` (e.g. an npm dev +/// server spawned with `detached: true`) creates its own process group and +/// escapes it. Traversing by PPID finds those processes so they can be +/// killed by PID directly. +pub(crate) fn descendants_from_tree(tree: &HashMap>, root_pid: i32) -> Vec { + let mut result = Vec::new(); + let mut queue = VecDeque::new(); + queue.push_back(root_pid); + while let Some(pid) = queue.pop_front() { + if let Some(kids) = tree.get(&pid) { + for &kid in kids { + result.push(kid); + queue.push_back(kid); + } + } + } + result +} + +async fn collect_process_descendants(root_pid: i32) -> Vec { + let tree = snapshot_process_tree().await; + descendants_from_tree(&tree, root_pid) +} + /// Fallback fd upper bound when sysconf(_SC_OPEN_MAX) fails or overflows i32. const FD_CLOSE_UPPER_BOUND: i32 = 10240; @@ -252,8 +312,22 @@ impl NativePtyHost { })?; let pid = Pid::from_raw(raw_pid); - // Send SIGTERM first (graceful shutdown) - signal::kill(pid, Signal::SIGTERM).ok(); + // Snapshot the full descendant tree before signalling. Any process that + // called setsid()/setpgid() (e.g. an npm dev server) has a different PGID + // and would escape killpg — we'll kill those by PID directly below. + let descendants = collect_process_descendants(raw_pid).await; + + // Send SIGTERM to the entire process group (graceful shutdown). + // The child setsid()'d at spawn, so its PID is also its PGID. Targeting the + // group reaps grandchildren (e.g. vitest worker forks under a user shell) + // that would otherwise be reparented to launchd and leak. + signal::killpg(pid, Signal::SIGTERM).ok(); + // SIGTERM any escaped descendants (separate process groups). + for &desc in &descendants { + unsafe { + libc::kill(desc, libc::SIGTERM); + } + } // Poll for exit let deadline = tokio::time::Instant::now() + grace_period; @@ -267,8 +341,18 @@ impl NativePtyHost { tokio::time::sleep(Duration::from_millis(200)).await; } - // Force kill - signal::kill(pid, Signal::SIGKILL).ok(); + // Force kill the whole group. + signal::killpg(pid, Signal::SIGKILL).ok(); + // Force kill escaped descendants. Check liveness first (kill(pid, 0)) + // to avoid SIGKILL hitting a recycled PID that replaced the original + // process during the grace period. + for &desc in &descendants { + unsafe { + if libc::kill(desc, 0) == 0 { + libc::kill(desc, libc::SIGKILL); + } + } + } tokio::time::sleep(Duration::from_millis(100)).await; self.check(handle).await } @@ -627,4 +711,117 @@ mod tests { host.kill(&handle, Duration::from_secs(1)).await.ok(); } + + #[tokio::test(flavor = "current_thread")] + async fn given_killed_agent_should_reap_grandchildren() { + let host = NativePtyHost::new(); + let tmp = tempfile::NamedTempFile::new().unwrap(); + let path = tmp.path().to_string_lossy().into_owned(); + + // Shell that backgrounds a long sleep, records its PID, then waits. + // The sleep is a grandchild of the kill target — under the old single-PID + // kill it would orphan to launchd; under killpg it should be reaped. + let handle = host + .spawn(SpawnConfig { + command: "/bin/sh".into(), + args: vec!["-c".into(), format!("sleep 300 & echo $! > {path}; wait")], + cwd: "/tmp".into(), + env: vec![], + env_remove: vec![], + cols: 80, + rows: 24, + }) + .await + .unwrap(); + + let mut grandchild_pid: i32 = 0; + for _ in 0..50 { + tokio::time::sleep(Duration::from_millis(50)).await; + if let Ok(s) = std::fs::read_to_string(&path) + && let Ok(pid) = s.trim().parse::() + && pid > 0 + { + grandchild_pid = pid; + break; + } + } + assert!(grandchild_pid > 0, "grandchild PID was not recorded"); + + let alive_before = unsafe { libc::kill(grandchild_pid, 0) }; + assert_eq!(alive_before, 0, "grandchild should be alive before kill"); + + host.kill(&handle, Duration::from_secs(2)).await.unwrap(); + tokio::time::sleep(Duration::from_millis(200)).await; + + let alive_after = unsafe { libc::kill(grandchild_pid, 0) }; + assert_ne!( + alive_after, 0, + "grandchild PID {grandchild_pid} should be dead after group kill" + ); + } + + #[tokio::test(flavor = "current_thread")] + async fn given_killed_agent_should_reap_setsid_escaped_descendants() { + // Simulates an npm dev server: a descendant that called setsid() and is + // therefore in its own process group, escaping killpg. The tree-walk kill + // should still find and terminate it by PID. + let host = NativePtyHost::new(); + let tmp = tempfile::NamedTempFile::new().unwrap(); + let path = tmp.path().to_string_lossy().into_owned(); + + // Shell that backgrounds a perl process which immediately calls setsid() + // (escaping the agent's process group), records its PID, then sleeps. + // The shell stays alive via `wait` so perl remains its child in the ppid + // tree long enough for collect_process_descendants to find it. + let handle = host + .spawn(SpawnConfig { + command: "/bin/sh".into(), + args: vec![ + "-c".into(), + format!( + r#"perl -MPOSIX -e 'POSIX::setsid(); open my $f, q(>), q({path}); print $f $$; close $f; sleep 300' & wait"#, + path = path + ), + ], + cwd: "/tmp".into(), + env: vec![], + env_remove: vec![], + cols: 80, + rows: 24, + }) + .await + .unwrap(); + + let mut escaped_pid: i32 = 0; + for _ in 0..50 { + tokio::time::sleep(Duration::from_millis(50)).await; + if let Ok(s) = std::fs::read_to_string(&path) + && let Ok(pid) = s.trim().parse::() + && pid > 0 + { + escaped_pid = pid; + break; + } + } + assert!(escaped_pid > 0, "escaped child PID was not recorded"); + + // Confirm it escaped to its own process group. + let escaped_pgid = unsafe { libc::getpgid(escaped_pid) }; + assert_ne!( + escaped_pgid, handle.pid as libc::pid_t, + "escaped child should be in a different process group than the agent" + ); + + let alive_before = unsafe { libc::kill(escaped_pid, 0) }; + assert_eq!(alive_before, 0, "escaped child should be alive before kill"); + + host.kill(&handle, Duration::from_secs(2)).await.unwrap(); + tokio::time::sleep(Duration::from_millis(200)).await; + + let alive_after = unsafe { libc::kill(escaped_pid, 0) }; + assert_ne!( + alive_after, 0, + "escaped child PID {escaped_pid} should be dead after tree-walk kill" + ); + } } diff --git a/deny.toml b/deny.toml index afee6a5..7d7ade1 100644 --- a/deny.toml +++ b/deny.toml @@ -2,7 +2,11 @@ all-features = true [advisories] -ignore = [] +ignore = [ + # serde_yml is unsound (RUSTSEC-2025-0068) and unmaintained, but no safe + # upgrade exists. Tracked for replacement in a dedicated follow-up. + "RUSTSEC-2025-0068", +] [licenses] allow = [