fix(cli/tui): split MRU resume + actual exit session - #16336
Conversation
- order session listing by computed last_active in SessionDB so callers get MRU rows directly - keep _resolve_last_session as a single-row lookup and add regression coverage for >20 session sampling
There was a problem hiding this comment.
Pull request overview
This PR fixes hermes -c session resolution to use true MRU semantics by having the state DB compute a last_active value per session (based on latest message timestamp, falling back to started_at) and ordering session queries by that value.
Changes:
- Update
SessionDB.search_sessions()to computelast_activeand order results by MRU. - Simplify
_resolve_last_session()to a single-row lookup (limit=1) that trusts DB ordering. - Add regression tests to ensure MRU selection works even when the true MRU falls outside the newest-started 20 sessions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/hermes_cli/test_resolve_last_session.py | Adds regression coverage for -c selecting the true MRU session, including the >20 sessions case. |
| hermes_state.py | Computes last_active in search_sessions() and sorts sessions by MRU. |
| hermes_cli/main.py | Changes -c resolution to fetch only the top MRU session from the DB. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
OutThisLife
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- use a grouped last_active join in search_sessions to avoid per-row correlated max lookups - always close SessionDB in _resolve_last_session via finally and add regression coverage for search failure cleanup
- create HERMES_TUI_ACTIVE_SESSION_FILE with mkstemp instead of a predictable tmp path and always cleanup in finally - add assertions that launch wiring uses a randomized session file path and removes it on exit
alt-glitch
added
type/bug
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Validate that the temp active-session file exists while the TUI subprocess runs and is removed after launch cleanup to match mkstemp semantics.
|
Merged via #16626 — your 6 commits were cherry-picked onto current main with your authorship fully preserved in git log. GitHub refused the direct rebase-merge on your branch because it contained a merge commit, so we did a clean cherry-pick onto a fresh branch to keep per-commit attribution. Thanks Brooklyn! |
Summary
SessionDB.search_sessions()compute and order bylast_active(MAX(messages.timestamp)fallbackstarted_at) so session queries are true MRU._resolve_last_session()as a single-row lookup (limit=1) instead of sampling and client-side sorting._read_tui_active_session_file,HERMES_TUI_ACTIVE_SESSION_FILE, andwriteActiveSessionFileso the shell epilogue reports the actual active session.Test plan
scripts/run_tests.sh tests/hermes_cli/test_resolve_last_session.py tests/hermes_cli/test_tui_resume_flow.pynpm test(inui-tui)