fix(cli-mode): make the CLI session reaper actually reap#18
Open
miguelrisero wants to merge 1 commit into
Open
fix(cli-mode): make the CLI session reaper actually reap#18miguelrisero wants to merge 1 commit into
miguelrisero wants to merge 1 commit into
Conversation
The periodic reaper shipped in #17 never killed anything in production. Its pre-kill TOCTOU recheck used `tmux display-message -p -t =<session> -F '#{session_attached}\t#{session_activity}'`, but display-message resolves formats in a client/pane context and returns EMPTY for those session-scoped fields. The recheck therefore failed to parse, returned None, and the reaper treated every session as "already gone" and skipped the kill. Net effect: orphaned / archived / idle>48h sessions were never reaped (verified live — a 22h orphan and a 71h-idle session survived ~44 reap cycles). Fix: implement cli_tmux_session_liveness via the existing `list-sessions` path, which populates `#{session_attached}` / `#{session_activity}` correctly and already no-ops cleanly when tmux is absent, instead of display-message. Add parse_cli_session_line regression tests covering the empty-field failure mode. kill-on-delete, kill-on-archive, and attached-skip were unaffected and worked.
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.
Summary
Follow-up to #17. The periodic CLI-session reaper shipped there but never reaped anything in
production — a bug in the very TOCTOU recheck that was added to harden it.
Root cause
The pre-kill recheck
cli_tmux_session_livenessused:display-messageresolves formats in a client/pane context and returns those session-scopedfields empty (verified live: output is
|for every session, whereaslist-sessionsreturnsreal values). The empty
session_activityfailed to parse → the recheck returnedNone→ thereaper treats
Noneas "session already gone" and skips the kill. So every candidate wasskipped, on every 30-min cycle.
Live evidence: with the #17 build running ~22h, a 22h orphan (workspace deleted), a detached
archived session, and a 71h-idle session were all still alive — ~44 reap cycles, zero kills.
Fix
cli_tmux_session_livenessvia the existinglist_cli_tmux_sessions()(list-sessions)path, which populates the fields correctly and already no-ops when tmux is absent.
parse_cli_session_lineregression tests, including the empty-field case that caused this.Unaffected and confirmed working: kill-on-delete, kill-on-archive, attached-skip, classification.
Test plan
cargo check -p local-deployment,cargo clippy -p local-deployment— green.cargo test -p local-deployment --lib parse_cli_session_line— green.by hand for immediate relief; this PR makes the periodic reaper do it automatically.
Deploy note
Fork CI is dormant; verified locally.
/optneeds a rebuild + restart to pick this up.