mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 08:03:58 +00:00
Neil
a907dd2ba2
* fix(pty): key buffered pre-attach exits on the PTY incarnation, not a clock A restarted SSH relay renumbers PTYs from pty-1, so a fresh spawn is routinely handed an id whose previous shell is still emitting a late exit. #16970 stopped that exit blanking the new tab by dating every buffered record and dropping anything older than the spawn request. That fence is a clock, so it cannot judge a stale exit that arrives AFTER the request left — the residual risk #16970 documented. Thread the incarnation main already puts on the pty:exit payload (and the pty:spawn reply) through preload to the pre-handler buffer, so a buffered exit names which lifetime of the id died. An exit disagreeing with the incarnation now attaching is discarded whenever it arrived. Only a positive disagreement discards: absence stays "unknown", never a mismatch, so hosts that predate the field keep #16970's behaviour exactly. The fence is retained for the two cases with no incarnation to compare — buffered bytes (pty:data carries none) and unnamed exits. No wire change: incarnationId was already published on the relay's pty.exit notification and pty.spawn reply, and already forwarded over the in-process pty:exit / pty:spawn IPC. Only the preload types and the renderer read it now. * fix(pty): read the incarnation through the shared guard, not truthiness A malformed incarnation is evidence of nothing, so it must read as "unknown" rather than as a value that disagrees with every well-formed one — otherwise a non-string on the payload would discard the very exits the buffer exists to deliver. Route both the record and the comparison through the existing isPtyIncarnationId guard. * refactor(pty): name the bounded-map helper after what it does It evicts the oldest entry when the map is full and the id is new; it reserves nothing. Rename only — no behaviour change. * fix(pty): key the buffered-exit STORAGE on the incarnation too, not just the check Review caught a swallowed exit. Keying only the comparison on the incarnation while the storage stayed one slot per pty id left the two races this buffer exists for able to cancel each other out: 1. the freshly spawned shell dies before the pane attaches -> its exit (X) is buffered; 2. the relay flushes the previous owner's exit for the same recycled id (W), which OVERWRITES X in the single slot; 3. the spawn reply names X, so the identity discard drops W -- the only record left. registerExit then finds nothing and the pane binds to a PTY that is dead and will never be reported dead: a hang instead of the blank tab #16970 fixed. Store one record per lifetime, capped at 4 per id, so W can never evict X. A duplicate exit for a lifetime replaces that lifetime's record rather than crowding out another's; drain still delivers the newest survivor, preserving the last-write-wins behaviour a single slot always had. * fix(pty): filter buffered exits by lifetime inside the buffer, not at call sites Review found the identity was enforced only where connectIpcPty calls the discard, while preHandlerPtyExit has several other consumers. The severe one is registerEagerPtyBuffer: both background launchers spawn directly and then drain whatever is buffered for the returned id, so a relay-recycled id holding the previous owner's exit tore a freshly launched agent session down seconds after it started -- no fence, no admitPtyId, no identity check at all. Move the rule into the buffer: every read goes through admissiblePreHandlerPtyExits, so a record proven to belong to another lifetime is unreachable by construction rather than because each caller remembered to discard first. hasPreHandlerPtyExit/drainPreHandlerPtyExit take the asking lifetime; registerEagerPtyBuffer and registerExit thread it through, and both background launchers pass the incarnation their own spawn returned. A reader that cannot name an incarnation still sees everything, which is the honest answer -- it holds no evidence to discriminate with. That keeps the pre-spawn fast path in connectIpcPty behaving exactly as it does today; see the PR for the consumers this still does not cover. * chore: drop unrelated formatter churn in reliability-gates.jsonc Repo-wide oxfmt reindented pre-existing entries in a file this change never touches. Keep the diff to the PTY incarnation work.