From d3a753df30e2805ed5e00ba0fae7f24fe40efcee Mon Sep 17 00:00:00 2001 From: ClintonEmok <54935030+ClintonEmok@users.noreply.github.com> Date: Tue, 8 Sep 2026 08:26:44 +0200 Subject: [PATCH] fix(gateway): don't end durable session row on automatic Desktop cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ws_orphan_reap, idle_timeout, lru_evict, ws_disconnect, and tui_shutdown are runtime/connection GC — not user intent. Ending the durable session row during these automatic cleanup reasons confuses GC with user action, causing secondary bot chats to vanish from the sidebar even though the transcript is intact in state.db. The canonical Bot Chat already has resurrection paths for accidental ws_orphan_reap ends, but non-canonical secondary chats do not, so they are hit harder. Skip db.end_session() when _desktop_automatic_cleanup is True (automatic cleanup reason + Desktop source). Runtime is still reclaimed, the session.reclaimed event still fires, and explicit user close/archive/reset still ends the durable row normally. Fixes #105588 --- .../test_cross_process_orphan_ownership.py | 8 +- .../test_finalize_session_persist.py | 133 ++++++++++++++++++ tui_gateway/session_lifecycle.py | 5 +- 3 files changed, 143 insertions(+), 3 deletions(-) diff --git a/tests/tui_gateway/test_cross_process_orphan_ownership.py b/tests/tui_gateway/test_cross_process_orphan_ownership.py index 1a9de04ff9..750fc6a86c 100644 --- a/tests/tui_gateway/test_cross_process_orphan_ownership.py +++ b/tests/tui_gateway/test_cross_process_orphan_ownership.py @@ -344,7 +344,8 @@ def test_automatic_cleanup_reclaims_own_orphan_lease_not_treated_as_sibling( server._finalize_session(session, end_reason="ws_orphan_reap") - assert ended == [(session_id, "ws_orphan_reap")] + # Automatic Desktop cleanup must NOT end the durable row (#105588). + assert ended == [] assert active_session_registry_snapshot(registry_home=profile_home) == [] @@ -501,6 +502,9 @@ def test_automatic_desktop_cleanup_preserves_sibling_and_ends_sole_owner( server._finalize_session(_session(sole_lease), end_reason=reason) assert active_session_registry_snapshot(registry_home=profile_home) == [] - assert ended == [(session_id, reason) for reason in reasons] + # Automatic Desktop cleanup must NOT end the durable row, even for + # sole owners — the conversation stays open until the user explicitly + # closes or archives it. (#105588) + assert ended == [] finally: _stop_child(child, release_file) diff --git a/tests/tui_gateway/test_finalize_session_persist.py b/tests/tui_gateway/test_finalize_session_persist.py index f5ddf7aa35..9a02d574ef 100644 --- a/tests/tui_gateway/test_finalize_session_persist.py +++ b/tests/tui_gateway/test_finalize_session_persist.py @@ -268,3 +268,136 @@ class TestOnSessionEndHook: model="claude-sonnet-4", platform="tui", ) + + +class TestDesktopAutomaticCleanupPreservesDurableRow: + """Automatic Desktop cleanup (ws_orphan_reap, idle_timeout, etc.) must NOT + end the durable session row — the conversation stays open and resumable + until the user explicitly closes or archives it. + + Regression test for #105588. + """ + + @patch("tui_gateway.server._other_runtime_lease_guard") + @patch("tui_gateway.server._get_db") + @patch("tui_gateway.server._session_source", return_value="desktop") + def test_ws_orphan_reap_skips_end_session( + self, _mock_source, mock_get_db, mock_lease_guard + ): + """ws_orphan_reap on a Desktop session must not call db.end_session.""" + from tui_gateway.server import _finalize_session + + mock_db = MagicMock() + mock_get_db.return_value = mock_db + # Simulate no other runtime owning the lifecycle (TUI owns it). + mock_lease_guard.__enter__ = MagicMock(return_value=False) + mock_lease_guard.__exit__ = MagicMock(return_value=False) + + agent = _make_agent(session_id="sess_orphan_001") + session = _make_session( + agent=agent, + history=[{"role": "user", "content": "hello"}], + ) + session["source"] = "desktop" + + _finalize_session(session, end_reason="ws_orphan_reap") + + mock_db.end_session.assert_not_called() + + @patch("tui_gateway.server._other_runtime_lease_guard") + @patch("tui_gateway.server._get_db") + @patch("tui_gateway.server._session_source", return_value="desktop") + def test_idle_timeout_skips_end_session( + self, _mock_source, mock_get_db, mock_lease_guard + ): + """idle_timeout on a Desktop session must not call db.end_session.""" + from tui_gateway.server import _finalize_session + + mock_db = MagicMock() + mock_get_db.return_value = mock_db + mock_lease_guard.__enter__ = MagicMock(return_value=False) + mock_lease_guard.__exit__ = MagicMock(return_value=False) + + agent = _make_agent(session_id="sess_idle_001") + session = _make_session( + agent=agent, + history=[{"role": "user", "content": "hello"}], + ) + session["source"] = "desktop" + + _finalize_session(session, end_reason="idle_timeout") + + mock_db.end_session.assert_not_called() + + @patch("tui_gateway.server._other_runtime_lease_guard") + @patch("tui_gateway.server._get_db") + @patch("tui_gateway.server._session_source", return_value="desktop") + def test_lru_evict_skips_end_session( + self, _mock_source, mock_get_db, mock_lease_guard + ): + """lru_evict on a Desktop session must not call db.end_session.""" + from tui_gateway.server import _finalize_session + + mock_db = MagicMock() + mock_get_db.return_value = mock_db + mock_lease_guard.__enter__ = MagicMock(return_value=False) + mock_lease_guard.__exit__ = MagicMock(return_value=False) + + agent = _make_agent(session_id="sess_lru_001") + session = _make_session( + agent=agent, + history=[{"role": "user", "content": "hello"}], + ) + session["source"] = "desktop" + + _finalize_session(session, end_reason="lru_evict") + + mock_db.end_session.assert_not_called() + + @patch("tui_gateway.server._other_runtime_lease_guard") + @patch("tui_gateway.server._get_db") + @patch("tui_gateway.server._session_source", return_value="desktop") + def test_explicit_close_still_calls_end_session( + self, _mock_source, mock_get_db, _mock_lease_guard + ): + """Explicit user close (tui_close) must still call db.end_session — + only automatic cleanup reasons skip it.""" + from tui_gateway.server import _finalize_session + + mock_db = MagicMock() + mock_get_db.return_value = mock_db + + agent = _make_agent(session_id="sess_close_001") + session = _make_session( + agent=agent, + history=[{"role": "user", "content": "hello"}], + ) + session["source"] = "desktop" + + _finalize_session(session, end_reason="tui_close") + + mock_db.end_session.assert_called_once_with("sess_close_001", "tui_close") + + @patch("tui_gateway.server._other_runtime_lease_guard") + @patch("tui_gateway.server._get_db") + @patch("tui_gateway.server._session_source", return_value="tui") + def test_non_desktop_source_still_calls_end_session( + self, _mock_source, mock_get_db, _mock_lease_guard + ): + """ws_orphan_reap on a NON-desktop source (e.g. TUI) must still call + db.end_session — the fix only applies to Desktop sessions.""" + from tui_gateway.server import _finalize_session + + mock_db = MagicMock() + mock_get_db.return_value = mock_db + + agent = _make_agent(session_id="sess_tui_001") + session = _make_session( + agent=agent, + history=[{"role": "user", "content": "hello"}], + ) + # No source set → falls back to platform which we patch as "tui" + + _finalize_session(session, end_reason="ws_orphan_reap") + + mock_db.end_session.assert_called_once_with("sess_tui_001", "ws_orphan_reap") diff --git a/tui_gateway/session_lifecycle.py b/tui_gateway/session_lifecycle.py index dda33e2dd8..4ce964e111 100644 --- a/tui_gateway/session_lifecycle.py +++ b/tui_gateway/session_lifecycle.py @@ -271,7 +271,10 @@ def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> No # compression splits back to the reaped child, forever). if _is_gateway_owned_source((db.get_session(session_id) or {}).get("source", "")): _tui_owns_lifecycle = False - elif _tui_owns_lifecycle: + elif _tui_owns_lifecycle and not _desktop_automatic_cleanup: + # Automatic Desktop cleanup (ws_orphan_reap, idle_timeout, etc.) reclaims + # runtime but must not end the durable row — the conversation stays open + # and resumable until the user explicitly closes or archives it. #105588 db.end_session(session_id, end_reason) # In-flight async delegations end WITH the session (no return address left). Always interrupt by THIS live UI # sid; by durable session_key only when the TUI owns the lifecycle — a viewer tab must not kill gateway work.