Skip to content

fix(runtime): sweep node wrapper processes hosting managed agent shims#1296

Merged
wpfleger96 merged 5 commits into
mainfrom
duncan/orphan-reaper-node
Jun 25, 2026
Merged

fix(runtime): sweep node wrapper processes hosting managed agent shims#1296
wpfleger96 merged 5 commits into
mainfrom
duncan/orphan-reaper-node

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Problem

codex-acp and claude-agent-acp are npm shims — their OS process name is node, not the binary name. When the Buzz app restarts, buzz-acp dies and the node wrapper is reparented to launchd (PPID=1). The orphan reaper never killed it because process_belongs_to_us() checked the process name against KNOWN_AGENT_BINARIES"node" was not in the list.

The node wrapper kept the native codex-acp binary alive as its child, and that child had no network_proxy flags (spawned before the update), causing relay network failures for Thufir and any other codex-based agent after an app update.

Fix

Commit 1 — interpreter recognition:
Add KNOWN_SCRIPT_INTERPRETERS = ["node"] and a name_matches_interpreter() helper. Extend both macOS (proc_name()) and Linux (/proc/comm + /proc/exe) variants of process_belongs_to_us() to return true for interpreter names, so the sweep proceeds to process_has_buzz_marker().

Commit 2 — marker check at record-management paths:
Three call sites used process_belongs_to_us() without a subsequent process_has_buzz_marker() check. With node now returning true from process_belongs_to_us(), these sites could falsely act on unrelated node processes (dev servers, build tools) whose PID happened to collide with a stale agent record:

  • kill_stale_tracked_processes: would kill the foreign process
  • sync_managed_agent_processes: would skip clearing runtime_pid, blocking respawn
  • start_managed_agent_process: would skip respawning the agent

Add instance_id: &str to kill_stale_tracked_processes and sync_managed_agent_processes; thread current_instance_id(app) through all 11 call sites. Add process_has_buzz_marker() at all three check sites. The sweep paths already gated on the marker — this brings the record-management paths to the same standard.

Why not add "node" to KNOWN_AGENT_BINARIES? That would sweep all node processes on the machine regardless of ownership — catastrophic false positives.

Future-proofing

KNOWN_SCRIPT_INTERPRETERS is a named constant. Adding python3, deno, or bun support if those become harness runtimes is a one-liner.

Tests

4 new unit tests in runtime/tests.rs:

  • name_matches_known_binary_rejects_node — guards against accidentally adding node to KNOWN_AGENT_BINARIES
  • name_matches_interpreter_accepts_nodenode is recognized as an interpreter
  • name_matches_interpreter_rejects_unknownpython3, deno, bun do not match
  • name_matches_interpreter_rejects_node_prefixnode_modules, nodejs, node-gyp do not match (exact equality required)

All 40 managed_agents::runtime tests pass.

npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 and others added 5 commits June 25, 2026 16:29
codex-acp and claude-agent-acp are npm shims — their OS process name is
`node`, not the binary name. When the Buzz app restarts, buzz-acp dies and
the node wrapper is reparented to launchd (PPID=1). The orphan reaper never
killed it because `process_belongs_to_us()` checked the process name against
`KNOWN_AGENT_BINARIES` — `"node"` was not in the list. The node wrapper kept
the native codex-acp binary alive as its child, and that child had no
`network_proxy` flags (spawned before the update), causing relay failures.

Add `KNOWN_SCRIPT_INTERPRETERS = ["node"]` and `name_matches_interpreter()`.
Extend both macOS and Linux `process_belongs_to_us()` to return `true` for
interpreter names so the sweep proceeds to `process_has_buzz_marker()`, which
gates on `BUZZ_MANAGED_AGENT` ownership. Unrelated node processes are never
swept — zero false positives.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
process_belongs_to_us() now returns true for node processes (interpreter
support added in the previous commit). Three call sites acted on that result
without a subsequent process_has_buzz_marker() check:

- kill_stale_tracked_processes: would kill any node process whose PID
  collided with a stale agent record
- sync_managed_agent_processes: would skip clearing runtime_pid for a
  foreign node process, leaving a stale PID that blocks respawn
- start_managed_agent_process: would skip respawning if a foreign node
  process happened to hold the recorded PID

Add instance_id: &str to kill_stale_tracked_processes and
sync_managed_agent_processes; thread current_instance_id(app) through all
11 call sites. Add process_has_buzz_marker() to all three check sites.
The sweep paths (collect_same_instance_orphans, reap_dead_instance_agents)
already gated on the marker — this brings the record-management paths to
the same standard.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Import use-lists in agent_models.rs, agents.rs, lib.rs, and restore.rs
were wrapped at slightly different column boundaries after the
instance_id threading changes. No logic changes — pure formatting.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
interpreter-orphan reaper adds ~30 lines to runtime.rs (new constant,
helper, and marker-guard threading); instance_id threading adds ~8
lines to lib.rs. Both files already had approved overrides; bumping
to actual line count.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96 wpfleger96 merged commit f072032 into main Jun 25, 2026
25 checks passed
@wpfleger96 wpfleger96 deleted the duncan/orphan-reaper-node branch June 25, 2026 21:07
tellaho pushed a commit that referenced this pull request Jun 25, 2026
…ebar

* origin/main:
  fix(sidebar): non-selectable channel names + copy/leave context menu actions (#1260)
  fix(runtime): sweep node wrapper processes hosting managed agent shims (#1296)
  fix(buzz-agent): follow symlinks when discovering skill directories (#1295)
  chore: add grab-emoji.sh to register Slack emoji in Buzz (#1292)
  Fix cross-pod membership notification fanout (#1291)
  fix(buzz-acp): strengthen agent communication rules in base prompt (#1293)
  chore(release): release Buzz Desktop version 0.3.34 (#1289)
  feat(desktop): refresh Agents tab live on inbound relay sync (#1256)
  fix(buzz-acp): inject Codex network allowlist for relay hostname at spawn time (#1287)

Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>

# Conflicts:
#	desktop/src-tauri/src/commands/agent_models.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant