diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 9fa46e5ce3..b6ecbd7aa8 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2764,6 +2764,19 @@ fn handle_prompt_result( PromptOutcome::Cancelled => "cancelled", }; let agent_index = result.agent.index; + // Capture the spawn-time configured model and our PID before the agent is + // moved into match arms below. `desired_model` reflects the config/persona + // model at spawn time — it does NOT reflect `session/set_model` overrides, + // which live in buzz-agent's session state and are what `llm: (model) …` + // errors carry. The two can legitimately differ; `configured_model=` is + // still valuable for identifying a stale orphan running an old model. + let harness_configured_model = result + .agent + .desired_model + .as_deref() + .unwrap_or("") + .to_string(); + let harness_pid = std::process::id(); let channel_id = match &result.source { PromptSource::Channel(ch) => Some(*ch), @@ -2798,6 +2811,8 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, + configured_model = %harness_configured_model, + pid = harness_pid, "agent_returned — respawning" ); let death_message = match outcome_label { @@ -2842,6 +2857,8 @@ fn handle_prompt_result( tracing::debug!( agent = agent_index, outcome = outcome_label, + configured_model = %harness_configured_model, + pid = harness_pid, "agent_returned (cancelled)" ); pool.return_agent(result.agent); @@ -2858,6 +2875,8 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, + configured_model = %harness_configured_model, + pid = harness_pid, error = %e, "transport/protocol error — respawning agent" ); @@ -2882,6 +2901,8 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, + configured_model = %harness_configured_model, + pid = harness_pid, error = %e, "agent_returned (application error — pipe intact)" ); diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 76b39e8fe3..07c2071aa2 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -72,7 +72,7 @@ impl Llm { effective_model: &str, ) -> Result { let effort = cfg.thinking_effort; - match cfg.provider { + let result = match cfg.provider { Provider::Anthropic => { let v = self .post_anthropic( @@ -140,7 +140,17 @@ impl Llm { }) .await } - } + }; + // Stamp the effective model into Llm errors so log lines carry + // `llm: (model-name) 404 Not Found: …` instead of the bare status. + // The `llm: ` prefix comes from `Display for AgentError::Llm`; the + // map_err here prepends `(model-name) ` to the inner string only. + // This is the single place all provider paths converge, so the mapping + // is centralized and never needs to be repeated in each provider arm. + result.map_err(|e| match e { + AgentError::Llm(s) => AgentError::Llm(format!("({effective_model}) {s}")), + other => other, + }) } pub async fn summarize( diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 85f5391128..a90ecb1dd9 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -132,6 +132,17 @@ pub async fn restore_managed_agents_on_launch( // whose desktop process is no longer running and reap them. super::reap_dead_instance_agents(&super::current_instance_id(app), &tracked_pids); + // Exact-path sweep: kill any buzz-acp process whose executable path + // matches this bundle's harness binary but is not in the tracked set. + // Complements the env-var sweep above — catches orphans that predate + // BUZZ_MANAGED_AGENT injection or lost their PID-file receipt. + // + // TODO: the three sweeps above each walk the PID table independently. + // A future consolidation should collect a single shared process snapshot + // at the top of this block and thread it through all sweep functions, + // replacing the three separate kernel enumerations. + super::sweep_untracked_bundle_harnesses(&tracked_pids); + let candidates: Vec = records .iter() .filter(|record| record.start_on_app_launch && record.backend == BackendKind::Local) diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index dbf3bc2301..78d9e9e603 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -16,6 +16,10 @@ use crate::{ mod path; use path::build_augmented_path; +mod sweep; +pub(crate) use sweep::sweep_untracked_bundle_harnesses; +pub use sweep::{expected_harness_exe_path, select_untracked_bundle_harnesses, ProcessSnapshot}; + type RespondToEnv = (Vec<(&'static str, String)>, Vec<&'static str>); /// Binary name fragments for all known agent/harness processes that Buzz @@ -169,40 +173,9 @@ fn buzz_marker_entry(instance_id: &str) -> Vec { #[cfg(target_os = "macos")] fn process_has_buzz_marker(pid: u32, instance_id: &str) -> bool { let marker = buzz_marker_entry(instance_id); - - let mut mib: [libc::c_int; 3] = [libc::CTL_KERN, libc::KERN_PROCARGS2, pid as libc::c_int]; - let mut buf_size: libc::size_t = 0; - - // First call: get required buffer size. - if unsafe { - libc::sysctl( - mib.as_mut_ptr(), - 3, - std::ptr::null_mut(), - &mut buf_size, - std::ptr::null_mut(), - 0, - ) - } != 0 - { + let Some(buf) = sweep::procargs2_buffer(pid) else { return false; - } - - let mut buf: Vec = vec![0; buf_size]; - if unsafe { - libc::sysctl( - mib.as_mut_ptr(), - 3, - buf.as_mut_ptr() as *mut libc::c_void, - &mut buf_size, - std::ptr::null_mut(), - 0, - ) - } != 0 - { - return false; - } - buf.truncate(buf_size); + }; // Buffer layout: [i32 argc][exec_path\0][null padding][argv\0...][env\0...] if buf.len() < std::mem::size_of::() { @@ -463,11 +436,16 @@ pub(crate) fn sweep_orphaned_agent_processes(app: &AppHandle, _skip_pids: &[u32] } // ── macOS process-info FFI (shared by all sweep/reap functions) ────────── +// +// `proc_listallpids` lives in `sweep.rs` (which owns `collect_all_pids`). +// All callers in this file reach it through `sweep::collect_all_pids()`. +// `proc_pidinfo` and `BSDInfo` are declared here as `pub(super)` so that +// `sweep.rs` can call `super::proc_pidinfo` / use `super::BSDInfo` without +// redefining the struct layout in two places. #[cfg(target_os = "macos")] extern "C" { - fn proc_listallpids(buffer: *mut libc::c_int, buffersize: libc::c_int) -> libc::c_int; - fn proc_pidinfo( + pub(super) fn proc_pidinfo( pid: libc::c_int, flavor: libc::c_int, arg: u64, @@ -480,11 +458,11 @@ extern "C" { /// against the macOS SDK — total size 136 bytes. #[cfg(target_os = "macos")] #[repr(C)] -struct BSDInfo { +pub(super) struct BSDInfo { _flags_status_xstatus: [u8; 12], // pbi_flags + pbi_status + pbi_xstatus - pbi_pid: u32, // offset 12 - pbi_ppid: u32, // offset 16 - pbi_uid: u32, // offset 20 + pub(super) pbi_pid: u32, // offset 12 + pub(super) pbi_ppid: u32, // offset 16 + pub(super) pbi_uid: u32, // offset 20 _rest: [u8; 112], } @@ -492,7 +470,7 @@ struct BSDInfo { const _: () = assert!(std::mem::size_of::() == 136); #[cfg(target_os = "macos")] -const PROC_PIDTBSDINFO: libc::c_int = 3; +pub(super) const PROC_PIDTBSDINFO: libc::c_int = 3; /// Enumerate all processes on the system owned by the current user and kill any /// agent binary stamped with *this* instance's `BUZZ_MANAGED_AGENT` marker @@ -503,33 +481,10 @@ const PROC_PIDTBSDINFO: libc::c_int = 3; #[cfg(target_os = "macos")] pub(crate) fn sweep_system_agent_processes(instance_id: &str, skip_pids: &[u32]) { let my_uid = unsafe { libc::getuid() }; - - // Loop until the buffer is large enough to hold all PIDs. Under a fork - // storm the process table can outgrow the initial estimate between the - // probe and the fill call. - let mut pids: Vec; - loop { - let count = unsafe { proc_listallpids(std::ptr::null_mut(), 0) }; - if count <= 0 { - return; - } - let buf_len = (count as usize) * 2; - pids = vec![0; buf_len]; - let actual = unsafe { - proc_listallpids( - pids.as_mut_ptr(), - (buf_len * std::mem::size_of::()) as libc::c_int, - ) - }; - if actual <= 0 { - return; - } - pids.truncate(actual as usize); - if (actual as usize) < buf_len { - break; - } + let pids = sweep::collect_all_pids(); + if pids.is_empty() { + return; } - let my_pid = std::process::id() as i32; let mut orphans: Vec = Vec::new(); @@ -729,27 +684,9 @@ pub(crate) fn collect_same_instance_orphans( let my_pid = std::process::id() as i32; let mut orphans = std::collections::HashSet::new(); - let mut pids: Vec; - loop { - let count = unsafe { proc_listallpids(std::ptr::null_mut(), 0) }; - if count <= 0 { - return orphans; - } - let buf_len = (count as usize) * 2; - pids = vec![0; buf_len]; - let actual = unsafe { - proc_listallpids( - pids.as_mut_ptr(), - (buf_len * std::mem::size_of::()) as libc::c_int, - ) - }; - if actual <= 0 { - return orphans; - } - pids.truncate(actual as usize); - if (actual as usize) < buf_len { - break; - } + let pids = sweep::collect_all_pids(); + if pids.is_empty() { + return orphans; } for &pid in &pids { @@ -904,39 +841,7 @@ fn buffer_contains_identifier(buf: &[u8], id: &[u8]) -> bool { #[cfg(target_os = "macos")] fn extract_buzz_marker_value(pid: u32) -> Option { let prefix = b"BUZZ_MANAGED_AGENT="; - - let mut mib: [libc::c_int; 3] = [libc::CTL_KERN, libc::KERN_PROCARGS2, pid as libc::c_int]; - let mut buf_size: libc::size_t = 0; - - if unsafe { - libc::sysctl( - mib.as_mut_ptr(), - 3, - std::ptr::null_mut(), - &mut buf_size, - std::ptr::null_mut(), - 0, - ) - } != 0 - { - return None; - } - - let mut buf: Vec = vec![0; buf_size]; - if unsafe { - libc::sysctl( - mib.as_mut_ptr(), - 3, - buf.as_mut_ptr() as *mut libc::c_void, - &mut buf_size, - std::ptr::null_mut(), - 0, - ) - } != 0 - { - return None; - } - buf.truncate(buf_size); + let buf = sweep::procargs2_buffer(pid)?; if buf.len() < std::mem::size_of::() { return None; @@ -1009,27 +914,9 @@ fn desktop_is_alive_for_instance(instance_id: &str) -> bool { let my_uid = unsafe { libc::getuid() }; let identifier_bytes = instance_id.as_bytes(); - let mut pids: Vec; - loop { - let count = unsafe { proc_listallpids(std::ptr::null_mut(), 0) }; - if count <= 0 { - return false; - } - let buf_len = (count as usize) * 2; - pids = vec![0; buf_len]; - let actual = unsafe { - proc_listallpids( - pids.as_mut_ptr(), - (buf_len * std::mem::size_of::()) as libc::c_int, - ) - }; - if actual <= 0 { - return false; - } - pids.truncate(actual as usize); - if (actual as usize) < buf_len { - break; - } + let pids = sweep::collect_all_pids(); + if pids.is_empty() { + return false; } for &pid in &pids { @@ -1072,36 +959,9 @@ fn desktop_is_alive_for_instance(instance_id: &str) -> bool { } // Check if this desktop process's args/env contain the identifier. // The KERN_PROCARGS2 buffer holds argv + environ as null-delimited strings. - let mut mib: [libc::c_int; 3] = [libc::CTL_KERN, libc::KERN_PROCARGS2, pid]; - let mut buf_size: libc::size_t = 0; - if unsafe { - libc::sysctl( - mib.as_mut_ptr(), - 3, - std::ptr::null_mut(), - &mut buf_size, - std::ptr::null_mut(), - 0, - ) - } != 0 - { + let Some(args_buf) = sweep::procargs2_buffer(pid as u32) else { continue; - } - let mut args_buf: Vec = vec![0; buf_size]; - if unsafe { - libc::sysctl( - mib.as_mut_ptr(), - 3, - args_buf.as_mut_ptr() as *mut libc::c_void, - &mut buf_size, - std::ptr::null_mut(), - 0, - ) - } != 0 - { - continue; - } - args_buf.truncate(buf_size); + }; // Boundary-anchored search: the identifier in the config JSON is // followed by a non-identifier char (typically `"`). A raw substring // match would let `...app` match inside `...app.dev`. @@ -1168,27 +1028,9 @@ pub(crate) fn reap_dead_instance_agents(our_instance_id: &str, skip_pids: &[u32] let my_uid = unsafe { libc::getuid() }; let my_pid = std::process::id() as i32; - let mut pids: Vec; - loop { - let count = unsafe { proc_listallpids(std::ptr::null_mut(), 0) }; - if count <= 0 { - return; - } - let buf_len = (count as usize) * 2; - pids = vec![0; buf_len]; - let actual = unsafe { - proc_listallpids( - pids.as_mut_ptr(), - (buf_len * std::mem::size_of::()) as libc::c_int, - ) - }; - if actual <= 0 { - return; - } - pids.truncate(actual as usize); - if (actual as usize) < buf_len { - break; - } + let pids = sweep::collect_all_pids(); + if pids.is_empty() { + return; } // Collect (pid, instance_id) for all foreign agent processes. @@ -1311,6 +1153,8 @@ pub(crate) fn reap_dead_instance_agents(our_instance_id: &str, skip_pids: &[u32] #[cfg(not(unix))] pub(crate) fn reap_dead_instance_agents(_our_instance_id: &str, _skip_pids: &[u32]) {} +// Exact-path harness sweep lives in runtime/sweep.rs (re-exported above). + /// Kill stale agent processes from a previous session whose PID is still alive /// but not tracked in the current `runtimes` map. Updates the record fields and /// returns `true` if any records were modified. diff --git a/desktop/src-tauri/src/managed_agents/runtime/sweep.rs b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs new file mode 100644 index 0000000000..2e026ec5fc --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs @@ -0,0 +1,500 @@ +//! Boot-time sweep for untracked same-bundle harness processes. +//! +//! The env-var and PID-file sweeps cannot see a harness whose receipt is gone +//! or that predates `BUZZ_MANAGED_AGENT` injection. This sweep derives the +//! expected `buzz-acp` path from the running executable and kills any process +//! whose exe matches exactly, minus the tracked set. + +use std::path::{Path, PathBuf}; + +// Re-declare the macOS process-info FFI so sweep.rs can call it independently. +// Multiple extern "C" declarations of the same symbol are legal in Rust; the +// linker sees one symbol regardless of how many translation units declare it. +#[cfg(target_os = "macos")] +extern "C" { + fn proc_listallpids(buffer: *mut libc::c_int, buffersize: libc::c_int) -> libc::c_int; + fn proc_name(pid: libc::c_int, buffer: *mut libc::c_void, buffersize: u32) -> libc::c_int; +} + +// ── Shared low-level helpers ────────────────────────────────────────────── + +/// Collect all PIDs currently on the system. +/// +/// Loops until the buffer is large enough to hold all PIDs — under a fork +/// storm the count can grow between the probe and the fill call. Returns an +/// empty vec on any kernel error. +#[cfg(target_os = "macos")] +pub(super) fn collect_all_pids() -> Vec { + let mut pids: Vec; + loop { + let count = unsafe { proc_listallpids(std::ptr::null_mut(), 0) }; + if count <= 0 { + return Vec::new(); + } + let buf_len = (count as usize) * 2; + pids = vec![0; buf_len]; + let actual = unsafe { + proc_listallpids( + pids.as_mut_ptr(), + (buf_len * std::mem::size_of::()) as libc::c_int, + ) + }; + if actual <= 0 { + return Vec::new(); + } + pids.truncate(actual as usize); + if (actual as usize) < buf_len { + return pids; + } + } +} + +/// Read the raw `KERN_PROCARGS2` buffer for the given PID. +/// +/// Two-phase sysctl: first probe for the required buffer size, then fill. +/// A process's arguments are immutable after `execve`, so the size reported +/// by the probe is stable — no grow-and-retry loop is needed. +/// +/// Returns `None` if either sysctl call fails (e.g. the process has already +/// exited or we lack permission). +#[cfg(target_os = "macos")] +pub(super) fn procargs2_buffer(pid: u32) -> Option> { + let mut mib: [libc::c_int; 3] = [libc::CTL_KERN, libc::KERN_PROCARGS2, pid as libc::c_int]; + let mut buf_size: libc::size_t = 0; + + if unsafe { + libc::sysctl( + mib.as_mut_ptr(), + 3, + std::ptr::null_mut(), + &mut buf_size, + std::ptr::null_mut(), + 0, + ) + } != 0 + { + return None; + } + + let mut buf: Vec = vec![0; buf_size]; + if unsafe { + libc::sysctl( + mib.as_mut_ptr(), + 3, + buf.as_mut_ptr() as *mut libc::c_void, + &mut buf_size, + std::ptr::null_mut(), + 0, + ) + } != 0 + { + return None; + } + buf.truncate(buf_size); + Some(buf) +} + +// ── ProcessSnapshot and pure decision function ──────────────────────────── + +/// A snapshot of one process for the pure kill-decision function. Holds only +/// the fields needed to decide whether a process is an untracked same-bundle +/// harness — no live process handles, no system calls. +#[derive(Debug, Clone)] +pub struct ProcessSnapshot { + /// PID of the process. + pub pid: u32, + /// Full executable path, as reported by the kernel. + pub exe_path: PathBuf, +} + +/// Strip the kernel-appended `" (deleted)"` suffix from an executable path. +/// +/// On Linux, `read_link("/proc//exe")` returns `…/buzz-acp (deleted)` +/// when the on-disk binary has been replaced since the process launched. +/// That is exactly the class of stale-install orphan this sweep targets, so +/// we must strip the suffix before comparing against the expected path. +/// Falls back to the original path unchanged when the suffix is absent. +// Available on Linux (where it is called from collect_process_snapshots) and +// in test builds on all platforms (including macOS). Dead on macOS outside +// tests — suppress the warning there. +#[cfg(any(all(unix, not(target_os = "macos")), test))] +pub(super) fn strip_deleted_suffix(path: PathBuf) -> PathBuf { + const SUFFIX: &str = " (deleted)"; + match path.to_str() { + Some(s) if s.ends_with(SUFFIX) => PathBuf::from(&s[..s.len() - SUFFIX.len()]), + _ => path, + } +} + +/// Pure kill-decision function: given a slice of process snapshots, the +/// expected harness executable path, and the set of tracked pids to spare, +/// returns the pids of processes that should be reaped. +/// +/// Selection criteria: +/// - `exe_path` exactly matches `harness_exe` (same-bundle harness only). +/// - `pid` is not in `tracked_pids` (untracked — not owned by this session). +/// +/// Children of tracked parents die when their parent's process group is +/// signalled — this function deliberately targets only harness-level processes +/// so we never directly kill a child of a live tracked parent. +pub fn select_untracked_bundle_harnesses( + snapshots: &[ProcessSnapshot], + harness_exe: &Path, + tracked_pids: &[u32], +) -> Vec { + snapshots + .iter() + .filter(|s| s.exe_path == harness_exe && !tracked_pids.contains(&s.pid)) + .map(|s| s.pid) + .collect() +} + +// ── Process-table enumeration ───────────────────────────────────────────── + +/// Extract the executable path from a process's `KERN_PROCARGS2` buffer. +/// +/// The buffer layout is: `[i32 argc][exec_path\0][null-pad][argv\0…][env\0…]`. +/// The exec path is therefore the first null-terminated string immediately +/// after the leading `i32` — no argv traversal is needed, unlike +/// `extract_buzz_marker_value` / `process_has_buzz_marker` which must skip +/// past both argv and the exec path to reach the environment entries. +/// +/// Returns `None` if the buffer is unreadable or malformed. +#[cfg(target_os = "macos")] +fn proc_exe_path_from_procargs2(pid: u32) -> Option { + let buf = procargs2_buffer(pid)?; + if buf.len() < std::mem::size_of::() { + return None; + } + // Skip the argc i32 at the start of the buffer. + let pos = std::mem::size_of::(); + // The exec path immediately follows — scan to the first null byte. + let end = buf[pos..].iter().position(|&b| b == 0).map(|i| pos + i)?; + let path_bytes = &buf[pos..end]; + if path_bytes.is_empty() { + return None; + } + // KERN_PROCARGS2 exec paths are always absolute UTF-8 on macOS. + let s = std::str::from_utf8(path_bytes).ok()?; + Some(PathBuf::from(s)) +} + +/// Collect process snapshots for all user-owned processes on macOS. +/// +/// Applies a cheap `proc_name` pre-filter before the expensive +/// `KERN_PROCARGS2` sysctl: only PIDs whose binary name matches the harness +/// binary name (`buzz-acp`) proceed to the full exe-path fetch. This mirrors +/// the pattern `sweep_system_agent_processes` uses and cuts the expensive +/// two-sysctl call by ~99.9% (there are typically O(hundreds) of user +/// processes but at most a handful of `buzz-acp` instances). +#[cfg(target_os = "macos")] +fn collect_process_snapshots(harness_name: &str) -> Vec { + let my_uid = unsafe { libc::getuid() }; + let my_pid = std::process::id() as i32; + let mut snapshots = Vec::new(); + + let pids = collect_all_pids(); + + for &pid in &pids { + if pid <= 0 || pid == my_pid { + continue; + } + // Cheap name pre-filter: only proceed to the expensive KERN_PROCARGS2 + // sysctl for PIDs whose binary name matches the harness binary name. + let mut name_buf = [0u8; 1024]; + let len = unsafe { + proc_name( + pid, + name_buf.as_mut_ptr() as *mut libc::c_void, + name_buf.len() as u32, + ) + }; + if len <= 0 { + continue; + } + let name = String::from_utf8_lossy(&name_buf[..len as usize]); + if name != harness_name { + continue; + } + // Verify UID to avoid inspecting processes owned by other users. + let upid = pid as u32; + let mut info = std::mem::MaybeUninit::::zeroed(); + let ret = unsafe { + super::proc_pidinfo( + pid, + super::PROC_PIDTBSDINFO, + 0, + info.as_mut_ptr() as *mut libc::c_void, + std::mem::size_of::() as libc::c_int, + ) + }; + if ret <= 0 { + continue; + } + let info = unsafe { info.assume_init() }; + if info.pbi_uid != my_uid { + continue; + } + if let Some(exe_path) = proc_exe_path_from_procargs2(upid) { + snapshots.push(ProcessSnapshot { + pid: upid, + exe_path, + }); + } + } + snapshots +} + +/// Collect process snapshots for all user-owned processes on Linux via /proc. +/// +/// On Linux `/proc//exe` is a symlink to the executable path. The kernel +/// appends `" (deleted)"` when the on-disk binary has been replaced since +/// launch — exactly the stale-install class this sweep targets. The suffix is +/// stripped so the comparison against `expected_harness_exe_path` succeeds. +#[cfg(all(unix, not(target_os = "macos")))] +fn collect_process_snapshots(harness_name: &str) -> Vec { + let my_uid = unsafe { libc::getuid() }; + let my_pid = std::process::id() as i32; + let mut snapshots = Vec::new(); + + let Ok(entries) = std::fs::read_dir("/proc") else { + return snapshots; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(name_str) = name.to_str() else { + continue; + }; + let Ok(pid) = name_str.parse::() else { + continue; + }; + if pid <= 0 || pid == my_pid { + continue; + } + let upid = pid as u32; + // Cheap name pre-filter via /proc//comm (15-char truncated, but + // "buzz-acp" is 8 chars so it's always preserved). + let Ok(comm) = std::fs::read_to_string(format!("/proc/{upid}/comm")) else { + continue; + }; + if comm.trim() != harness_name { + continue; + } + // Check ownership. + let Ok(meta) = entry.metadata() else { + continue; + }; + use std::os::unix::fs::MetadataExt; + if meta.uid() != my_uid { + continue; + } + // Resolve the executable path via the /proc symlink. + // Strip " (deleted)" so orphans whose binary was replaced still match. + if let Ok(exe_path) = std::fs::read_link(format!("/proc/{upid}/exe")) { + snapshots.push(ProcessSnapshot { + pid: upid, + exe_path: strip_deleted_suffix(exe_path), + }); + } + } + snapshots +} + +// ── expected_harness_exe_path ───────────────────────────────────────────── + +/// Derive the expected path of the `buzz-acp` harness binary next to the +/// current executable. Returns `None` if `current_exe()` fails or has no +/// parent directory. +/// +/// In a `.app` bundle: `.../Contents/MacOS/buzz-acp`. +/// In a dev checkout: `/debug/buzz-acp` or similar. +/// Never hardcoded — always derived from the running process. +/// +/// Attempts `std::fs::canonicalize` to resolve symlinks so the path +/// comparison in `select_untracked_bundle_harnesses` is stable even when the +/// bundle is accessed through a symlink. Falls back to the raw (unresolved) +/// path on `canonicalize` failure — canonicalization can fail for paths that +/// exist only as kernel metadata (e.g. a process launched from a path that +/// has since been moved), so failure must never prevent the sweep from running; +/// it only narrows the comparison to raw paths, which is still correct for +/// the common case. +/// +/// Residual false-negative: if the bundle itself has been translocated or +/// moved since launch, `current_exe()` reflects the new path but a running +/// harness may report the old path. In that case the exe-path comparison +/// fails harmlessly — the orphan is not killed, but neither is anything +/// incorrectly killed. Similarly, an orphan spawned by an older install of +/// the same app (different bundle path, e.g. a prior DMG) will not match +/// this path — that class is handled by `sweep_system_agent_processes`, which +/// scopes by `BUZZ_MANAGED_AGENT` instance ID rather than exe path. +pub fn expected_harness_exe_path() -> Option { + let exe = std::env::current_exe().ok()?; + let dir = exe.parent()?; + let raw = dir.join("buzz-acp"); + // Canonicalize if possible; fall back to the raw path on failure. + Some(std::fs::canonicalize(&raw).unwrap_or(raw)) +} + +/// The basename of the harness binary — used for the cheap name pre-filter in +/// `collect_process_snapshots` before the expensive exe-path lookup. +const HARNESS_BINARY_NAME: &str = "buzz-acp"; + +// ── sweep_untracked_bundle_harnesses ───────────────────────────────────── + +/// Sweep and kill harness processes that share this bundle's exact `buzz-acp` +/// executable path but are not in `skip_pids`. +/// +/// Complements the env-var-based `sweep_system_agent_processes`: this sweep +/// catches orphans that predate the `BUZZ_MANAGED_AGENT` env var injection +/// and any that lost their PID-file receipt. +/// +/// **Boot-time only.** This function is called once, under the store lock, +/// before Phase B spawns any new agents — there is no window for a legitimate +/// harness to have started between the tracked-pid snapshot and this scan, so +/// no grace mechanism is needed. A future periodic caller would need the +/// `sweep_system_agent_processes_with_grace`-style two-tick grace to avoid +/// killing a harness that is legitimately starting up between the skip-list +/// snapshot and the scan. +/// +/// Scoping guarantee: only processes whose exe path (after symlink resolution +/// where possible) equals `/buzz-acp` are candidates. Dev builds +/// at a different path, other installs, and children of tracked parents are +/// never directly targeted. Children die with their parent's process group +/// when `resolve_pgids_and_kill` signals the PGID. +#[cfg(unix)] +pub(crate) fn sweep_untracked_bundle_harnesses(skip_pids: &[u32]) { + let Some(harness_exe) = expected_harness_exe_path() else { + return; + }; + let snapshots = collect_process_snapshots(HARNESS_BINARY_NAME); + let to_kill = select_untracked_bundle_harnesses(&snapshots, &harness_exe, skip_pids); + if to_kill.is_empty() { + return; + } + eprintln!( + "buzz-desktop: sweep_untracked_bundle_harnesses: reaping {} stale harness process(es) {:?} (exe: {})", + to_kill.len(), + to_kill, + harness_exe.display(), + ); + // Small snapshot→kill PID-reuse window: a PID in `to_kill` could be + // recycled between the snapshot and the kill call. This matches the + // precedent set by the neighboring sweeps; `resolve_pgids_and_kill`'s + // PGID-recycling retain guard (skip a resolved PGID that is alive but + // not one of our orphan candidates) narrows the window further. + let to_kill_i32: Vec = to_kill.iter().map(|&p| p as i32).collect(); + super::resolve_pgids_and_kill(&to_kill_i32); +} + +#[cfg(not(unix))] +pub(crate) fn sweep_untracked_bundle_harnesses(_skip_pids: &[u32]) {} + +// ── Tests ───────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + // ── strip_deleted_suffix ───────────────────────────────────────────── + + #[test] + fn strip_deleted_suffix_removes_kernel_suffix() { + // Linux appends " (deleted)" when the binary has been replaced since + // launch — this is exactly the stale orphan class we want to reap. + let p = PathBuf::from("/Applications/Buzz.app/Contents/MacOS/buzz-acp (deleted)"); + assert_eq!( + strip_deleted_suffix(p), + PathBuf::from("/Applications/Buzz.app/Contents/MacOS/buzz-acp") + ); + } + + #[test] + fn strip_deleted_suffix_leaves_normal_path_unchanged() { + let p = PathBuf::from("/Applications/Buzz.app/Contents/MacOS/buzz-acp"); + assert_eq!(strip_deleted_suffix(p.clone()), p,); + } + + #[test] + fn strip_deleted_suffix_does_not_strip_partial_match() { + // "(deleted)" without the leading space must not be stripped. + let p = PathBuf::from("/some/path/buzz-acp(deleted)"); + assert_eq!(strip_deleted_suffix(p.clone()), p,); + } + + // ── select_untracked_bundle_harnesses ──────────────────────────────── + + const BUNDLE_HARNESS: &str = "/Applications/Buzz.app/Contents/MacOS/buzz-acp"; + const DEV_HARNESS: &str = "/Users/dev/buzz/.worktrees/main/target/debug/buzz-acp"; + + fn snap(pid: u32, path: &str) -> ProcessSnapshot { + ProcessSnapshot { + pid, + exe_path: PathBuf::from(path), + } + } + + #[test] + fn untracked_same_bundle_harness_is_killed() { + let snapshots = vec![snap(1001, BUNDLE_HARNESS)]; + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &[]); + assert_eq!(result, vec![1001]); + } + + #[test] + fn tracked_harness_is_spared() { + let snapshots = vec![snap(1002, BUNDLE_HARNESS)]; + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &[1002]); + assert!(result.is_empty()); + } + + #[test] + fn different_bundle_path_is_spared() { + let snapshots = vec![snap(1003, DEV_HARNESS)]; + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &[]); + assert!(result.is_empty()); + } + + #[test] + fn child_of_tracked_parent_not_directly_targeted() { + // A non-harness binary is never selected regardless of tracked state. + let snapshots = vec![snap(1004, "/Applications/Buzz.app/Contents/MacOS/goose")]; + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &[]); + assert!(result.is_empty()); + } + + #[test] + fn empty_process_list_returns_empty() { + let result = select_untracked_bundle_harnesses(&[], &PathBuf::from(BUNDLE_HARNESS), &[]); + assert!(result.is_empty()); + } + + #[test] + fn mixed_snapshot_kills_only_untracked_same_bundle() { + let snapshots = vec![ + snap(2001, BUNDLE_HARNESS), // tracked → spared + snap(2002, BUNDLE_HARNESS), // untracked → killed + snap(2003, DEV_HARNESS), // different path → spared + snap(2004, "/usr/bin/goose"), // unrelated → spared + ]; + let mut result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &[2001]); + result.sort(); + assert_eq!(result, vec![2002]); + } + + #[test] + fn deleted_suffix_stripped_path_matches_expected() { + // Snapshot with " (deleted)" suffix stripped → should match the clean expected path. + let raw = PathBuf::from("/Applications/Buzz.app/Contents/MacOS/buzz-acp (deleted)"); + let snaps = vec![ProcessSnapshot { + pid: 3001, + exe_path: strip_deleted_suffix(raw), + }]; + let result = select_untracked_bundle_harnesses(&snaps, &PathBuf::from(BUNDLE_HARNESS), &[]); + assert_eq!(result, vec![3001]); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 342bb1596c..f675ed0a10 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -179,6 +179,8 @@ fn build_env_owner_only_sets_mode_and_removes_others() { assert!(remove.contains(&"BUZZ_ACP_AGENT_OWNER")); } +// select_untracked_bundle_harnesses tests live in runtime/sweep.rs (mod tests). + #[test] fn build_env_allowlist_sets_both_envs_and_joins() { let a = "a".repeat(64);