Orphan-reap responsive-but-unclaimed shellpers at startup to fix PTY pool exhaustion

[amrmelsayed]

Background

Surfaced from PR #1031 review and issue #1030. The contributor's diagnosis of the PTY-exhaustion symptom (posix_spawnp failed / Invalid shellper info JSON after enough Tower restart cycles) is correct, but the proposed fix in #1031 (kill all scoped shellpers on tower stop) breaks the documented "Tower stop does NOT kill shellpers" contract that the persistence work in #274, #832, PR #999, and #991 was built on.

This issue tracks the right place to fix the leak: the startup orphan-reaper.

The leak

killOrphanedShellpers() in packages/codev/src/agent-farm/servers/tower-server.ts (~line 411) runs at Tower startup after reconcileTerminalSessions(). It scans shellper-main processes scoped to this Tower's socketDir and currently:

• Kills shellpers whose socket doesn't respond → correct, they're corpses.
• Spares shellpers whose socket responds, regardless of whether reconciliation claimed them → the leak.

The "spare responsive shellpers" guard was added in #341 because a corrupt or empty SQLite would have made every live shellper look unclaimed, and the reaper would have nuked an entire active workspace. The safe fallback was "if it responds, assume it belongs to somebody, don't touch it."

That fallback is overly conservative. A responsive shellper from a Tower instance that no longer exists (workspace deleted, prior run that left files behind, socketDir overlap across users on a multi-user box, etc.) will never be claimed by any future Tower either. Each one holds one macOS PTY. macOS has a small fixed PTY pool — these accumulate and eventually exhaust it.

Proposed fix

After reconcileTerminalSessions() completes and the _reconciling barrier in packages/codev/src/agent-farm/servers/tower-terminals.ts is dropped (so no client can racingly reattach during the reap), any scoped responsive shellper still not in the active sessions map is provably orphaned: nothing in Tower's reconciliation path will claim it later.

Two options:

1. Inline includeResponsive: true second sweep. Add a parameter to killOrphanedShellpers(). The first sweep stays as-is (unresponsive-only, runs before _reconciling drops). The second runs after reconciliation and the barrier-drop, with includeResponsive: true, killing whatever's left in scope that's still unclaimed.

2. Log-only at startup + explicit operator verb. Don't auto-kill on startup. Log a warning for each detected orphan. Expose afx terminal gc (or similar) for operators to run when they see the symptom. Lower-risk: preserves Orphaned shellper and consult processes accumulate over time #341's conservatism, gives operators an explicit verb to reclaim.

Option 1 is the better default for a healthy long-run state; option 2 is the safer first step. Either way, the implementation reuses SessionManager.findShellperProcesses() and the existing socketDir marker scoping. The actual kill is a process-group SIGTERM → 5s → SIGKILL (PR #1031's killScopedShellpers() is a good shape if reused).

Testing surface

• Corrupt / empty SQLite at startup — reconciliation produces an empty active map; reaper must NOT kill responsive shellpers in this case (so the new pass should run only when reconciliation completed cleanly, not when it raised).
• Partial reconciliation — some sessions reattached, some failed. Reaper kills only the unclaimed-after-success set.
• Race against client reattach — verify the _reconciling barrier in tower-terminals.ts keeps clients out until the reap completes.
• Multi-Tower-on-same-machine — verify socketDir scoping prevents cross-Tower interference.

References

• Original issue: afx tower start/stop should report startup failure and reclaim shellper PTYs on explicit stop #1030
• PR with the wrong-layer fix: Fix Tower startup checks and shellper cleanup #1031
• Prior persistence work: Architect terminal should survive Tower restarts (shellper persistence) #274, Multi-architect conversation resume: disambiguate via per-architect session UUID #832, Terminals survive a Tower restart: preserve terminal id + stop afx tower stop killing port clients (#991) #999, terminal: stale tab on a pre-restart terminal id can't self-recover without a manual state re-fetch #991
• The Orphaned shellper and consult processes accumulate over time #341 safety guard this loosens

Out of scope

Behavior change to afx tower stop — keep the existing contract (Tower stops, shellpers survive, next start reattaches). This is purely a startup-time orphan-detection improvement.

[amrmelsayed]

Conceptual sibling: #1047

Worth reading alongside this issue. Different observable symptoms, but they share architectural territory.

	This issue (#1038)	#1047
Symptom	New terminal spawns fail (posix_spawnp failed)	Existing terminals freeze, restart resolves
State that leaks	OS-level shellper processes	In-memory JS bookkeeping in Tower
Lifetime of the leak	Across many Tower restart cycles	Within one Tower's lifetime

Both trace back to the same persistence-across-restart design line (#274 / #832 / PR #991 / PR #999) that made shellpers detach from Tower and survive its restart. Two related bookkeeping responsibilities fall out of that design:

• Tower decides "which shellpers to reap on next start" — if that decision is too conservative, you get this issue's symptom (responsive-but-unclaimed shellpers accumulate across Tower lifetimes).
• Tower tracks "shellpers I know about" in-memory during runtime — if that bookkeeping leaks, you get tower + vscode: terminal freeze from oversized-replay reconnect storm (unbounded no-newline buffers + client backpressure loop) #1047's symptom (CPU climbs over hours within one Tower lifetime).

PIR #991 is also the strongest candidate for #1047 per its triage comment, and that's the same surface this issue's orphan-reap fix lives on. A fix for this issue's reap semantics could shift where #1047's leak surface lives, and vice versa.

Practical implication for whoever picks this up: read #1047 before designing the orphan-reap change. If responsive-but-unclaimed shellpers from prior lifetimes are also leaking in-memory bookkeeping on the current Tower's side (the #1047 mechanism), the reap design should account for the in-memory consistency, not just the OS-process cleanup.

[amrmelsayed]

Confirmed independent from #1047

#1047 was initially flagged as a conceptual sibling of this issue — both trace to the persistence-across-restart design line (#274 / #832 / #991 / #999), and there was an open question of whether they shared a root cause in shellper tracking and would need a coordinated fix.

That's now resolved: #1047's root cause was pinned to unbounded no-newline terminal buffers producing an oversized reconnection replay that drove the VSCode terminal client into a backoff-free reconnect storm (CPU saturation, all terminals freeze). Its fix touches the ring buffer, the shellper replay buffer, the WebSocket replay framing, and the VSCode client — not shellper process spawning or the startup orphan-reaping this issue is about.

So the two are independent bugs in different layers:

	#1047	#1038 (this issue)
Leak	In-memory buffers + a client reconnect loop	OS-level shellper processes across restarts
Symptom	Existing terminals freeze	New spawns fail (PTY pool / posix_spawnp exhaustion)
Fix surface	buffers, WS replay bracket, client backpressure	startup orphan-reaping (SessionManager.killOrphanedShellpers)

This issue remains open and unaffected by the #1047 work — the code paths are disjoint, so neither blocks nor regresses the other.