fix(escrow): release a session when the user closes it for good - #291
Merged
Conversation
Closing a tab or a workspace left its shell running. The escrow holder keeps a dup of the pty master, so freeing the app-side surface never hangs up the terminal: the shell, and whatever agent was running in it, survived with nothing referencing it. Measured live: three orphaned Claude Code sessions, one of them 24 hours old, still polling the app's socket and being refused because they no longer belong to any window. The protocol had no way to say this. Its three frame types cover handing a session over, asking for it back, and the reply -- nothing means "this one is finished". `unclaimedSessionTTL` does not cover it either: that only starts once a session is draining, which happens when the owning app dies. A session closed while the app keeps running was bounded by nothing at all. Adds frame 0x05, sent from the app when a surface tears down for good; the holder authenticates it by token, closes its pty master, and drops the session, so the shell finally sees SIGHUP. Gated on two conditions, because getting this wrong destroys live work: - only from `teardownSurface()`, which is the path `ClosedTerminalUndoStore` runs on `finalize` -- so the close-undo grace period has already elapsed and nothing can restore the surface. `deinit` is excluded. - never while the app is terminating. Sessions open at quit must stay escrowed; releasing them there would kill every running agent on every update, which is the exact regression escrow exists to prevent. The second condition needed a flag the session machinery can read, so `AppDelegate`'s private termination state is mirrored onto `SessionMachineryGate` and reset in lockstep when a quit is cancelled. Holders predating this frame land in `serve`'s `default` branch, which logs and skips, degrading to the previous behavior.
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
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.
What this does
Closing a terminal tab or a workspace now actually ends what was running in it.
Until now it didn't. The shell survived the tab disappearing, along with any agent inside it: invisible to the app, still holding memory, and unable to talk back over the socket. On this machine that had accumulated three orphaned Claude Code sessions, one of them 24 hours old, all still polling and being refused.
Quitting the app is unchanged and still preserves sessions, so they come back on the next launch.
Why it happened
The escrow holder keeps a dup of the pty master. Freeing the app-side surface closes only the app's copy, so the slave never sees SIGHUP and the shell keeps running.
The protocol had no way to say otherwise. Its three frame types cover handing a session over (
escrowType), asking for it back (retrieveRequestType), and the reply — nothing means "this one is finished", and nothing in the teardown path contacted escrow at all.unclaimedSessionTTLdoesn't cover it either. That timer only starts once a session is draining, which happens when the owning app dies. A session closed while the app keeps running was bounded by nothing at all — it would be held until the holder exited.Summary
0x05(releaseType): session id + token, no ancillary fd, no response.TerminalSurfacekeeps the token from escrow so a close can authenticate.The gate
Getting this wrong destroys live work, so it requires both:
reason == "teardown"— the pathClosedTerminalUndoStore'sfinalizeruns, so the close-undo grace period has already elapsed and nothing can restore the surface.deinitis excluded: deallocation isn't necessarily a user-visible close.Condition 2 needed a flag the session machinery can read, so
AppDelegate's private termination state is mirrored ontoSessionMachineryGate, reset in lockstep when a quit is cancelled (otherwise a declined quit would disable release for the rest of the session).Compatibility
Holders predating this frame fall into
serve'sdefaultbranch, which closes any stray fd, logs, and skips — degrading to the previous behavior. This matters because the holder outlives the app and routinely talks to a client from a newer build.Test plan
SessionEscrowReattachRegressionTests— 3 new cases pass (frame round-trip, malformed input rejected, the release gate truth table including the quit case)pgrep -f claude), andescrow.release ... outcome=releasedappears in~/Library/Logs/Programa/diagnostics.logRelated
Complementary to #239, not a duplicate. That issue covers reconciling after a crash or quit, where abandonment has to be inferred from claims. A user closing a tab is explicit and unambiguous, so it needs no claim protocol.