diff --git a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs index 8dddf9f715..ee1d021d22 100644 --- a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs +++ b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs @@ -1,6 +1,7 @@ //! Windows process-tree lifecycle primitives for managed agents. //! -//! The Unix teardown uses `process_group(0)` + group signals (in `runtime.rs`). +//! The Unix teardown uses `setsid()` (own session + process group) + group +//! signals (in `runtime.rs`). //! Windows has no process groups, so the harness's 24 agent workers + MCP //! servers are reaped two ways here: //! - [`JobHandle`] / [`create_job_for_child`] — the in-process stop path. A @@ -17,7 +18,7 @@ use windows_sys::Win32::Foundation::HANDLE; /// Win32 Job Object that owns the harness process and (via Windows' default /// child-inheritance) every process it spawns. Dropping the handle with /// `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` set kills the whole tree — the Windows -/// mirror of the Unix `process_group(0)` + group-signal teardown. This is what +/// mirror of the Unix `setsid()` + group-signal teardown. This is what /// guarantees the 24 agent workers + MCP servers die when we stop or when the /// app exits, instead of being orphaned by a bare `Child::kill()`. pub struct JobHandle(HANDLE); diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 6687cbbcf2..10b8f1c173 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -2011,12 +2011,41 @@ pub fn spawn_agent_child( .env("BUZZ_MANAGED_AGENT", current_instance_id(app)) .env("BUZZ_MANAGED_AGENT_START_NONCE", &start_nonce); - // Spawn the harness in its own process group so we can kill the entire - // tree (harness + MCP servers + agent subprocesses) on shutdown. + // Spawn the harness in its OWN SESSION (setsid) so the entire agent tree + // (harness + adapter + engine + MCP servers) has no controlling terminal. + // + // WHY setsid and not process_group(0): when the desktop app is launched + // from a terminal (`just dev`), the harness subtree inherits that + // terminal's controlling TTY. As background process groups, macOS job + // control intermittently STOPS the actively-working agent's group + // (SIGTTIN/SIGTTOU-class stops, seen as `T` state in ps), freezing agent + // turns indefinitely. setsid() makes the child a new session leader with + // NO controlling terminal, so /dev/tty access can never stop the tree. + // Packaged .app launches have no controlling TTY and were unaffected. + // + // setsid() also makes the child both session AND process-group leader + // (pid == pgid), so the group-kill paths in `sigterm_then_sigkill` + // (which signal `-pid`) still reach the whole tree — the same invariant + // process_group(0) provided. Note we do NOT set process_group(0) here: + // std may apply setpgid() before pre_exec runs, which would make the child + // a group leader and cause setsid() to fail with EPERM. #[cfg(unix)] { use std::os::unix::process::CommandExt; - command.process_group(0); + // SAFETY: setsid/setpgid are async-signal-safe and touch only the + // child's own session/group. This file already relies on raw libc + // process control (see `sigterm_then_sigkill`'s `libc::kill`). + unsafe { + command.pre_exec(|| { + if libc::setsid() == -1 { + // EPERM: already a group leader. Fall back to a best-effort + // new process group so group-kill still works; do not fail + // the spawn over it. + let _ = libc::setpgid(0, 0); + } + Ok(()) + }); + } } // Windows: suppress the harness console window. Without this a bare // terminal pops for buzz-acp.exe and lingers (the app itself sets diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index fd758047c3..3ed74cfc84 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -857,6 +857,71 @@ fn own_group_grandchild_detected_by_ancestor_walk() { let _ = intermediate.wait(); } +/// Validates the TTY-detach fix: a child spawned with the same `pre_exec` +/// `setsid()` the harness uses becomes its own SESSION leader (sid == pid) in +/// a session distinct from the test process. This is what frees the agent tree +/// from the launching terminal's controlling TTY so background-group job +/// control (SIGTTIN/SIGTTOU stops) can no longer freeze agent turns. +/// +/// Note: this exercises the setsid pattern on a stand-in child, not +/// `spawn_agent_child` itself (which needs an `AppHandle`). It will NOT catch +/// a regression that re-adds `process_group(0)` to the harness `Command` — +/// that would make `setsid()` fail EPERM at spawn time and silently reattach +/// the tree to the launching terminal's session. Keep the spawn free of +/// `process_group` calls. +#[cfg(unix)] +#[test] +fn spawned_harness_is_session_leader_in_new_session() { + use std::os::unix::process::CommandExt; + use std::process::Command; + + let parent_sid = unsafe { libc::getsid(0) }; + + let mut child = { + let mut cmd = Command::new("sleep"); + cmd.arg("30").stdin(std::process::Stdio::null()); + // Mirror the production spawn: detach into a new session with no + // controlling terminal. + unsafe { + cmd.pre_exec(|| { + if libc::setsid() == -1 { + let _ = libc::setpgid(0, 0); + } + Ok(()) + }); + } + cmd.spawn().expect("spawn detached sleep") + }; + + let child_pid = child.id() as i32; + + // The child is a session leader: its session id equals its own pid. + let child_sid = unsafe { libc::getsid(child_pid) }; + assert_eq!( + child_sid, child_pid, + "child should be its own session leader (sid == pid) after setsid()" + ); + + // And it is a DIFFERENT session than the test process — no shared + // controlling terminal. + assert_ne!( + child_sid, parent_sid, + "child session must differ from the test process session" + ); + + // setsid also makes the child a process-group leader (pid == pgid), so the + // production group-kill path (`kill(-pid, ...)`) still reaches the tree. + let child_pgid = unsafe { libc::getpgid(child_pid) }; + assert_eq!( + child_pgid, child_pid, + "child should be its own process-group leader (pgid == pid) after setsid()" + ); + + // Cleanup: kill the child's process group. + unsafe { libc::kill(-child_pid, libc::SIGKILL) }; + let _ = child.wait(); +} + // ── pair receipt validation tests ─────────────────────────────────────── fn receipt_fixture(