From 26324fb1e0567deb9dfb7b337d22e305dd74e777 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 11:33:18 -0400 Subject: [PATCH 1/6] feat(acp,buzz-agent): include effective model and pid in agent turn error context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an agent turn fails, the `llm:` error prefix was stripped of any model context — the incident log read `llm: 404 Not Found: …` while all live config showed a different model, making attribution take hours. buzz-agent's `Llm::complete()` now maps `AgentError::Llm` at the single provider-convergence point to prefix the model: `llm: (model-name) 404 Not Found: …`. buzz-acp's `agent_returned` log sites gain `model=` and `pid=` structured fields so a future incident log immediately surfaces which model the harness was configured for and which OS process emitted the error. --- crates/buzz-acp/src/lib.rs | 19 +++++++++++++++++++ crates/buzz-agent/src/llm.rs | 12 ++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 9fa46e5ce3..bbdf3c8f64 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2764,6 +2764,17 @@ fn handle_prompt_result( PromptOutcome::Cancelled => "cancelled", }; let agent_index = result.agent.index; + // Capture the desired model and our PID before the agent is moved into match + // arms below. These two fields let a post-incident log grep show exactly + // which model the harness was configured for and which process emitted the + // error — essential for diagnosing a stale orphan running an old model. + let harness_desired_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 +2809,8 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, + model = %harness_desired_model, + pid = harness_pid, "agent_returned — respawning" ); let death_message = match outcome_label { @@ -2842,6 +2855,8 @@ fn handle_prompt_result( tracing::debug!( agent = agent_index, outcome = outcome_label, + model = %harness_desired_model, + pid = harness_pid, "agent_returned (cancelled)" ); pool.return_agent(result.agent); @@ -2858,6 +2873,8 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, + model = %harness_desired_model, + pid = harness_pid, error = %e, "transport/protocol error — respawning agent" ); @@ -2882,6 +2899,8 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, + model = %harness_desired_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..06d1ac3c0b 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,15 @@ 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. + // 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( From f26a77f9652e47a699350a69a2f4833ef9924b88 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 11:33:33 -0400 Subject: [PATCH 2/6] fix(desktop): reap untracked same-bundle harness processes at startup The env-var and PID-file sweeps at boot cannot see a harness that predates BUZZ_MANAGED_AGENT injection or whose receipt was overwritten by a respawn. The incident orphan survived multiple in-app agent restarts and at least one sweep-bearing boot, keeping its weeks-old in-process config alive. Adds sweep_untracked_bundle_harnesses: derives the expected buzz-acp path from std::env::current_exe() at runtime (never hardcoded), collects per- process exe paths from KERN_PROCARGS2/proc//exe, runs them through the pure select_untracked_bundle_harnesses decision function (exact exe-path match minus tracked pids), and kills survivors by process group via the existing resolve_pgids_and_kill machinery. Exact-path scoping guarantees dev builds, other installs, and children of tracked parents are never touched. Wired into restore.rs after the existing sweeps with logging. --- .../src-tauri/src/managed_agents/restore.rs | 6 + .../src-tauri/src/managed_agents/runtime.rs | 246 ++++++++++++++++++ .../src/managed_agents/runtime/tests.rs | 84 ++++++ 3 files changed, 336 insertions(+) diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 85f5391128..6efc96bf8c 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -132,6 +132,12 @@ 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. + 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..ace70a8827 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1311,6 +1311,252 @@ 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 ────────────────────────────────────────────── +// +// The PID-file and env-var sweeps above cannot see a harness whose receipt is +// gone or that predates the BUZZ_MANAGED_AGENT env var injection (e.g. a very +// old orphan from before tracking was added). This sweep derives the expected +// harness binary path from the running executable and kills any process whose +// exe matches exactly, minus the tracked set. Exact-path scoping means dev +// builds, other installs, and children of tracked parents are never touched. + +/// 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: std::path::PathBuf, +} + +/// 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: &std::path::Path, + tracked_pids: &std::collections::HashSet, +) -> Vec { + snapshots + .iter() + .filter(|s| s.exe_path == harness_exe && !tracked_pids.contains(&s.pid)) + .map(|s| s.pid) + .collect() +} + +/// Extract the executable path from a process's `KERN_PROCARGS2` buffer. +/// The first null-terminated string after the `i32 argc` field is the exec +/// path. Returns `None` if the buffer is unreadable or malformed. +#[cfg(target_os = "macos")] +fn proc_exe_path_from_procargs2(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); + + 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(std::path::PathBuf::from(s)) +} + +/// Collect process snapshots for all user-owned processes on macOS. +#[cfg(target_os = "macos")] +fn collect_process_snapshots() -> Vec { + let my_uid = unsafe { libc::getuid() }; + let mut snapshots = Vec::new(); + + let mut pids: Vec; + loop { + let count = unsafe { proc_listallpids(std::ptr::null_mut(), 0) }; + if count <= 0 { + return snapshots; + } + 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 snapshots; + } + pids.truncate(actual as usize); + if (actual as usize) < buf_len { + break; + } + } + + let my_pid = std::process::id() as i32; + for &pid in &pids { + if pid <= 0 || pid == my_pid { + continue; + } + let upid = pid as u32; + // Verify UID to avoid inspecting processes owned by other users. + let mut info = std::mem::MaybeUninit::::zeroed(); + let ret = unsafe { + proc_pidinfo( + pid, + 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. +#[cfg(all(unix, not(target_os = "macos")))] +fn collect_process_snapshots() -> 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; + // 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. + if let Ok(exe_path) = std::fs::read_link(format!("/proc/{upid}/exe")) { + snapshots.push(ProcessSnapshot { + pid: upid, + exe_path, + }); + } + } + snapshots +} + +/// 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. +pub fn expected_harness_exe_path() -> Option { + let exe = std::env::current_exe().ok()?; + let dir = exe.parent()?; + Some(dir.join("buzz-acp")) +} + +/// 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 receipt. +/// +/// Scoping guarantee: only processes whose `/proc//exe` or `KERN_PROCARGS2` +/// exec path equals `/buzz-acp` are considered. Dev builds, other +/// installs, and children of tracked parents are never directly targeted. +#[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(); + let tracked: std::collections::HashSet = skip_pids.iter().copied().collect(); + let to_kill = select_untracked_bundle_harnesses(&snapshots, &harness_exe, &tracked); + 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(), + ); + let to_kill_i32: Vec = to_kill.iter().map(|&p| p as i32).collect(); + resolve_pgids_and_kill(&to_kill_i32); +} + +#[cfg(not(unix))] +pub(crate) fn sweep_untracked_bundle_harnesses(_skip_pids: &[u32]) {} + /// 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/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 342bb1596c..c16099161f 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -179,6 +179,90 @@ fn build_env_owner_only_sets_mode_and_removes_others() { assert!(remove.contains(&"BUZZ_ACP_AGENT_OWNER")); } +// ── select_untracked_bundle_harnesses tests ───────────────────────────── + +use super::{select_untracked_bundle_harnesses, ProcessSnapshot}; +use std::collections::HashSet; +use std::path::PathBuf; + +fn snap(pid: u32, path: &str) -> ProcessSnapshot { + ProcessSnapshot { + pid, + exe_path: PathBuf::from(path), + } +} + +const BUNDLE_HARNESS: &str = "/Applications/Buzz.app/Contents/MacOS/buzz-acp"; +const DEV_HARNESS: &str = "/Users/dev/buzz/.worktrees/main/target/debug/buzz-acp"; + +#[test] +fn untracked_same_bundle_harness_is_killed() { + // A process with the exact bundle harness path and not in tracked set → + // should be selected for reaping. + let snapshots = vec![snap(1001, BUNDLE_HARNESS)]; + let tracked: HashSet = HashSet::new(); + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); + assert_eq!(result, vec![1001]); +} + +#[test] +fn tracked_harness_is_spared() { + // Same path but pid is in the tracked set → must NOT be selected. + let snapshots = vec![snap(1002, BUNDLE_HARNESS)]; + let tracked: HashSet = [1002].into_iter().collect(); + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); + assert!(result.is_empty()); +} + +#[test] +fn different_bundle_path_is_spared() { + // A dev-build harness at a different path — must NOT be selected even + // though the binary name is the same. Exact path match only. + let snapshots = vec![snap(1003, DEV_HARNESS)]; + let tracked: HashSet = HashSet::new(); + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); + assert!(result.is_empty()); +} + +#[test] +fn child_of_tracked_parent_not_directly_targeted() { + // A process that is NOT the harness binary (different exe) — never + // selected regardless of tracked state. Children of tracked harnesses + // die with the harness's process group; we never need to target them. + let snapshots = vec![snap(1004, "/Applications/Buzz.app/Contents/MacOS/goose")]; + let tracked: HashSet = HashSet::new(); + let result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); + assert!(result.is_empty()); +} + +#[test] +fn empty_process_list_returns_empty() { + let result = + select_untracked_bundle_harnesses(&[], &PathBuf::from(BUNDLE_HARNESS), &HashSet::new()); + assert!(result.is_empty()); +} + +#[test] +fn mixed_snapshot_kills_only_untracked_same_bundle() { + // Four processes: tracked same-bundle, untracked same-bundle, dev-bundle, + // unrelated binary. Only the untracked same-bundle should be selected. + 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 tracked: HashSet = [2001].into_iter().collect(); + let mut result = + select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); + result.sort(); + assert_eq!(result, vec![2002]); +} + #[test] fn build_env_allowlist_sets_both_envs_and_joins() { let a = "a".repeat(64); From 46ac4f5afeecb9cd09a0c97e3f4f3234fd4b0820 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 11:38:23 -0400 Subject: [PATCH 3/6] fix(desktop): update runtime.rs file-size override for sweep addition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new sweep_untracked_bundle_harnesses function and its supporting types add ~246 lines to runtime.rs. Bump the narrowly-scoped file-size override from 2208 → 2454 with a comment noting the load-bearing security nature and the follow-up split into runtime/sweep.rs. --- desktop/scripts/check-file-sizes.mjs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 54e51ebbdb..96574440eb 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -91,7 +91,12 @@ const overrides = new Map([ // activity-feed threads avatar_url into build_managed_agent_summary for the // assistant-bubble pinned snapshot. // +1 for agent_pubkey field in setup payload (config-nudge card wire). - ["src-tauri/src/managed_agents/runtime.rs", 2208], + // +246: sweep_untracked_bundle_harnesses — exact-exe-path boot sweep for + // receiptless harness orphans (ProcessSnapshot + select_untracked + macOS/Linux + // collect_process_snapshots + proc_exe_path_from_procargs2 + expected_harness_exe_path + // + sweep fn + 6 unit tests). Load-bearing security fix; queued to split + // process-table sweep logic into a dedicated runtime/sweep.rs module. + ["src-tauri/src/managed_agents/runtime.rs", 2454], // config-bridge setup-payload env-boundary fix adds readiness wiring in // spawn_agent_child; load-bearing security fix, queued to split. ["src-tauri/src/managed_agents/config_bridge/reader.rs", 1016], From 5e227e7648ab0ac9569da1008f9ac6921613e69a Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 12:26:11 -0400 Subject: [PATCH 4/6] fix(acp): rename agent_returned tracing field model= to configured_model= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spawn-time `desired_model` captured on the four agent_returned sites does not reflect session/set_model overrides — those live in buzz-agent's session state and are what the llm(model) error prefix carries. Renaming to configured_model= makes the scope explicit and avoids confusion when the two differ (e.g. a stale orphan whose model was later changed via the model-switcher in an active session). --- crates/buzz-acp/src/lib.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index bbdf3c8f64..b6ecbd7aa8 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2764,11 +2764,13 @@ fn handle_prompt_result( PromptOutcome::Cancelled => "cancelled", }; let agent_index = result.agent.index; - // Capture the desired model and our PID before the agent is moved into match - // arms below. These two fields let a post-incident log grep show exactly - // which model the harness was configured for and which process emitted the - // error — essential for diagnosing a stale orphan running an old model. - let harness_desired_model = result + // 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() @@ -2809,7 +2811,7 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, - model = %harness_desired_model, + configured_model = %harness_configured_model, pid = harness_pid, "agent_returned — respawning" ); @@ -2855,7 +2857,7 @@ fn handle_prompt_result( tracing::debug!( agent = agent_index, outcome = outcome_label, - model = %harness_desired_model, + configured_model = %harness_configured_model, pid = harness_pid, "agent_returned (cancelled)" ); @@ -2873,7 +2875,7 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, - model = %harness_desired_model, + configured_model = %harness_configured_model, pid = harness_pid, error = %e, "transport/protocol error — respawning agent" @@ -2899,7 +2901,7 @@ fn handle_prompt_result( tracing::warn!( agent = agent_index, outcome = outcome_label, - model = %harness_desired_model, + configured_model = %harness_configured_model, pid = harness_pid, error = %e, "agent_returned (application error — pipe intact)" From 289257bd789dd4692c361d3473b68e17f60476ec Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 12:26:24 -0400 Subject: [PATCH 5/6] refactor(desktop): extract sweep.rs, add name pre-filter and shared helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the exact-path harness sweep into runtime/sweep.rs so runtime.rs stays within its file-size budget (reverts the override bump). Extract two shared low-level helpers — collect_all_pids() and procargs2_buffer() — and convert all four duplicate proc_listallpids loop sites (sweep_system_ agent_processes, collect_same_instance_orphans, reap_dead_instance_agents, desktop_is_alive_for_instance) and the two duplicate KERN_PROCARGS2 two-phase sysctl sites (process_has_buzz_marker, extract_buzz_marker_value) to use them. Add a cheap proc_name pre-filter in collect_process_snapshots before the expensive KERN_PROCARGS2 exe-path fetch — mirrors the existing pattern in sweep_system_agent_processes and cuts the expensive two-sysctl calls by ~99.9%. Strip " (deleted)" from Linux /proc//exe paths so stale orphans whose binary was replaced still match. Canonicalize expected_harness_exe_path with a raw-path fallback so symlinked bundles compare correctly. Unify the skip_pids calling convention to &[u32] end-to-end. Add a note at the restore.rs call site about future consolidation to a single shared snapshot. --- desktop/scripts/check-file-sizes.mjs | 7 +- .../src-tauri/src/managed_agents/restore.rs | 5 + .../src-tauri/src/managed_agents/runtime.rs | 470 ++--------------- .../src/managed_agents/runtime/sweep.rs | 492 ++++++++++++++++++ .../src/managed_agents/runtime/tests.rs | 84 +-- 5 files changed, 533 insertions(+), 525 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/runtime/sweep.rs diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 96574440eb..54e51ebbdb 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -91,12 +91,7 @@ const overrides = new Map([ // activity-feed threads avatar_url into build_managed_agent_summary for the // assistant-bubble pinned snapshot. // +1 for agent_pubkey field in setup payload (config-nudge card wire). - // +246: sweep_untracked_bundle_harnesses — exact-exe-path boot sweep for - // receiptless harness orphans (ProcessSnapshot + select_untracked + macOS/Linux - // collect_process_snapshots + proc_exe_path_from_procargs2 + expected_harness_exe_path - // + sweep fn + 6 unit tests). Load-bearing security fix; queued to split - // process-table sweep logic into a dedicated runtime/sweep.rs module. - ["src-tauri/src/managed_agents/runtime.rs", 2454], + ["src-tauri/src/managed_agents/runtime.rs", 2208], // config-bridge setup-payload env-boundary fix adds readiness wiring in // spawn_agent_child; load-bearing security fix, queued to split. ["src-tauri/src/managed_agents/config_bridge/reader.rs", 1016], diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 6efc96bf8c..a90ecb1dd9 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -136,6 +136,11 @@ pub async fn restore_managed_agents_on_launch( // 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 diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index ace70a8827..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 - { - 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 - { + let Some(args_buf) = sweep::procargs2_buffer(pid as u32) else { 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,251 +1153,7 @@ 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 ────────────────────────────────────────────── -// -// The PID-file and env-var sweeps above cannot see a harness whose receipt is -// gone or that predates the BUZZ_MANAGED_AGENT env var injection (e.g. a very -// old orphan from before tracking was added). This sweep derives the expected -// harness binary path from the running executable and kills any process whose -// exe matches exactly, minus the tracked set. Exact-path scoping means dev -// builds, other installs, and children of tracked parents are never touched. - -/// 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: std::path::PathBuf, -} - -/// 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: &std::path::Path, - tracked_pids: &std::collections::HashSet, -) -> Vec { - snapshots - .iter() - .filter(|s| s.exe_path == harness_exe && !tracked_pids.contains(&s.pid)) - .map(|s| s.pid) - .collect() -} - -/// Extract the executable path from a process's `KERN_PROCARGS2` buffer. -/// The first null-terminated string after the `i32 argc` field is the exec -/// path. Returns `None` if the buffer is unreadable or malformed. -#[cfg(target_os = "macos")] -fn proc_exe_path_from_procargs2(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); - - 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(std::path::PathBuf::from(s)) -} - -/// Collect process snapshots for all user-owned processes on macOS. -#[cfg(target_os = "macos")] -fn collect_process_snapshots() -> Vec { - let my_uid = unsafe { libc::getuid() }; - let mut snapshots = Vec::new(); - - let mut pids: Vec; - loop { - let count = unsafe { proc_listallpids(std::ptr::null_mut(), 0) }; - if count <= 0 { - return snapshots; - } - 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 snapshots; - } - pids.truncate(actual as usize); - if (actual as usize) < buf_len { - break; - } - } - - let my_pid = std::process::id() as i32; - for &pid in &pids { - if pid <= 0 || pid == my_pid { - continue; - } - let upid = pid as u32; - // Verify UID to avoid inspecting processes owned by other users. - let mut info = std::mem::MaybeUninit::::zeroed(); - let ret = unsafe { - proc_pidinfo( - pid, - 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. -#[cfg(all(unix, not(target_os = "macos")))] -fn collect_process_snapshots() -> 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; - // 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. - if let Ok(exe_path) = std::fs::read_link(format!("/proc/{upid}/exe")) { - snapshots.push(ProcessSnapshot { - pid: upid, - exe_path, - }); - } - } - snapshots -} - -/// 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. -pub fn expected_harness_exe_path() -> Option { - let exe = std::env::current_exe().ok()?; - let dir = exe.parent()?; - Some(dir.join("buzz-acp")) -} - -/// 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 receipt. -/// -/// Scoping guarantee: only processes whose `/proc//exe` or `KERN_PROCARGS2` -/// exec path equals `/buzz-acp` are considered. Dev builds, other -/// installs, and children of tracked parents are never directly targeted. -#[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(); - let tracked: std::collections::HashSet = skip_pids.iter().copied().collect(); - let to_kill = select_untracked_bundle_harnesses(&snapshots, &harness_exe, &tracked); - 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(), - ); - let to_kill_i32: Vec = to_kill.iter().map(|&p| p as i32).collect(); - resolve_pgids_and_kill(&to_kill_i32); -} - -#[cfg(not(unix))] -pub(crate) fn sweep_untracked_bundle_harnesses(_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 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..d32eefb80a --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs @@ -0,0 +1,492 @@ +//! 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. +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(), + ); + 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 c16099161f..f675ed0a10 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -179,89 +179,7 @@ fn build_env_owner_only_sets_mode_and_removes_others() { assert!(remove.contains(&"BUZZ_ACP_AGENT_OWNER")); } -// ── select_untracked_bundle_harnesses tests ───────────────────────────── - -use super::{select_untracked_bundle_harnesses, ProcessSnapshot}; -use std::collections::HashSet; -use std::path::PathBuf; - -fn snap(pid: u32, path: &str) -> ProcessSnapshot { - ProcessSnapshot { - pid, - exe_path: PathBuf::from(path), - } -} - -const BUNDLE_HARNESS: &str = "/Applications/Buzz.app/Contents/MacOS/buzz-acp"; -const DEV_HARNESS: &str = "/Users/dev/buzz/.worktrees/main/target/debug/buzz-acp"; - -#[test] -fn untracked_same_bundle_harness_is_killed() { - // A process with the exact bundle harness path and not in tracked set → - // should be selected for reaping. - let snapshots = vec![snap(1001, BUNDLE_HARNESS)]; - let tracked: HashSet = HashSet::new(); - let result = - select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); - assert_eq!(result, vec![1001]); -} - -#[test] -fn tracked_harness_is_spared() { - // Same path but pid is in the tracked set → must NOT be selected. - let snapshots = vec![snap(1002, BUNDLE_HARNESS)]; - let tracked: HashSet = [1002].into_iter().collect(); - let result = - select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); - assert!(result.is_empty()); -} - -#[test] -fn different_bundle_path_is_spared() { - // A dev-build harness at a different path — must NOT be selected even - // though the binary name is the same. Exact path match only. - let snapshots = vec![snap(1003, DEV_HARNESS)]; - let tracked: HashSet = HashSet::new(); - let result = - select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); - assert!(result.is_empty()); -} - -#[test] -fn child_of_tracked_parent_not_directly_targeted() { - // A process that is NOT the harness binary (different exe) — never - // selected regardless of tracked state. Children of tracked harnesses - // die with the harness's process group; we never need to target them. - let snapshots = vec![snap(1004, "/Applications/Buzz.app/Contents/MacOS/goose")]; - let tracked: HashSet = HashSet::new(); - let result = - select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); - assert!(result.is_empty()); -} - -#[test] -fn empty_process_list_returns_empty() { - let result = - select_untracked_bundle_harnesses(&[], &PathBuf::from(BUNDLE_HARNESS), &HashSet::new()); - assert!(result.is_empty()); -} - -#[test] -fn mixed_snapshot_kills_only_untracked_same_bundle() { - // Four processes: tracked same-bundle, untracked same-bundle, dev-bundle, - // unrelated binary. Only the untracked same-bundle should be selected. - 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 tracked: HashSet = [2001].into_iter().collect(); - let mut result = - select_untracked_bundle_harnesses(&snapshots, &PathBuf::from(BUNDLE_HARNESS), &tracked); - result.sort(); - assert_eq!(result, vec![2002]); -} +// select_untracked_bundle_harnesses tests live in runtime/sweep.rs (mod tests). #[test] fn build_env_allowlist_sets_both_envs_and_joins() { From 80b26f14eafe150c1377aaa67a95e5671eb95a67 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 12:39:58 -0400 Subject: [PATCH 6/6] docs(buzz-agent,desktop): correct llm error format comment and note sweep pid-reuse window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a comment in llm.rs that claimed `llm(model-name): 404` when the actual Display output is `llm: (model-name) 404 …` (the map_err prepends `(model-name) ` to the inner string; AgentError::Llm Display adds `llm: `). Add a clarifying sentence explaining the two-part construction. Add a brief comment in sweep_untracked_bundle_harnesses before the resolve_pgids_and_kill call acknowledging the small snapshot→kill PID-reuse window, noting it matches the neighboring sweeps' precedent and that resolve_pgids_and_kill's PGID-recycling retain guard narrows it further. Extend the expected_harness_exe_path doc comment to explicitly cover the "orphan from an older install at a different bundle path" case, naming sweep_system_agent_processes (instance-ID-scoped) as the covering defense. --- crates/buzz-agent/src/llm.rs | 4 +++- desktop/src-tauri/src/managed_agents/runtime/sweep.rs | 10 +++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 06d1ac3c0b..07c2071aa2 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -142,7 +142,9 @@ impl Llm { } }; // Stamp the effective model into Llm errors so log lines carry - // `llm(model-name): 404 Not Found: …` instead of the bare status. + // `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 { diff --git a/desktop/src-tauri/src/managed_agents/runtime/sweep.rs b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs index d32eefb80a..2e026ec5fc 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/sweep.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs @@ -323,7 +323,10 @@ fn collect_process_snapshots(harness_name: &str) -> Vec { /// 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. +/// 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()?; @@ -374,6 +377,11 @@ pub(crate) fn sweep_untracked_bundle_harnesses(skip_pids: &[u32]) { 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); }