Shellper reconnect error is swallowed: terminal becomes a silent zombie (no input/output, 'Message sent' logged for dropped frames) until next Tower restart

[amrmelsayed]

Symptom

After a Tower restart, a persistent (shellper-backed) terminal can become a silent zombie: viewers (VS Code adapter AND web dashboard) render a blank screen, keystrokes and afx send messages go nowhere, yet Tower reports the session as status: "running", persistent: true and the message router logs "Message sent". No error or warning appears anywhere. The underlying shellper and its Claude child are healthy the whole time, with the full conversation intact.

Observed 2026-07-18 on the shannon workspace's app architect (terminal dd7f81ad, shellper pid 4509, claude child 4640 alive since Jul 13). At least three messages were dropped into the void between the 06:25Z restart and diagnosis (two from builder pir-2579, one cross-workspace test), each logged as successfully sent.

Evidence trail

• Tower startup log 06:25:18.470Z: Session dd7f81ad… reconnected: pid=4509 — claims success.
• GET /api/terminals/dd7f81ad…/output → {"lines":[],"total":0}. Every healthy architect terminal returns total=1 (the reconnect replay frame).
• POST /api/terminals/dd7f81ad…/resize succeeds and updates cols/rows in the DB, but the output buffer stays empty: no SIGWINCH ever reaches the PTY, so the resize is a DB-only mutation.
• fd asymmetry: every healthy shellper socket shows 3 fds (lsof on the socket path); app's shows 2. The missing fd is the live connection from the current Tower.
• No shellper disconnected unexpectedly line for this session anywhere in the log. The failure left zero trace.
• Context: TWO Tower starts that morning, 06:18:41Z and 06:25:18Z. The zombie was minted during the second start's adoption pass.

Root cause: an error-path close is swallowed, and everything downstream hangs off it

packages/codev/src/terminal/shellper-client.ts:

1. On any post-handshake socket or parser error, onError runs safeEmitError(err) then cleanup().

2. cleanup() (line ~266) sets this._connected = false before destroying the socket.

3. The socket's subsequent 'close' event hits the guard (lines ~153-163):

   socket.on('close', () => {
     const wasConnected = this._connected;   // already false — cleanup ran first
     this.cleanup();
     if (wasConnected) { this.emit('close'); }   // skipped on every error-path close

   So the client suppresses its own 'close' emission exactly when it matters most.

4. session-manager.ts's client.on('close') handler (reconnect path, ~406-413) is the ONLY code that logs shellper disconnected unexpectedly and calls removeDeadSession. It never runs. The session stays in the map forever.

5. The 'error' the client DID emit is re-emitted by session-manager as 'session-error', which has no consumer in the Tower server layer — not logged, not acted on.

6. From then on, write() and resize() are silent no-ops by design: if (!this._connected || !this.socket) return; (lines ~274-282). The message router cannot distinguish delivered from dropped, so it logs "Message sent" for messages that were dropped.

Net: a transient one-shot socket error is converted into a permanent, invisible outage of one terminal, with success-shaped logging on top. There is no retry path in the running process; only a full Tower restart (which discards the poisoned in-memory client and re-runs adoption) clears it — and each restart is a fresh roll of the same dice for every persistent terminal.

Contributing race: afx tower stop does not wait

packages/codev/src/agent-farm/commands/tower.ts (~346-358): towerStop sends SIGTERM and returns immediately; there is no poll for process exit. towerStart tolerates ~1s of port-in-use. So afx tower stop && afx tower start guarantees an overlap window in which the new Tower's adoption pass (all 20 shellper connects within ~50ms of boot) runs while the old Tower is still tearing down its 20 connections. Each shellper briefly services a dying client and a new handshake concurrently — a plausible source of the transient post-handshake error. This is the likely trigger for today's instance (timing fits; exact errno not recoverable because nothing was logged).

Proposed fix (in priority order)

1. Fix the swallowed close. Emit 'close' based on a "was ever connected" flag that cleanup() does not clear, or drive the dead-session path from cleanup() itself. An error-path close must produce the same removal + logging as a clean disconnect.
2. Consume and log 'session-error' in the Tower layer. A reconnect that dies must leave a trace.
3. Make towerStop wait: SIGTERM, poll until the PID exits, escalate to SIGKILL after a timeout. stop && start should serialize, not overlap.
4. Make write() failure observable (return boolean or throw) so the message router stops logging "Message sent" for drops.
5. Optional hardening: the protocol already has PING/PONG; Tower could detect zombie sessions via missed heartbeats and re-run the reconnect for that session instead of waiting for a full restart.

Why this matters beyond one blank tab

Gate notifications, builder wake-ups, afx send, and all sibling-architect coordination ride this same PTY-injection path. The current failure mode is "silently degrade one terminal until the next full restart, which may silently degrade a different one", with the router attesting success throughout. #1194 (gate-event notifications) will increase reliance on this path.

Remediation for a currently-stuck terminal

Tower restart (discards the zombie client; adoption re-connects). Verify afterwards, do not trust appearances: every persistent terminal's GET /api/terminals/:id/output should be non-empty; any total=0 terminal is a fresh zombie (confirm via fd count on its shellper socket).

[amrmelsayed]

Fleet-wide sweep results (all 21 persistent terminals, same day)

Probed every terminal's output buffer, then behavior-tested all total=0 suspects (message injection for same-workspace, resize/SIGWINCH for other workspaces):

• 1 true zombie: shannon/app (no output after both message injection AND resize; missing live fd).
• 6 false positives: codev/demos, codev/reviewer, codev-ide/main, kidscanspell/main, MPPS2/main, MPPS2/mobile — all had total=0 but responded to injected input/resize immediately (buffer 0→1). All builders healthy (one at total=10001).

Refined diagnostic (the issue body's total=0 check alone overcounts):

1. GET /api/terminals/:id/output → total=0 marks a suspect, not a zombie.
2. Confirm by injecting input (afx send) or POSTing a resize (quieter; no conversation pollution), wait ~5s, re-check.
3. Still total=0 after both → zombie. Cross-check: shellper socket missing the live-Tower fd via lsof.

Secondary observation worth folding into the fix: reconnect does not reliably replay the shellper's screen buffer. Idle-but-healthy terminals can sit at total=0 indefinitely (demos/reviewer here), rendering blank in every viewer until something forces a repaint. That is a UX bug in its own right (a blank tab is indistinguishable from a zombie without probing) and amplifies this issue's severity: the one real zombie hid among six blank-but-fine terminals. A replay-on-reconnect (or synthetic SIGWINCH after adoption) would fix both the blank-until-poked UX and provide a natural liveness probe.

[amrmelsayed]

On it! Working on this with the PIR protocol (plan + dev-approval gates before PR).

[amrmelsayed]

On it! Working on this with the PIR protocol (plan + dev-approval gates before PR).