fix(acp): kill agent process trees synchronously on quit

[noxenys]

Fixes the ~30s orphan window after app quit: agent CLI subprocesses (and their own children — MCP servers, forked node, etc.) linger after the app closes and only self-exit when they independently notice stdin EOF.

Problem

The graceful teardown chain is correct but doesn't get to run at process exit:

• run_connection is driven on a dedicated std::thread (not a tokio task).
• The kill happens in sacp-tokio's ChildGuard::drop → kill_tree, which only runs when that driver thread unwinds.
• ConnectionManager::disconnect_all is fire-and-forget: it sends ConnectionCommand::Disconnect to each connection and returns immediately.
• Tauri's ExitRequested handler returns right after, the process terminates, and the driver threads are killed mid-flight — usually before they reach ChildGuard::drop. The agent CLI is reparented and lingers (~30s for Claude Code / node until stdin EOF).

Not a permanent leak, but the window means quit-then-relaunch leaves many orphaned node.exe alive, and a user watching Task Manager sees processes that look stuck.

Change

1. vendor/sacp-tokio: add AcpAgent::on_spawn(cb) — invokes cb(pid) once with the child's OS pid right after spawn. Non-breaking (new optional field defaulting to None; the child is still owned by ChildGuard).

2. connection.rs: attach on_spawn to store the pid in a new AgentConnection.child_pid: Arc<AtomicU32> (0 = not spawned / unknown).

3. manager.rs: disconnect_all now snapshots the pids, fires every Disconnect, sleeps a DISCONNECT_ALL_GRACE (500ms) window to let the graceful drops win the race, then kill_trees any survivor on a spawn_blocking thread. Already-dead pids are a best-effort no-op (logged at debug), so cleanly-closed connections cost nothing.

Notes

• Backstop only touches pids this manager spawned + their descendants.
• kill_tree::blocking is already used by ChildGuard::drop; no new deps or features.
• 500ms is a const (DISCONNECT_ALL_GRACE) — tune if quit feels sluggish.

Test plan

• cargo build --release
• Quit app with several active agent sessions → Task Manager shows no residual node.exe (previously several, gone only after ~30s)
• Normal single-connection disconnect still exits cleanly within the window (backstop kill_tree is a no-op)

[xintaofei]

Reviewed this and pushed a follow-up commit (3998630) rather than just leaving notes — the diagnosis here is right, and the graceful-teardown chain really doesn't get to run at exit, but the backstop as written had two problems and one of them made it fire in the wrong direction.

1. The recorded pid was never retracted. disconnect_all drains the map first and then sleeps, so a connection that closes cleanly inside the window still has its pid on the kill list — and by then the process has been killed and reaped. The comment says as much (Already-dead pid is the common, expected case), which means every quit rolled the dice on kill_tree-ing a number the OS had already handed to someone else. That's the exact hazard supervise.rs:195 guards against with WORKER_PID.store(0) after waitpid.

The fix has to live in the layer that owns the process, and "the driver returned" is not "the agent is gone": kill_tree only signals (SIGTERM on Unix) and doesn't wait. So the vendored ChildGuard now keeps the Child until it observes a real reap and reports that through a new AcpAgent::on_exit, paired with your on_spawn. A still-running agent keeps its pid published and still gets swept; only an observed reap clears it. Letting the child fall into tokio's orphan queue instead isn't an option — Reaper::drop (tokio 1.49 process/unix/reap.rs:117) pushes it there unconditionally and reaps it out of sight, which would leave us holding a pid that no longer names it.

2. Pids were read before the grace window. A connection still Connecting when quit begins publishes its pid after the drain, so the snapshot read 0 and skipped it — precisely the orphan this is for. The Arcs live across the sleep (the senders are only borrowed, never consumed), so the load moved into the kill loop.

Also switched the shutdown broadcast to try_send: it's serial send().await on a bounded channel today, so a connection with a full queue could park the quit before the backstop ever runs — the wedged connection whose tree most needs killing.

Tests. Three in manager.rs (tree kill; pid published during the window; cleared pid left alone) — I checked they're not vacuous by reverting disconnect_all to its current form, where two of the three fail. Three in the vendored crate covering the exit contract, including an agent that ignores SIGTERM and must keep its pid published. That crate is a path dependency and not a workspace member, so cargo test never reached its tests; added a CI step so it does — drop that hunk if you'd rather keep it out.

Verified at 3998630: clippy --all-targets --features test-utils -- -D warnings clean, cargo test --features test-utils --lib 1871 passed / 0 failed, vendor --lib 12 passed (5 consecutive runs, the timing-sensitive ones are stable). I couldn't build the tests/ integration binaries or the server cell locally (ran out of disk) — CI covers both, and nothing here is cfg(feature)-gated.

One residual, deliberately not addressed here: between loading a live pid and issuing the kill, that process could exit and its number be reused. It's microseconds now instead of a 500ms window on a pid we expected to be dead, and it's inherent to killing a tree — descendants aren't our children, so no handle or pidfd binds their identity. Fixing it properly means containment at spawn (Job Object on Windows, setsid + killpg on Unix), which is a launch-semantics change and belongs in its own PR.

Separately: codeg-server never calls disconnect_all at all, so the same orphans persist there. Out of scope for this, worth a follow-up.

[xintaofei]

感谢PR