From 4b281409123490a2f10685c0fd8214258bcc4bd9 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Apr 2026 22:49:49 -0500 Subject: [PATCH] fix(cli): tighten MRU lookup and session DB cleanup - 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 --- hermes_cli/main.py | 8 +++++++- hermes_state.py | 19 +++++++++++-------- tests/hermes_cli/test_resolve_last_session.py | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index cb877fe60..cb1ad1596 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -597,15 +597,21 @@ def _session_browse_picker(sessions: list) -> Optional[str]: def _resolve_last_session(source: str = "cli") -> Optional[str]: """Look up the most recently-used session ID for a source.""" + db = None try: from hermes_state import SessionDB db = SessionDB() sessions = db.search_sessions(source=source, limit=1) - db.close() return sessions[0]["id"] if sessions else None except Exception: pass + finally: + if db is not None: + try: + db.close() + except Exception: + pass return None diff --git a/hermes_state.py b/hermes_state.py index 226c44e71..bfa36d599 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1487,22 +1487,25 @@ class SessionDB: message timestamp for the session, falling back to ``started_at``), ordered by most-recently-used first. """ - select_last_active = ( - "COALESCE(" - "(SELECT MAX(m.timestamp) FROM messages m WHERE m.session_id = s.id)," - " s.started_at" - ") AS last_active" + select_with_last_active = ( + "SELECT s.*, COALESCE(m.last_active, s.started_at) AS last_active " + "FROM sessions s " + "LEFT JOIN (" + "SELECT session_id, MAX(timestamp) AS last_active " + "FROM messages GROUP BY session_id" + ") m ON m.session_id = s.id " ) with self._lock: if source: cursor = self._conn.execute( - f"SELECT s.*, {select_last_active} FROM sessions s " - "WHERE s.source = ? ORDER BY last_active DESC, s.started_at DESC, s.id DESC LIMIT ? OFFSET ?", + f"{select_with_last_active}" + "WHERE s.source = ? " + "ORDER BY last_active DESC, s.started_at DESC, s.id DESC LIMIT ? OFFSET ?", (source, limit, offset), ) else: cursor = self._conn.execute( - f"SELECT s.*, {select_last_active} FROM sessions s " + f"{select_with_last_active}" "ORDER BY last_active DESC, s.started_at DESC, s.id DESC LIMIT ? OFFSET ?", (limit, offset), ) diff --git a/tests/hermes_cli/test_resolve_last_session.py b/tests/hermes_cli/test_resolve_last_session.py index 4ba54003e..1a82d1a79 100644 --- a/tests/hermes_cli/test_resolve_last_session.py +++ b/tests/hermes_cli/test_resolve_last_session.py @@ -89,6 +89,24 @@ def test_resolve_last_session_returns_none_when_empty(monkeypatch): assert _resolve_last_session("cli") is None +def test_resolve_last_session_closes_db_on_search_error(monkeypatch): + class _FailingDB: + def __init__(self): + self.closed = False + + def search_sessions(self, source=None, limit=20, **_kw): + raise RuntimeError("boom") + + def close(self): + self.closed = True + + db = _FailingDB() + monkeypatch.setattr("hermes_state.SessionDB", lambda: db) + + assert _resolve_last_session("cli") is None + assert db.closed is True + + def test_resolve_last_session_falls_back_to_started_at(monkeypatch): # When last_active is missing entirely (legacy row), fall back to # started_at so the helper still picks the newest session.