Fix #1686: reconcile must confirm death before deleting/SIGTERMing a shellper row

[amrmelsayed]

Summary

Tower's terminal reconcile treated any failed shellper reconnect as death: it SIGTERMed the (still-live) pid and deleted the DB row. In the #1629 incident this destroyed 53/56 live sessions. The fix makes death provable — a live pid is never signaled.

Fixes #1686

Root Cause

SessionManager.reconnectSession() (session-manager.ts:535-577) returns null for five distinct reasons, only some of which mean the process is dead:

1. process dead, 2. PID reused (start-time mismatch), 3. socket not a socket, 4. socket missing, and — the incident case — 5. client.connect() refused because another Tower client already owns the shellper (one-client-per-shellper), or a transient boot-time socket/fd hiccup (see tower startup can hang unbounded and unlogged in killOrphanedShellpers, tripping the 30s launcher timeout (field report on 3.3.3) #1685's environment).

Reconcile Phase 1 collapsed all five into "stale (PID/socket dead)", and Phase 2's sweep then SIGTERMed session.pid (alive) and deleted the row — signalling exactly the live processes it should have preserved. The same delete-on-null defect lived behind a second door: getTerminalsForWorkspace()'s on-the-fly reconnect deleted the row (no SIGTERM) on any null, so the first /api/state//api/overview read would delete a row Phase 2 had just preserved.

Fix

Destroy only what is proven dead, and the sole proof of death is the pid being down. A live shellper pid is never signaled and its row is never deleted, regardless of socket-file state — a transiently absent socket is the same transient-failure class and must not kill a live agent. A pid-dead row still deletes as before. The row is left in place (WARN) and retried on a later reconcile/adoption pass.

Applied at both sites that read reconnect-failure as death:

• reconcile Phase 2 sweep — the SIGTERM + DELETE path from the incident.
• getTerminalsForWorkspace() on-the-fly reconnect — the DELETE-only sibling path (folded in per review: same root cause; without it Phase 2's "retried next pass" promise is false).

Also: the Phase 1 null-reconnect log no longer asserts "is stale (PID/socket dead)" (it defers the verdict to the Phase 2 sweep), and a new unconfirmed counter keeps a preserve-only reconcile from logging "No terminal sessions to reconcile".

Note on the issue text: #1686 contained an internal tension — "positive evidence of death: pid dead AND/OR socket absent" vs. "a row whose pid is alive is left in place … never signaled". This PR resolves it (per review + owner ruling) to the invariant above: proof of death = pid down; socket-file state does not contribute to the kill decision.

Relationship to #1629: that issue's owner-lock guard stops the second-Tower amplifier; this removes the destructive mechanism, which a single-Tower transient can also trigger.

Known residuals / decisions

• Unbounded retention (accepted): a live but permanently-unreconnectable socketless shellper keeps its row for the rest of the Tower session. It is reaped at the next boot's orphan sweep (killOrphanedShellpers, tower-server.ts:725), which skips shellpers with a responsive socket (the incident case, correctly preserved) and SIGTERMs the socket-dead ones; the following reconcile / /api/state read then finds shellperAlive === false and deletes the row. (Note: the Codev process fleet drives macOS memory-pressure kills: stranded shellper husks accumulate per restart + no idle-fleet memory policy #1227 husk sweep does not reap it — the retained row makes that pid count as "registered" and thus husk-exempt.) Deliberate: leak a row rather than kill a session.
• PID reuse: processExists(shellper_pid) can't distinguish a recycled pid. Kept conservative (a live-looking pid preserves the row); tightening via getProcessStartTime() would re-open the false-death path this fix exists to close.
• Legacy edge: a row with shellper_socket set but shellper_pid === null yields shellperAlive=false and falls through to SIGTERM of session.pid — ~unreachable given saveTerminalSession's call sites.

Test Plan

• Regression tests added — five: both guarded sites with both a present and an absent socket (four preserve-cases), plus a pid-dead case proving the guard still sweeps genuinely dead rows (bounds it from the over-preservation side). Each preserve-case verified to fail without its guard (Phase 2: SIGTERMs the live pid + DELETEs the row; on-the-fly: DELETEs the row; socket-absent cases: fail under the earlier && socketPresent guard). Deterministic (no timers); the process.kill spy passes signal 0 through to real liveness but swallows SIGTERM so a regression can't kill the test runner; onTestFinished restores the mock.
• Build passes (pnpm build, exit 0).
• All tests pass — tower-terminals.test.ts 65/65; tsc --noEmit clean; full agent-farm suite green (176 files, 3683 tests, 0 failures) after pnpm build. Note: running vitest before pnpm build surfaces 5 getRolesDir → "Roles directory not found" failures in consolidate.test.ts / spawn-retirement.test.ts — those tests depend on the skeleton copied by bundle-assets, so they only fail when tests run ahead of the build (a pre-existing build-prerequisite artifact, reproduced identically on the clean base, unrelated to this change; CI builds before testing).