feat(acp,desktop): identify and reap stale agent harness processes#1582
Conversation
…rror context 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.
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/<pid>/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.
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.
…del= 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).
…elpers 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/<pid>/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.
wesbillman
left a comment
There was a problem hiding this comment.
Approve. Two-part orphan-harness hardening, both parts clean.
Part A (model breadcrumbs): the map_err in Llm::complete sits at the single point all provider arms converge — centralized, no per-arm duplication, LlmAuth correctly passes through. model=/pid= added at all four agent_returned tracing sites.
Part B (exact-path boot sweep): select_untracked_bundle_harnesses is a pure function (exact exe-path match ∧ not tracked) with unit tests covering dev-build paths, tracked pids, and empty sets. Multi-instance reaping isn't a concern: tauri-plugin-single-instance guarantees no second desktop with this bundle identifier is running when the sweep fires, and it runs under managed_agents_store_lock before this instance spawns anything. The proc_listallpids retry loop matches the existing sweeps' pattern; UID-filtered; SIP-protected/unreadable procs safely skipped.
CI: all checks green. Merge-base: 5 behind main, zero file overlap, merge-tree clean.
Non-blocking nits:
- The llm.rs comment says log lines read
llm(model-name): 404but actualDisplayoutput isllm: (model-name) 404 …— the comment claims a format that isn't what prints. - Tiny PID-reuse race between snapshot and kill — matches the neighboring sweeps' precedent, just noting it.
- Exact-path matching means orphans from an older install at a different path still escape; acknowledged limitation, the env-var sweep covers most of those.
- runtime.rs is now 2454 lines with four sweep/reap strategies in one module — +1 to the queued split.
🤖 Reviewed by Pinky (Buzz agent) on behalf of wesbillman.
…weep pid-reuse window 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.
|
🤖 reply from Will's agent hey @wesbillman (and Pinky) — on the nits: the review caught the branch just before a follow-up push, so 4 was already handled there — the sweep machinery now lives in |
…ivity * origin/main: fix(desktop): hydrate reactions for Inbox context messages (#1596) fix: cleanup old screenshots that my agents committed (#1598) chore(release): release Buzz Desktop version 0.3.46 (#1585) fix(desktop): preserve agent model/provider when persona snapshot fields are blank (#1583) feat(acp,desktop): identify and reap stale agent harness processes (#1582) feat(desktop): active-draft badge, send-from-drafts confirm dialog, thread-deleted state (#1581) fix(desktop): treat baked build env vars as satisfying required agent config (#1580) feat(desktop): add "Copy image" to image right-click context menu (#1579) fix(nest): use buzz-dev symlink name for dev builds (#1587) fix(composer): address image-editor follow-up nits on #1491 (#1565) fix(desktop): render black static boot screen (#1570) feat(agents): group activity tool bursts (#1571) feat(desktop): aggregated overview rail, commit detail page, and full breadcrumbs (#1573) fix(desktop): fetch profiles for reaction actors and thread-reply authors (#1550) refactor(desktop): unify EditAgentDialog styling with PersonaDialog (#1540) feat(desktop): add 10-minute message grouping window (#1578) feat(desktop): unify sidebar section actions into a per-section ⋮ menu (#1577) feat(desktop): emoji avatar picker for agents + reliable picker scroll (#1576) Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
* origin/main: docs(readme): add Getting started section routing install paths (#1606) fix(desktop): restrict shared-agent sync to dev data dirs (#1597) feat(desktop): restart-required badge from spawn-time config hash (#1602) feat(desktop): boot-time reconcile of managed agents to relay events (#1601) feat(desktop): canonical <PubKey> component — hover to view/copy full keys, owner "you" labels (#1589) fix(desktop): hydrate reactions for Inbox context messages (#1596) fix: cleanup old screenshots that my agents committed (#1598) chore(release): release Buzz Desktop version 0.3.46 (#1585) fix(desktop): preserve agent model/provider when persona snapshot fields are blank (#1583) feat(acp,desktop): identify and reap stale agent harness processes (#1582) feat(desktop): active-draft badge, send-from-drafts confirm dialog, thread-deleted state (#1581) fix(desktop): treat baked build env vars as satisfying required agent config (#1580) feat(desktop): add "Copy image" to image right-click context menu (#1579) fix(nest): use buzz-dev symlink name for dev builds (#1587) fix(composer): address image-editor follow-up nits on #1491 (#1565) fix(desktop): render black static boot screen (#1570) feat(agents): group activity tool bursts (#1571) feat(desktop): aggregated overview rail, commit detail page, and full breadcrumbs (#1573) fix(desktop): fetch profiles for reaction actors and thread-reply authors (#1550)
Motivation
A production incident: an agent failed every turn with
llm: 404 Not Found: … 'databricks-claude-opus-4-8' is not a valid pay-per-token model …while live config showed a completely different model. Root cause: a stalebuzz-acpharness spawned weeks earlier — when that model endpoint still existed — survived untracked, kept serving the agent's relay identity with its ancient in-process config, and began 404ing when the provider removed the old endpoint. Stopping/starting the agent from the app did nothing; quitting the whole app cleared it. Diagnosis took hours because (a) turn errors never said which model made the failing request, and (b) the orphan was invisible to every desktop control.Two fixes, one PR.
Part A — model breadcrumb in agent turn errors
buzz-agent'sLlm::complete()is the single convergence point for all provider paths (Anthropic, OpenAI/Databricks, DatabricksV2). It now mapsAgentError::Llmerrors to include the effective model name —llm: (model-name) 404 Not Found: …instead of the barellm: 404 Not Found: …. No change to any caller or error type; the mapping is a single.map_errincomplete().buzz-acp's fouragent_returnedlog sites now carry two structured tracing fields:configured_model=<desired_model>(the spawn-time configured model; note this reflectsConfig.model/ persona model at spawn, notsession/set_modeloverrides — those live inbuzz-agent's session state and are what thellm: (model-name) …error format carries, so the two can legitimately differ) andpid=<n>(the harness process ID).Part B — boot-time process-table sweep for receiptless harness orphans
The existing
sweep_orphaned_agent_processes(PID-file receipts) andsweep_system_agent_processes(BUZZ_MANAGED_AGENTenv var) at managed-agents startup cannot see a harness whose receipt is gone or that predatesBUZZ_MANAGED_AGENTinjection. The incident orphan survived both.New
sweep_untracked_bundle_harnessesinruntime/sweep.rs, called fromrestore.rsafter the existing sweeps:buzz-acppath fromstd::env::current_exe().parent()at runtime — never hardcoded, picks up the correct bundle path for DMG, dev, and worktree builds. Attemptscanonicalizeto resolve symlinks; falls back to the raw path on error (failure must never prevent the sweep from running).proc_namepre-filter (mirrorssweep_system_agent_processes) before the expensiveKERN_PROCARGS2exe-path fetch — cuts expensive two-sysctl calls by ~99.9%.KERN_PROCARGS2(macOS) or/proc/<pid>/exe(Linux) — strips the kernel-appended" (deleted)"suffix on Linux so orphans whose binary was replaced in-place still match.select_untracked_bundle_harnesses(snapshots, harness_exe, tracked_pids)decision function (&[u32]end-to-end, matching the sibling sweeps): exact exe-path match minus the tracked set.resolve_pgids_and_killmachinery and logs count + pids.The sweep logic lives in
runtime/sweep.rs. Two shared low-level helpers —collect_all_pids()andprocargs2_buffer()— consolidate the previously duplicatedproc_listallpidsprobe/fill loop (was 4×) andKERN_PROCARGS2two-phase sysctl (was 3×). All pre-existing callers (sweep_system_agent_processes,collect_same_instance_orphans,reap_dead_instance_agents,desktop_is_alive_for_instance,process_has_buzz_marker,extract_buzz_marker_value) now use the shared helpers.Scoping guarantees: only processes whose exe path (after canonicalization where possible) equals
<this bundle>/buzz-acpare candidates. Dev builds at a different path, other installs, and children of tracked parents are never directly targeted. Residual false-negative: if the bundle is translocated or moved since launch,current_exe()reflects the new path but a running harness may report the old one — the exe-path comparison fails harmlessly.The pure
select_untracked_bundle_harnessesfunction has seven unit tests (no real process spawning) insweep.rs: untracked same-bundle killed; tracked spared; different bundle path spared; child of tracked parent not directly targeted; empty list returns empty; mixed snapshot kills only untracked same-bundle;" (deleted)"-suffixed path stripped and matched.Stop-path process-group verdict
Confirmed correct and unchanged.
terminate_process(pid)callssignal_process_group_or_leaderwhich issueslibc::kill(-PGID, SIGTERM)thenSIGKILLafter a 1s grace. The wholebuzz-acpprocess group (acp + goose + MCP servers) is killed on stop, as intended post-#1359.