fix(sessions): pty list must not reap a live session on a transient pid read - #117
Merged
Conversation
…id read Under concurrent multi-agent load, listSessions could destroy a LIVE session's socket/pid when readPid transiently returned null: the daemon creates its .sock (listen) BEFORE writing its .pid (server.ts), and the non-atomic pidfile write can be caught mid-flight, so a `pty list` landing in that window read a null pid and ran cleanupSocket — making the still-running daemon invisible, so consumers that reconcile on not-running (st2, convoy) GC'd + re-launched a healthy session. Even a reachable socket didn't save it (alive required pid !== null). Fix: destruction now requires POSITIVE proof of death — a readable pid whose process is gone AND an unreachable socket. A reachable control socket alone proves the daemon is alive (-> running); an unreadable pid + unreachable socket is indeterminate (mid-startup / raced write) -> reported running defensively, not destroyed. Fixes it at the source for every list/--json consumer. Test: tests/list-live-session-race.test.ts — a live session with a transiently missing pidfile stays running + its socket is kept; a genuinely-dead (SIGKILLed) session's stale socket is still reaped and it stays addressable as vanished.
myobie
added a commit
that referenced
this pull request
Jul 22, 2026
…s life (#118) `listSessions` computed `socketReachable` unconditionally, so `pty list` did a wasted per-session socket connect for every live-pid entry — the probe result is ignored (running either way) and it adds exactly the socket-connect contention that gets flaky under concurrent multi-agent list load. Probe the control socket ONLY when the pid doesn't already prove life (dead-looking / unreadable), where it is the deciding signal (the #117 reachable-socket rescue). Behavior-preserving: full suite green (1417 passed), typecheck clean. Mirrors the pty-rust port's approach (4fba12f). Genuine defense-in-depth for the hot list path under the concurrent multi-agent load #117 was about.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pty listno longer reaps a live session on a transient pid readBug (found in st2 eval work, reported by CoS): under concurrent multi-agent
load,
pty list/--jsoncould transiently misreport a LIVE session asnot-running and even destroy it — a consumer that reconciles on not-running
(st2, convoy) then GCs a healthy session. Confirmed empirically: a live
session's pidfile went missing after a spurious GC.
Root cause (a destructive read):
listSessionsinsrc/sessions.tsdetermined liveness as
pid !== null && isProcessAlive(pid) && socketReachable,and its
elsebranch rancleanupSocket(name)— unlinking the daemon's.sockAND
.pid. So wheneverreadPidtransiently returned null, a live sessionwas dropped from the list AND had its control socket reaped out from under it,
making the still-running daemon invisible → re-launched by consumers.
readPidreturns null for a healthy session in a real window: the daemon createsits
.sock(listen) before it writes its.pid(server.ts:550-552), andthe plain (non-atomic) pidfile write can be caught mid-flight. Under concurrent
load a
pty listlands in that window. Worse: even a reachable socket (proofthe daemon is alive) didn't save it, because
aliverequiredpid !== null.Fix: destruction now requires positive proof of death — a readable pid
whose process is gone AND an unreachable socket. A reachable control socket alone
proves the daemon is alive (→ running). An unreadable pid + unreachable socket is
treated as indeterminate (most likely mid-startup / raced write) → reported
running defensively, NOT destroyed; a later read resolves it. This fixes it at
the SOURCE for every
list/--jsonconsumer.Tests:
tests/list-live-session-race.test.ts— (1) a live session with atransiently-missing pidfile stays
runningand its socket is NOT reaped;(2) a genuinely dead (SIGKILLed) session's stale socket IS still reaped
(positive-proof path preserved) and it stays addressable as
vanished.Full suite green; typecheck clean. Flagging to pty-rust so rust's list read
doesn't inherit the same reap-on-unreadable-pid behavior.