[waleedkadous]

Summary

After a reboot, a persisted terminal_sessions row whose shellper_pid now belongs to an unrelated process was kept forever and retried on every reconcile and /api/state poll. On this machine that was two log lines every ~2s, and tower.log grew to 354 MB. Now that row is deleted the first time Tower sees it, with one INFO line, and the pid is never signaled.

Fixes #1877

Root Cause

SessionManager.reconnectSession() already spots the reuse (PID 2217 reused (start time mismatch)), but it returns only null. Both Bugfix #1686 guards in tower-terminals.ts then checked only processExists(shellper_pid):

• reconcile Phase 2
• the on-the-fly reconnect in getTerminalsForWorkspace()

A reused pid is alive, so the row was kept (unconfirmed++ / WARN) and retried, forever.

Reproduced live, read-only. Row fea2d3f9… has pid = shellper_pid = 2217 with a recorded start of 2026-10-13. ps -p 2217 shows /usr/libexec/diagnosticextensionsd, started 2026-10-24 after that day's boot. The socket file is gone.

Fix

• New isShellperPidReused(pid, recordedStartTime) in terminal/session-manager.ts, beside the reuse rule it shares (PID_REUSE_TOLERANCE_MS; the two 2000 literals now use this constant). It returns true only for a known mismatch. If the start time is unknown on either side (null recorded, or ps can't read it), it returns false, so a transient ps failure can't switch off the tower reconcile: failed reconnect triggers delete + SIGTERM without verifying the shellper is dead (destroyed 53 live sessions in the #1629 incident) #1686 rule.
• Both guard sites, inside the shellperAlive branch: a reused pid gets an INFO line, the row is deleted, and the loop moves on (continue). In Phase 2 this happens before the stale-row branch, which SIGTERMs session.pid. Shellper rows store pid == shellper_pid, so just skipping the guard would have signaled diagnosticextensionsd.
• Every other reconnect failure keeps the tower reconcile: failed reconnect triggers delete + SIGTERM without verifying the shellper is dead (destroyed 53 live sessions in the #1629 incident) #1686 behaviour: WARN, keep the row, never signal.

Departure from the issue's suggested shape. The sweep re-checks the start time with the shared helper instead of passing a pid-reused reason out of reconnectSession(). Two reasons:

• It leaves reconnectSession's long-standing client | null return unchanged (5 callers, about 50 test mocks).
• It is stricter. reconnectSession treats "start time unknown" the same as "mismatch", and deleting a row on an unknown start time would weaken tower reconcile: failed reconnect triggers delete + SIGTERM without verifying the shellper is dead (destroyed 53 live sessions in the #1629 incident) #1686.

The behaviour the issue prescribes is implemented as written.

Out of scope, flagged for follow-up (same hazard, not part of this issue):

• The Phase 1 pre-filter (workspace path missing, or in a temp dir) SIGTERMs shellper_pid based on processExists alone.
• The Never activate Codev inside a builder worktree; hide the Agents view there #1809 liveSessionsForWorkspace treats a live reused pid as a live session.

Test Plan

• Regression tests added in tower-terminals.test.ts, one each for the reconcile and on-the-fly paths. Setup: pid alive, recorded start time a day before the real one. Expected: row deleted, no signal other than 0, INFO logged, no "leaving row untouched" WARN. Both fail on the unfixed code (expected "vi.fn()" to be called with arguments: [ 'bugfix-1877-reused-session' ]) and pass with the fix.
• The four tower reconcile: failed reconnect triggers delete + SIGTERM without verifying the shellper is dead (destroyed 53 live sessions in the #1629 incident) #1686 live-pid tests now record the process's real start time, so they serve as the "start time matches → row kept, never signaled" regression for tower reconcile: failed reconnect triggers delete + SIGTERM without verifying the shellper is dead (destroyed 53 live sessions in the #1629 incident) #1686. A real live shellper always has a matching start time, and their old Date.now() would now correctly read as reused.
• Direct isShellperPidReused tests: mismatch → true; match → false; null recorded → false; unreadable live start time → false.
• Build passes (pnpm build)
• All tests pass: 7228 passed, 49 skipped, 0 failed

Once this ships, the stale row on this machine clears itself after the next Tower restart. No hand edit of global.db is needed.

🤖 Generated with Claude Code

[waleedkadous]

Thanks, this is a tidy fix and the write-up made it easy to review. Integration review ran 3-way (gemini, codex, claude): all three APPROVE, none blocking.

Two small things I'd like before merge, both one-liners:

1. shellper-husk-sweep.ts still defines its own PID_REUSE_TOLERANCE_MS = 2000. Import the constant you just exported from session-manager.ts so the rule has one home.
2. In the isShellperPidReused doc comment, add a sentence that the comparison is between two clocks (Date.now() at shellper start vs ps -o lstart, second-truncated on macOS), so the 2 s tolerance is understood as a skew budget, not a magic number. A false "reused" verdict now deletes a row that was previously only kept-and-retried; bounded (nothing is signaled, and the row was already unreconnectable), but worth stating.

Already covered elsewhere, no action here: the Phase 1 pre-filter still SIGTERMs on processExists alone, which is your follow-up #1879. Good call splitting it.

Waleed has the gate; I'll relay his decision.