Skip to content

fix(engine): kill escaped child processes on worktree cleanup #157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
2witstudios merged 6 commits into from
May 24, 2026
Merged

fix(engine): kill escaped child processes on worktree cleanup #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
418bbaa
fix(engine): kill escaped child processes on worktree cleanup
2witstudios May 24, 2026
acc1bcf
style: rustfmt formatting fixes in pty_manager
2witstudios May 24, 2026
d4fe963
chore: fix pre-existing Dependency Audit CI failures
2witstudios May 24, 2026
b1762e0
refactor(engine): share ps snapshot across all kill paths
2witstudios May 24, 2026
a875d30
fix(engine): guard descendant SIGKILL with liveness check to avoid PI…
2witstudios May 24, 2026
9b1d03a
fix(deps): bump neon to latest main for Swift 6.1 concurrency compati…
2witstudios May 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down
2 changes: 1 addition & 1 deletion ...macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 68 additions & 3 deletions crates/pu-engine/src/engine/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<AgentHandle> = {
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<i32> = 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);
}
Comment on lines +237 to +241

@chatgpt-codex-connector chatgpt-codex-connector Bot May 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reap escaped descendants in managed shutdown path

main.rs now calls Engine::kill_all_sessions() when the app parent dies, but this helper only sends killpg to each session PID. Descendants that called setsid()/setpgid() (the exact escaped-child case handled in NativePtyHost::kill) are outside that process group, so they survive this shutdown path and remain orphaned after process::exit(0). This means force-quitting the macOS app can still leak detached node/dev-server processes.

Useful? React with 👍 / 👎.

@2witstudios 2witstudios May 24, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit b1762e0. kill_all_sessions now takes a shared ps snapshot via snapshot_process_tree(), collects descendants for all handles with descendants_from_tree(), and sends SIGTERM then SIGKILL to both process groups (killpg) and escaped descendants. The single snapshot is shared across all handles to avoid multiple ps invocations.

}
}
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 {
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
38 changes: 23 additions & 15 deletions crates/pu-engine/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
Expand Down
Loading
Loading