fix(desktop): check PGID in orphan sweep and signal correct process groups (#1359)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>

desktop/scripts/check-file-sizes.mjs

@@ -72,7 +72,8 @@ const overrides = new Map([
   // path here. Load-bearing feature growth; queued to split in the resolver
   // unify refactor followup. +26 for resolve_effective_prompt_model_provider
   // re-introduced after 826d735fe removal (config-bridge caller still needs it).
-  ["src-tauri/src/managed_agents/runtime.rs", 2036],
+  // PGID resolution helper + PID-recycling safety guard added for orphan sweep.
+  ["src-tauri/src/managed_agents/runtime.rs", 2150],
   // Phase-2 inbound reconcile + review-fix cycle: reconcile_inbound_persona_event
   // dispatches 30175/30176/30177 inbound plus kind:5 tombstone consume
   // (reconcile_inbound_tombstone), the two apply_inbound_* fns, the

desktop/src-tauri/src/managed_agents/runtime.rs

@@ -355,9 +355,71 @@ fn sigterm_then_sigkill(pids: &[i32]) {
     }
 }
 
+/// Resolve orphan candidate PIDs to their actual process group IDs, dedupe,
+/// and signal the groups. An orphaned grandchild (e.g. `goose` or `buzz-dev-mcp`)
+/// whose harness has exited retains the harness's PGID — signaling that PGID
+/// kills the entire orphaned subtree. Falls back to the candidate PID itself
+/// when PGID resolution fails (process may have exited between detection and
+/// kill).
+#[cfg(target_os = "macos")]
+fn resolve_pgids_and_kill(candidate_pids: &[i32]) {
+    let candidate_set: std::collections::HashSet<i32> = candidate_pids.iter().copied().collect();
+    let mut pgids = std::collections::HashSet::new();
+    for &pid in candidate_pids {
+        let pgid = unsafe { libc::getpgid(pid) };
+        if pgid > 0 {
+            pgids.insert(pgid);
+        } else {
+            // Process may have exited; try signaling it directly as a group.
+            pgids.insert(pid);
+        }
+    }
+    // PID-recycling guard: if a resolved PGID is alive but isn't one of our
+    // orphan candidates, the old harness PID was recycled by a new process
+    // that called setsid() — skip it to avoid killing an unrelated group.
+    pgids.retain(|&pgid| {
+        if candidate_set.contains(&pgid) {
+            return true;
+        }
+        let alive = unsafe { libc::kill(pgid, 0) } == 0;
+        !alive
+    });
+    let unique: Vec<i32> = pgids.into_iter().collect();
+    sigterm_then_sigkill(&unique);
+}
+
+/// Resolve orphan candidate PIDs to their actual process group IDs, dedupe,
+/// and signal the groups. Linux variant reads PGID from /proc/<pid>/stat.
+#[cfg(all(unix, not(target_os = "macos")))]
+fn resolve_pgids_and_kill(candidate_pids: &[i32]) {
+    let candidate_set: std::collections::HashSet<i32> = candidate_pids.iter().copied().collect();
+    let mut pgids = std::collections::HashSet::new();
+    for &pid in candidate_pids {
+        if let Some(pgid) = read_pgid_linux(pid as u32) {
+            pgids.insert(pgid as i32);
+        } else {
+            // Process may have exited; try signaling it directly as a group.
+            pgids.insert(pid);
+        }
+    }
+    // PID-recycling guard: if a resolved PGID is alive but isn't one of our
+    // orphan candidates, the old harness PID was recycled by a new process
+    // that called setsid() — skip it to avoid killing an unrelated group.
+    pgids.retain(|&pgid| {
+        if candidate_set.contains(&pgid) {
+            return true;
+        }
+        let alive = unsafe { libc::kill(pgid, 0) } == 0;
+        !alive
+    });
+    let unique: Vec<i32> = pgids.into_iter().collect();
+    sigterm_then_sigkill(&unique);
+}
+
 /// Kill orphaned agent processes using PID file receipts. Reads all files from
 /// `agent-pids/`, verifies each PID still belongs to a known agent binary,
-/// then kills the process group. Deletes the PID file after killing.
+/// then resolves each candidate's actual PGID and signals the process group.
+/// Deletes the PID file after killing.
 ///
 /// `skip_pids` are PIDs already handled by the tracked-agent path.
 #[cfg(unix)]
@@ -379,7 +441,7 @@ pub(crate) fn sweep_orphaned_agent_processes(app: &AppHandle, skip_pids: &[u32])
         .collect();
 
     if !targets.is_empty() {
-        sigterm_then_sigkill(&targets);
+        resolve_pgids_and_kill(&targets);
     }
 
     // Clean up PID files for processes we just killed or that are already gone.
@@ -503,6 +565,13 @@ pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32])
         if skip_pids.contains(&info.pbi_ppid) {
             continue;
         }
