fix(tui): end_session on finalize to prevent ghost sessions in /resume (#18269)#18299
Closed
Bartok9 wants to merge 1 commit into
Closed
fix(tui): end_session on finalize to prevent ghost sessions in /resume (#18269)#18299Bartok9 wants to merge 1 commit into
Bartok9 wants to merge 1 commit into
Conversation
NousResearch#18269) TUI sessions were persisted to the DB via db.create_session() inside _start_agent_build/_build() but db.end_session() was never called when the TUI session closed. All three close paths — session.close RPC, Ctrl+C (atexit _shutdown_sessions), and /quit — called _finalize_session which committed memory and fired hooks but never marked the session as ended. The result: every session row accumulated with ended_at=NULL, appearing as resumable entries in /resume. Fix: add db.end_session(key, end_reason) to _finalize_session(). This is the correct mirror of how CLI handles session lifecycle (cli.py calls end_session on /new and on quit). _finalize_session already receives the session dict which carries session_key; we simply call end_session on that key at the end. An end_reason argument (default 'tui_close') is added so callers can pass a more specific reason when available. Internally _shutdown_sessions and session.close both use the default; future callers (e.g. session.resume ending the old session) can pass a distinct reason for audit clarity. Closes NousResearch#18269
Collaborator
Collaborator
|
Likely duplicate of #18283 |
Collaborator
Contributor
Author
|
Thanks @alt-glitch — confirmed superseded by #18370 which includes this fix with credit. Closing to keep the board clean. |
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
2 participants
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.
Problem
TUI sessions were eagerly persisted to
state.dbviadb.create_session()inside_start_agent_build()/_build(), butdb.end_session()was never called when a TUI session closed. All three close paths converge on_finalize_session():/quitsession.closeRPC →_finalize_session(session)atexit→_shutdown_sessions()→_finalize_session(session)/new(new session within TUI)_finalize_session()committed memory and fired hooks but never calleddb.end_session(). Every session row therefore accumulated withended_at = NULLand appeared as a resumable entry in/resume.On one user's install: 24 out of 52 TUI sessions (46%) were ghost rows with
message_count = 0.Fix
Add
db.end_session(key, end_reason)at the end of_finalize_session():This mirrors how the CLI handles session lifecycle —
cli.pyalready callsend_session()on/newand quit. A defaultend_reason="tui_close"is added so the audit trail matches CLI conventions.No risk to existing sessions
db.end_session()is a no-op whenended_atis already set (seehermes_state.py:547):So sessions that are ended through other paths (e.g. compression,
/branch) won't be double-ended.Verification
Closes #18269