fix(engine): kill escaped child processes on worktree cleanup by 2witstudios · Pull Request #157 · 2witstudios/PurePoint

2witstudios · 2026-05-24T13:40:20Z

Summary

Root cause: `killpg` only kills processes sharing the agent's PGID. npm dev servers (webpack, vite, etc.) commonly call `setsid()` or spawn with `detached: true`, creating a new process group and escaping `killpg` entirely. pnpm was unaffected because it typically `exec`s into the script process instead of forking detached children.
Fix: before sending any signals, snapshot all descendant PIDs by parsing `ps -ax -o pid,ppid` and walking the PPID tree. These are killed by PID directly alongside the existing `killpg`, regardless of process group.
Also hooks `kill_all_sessions()` into the managed-mode parent-died path so a force-quit of the macOS app reaps agent subtrees before the daemon calls `process::exit(0)`.
Updates the Xcode build script to also copy `pu-engine` to `~/.pu/bin/`.
Updates `rand` 0.10.0 → 0.10.1 (fixes RUSTSEC-2026-0097) and adds advisory ignore for `serde_yml` RUSTSEC-2025-0068 (no safe upgrade; tracked for replacement separately) to fix pre-existing Dependency Audit CI failure.

Test plan

New test `given_killed_agent_should_reap_setsid_escaped_descendants` — spawns a perl process that calls `POSIX::setsid()` (verifying it has a different PGID), kills the agent, confirms the escaped process is dead
Existing `given_killed_agent_should_reap_grandchildren` still passes
All 118 `pu-engine` unit tests + 11 integration tests pass: `cargo test -p pu-engine`
`cargo clippy --all` clean (no warnings)
`cargo fmt --check` clean
`cargo deny check advisories` passes
Manually: start an npm dev server in a worktree agent, delete the worktree — confirm no stale `node` processes remain

🤖 Generated with Claude Code

npm dev servers and other Node.js subprocesses commonly call setsid() or are spawned with detached:true, creating their own process groups. This caused them to survive killpg() on worktree deletion, leaving stale processes consuming resources. Fix: before signalling, snapshot the full descendant tree via `ps -ax -o pid,ppid` and walk PPIDs from the agent's root. These PIDs are killed directly alongside the existing killpg call, catching any process that escaped to a different process group. Covers both SIGTERM and SIGKILL phases. pnpm was unaffected because it typically execs into the script process rather than forking detached children. Also hooks kill_all_sessions() into the managed-mode parent-died path so a macOS app force-quit reaps agent subtrees before daemon exit, and updates the Xcode build script to copy pu-engine to ~/.pu/bin/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

coderabbitai · 2026-05-24T13:40:54Z

Warning

Review limit reached

@2witstudios, we couldn't start this review because you've used your available PR reviews for now.

Your plan currently allows 1 review/hour. Refill in 27 minutes and 52 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ea9bfcd2-8d5b-4c5e-ac79-b76ab50b260e

📥 Commits

Reviewing files that changed from the base of the PR and between 135459a and 9b1d03a.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj
apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
crates/pu-engine/src/engine/mod.rs
crates/pu-engine/src/main.rs
crates/pu-engine/src/pty_manager.rs
deny.toml

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch pu/fix-stale-processes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 418bbaa614

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-24T13:43:34Z

+        for handle in &handles {
+            if let Ok(pid) = i32::try_from(handle.pid) {
+                unsafe {
+                    libc::killpg(pid, libc::SIGTERM);
+                }


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 👍 / 👎.

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.

chatgpt-codex-connector · 2026-05-24T13:43:34Z

+        for &desc in &descendants {
+            unsafe {
+                libc::kill(desc, libc::SIGKILL);
+            }


Recompute descendant PIDs before forced SIGKILL

The descendant PID list is snapshotted once (collect_process_descendants) and then reused after the grace wait for the forced-kill pass. If any descendant exits during that window, its PID can be reused by an unrelated process, and the later libc::kill(desc, SIGKILL) may terminate the wrong process. This risk appears only under PID reuse races, but when it happens it can kill unrelated user workloads.

Useful? React with 👍 / 👎.

Fixed in commit a875d30. Both the kill() method in pty_manager.rs and kill_all_sessions in engine/mod.rs now gate each descendant SIGKILL on kill(desc, 0) == 0. A non-zero result means the PID no longer refers to the original process (exited or recycled), so we skip the SIGKILL. The descendant snapshot is still taken before SIGTERM — that's intentional to capture the full tree while processes are alive — but the per-PID liveness check before each SIGKILL prevents hitting a recycled PID.

- Update rand 0.10.0 → 0.10.1 to resolve RUSTSEC-2026-0097 (unsound with custom logger using rand::rng(); only triggered under very specific conditions but flagged by cargo-deny) - Add deny.toml ignore for RUSTSEC-2025-0068 (serde_yml unsound/ unmaintained, no safe upgrade exists — tracked for replacement separately) Both advisories were already failing on main before this PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Split collect_process_descendants into snapshot_process_tree (one ps call) + descendants_from_tree (pure tree walk). kill_all_sessions now takes a single snapshot and kills escaped descendants for all handles in one pass — consistent with the worktree-cleanup kill path and efficient regardless of how many agents are running. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…D reuse Before force-killing escaped descendants, verify the PID still refers to the original process via kill(pid, 0). If the PID was recycled during the grace period the liveness check fails and we skip the SIGKILL, preventing accidental kills of unrelated processes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…bility Xcode 26 (runner image 20260520) uses a newer Swift compiler that enforces strict concurrency more aggressively, causing TreeSitterClient to error with "sending 'result' risks causing data races". The latest neon main commit (484d6fb) uses nonisolated(unsafe) to address these Swift 6.1 stricter checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

style: rustfmt formatting fixes in pty_manager

acc1bcf

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

chatgpt-codex-connector Bot reviewed May 24, 2026

View reviewed changes

2witstudios and others added 4 commits May 24, 2026 08:45

2witstudios merged commit 5fa3067 into main May 24, 2026
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

fix(engine): kill escaped child processes on worktree cleanup#157
2witstudios merged 6 commits into
mainfrom
pu/fix-stale-processes

2witstudios commented May 24, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 24, 2026 •

edited

Loading

Review limit reached

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 24, 2026

Uh oh!

2witstudios May 24, 2026

Uh oh!

chatgpt-codex-connector Bot May 24, 2026

Uh oh!

2witstudios May 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

2witstudios commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

coderabbitai Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

2witstudios May 24, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

2witstudios May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

2witstudios commented May 24, 2026 •

edited

Loading

coderabbitai Bot commented May 24, 2026 •

edited

Loading