+        // Grandchild check: the harness is spawned with process_group(0), so
+        // all descendants share its PGID. If this process's PGID matches a
+        // tracked harness PID, it's a live descendant — not an orphan.
+        let pgid = unsafe { libc::getpgid(pid) };
+        if pgid > 0 && skip_pids.contains(&(pgid as u32)) {
+            continue;
+        }
         if !process_has_buzz_marker(upid, instance_id) {
             continue;
         }
@@ -514,7 +583,7 @@ pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32])
             "buzz-desktop: system sweep found {} orphaned agent process(es), cleaning up",
             orphans.len()
         );
-        sigterm_then_sigkill(&orphans);
+        resolve_pgids_and_kill(&orphans);
     }
 }
 
@@ -530,6 +599,17 @@ fn read_ppid_linux(pid: u32) -> Option<u32> {
     ppid_str.parse::<u32>().ok()
 }
 
+/// Read the process group ID from /proc/<pid>/stat. Same parsing strategy as
+/// `read_ppid_linux` — field 3 after the closing ')' is the PGID.
+#[cfg(all(unix, not(target_os = "macos")))]
+fn read_pgid_linux(pid: u32) -> Option<u32> {
+    let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?;
+    let after_comm = stat.rsplit_once(')')?.1;
+    // Fields after ')': " S ppid pgid ..."
+    let pgid_str = after_comm.split_whitespace().nth(2)?;
+    pgid_str.parse::<u32>().ok()
+}
+
 #[cfg(all(unix, not(target_os = "macos")))]
 pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32]) {
     let my_uid = unsafe { libc::getuid() };
@@ -575,6 +655,14 @@ pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32])
                 continue;
             }
         }
+        // Grandchild check: the harness is spawned with process_group(0), so
+        // all descendants share its PGID. If this process's PGID matches a
+        // tracked harness PID, it's a live descendant — not an orphan.
+        if let Some(pgid) = read_pgid_linux(upid) {
+            if skip_pids.contains(&pgid) {
+                continue;
+            }
+        }
         orphans.push(pid);
     }
 
@@ -583,7 +671,7 @@ pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32])
             "buzz-desktop: system sweep found {} orphaned agent process(es), cleaning up",
             orphans.len()
         );
-        sigterm_then_sigkill(&orphans);
+        resolve_pgids_and_kill(&orphans);
     }
 }
 
@@ -613,7 +701,7 @@ pub(crate) fn sweep_system_agent_processes_with_grace(
             "buzz-desktop: periodic sweep confirmed {} orphaned agent process(es), cleaning up",
             confirmed.len()
         );
-        sigterm_then_sigkill(&confirmed);
+        resolve_pgids_and_kill(&confirmed);
     }
     current
 }
@@ -694,6 +782,13 @@ pub(crate) fn collect_same_instance_orphans(
         if skip_pids.contains(&info.pbi_ppid) {
             continue;
         }
+        // Grandchild check: the harness is spawned with process_group(0), so
+        // all descendants share its PGID. If this process's PGID matches a
+        // tracked harness PID, it's a live descendant — not an orphan.
+        let pgid = unsafe { libc::getpgid(pid) };
+        if pgid > 0 && skip_pids.contains(&(pgid as u32)) {
+            continue;
+        }
         if process_has_buzz_marker(upid, instance_id) {
             orphans.insert(upid);
         }
@@ -747,6 +842,14 @@ pub(crate) fn collect_same_instance_orphans(
                 continue;
             }
         }
+        // Grandchild check: the harness is spawned with process_group(0), so
+        // all descendants share its PGID. If this process's PGID matches a
+        // tracked harness PID, it's a live descendant — not an orphan.
+        if let Some(pgid) = read_pgid_linux(upid) {
+            if skip_pids.contains(&pgid) {
+                continue;
+            }
+        }
         orphans.insert(upid);
     }
     orphans
@@ -1141,7 +1244,7 @@ pub(crate) fn reap_dead_instance_agents(our_instance_id: &str, skip_pids: &[u32]
             "buzz-desktop: reaping {} orphaned agent(s) from dead instance '{instance_id}'",
             agent_pids.len()
         );
-        sigterm_then_sigkill(agent_pids);
+        resolve_pgids_and_kill(agent_pids);
     }
 }
 
