From b02bd3dcfa7034c8f8dab27cd0a4a431ed48dca6 Mon Sep 17 00:00:00 2001 From: ClintonEmok <54935030+ClintonEmok@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:47:18 +0200 Subject: [PATCH] fix(gateway): ws-orphan reaper leaves the canonical Bot Chat resumable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot Mode's forever-chat is identified by (profile_name, title='Bot Chat'), not by stored session ids. When the desktop's WS stayed down past _WS_ORPHAN_REAP_GRACE_S, _finalize_session ended the row with end_reason='ws_orphan_reap', which archives it; on reopen findExistingCanonicalChat located the archived row but openSession rejected it, the recreate collided with idx_sessions_title_unique, and the bot fell back to auto-titled throwaway sessions — 511 messages of history invisible from every UI path (#92687). ws_orphan_reap is already classified as an accidental, recoverable end elsewhere (promote_to_session_reset, find_latest_gateway_session_for_peer); extend that leniency to the canonical chat at the write site: when an accidental reap reason targets a profile-scoped 'Bot Chat' row, skip only the DB end-write and log why. In-process teardown still runs, explicit user boundaries (tui_close, session.close, /new) still end it, ordinary sessions are unaffected, and the #60609 gateway-owner guard keeps precedence. Covered by unit tests mirroring test_gateway_owned_session_reap.py: canonical row spared on reap, explicit close still ends, non-canonical titles unaffected, gateway-owned rows keep their guard, missing rows keep legacy behavior. Fixes #92687 --- .../test_canonical_bot_chat_reap.py | 114 ++++++++++++++++++ tui_gateway/server.py | 55 ++++++++- 2 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 tests/tui_gateway/test_canonical_bot_chat_reap.py diff --git a/tests/tui_gateway/test_canonical_bot_chat_reap.py b/tests/tui_gateway/test_canonical_bot_chat_reap.py new file mode 100644 index 000000000000..5a405bb26002 --- /dev/null +++ b/tests/tui_gateway/test_canonical_bot_chat_reap.py @@ -0,0 +1,114 @@ +"""Tests for #92687: the ws-orphan reaper must not end the canonical Bot Chat. + +Bot Mode's forever-chat is identified by ``(profile_name, +title='Bot Chat')``, not by stored session ids. When the desktop's WS drops +for the reap grace window, ``_finalize_session(end_reason='ws_orphan_reap')`` +used to end that row — archiving it out from under every future open. The +plugin's recreate then collides with the global title UNIQUE index and forks +throwaway auto-titled sessions, so the bot appears to have "lost its memory". + +The fix: an *accidental* end reason (ws_orphan_reap) hitting a canonical Bot +Chat row skips only the DB end-write; explicit user boundaries still end it. +""" + +from unittest.mock import MagicMock, patch + +from tui_gateway.server import ( + _finalize_session, + _is_canonical_bot_chat_row, +) + + +def _make_session(session_id="sess_1"): + agent = MagicMock() + agent.session_id = session_id + return { + "agent": agent, + "history": [{"role": "user", "content": "x"}], + "history_lock": None, + "session_key": session_id, + } + + +def _canonical_row(**overrides): + row = {"id": "sess_1", "source": "desktop", "profile_name": "default", + "title": "Bot Chat"} + row.update(overrides) + return row + + +class TestIsCanonicalBotChatRow: + def test_matches_profile_scoped_title(self): + assert _is_canonical_bot_chat_row(_canonical_row()) is True + + def test_requires_a_profile(self): + assert _is_canonical_bot_chat_row(_canonical_row(profile_name="")) is False + assert _is_canonical_bot_chat_row(_canonical_row(profile_name=None)) is False + + def test_title_match_is_exact(self): + assert _is_canonical_bot_chat_row(_canonical_row(title="Bot Chat #2")) is False + assert _is_canonical_bot_chat_row(_canonical_row(title="bot chat")) is False + + def test_none_and_empty_rows_are_not(self): + assert _is_canonical_bot_chat_row(None) is False + assert _is_canonical_bot_chat_row({}) is False + + +class TestFinalizeSkipsAccidentalReapOfCanonicalBotChat: + @patch("tui_gateway.server._get_db") + def test_ws_orphan_reap_does_not_end_the_canonical_row(self, mock_get_db): + db = MagicMock() + db.get_session.return_value = _canonical_row() + mock_get_db.return_value = db + + _finalize_session(_make_session(), end_reason="ws_orphan_reap") + + db.end_session.assert_not_called() + + @patch("tui_gateway.server._get_db") + def test_explicit_user_close_still_ends_it(self, mock_get_db): + """tui_close / session.close are real boundaries — keep ending the + row so a deliberately closed Bot Chat doesn't haunt /resume.""" + db = MagicMock() + db.get_session.return_value = _canonical_row() + mock_get_db.return_value = 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_reap_of_an_ordinary_desktop_session_still_ends_it(self, mock_get_db): + """Only the canonical title is protected; every other desktop session + keeps the pre-existing reap behavior.""" + db = MagicMock() + db.get_session.return_value = _canonical_row(title="Tell me about yourself") + mock_get_db.return_value = db + + _finalize_session(_make_session(), end_reason="ws_orphan_reap") + + db.end_session.assert_called_once_with("sess_1", "ws_orphan_reap") + + @patch("tui_gateway.server._get_db") + def test_gateway_owned_sessions_keep_their_own_guard(self, mock_get_db): + """The #60609 gateway-owner guard must keep working for Bot rows that + ride a gateway platform source.""" + db = MagicMock() + db.get_session.return_value = _canonical_row(source="telegram") + mock_get_db.return_value = db + + _finalize_session(_make_session(), end_reason="ws_orphan_reap") + + db.end_session.assert_not_called() + + @patch("tui_gateway.server._get_db") + def test_missing_row_still_ended_on_reap(self, mock_get_db): + """No state.db row → can't be a canonical chat — keep the legacy + behavior.""" + db = MagicMock() + db.get_session.return_value = None + mock_get_db.return_value = db + + _finalize_session(_make_session(), end_reason="ws_orphan_reap") + + db.end_session.assert_called_once_with("sess_1", "ws_orphan_reap") diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 63b382c7dc36..1c5218af2cce 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -744,6 +744,34 @@ def _is_gateway_owned_source(source: str) -> bool: return False +# Bot Mode's per-profile forever-chat is identified by this exact title (the +# hermes-bots desktop plugin resolves canonical identity by +# ``(profile_name, title='Bot Chat')``, not by stored ids). +CANONICAL_BOT_CHAT_TITLE = "Bot Chat" + +# End reasons that describe an *accidental* transport loss rather than an +# intentional conversation boundary. ``ws_orphan_reap`` is already treated as +# recoverable for gateway-peer rows (#60609 family); the same leniency applies +# to Bot Chats below. +_ACCIDENTAL_END_REASONS = frozenset({"ws_orphan_reap"}) + + +def _is_canonical_bot_chat_row(row: dict | None) -> bool: + """True when the state.db row is the per-profile canonical Bot Chat. + + The row's ``profile_name`` scopes the identity; the title match is exact. + """ + if not row: + return False + + title = str(row.get("title") or "").strip() + + return bool( + str(row.get("profile_name") or "").strip() + and title == CANONICAL_BOT_CHAT_TITLE + ) + + def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> None: """Best-effort finalize hook + memory commit for a session. @@ -840,12 +868,33 @@ def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> No # Ending a gateway session in state.db triggers a Groundhog # Day routing loop: the gateway's #54878 self-heal detects # the stale entry, recovers to the parent session, context - # compression splits back to the reaped child, and the cycle - # repeats on every inbound message. (#60609) + # compression splits back to the reaped child, and the + # cycle repeats on every inbound message. (#60609) row = db.get_session(session_id) source = (row or {}).get("source", "") _tui_owns_lifecycle = not _is_gateway_owned_source(source) - if _tui_owns_lifecycle: + if ( + _tui_owns_lifecycle + and end_reason in _ACCIDENTAL_END_REASONS + and _is_canonical_bot_chat_row(row) + ): + # Don't END Bot Mode's canonical forever-chat on an + # accidental reap either (#92687): its durable identity + # is (profile_name, title='Bot Chat') — an ended row + # archives it out from under every future open, the + # plugin's recreate then collides with the title UNIQUE + # index and forks throwaway sessions instead. Leave the + # row open so reconnect resume finds it; the in-process + # teardown below proceeds unchanged. An explicit user + # boundary (tui_close, session.close, /new) still ends + # it normally. + logger.info( + "skipping %s end-write for canonical Bot Chat row " + "%s; leaving it open for reconnect resume", + end_reason, + session_id, + ) + elif _tui_owns_lifecycle: db.end_session(session_id, end_reason) except Exception: pass