From 330cd932ff15b706bc07b07569126143159464a4 Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Thu, 27 Aug 2026 08:05:23 +0700 Subject: [PATCH 1/2] fix(tui): keep Desktop sessions open across gateway reload Process-exit teardown was ending every live chat with tui_shutdown, and a WS 1012 service restart still armed the orphan reap. Neither path is a user delete, so populated Desktop rows must stay resumable after a profile-switch backend bounce. Log hard-deletes with a reason so a silent row loss is greppable. --- hermes_cli/web_routers/sessions.py | 2 +- hermes_state.py | 34 ++++++++++++++++++++++++++-- hermes_state_common.py | 6 +++++ tui_gateway/methods_session.py | 4 +++- tui_gateway/server.py | 36 +++++++++++++++++++++++++----- tui_gateway/ws.py | 29 ++++++++++++++++++++++-- 6 files changed, 100 insertions(+), 11 deletions(-) diff --git a/hermes_cli/web_routers/sessions.py b/hermes_cli/web_routers/sessions.py index 32ab0ee8cba75..d93228542949c 100644 --- a/hermes_cli/web_routers/sessions.py +++ b/hermes_cli/web_routers/sessions.py @@ -694,7 +694,7 @@ def _delete(): sid = db.resolve_session_id(session_id) if not sid: return {"ok": True, "already_absent": True} - db.delete_session(sid) + db.delete_session(sid, reason="api.sessions.delete") return {"ok": True} finally: db.close() diff --git a/hermes_state.py b/hermes_state.py index e5e35523fdaa4..31fdd7db3549a 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -12952,6 +12952,8 @@ def delete_session( session_id: str, sessions_dir: Optional[Path] = None, expected_delete_ids: Optional[List[str]] = None, + *, + reason: str = "", ) -> bool: """Delete a session and all its messages. @@ -12968,18 +12970,29 @@ def delete_session( inside the write transaction on purpose (TOCTOU guard); the cost is accepted for correctness. Returns True if the session was found and deleted. + + ``reason`` is an audit tag (``session.delete``, ``api.sessions.delete``, + …) logged with the id and whether the row had messages, so a silent + hard-delete is greppable after the fact (#95868). """ removed_delegate_ids: List[str] = [] expected_ids = ( set(expected_delete_ids) if expected_delete_ids is not None else None ) + stats: Dict[str, Any] = {"messages": None, "title_set": False} def _do(conn): cursor = conn.execute( - "SELECT 1 FROM sessions WHERE id = ? LIMIT 1", (session_id,) + "SELECT title, " + "(SELECT COUNT(*) FROM messages WHERE session_id = sessions.id) " + "FROM sessions WHERE id = ? LIMIT 1", + (session_id,), ) - if cursor.fetchone() is None: + row = cursor.fetchone() + if row is None: return False + stats["title_set"] = bool(row[0]) + stats["messages"] = int(row[1] or 0) if expected_ids is not None: actual_ids = { session_id, @@ -13001,6 +13014,13 @@ def _do(conn): deleted = self._execute_write(_do) if deleted: + logger.info( + "delete_session id=%s reason=%s messages=%s title=%s", + session_id, + reason or "-", + stats["messages"], + "set" if stats["title_set"] else "null", + ) for delegate_id in removed_delegate_ids: self._remove_session_files(sessions_dir, delegate_id) self._remove_session_files(sessions_dir, session_id) @@ -13010,6 +13030,8 @@ def delete_session_if_empty( self, session_id: str, sessions_dir: Optional[Path] = None, + *, + reason: str = "", ) -> bool: """Delete *session_id* only when it never gained resumable content. @@ -13024,6 +13046,9 @@ def delete_session_if_empty( children (delegate subagent runs) are preserved — a parent that spawned work is not "empty" even if its own transcript never flushed. Returns True if the session was deleted. + + ``reason`` is an audit tag logged when a row is actually removed + (#95868). """ def _do(conn): cursor = conn.execute( @@ -13047,6 +13072,11 @@ def _do(conn): deleted = self._execute_write(_do) if deleted: + logger.info( + "delete_session_if_empty id=%s reason=%s", + session_id, + reason or "-", + ) self._remove_session_files(sessions_dir, session_id) return bool(deleted) diff --git a/hermes_state_common.py b/hermes_state_common.py index c224b8202c856..86e9e715839b3 100644 --- a/hermes_state_common.py +++ b/hermes_state_common.py @@ -221,6 +221,12 @@ def _shape_preview(raw: Any) -> str: # row was closed at the next boot instead. Same accident class as # ws_orphan_reap — kept distinct for forensics — and equally resumable. "startup_orphan_reap", + # Process-exit teardown of the TUI/desktop gateway (atexit + # ``_shutdown_sessions``): a backend bounce, profile switch, or WS 1012 + # service restart is the same accident class as ``ws_orphan_reap``. + # Current code no longer stamps this on populated rows (#95868), but + # rows ended by older builds stay resumable. + "tui_shutdown", ) _RECOVERABLE_END_REASONS_SQL = ", ".join(f"'{reason}'" for reason in _RECOVERABLE_END_REASONS) diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index b82c4df717535..c11f339c593d7 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -1283,7 +1283,9 @@ def _(rid, params: dict) -> dict: else: sessions_dir = get_hermes_home() / "sessions" try: - deleted = db.delete_session(target, sessions_dir=sessions_dir) + deleted = db.delete_session( + target, sessions_dir=sessions_dir, reason="session.delete" + ) except Exception as e: return _err(rid, 5036, f"delete failed: {e}") if not deleted: diff --git a/tui_gateway/server.py b/tui_gateway/server.py index e6a669af62890..3b327490f0902 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -882,7 +882,18 @@ def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> No source = (row or {}).get("source", "") _tui_owns_lifecycle = not _is_gateway_owned_source(source) if _tui_owns_lifecycle: - db.end_session(session_id, end_reason) + # Keep-open is for an existing durable row only. + # A missing row cannot be gateway-owned (#60609) and + # still gets the pre-existing end_session reap. + if row is not None and end_reason in _KEEP_OPEN_END_REASONS: + logger.info( + "leaving session open through process teardown " + "sid=%s reason=%s", + session_id, + end_reason, + ) + else: + db.end_session(session_id, end_reason) except Exception: pass @@ -938,6 +949,15 @@ def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> No # deliberately absent: the client initiated those and already knows. _RECLAIM_END_REASONS = frozenset({"idle_timeout", "lru_evict", "ws_orphan_reap"}) +# Process-exit teardown is not a conversation end. atexit ``_shutdown_sessions`` +# used to ``end_session(..., "tui_shutdown")`` for every live Desktop chat when +# the backend bounced (profile switch, ``hermes serve`` reload, WS 1012). Those +# rows then vanished from the user's session list — ``tui_shutdown`` was not +# recoverable, and a raced persist left ``min_messages=1`` listings empty +# (#95868). Persist still runs in ``_finalize_session``; skip the durable close +# so the next process can resume. +_KEEP_OPEN_END_REASONS = frozenset({"tui_shutdown"}) + def _announce_session_reclaimed(session: dict, end_reason: str) -> None: """Tell connected clients a session was reclaimed out from under them. @@ -1385,6 +1405,11 @@ def _close_sessions_for_transport( the single WS-disconnect teardown entry point — there is no second independent reap loop in ``handle_ws``. + ``end_reason="ws_service_restart"`` (WS 1001/1012) parks without arming + the orphan reap: the backend is bouncing and atexit ``_shutdown_sessions`` + will persist in-memory state. Reaping here raced process exit and ended + durable Desktop chats that the next process should resume (#95868). + Returns ``(reaped, detached)`` counts for disconnect-path observability.""" with _sessions_lock: owned = [(sid, s) for sid, s in _sessions.items() if s.get("transport") is transport] @@ -1433,10 +1458,11 @@ def _close_sessions_for_transport( session["transport"] = _detached_ws_transport session.pop("_client_gone_interrupt_requested", None) detached += 1 - try: - _schedule_ws_orphan_reap(sid) - except Exception: - pass + if end_reason != "ws_service_restart": + try: + _schedule_ws_orphan_reap(sid) + except Exception: + pass return reaped, detached diff --git a/tui_gateway/ws.py b/tui_gateway/ws.py index ce0cbc70215a6..4730ef86ade44 100644 --- a/tui_gateway/ws.py +++ b/tui_gateway/ws.py @@ -43,6 +43,12 @@ async def ws(ws: WebSocket): _WS_WRITE_TIMEOUT_S = 10.0 _WS_LOG_PAYLOAD_PREVIEW = 240 +# RFC 6455 close codes that mean the *server* is going away, not that the user +# dropped the chat. 1001 Going Away / 1012 Service Restart fire on profile +# switch, ``hermes serve`` reload, and uvicorn worker recycle. Those sessions +# must be parked for resume, not orphan-reaped (#95868). +_WS_SERVICE_RESTART_CODES = frozenset({1001, 1012}) + # Per-token streaming frames are coalesced: buffered and flushed as a batch on # a short timer instead of waking the event loop once per token. A model reply # emits hundreds of these in a burst, and each one is a loop wakeup competing @@ -288,6 +294,23 @@ def _ws_peer_label(ws: Any) -> str: return f"{host}:{port}" if port is not None else host +def teardown_end_reason_for_ws_close(close_code) -> str: + """Map a WebSocket close code to the transport-teardown ``end_reason``. + + 1001 (Going Away) and 1012 (Service Restart) mean the backend is bouncing. + Park those sessions without arming the WS-orphan reap so atexit persist + plus the next process's ``session.resume`` can recover them (#95868). + Any other / missing code keeps the normal disconnect→reap path. + """ + try: + code = int(close_code) + except (TypeError, ValueError): + return "ws_disconnect" + if code in _WS_SERVICE_RESTART_CODES: + return "ws_service_restart" + return "ws_disconnect" + + def _disable_nagle(ws: Any) -> None: """Disable Nagle so streamed JSON-RPC frames go out individually. @@ -339,6 +362,7 @@ async def handle_ws( dispatch_crashes = 0 send_failures = 0 disconnect_reason = "not_connected" + ws_close_code = None try: if subprotocol: @@ -412,9 +436,10 @@ async def handle_ws( try: raw = await ws.receive_text() except _WebSocketDisconnect as exc: + ws_close_code = getattr(exc, "code", None) disconnect_reason = ( "client_disconnect(" - f"code={getattr(exc, 'code', None)}," + f"code={ws_close_code}," f"reason={getattr(exc, 'reason', None)})" ) break @@ -565,7 +590,7 @@ async def handle_ws( reaped_sessions, detached_sessions = await asyncio.to_thread( server._close_sessions_for_transport, transport, - end_reason="ws_disconnect", + end_reason=teardown_end_reason_for_ws_close(ws_close_code), ) except Exception: _log.exception("ws transport teardown failed peer=%s", peer) From 159fa0909d9fbc43c881a3759cc955b6ea9aba98 Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Thu, 27 Aug 2026 08:05:23 +0700 Subject: [PATCH 2/2] test(tui): cover reload keep-open and delete_session audit logs --- tests/test_empty_session_hygiene.py | 28 ++++++++ tests/test_tui_gateway_server.py | 49 +++++++++++-- tests/test_tui_gateway_ws.py | 18 +++++ .../test_finalize_session_persist.py | 69 +++++++++++++++++++ .../test_gateway_owned_session_reap.py | 11 +++ .../tui_gateway/test_startup_orphan_sweep.py | 2 + 6 files changed, 172 insertions(+), 5 deletions(-) diff --git a/tests/test_empty_session_hygiene.py b/tests/test_empty_session_hygiene.py index 4e4b575d740a6..56027808338b3 100644 --- a/tests/test_empty_session_hygiene.py +++ b/tests/test_empty_session_hygiene.py @@ -79,6 +79,34 @@ def test_no_file_cleanup_when_kept(self, db, tmp_path): assert (sessions_dir / "busy.json").exists() + def test_delete_session_logs_reason_and_message_count(self, db, caplog): + """Hard-deletes must be greppable after the fact (#95868).""" + db.create_session(session_id="logged", source="desktop", model="test") + db.append_message("logged", role="user", content="keep me") + db.set_session_title("logged", "Working chat") + + with caplog.at_level("INFO", logger="hermes_state"): + assert db.delete_session("logged", reason="session.delete") is True + + joined = "\n".join(caplog.messages) + assert "delete_session id=logged" in joined + assert "reason=session.delete" in joined + assert "messages=1" in joined + assert "title=set" in joined + assert db.get_session("logged") is None + + + def test_delete_session_if_empty_logs_reason(self, db, caplog): + db.create_session(session_id="draft", source="cli", model="test") + + with caplog.at_level("INFO", logger="hermes_state"): + assert db.delete_session_if_empty("draft", reason="cli_exit") is True + + joined = "\n".join(caplog.messages) + assert "delete_session_if_empty id=draft" in joined + assert "reason=cli_exit" in joined + + class TestCLIDiscardSessionIfEmpty: """Wiring tests for HermesCLI._discard_session_if_empty.""" diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 64d9157c16689..b3a89d8fcfe10 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -14082,7 +14082,7 @@ def test_session_delete_refuses_active_session(monkeypatch): called: list[str] = [] class _DB: - def delete_session(self, sid, sessions_dir=None): + def delete_session(self, sid, sessions_dir=None, **_kw): called.append(sid) return True @@ -14133,7 +14133,7 @@ def values(self): def test_session_delete_returns_4007_when_missing(monkeypatch): class _DB: - def delete_session(self, sid, sessions_dir=None): + def delete_session(self, sid, sessions_dir=None, **_kw): return False monkeypatch.setattr(server, "_get_db", lambda: _DB()) @@ -14148,7 +14148,7 @@ def delete_session(self, sid, sessions_dir=None): def test_session_delete_propagates_db_exception(monkeypatch): class _DB: - def delete_session(self, sid, sessions_dir=None): + def delete_session(self, sid, sessions_dir=None, **_kw): raise RuntimeError("disk full") monkeypatch.setattr(server, "_get_db", lambda: _DB()) @@ -14169,7 +14169,7 @@ def test_session_delete_success_returns_deleted_id(monkeypatch): captured: dict = {} class _DB: - def delete_session(self, sid, sessions_dir=None): + def delete_session(self, sid, sessions_dir=None, **_kw): captured["sid"] = sid captured["sessions_dir"] = sessions_dir return True @@ -14323,7 +14323,7 @@ class ProfileDB: def __init__(self, db_path=None): captured["db_path"] = db_path - def delete_session(self, sid, sessions_dir=None): + def delete_session(self, sid, sessions_dir=None, **_kw): captured["sid"] = sid captured["sessions_dir"] = sessions_dir return True @@ -17725,6 +17725,45 @@ def test_close_sessions_for_transport_closes_flagged_repoints_rest(monkeypatch): server._sessions.clear() +def test_close_sessions_for_transport_parks_without_reap_on_service_restart( + monkeypatch, +): + """WS 1012 / service restart must park Desktop sessions, not orphan-reap. + + The reporter's ``reaped_sessions=0 detached_sessions=7`` line is this + path: close_on_disconnect is off, so sessions are detached. Arming the + 20s orphan reap raced atexit ``_shutdown_sessions`` on a dying backend + and ended durable chats the next process should resume (#95868). + """ + reaps = [] + closed = [] + monkeypatch.setattr( + server, "_schedule_ws_orphan_reap", lambda sid: reaps.append(sid) + ) + monkeypatch.setattr( + server, + "_close_session_by_id", + lambda sid, *, end_reason: closed.append((sid, end_reason)) or True, + ) + transport = object() + server._sessions.clear() + server._sessions["chat"] = { + "transport": transport, + "close_on_disconnect": False, + } + try: + reaped, detached = server._close_sessions_for_transport( + transport, end_reason="ws_service_restart" + ) + assert reaped == 0 + assert detached == 1 + assert closed == [] + assert reaps == [] + assert server._sessions["chat"]["transport"] is server._detached_ws_transport + finally: + server._sessions.clear() + + def test_close_sessions_for_transport_skips_rebound_session(monkeypatch): """Rebind-between-snapshot-and-stomp (#77129 concept salvage). diff --git a/tests/test_tui_gateway_ws.py b/tests/test_tui_gateway_ws.py index 7dbebed65c100..9ce9816f32654 100644 --- a/tests/test_tui_gateway_ws.py +++ b/tests/test_tui_gateway_ws.py @@ -4,6 +4,8 @@ import threading import time +import pytest + from hermes_cli import mcp_startup from tui_gateway import server from tui_gateway import ws as ws_mod @@ -272,3 +274,19 @@ async def send_second(): asyncio.run(scenario()) +@pytest.mark.parametrize( + ("code", "expected"), + [ + (1012, "ws_service_restart"), + (1001, "ws_service_restart"), + (1000, "ws_disconnect"), + (1006, "ws_disconnect"), + (None, "ws_disconnect"), + ("", "ws_disconnect"), + ], +) +def test_teardown_end_reason_for_ws_close(code, expected): + """Service-restart close codes park sessions; other codes still reap.""" + assert ws_mod.teardown_end_reason_for_ws_close(code) == expected + + diff --git a/tests/tui_gateway/test_finalize_session_persist.py b/tests/tui_gateway/test_finalize_session_persist.py index 13dd73bfc9128..8e1196ca61f46 100644 --- a/tests/tui_gateway/test_finalize_session_persist.py +++ b/tests/tui_gateway/test_finalize_session_persist.py @@ -148,6 +148,32 @@ def test_db_end_session_still_called(self, mock_get_db): mock_db.end_session.assert_called_once_with("sess_123", "test") + @patch("tui_gateway.server._get_db") + def test_tui_shutdown_does_not_end_or_delete_session(self, mock_get_db): + """Process exit (#95868) must not close or hard-delete a durable row. + + atexit ``_shutdown_sessions`` finalizes every live Desktop chat with + ``tui_shutdown`` when the backend bounces. Ending that row made the + chat vanish from the sidebar after reload; deleting it would match + the reporter's empty ``SELECT id FROM sessions``. Persist still + runs; the row stays open for the next process to resume. + """ + from tui_gateway.server import _finalize_session + + mock_db = MagicMock() + mock_db.get_session.return_value = {"id": "sess_123", "source": "desktop"} + mock_get_db.return_value = mock_db + + agent = _make_agent(session_id="sess_123") + session = _make_session(agent=agent, history=[{"role": "user", "content": "x"}]) + + _finalize_session(session, end_reason="tui_shutdown") + + mock_db.end_session.assert_not_called() + mock_db.delete_session.assert_not_called() + mock_db.delete_session_if_empty.assert_not_called() + + class TestFinalizeSessionPersistE2E: """End-to-end: _finalize_session must actually land unflushed turns in state.db on disconnect/restart. @@ -247,6 +273,49 @@ def test_resumed_session_not_reflushed_as_duplicates(self, tmp_path, monkeypatch assert len(after) == 2, after + def test_tui_shutdown_keeps_populated_desktop_row_open(self, tmp_path, monkeypatch): + """A populated Desktop chat must survive process teardown (#95868). + + Profile switch / gateway reload closes the WS with 1012, then atexit + ``_shutdown_sessions`` finalizes in-memory sessions with + ``tui_shutdown``. The row must remain in ``sessions`` with + ``ended_at IS NULL`` and its messages intact so the next process + can list and resume it. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + from hermes_state import SessionDB + import tui_gateway.server as srv + + db = SessionDB(db_path=tmp_path / "state.db") + session_id = "20260826_132708_a12a19" + db.create_session(session_id=session_id, source="desktop") + db.append_message(session_id, role="user", content="hour-long working session") + db.append_message( + session_id, role="assistant", content="continuing the work…" + ) + monkeypatch.setattr(srv, "_get_db", lambda: db) + + agent = self._real_agent(db, session_id, []) + session = _make_session(agent=agent, history=[], session_key=session_id) + session["source"] = "desktop" + + srv._finalize_session(session, end_reason="tui_shutdown") + + ids = [ + r["id"] + for r in db._conn.execute("SELECT id FROM sessions").fetchall() + ] + assert session_id in ids + row = db.get_session(session_id) + assert row is not None + assert row["ended_at"] is None + assert row["end_reason"] is None + contents = [ + m.get("content") for m in db.get_messages_as_conversation(session_id) + ] + assert any("hour-long" in (c or "") for c in contents), contents + + class TestOnSessionEndHook: """Verify on_session_end plugin hook fires on finalize.""" diff --git a/tests/tui_gateway/test_gateway_owned_session_reap.py b/tests/tui_gateway/test_gateway_owned_session_reap.py index 06ff84d9a39ac..e4b2685a05269 100644 --- a/tests/tui_gateway/test_gateway_owned_session_reap.py +++ b/tests/tui_gateway/test_gateway_owned_session_reap.py @@ -72,3 +72,14 @@ def test_missing_row_still_ended(self, mock_get_db): _finalize_session(_make_session(), end_reason="tui_close") db.end_session.assert_called_once_with("sess_1", "tui_close") + + @patch("tui_gateway.server._get_db") + def test_missing_row_still_ended_on_tui_shutdown(self, mock_get_db): + """Keep-open (#95868) skips end_session only when a durable row exists.""" + db = MagicMock() + db.get_session.return_value = None + mock_get_db.return_value = db + + _finalize_session(_make_session(), end_reason="tui_shutdown") + + db.end_session.assert_called_once_with("sess_1", "tui_shutdown") diff --git a/tests/tui_gateway/test_startup_orphan_sweep.py b/tests/tui_gateway/test_startup_orphan_sweep.py index 38c3e11e7f019..9ffeb62a749e7 100644 --- a/tests/tui_gateway/test_startup_orphan_sweep.py +++ b/tests/tui_gateway/test_startup_orphan_sweep.py @@ -87,6 +87,8 @@ def test_swept_row_stays_resumable(self, monkeypatch, tmp_path): # The distinct reason is a recoverable accident, not a boundary. assert "startup_orphan_reap" in SessionDB.RECOVERABLE_END_REASONS + # Process-exit teardown is the same accident class (#95868). + assert "tui_shutdown" in SessionDB.RECOVERABLE_END_REASONS # Peer-keyed recovery still surfaces the swept row... recovered = db.find_latest_gateway_session_for_peer(