@@ -1199,7 +1302,7 @@ pub(crate) fn reap_dead_instance_agents(our_instance_id: &str, skip_pids: &[u32]
             "buzz-desktop: reaping {} orphaned agent(s) from dead instance '{instance_id}'",
             agent_pids.len()
         );
-        sigterm_then_sigkill(agent_pids);
+        resolve_pgids_and_kill(agent_pids);
     }
 }
 

desktop/src-tauri/src/managed_agents/runtime/tests.rs

@@ -512,3 +512,97 @@ fn name_matches_interpreter_rejects_node_prefix() {
     assert!(!super::name_matches_interpreter("nodejs"));
     assert!(!super::name_matches_interpreter("node-gyp"));
 }
+
+// ── PGID-based orphan sweep tests ───────────────────────────────────────
+
+/// Validates the kernel invariant that the orphan sweep PGID fix relies on:
+/// a grandchild process inherits the PGID of the process group leader (the
+/// harness), so checking PGID membership in `skip_pids` correctly identifies
+/// live descendants even when their ppid is an intermediate process (e.g.
+/// goose) rather than the harness itself.
+#[cfg(unix)]
+#[test]
+fn grandchild_inherits_pgid_of_process_group_leader() {
+    use std::os::unix::process::CommandExt;
+    use std::process::Command;
+
+    // Spawn a "harness" process in its own process group (mirrors
+    // `command.process_group(0)` in the real spawn path). The harness
+    // spawns an intermediate child which in turn spawns a grandchild.
+    // This mirrors the real tree: buzz-acp → goose → buzz-dev-mcp.
+    //
+    // The intermediate `sh` uses exec to replace itself with another sh
+    // that backgrounds the grandchild, so the grandchild's ppid is the
+    // intermediate (not the harness).
+    let mut harness = {
+        let mut cmd = Command::new("sh");
+        cmd.args(["-c", "sh -c 'sleep 10 & echo $!' & wait $!"])
+            .stdout(std::process::Stdio::piped())
+            .process_group(0);
+        cmd.spawn().expect("spawn harness")
+    };
+
+    // Read the grandchild PID from stdout.
+    use std::io::BufRead;
+    let stdout = harness.stdout.take().unwrap();
+    let reader = std::io::BufReader::new(stdout);
+    let grandchild_pid: i32 = reader
+        .lines()
+        .next()
+        .expect("should get a line")
+        .expect("should read line")
+        .trim()
+        .parse()
+        .expect("should parse grandchild PID");
+
+    let harness_pid = harness.id() as i32;
+
+    // The harness is the process group leader (PGID == its own PID).
+    let harness_pgid = unsafe { libc::getpgid(harness_pid) };
+    assert_eq!(
+        harness_pgid, harness_pid,
+        "harness should be its own process group leader"
+    );
+
+    // The grandchild's PGID should equal the harness PID — this is the
+    // invariant our orphan sweep fix relies on.
+    let grandchild_pgid = unsafe { libc::getpgid(grandchild_pid) };
+    assert_eq!(
+        grandchild_pgid, harness_pid,
+        "grandchild PGID should match harness PID (process group leader)"
+    );
+
+    // The grandchild's ppid is NOT the harness — it's the intermediate sh.
+    // This proves the ppid-only check would miss it (the regression path).
+    #[cfg(target_os = "macos")]
+    {
+        let mut info = std::mem::MaybeUninit::<super::BSDInfo>::zeroed();
+        let ret = unsafe {
+            super::proc_pidinfo(
+                grandchild_pid,
+                super::PROC_PIDTBSDINFO,
+                0,
+                info.as_mut_ptr() as *mut libc::c_void,
+                std::mem::size_of::<super::BSDInfo>() as libc::c_int,
+            )
+        };
+        assert!(ret > 0, "proc_pidinfo should succeed for grandchild");
+        let info = unsafe { info.assume_init() };
+        assert_ne!(
+            info.pbi_ppid as i32, harness_pid,
+            "grandchild ppid must NOT be the harness (it's the intermediate sh)"
+        );
+    }
+
+    // With skip_pids containing the harness PID, the grandchild's PGID
+    // is in skip_pids — so it would NOT be flagged as an orphan.
+    let skip_pids: Vec<u32> = vec![harness_pid as u32];
+    assert!(
+        skip_pids.contains(&(grandchild_pgid as u32)),
+        "grandchild's PGID should be found in skip_pids"
+    );
+
+    // Cleanup: kill the process group.
+    unsafe { libc::kill(-harness_pid, libc::SIGTERM) };
+    let _ = harness.wait();
+}