Retain detached shells for 15 minutes - #35
Merged
Merged
Conversation
Product decision from Nathan, and it matches what the measurements supported. Sixty seconds did not survive a coffee break. A held remote shell now survives a closed lid over lunch; a laptop left overnight still does not, which is the honest boundary rather than a promise the design cannot keep. The number comes from measured cost. An idle detached session buffers 0 bytes across a full detached window, so holding one costs a session struct and a PTY process and nothing that grows with time. A session still producing output is the expensive case and grows at whatever the remote writes: a pathological output loop measured about 19 KB/s, so roughly 17 MB over this window for one runaway shell. Both figures were taken from two real daemons rather than estimated. Behaviour past the window is unchanged and already proven: the client reports 'remote shell could not resume', names the expired session, and exits non-zero, so a dead session ends promptly instead of hanging. A daemon restart is the same case because the session store is in memory. The existing expiry proof forces expiry rather than waiting for it, so it tests the same thing at any retention value. A test now pins the value itself. A default that drifts silently is worse than one that is wrong on purpose: this is what a user's held shell survives, so changing it should require changing an assertion and saying why. The daemon override test now uses a non-default 120 to keep exercising the override path. Documented in the README and CHANGELOG with the cost figures and with the caveat that this TTL is still the only backstop against a runaway remote process, since the replay buffer has no cap of its own. That is the thing to fix before raising this much further.
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.
Product decision from Nathan; matches the measured recommendation.
Sixty seconds did not survive a coffee break. A held shell now survives a closed lid over lunch; overnight still does not, which is the honest boundary.
The number is measured, not chosen round. An idle detached session buffers 0 bytes across a full window, so holding one costs a session struct and a PTY process and nothing that grows with time. A session still producing output grows at whatever the remote writes — about 19 KB/s for a pathological loop, so roughly 17 MB over this window for one runaway shell. Both figures came from two real daemons.
Past the window is unchanged and already proven: the client reports
remote shell could not resume, names the expired session, and exits non-zero. Daemon restart is the same case (in-memory store). The existing expiry proof forces expiry rather than waiting, so it tests the same property at any retention value.A test pins the value itself — a default that drifts silently is worse than one that is wrong on purpose. The daemon override test now uses a non-default 120 so it keeps exercising the override path.
Caveat carried into README and CHANGELOG: this TTL is still the only backstop against a runaway remote process, because the replay buffer has no cap of its own. That is the thing to fix before raising this further.