Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions desktop/src-tauri/src/managed_agents/process_lifecycle.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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);
Expand Down
35 changes: 32 additions & 3 deletions desktop/src-tauri/src/managed_agents/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 65 additions & 0 deletions desktop/src-tauri/src/managed_agents/runtime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down