diff --git a/tests/tui_gateway/test_gateway_owned_session_reap.py b/tests/tui_gateway/test_gateway_owned_session_reap.py new file mode 100644 index 00000000000..81edd590f46 --- /dev/null +++ b/tests/tui_gateway/test_gateway_owned_session_reap.py @@ -0,0 +1,83 @@ +"""Tests for #60609: the TUI backend must not end gateway-owned sessions. + +``_finalize_session`` (and thus the ws-orphan reaper / session.close paths +that funnel into it) marks the session ended in state.db. For sessions the +messaging gateway owns (telegram, discord, ...), that write creates the +Groundhog Day routing loop described in #60609 — the gateway self-heal +drops the ended-but-routed entry, recovers hours-old parent context, and +loops. The TUI is only a viewer of those sessions. +""" + +from unittest.mock import MagicMock, patch + +from tui_gateway.server import _finalize_session, _is_gateway_owned_source + + +class TestIsGatewayOwnedSource: + def test_builtin_gateway_platforms_are_owned(self): + for src in ("telegram", "discord", "whatsapp", "slack", "signal", + "matrix", "mattermost", "bluebubbles", "sms", "email"): + assert _is_gateway_owned_source(src) is True, src + + def test_case_and_whitespace_normalized(self): + assert _is_gateway_owned_source(" Telegram ") is True + + def test_tui_owned_sources_are_not(self): + for src in ("tui", "cli", "webui", "desktop", "cron", "subagent", + "test", "acp", ""): + assert _is_gateway_owned_source(src) is False, src + + def test_local_and_server_endpoints_are_not(self): + # Platform enum members, but their sessions aren't owned by a remote + # chat surface — reaping them keeps /resume clean. + for src in ("local", "webhook", "api_server", "msgraph_webhook"): + assert _is_gateway_owned_source(src) is False, src + + def test_arbitrary_strings_are_not(self): + assert _is_gateway_owned_source("hermesbench-task-xyz") is False + assert _is_gateway_owned_source(None) is False + + +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, + } + + +class TestFinalizeSkipsGatewaySessions: + @patch("tui_gateway.server._get_db") + def test_gateway_session_not_ended(self, mock_get_db): + db = MagicMock() + db.get_session.return_value = {"id": "sess_1", "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_tui_session_still_ended(self, mock_get_db): + db = MagicMock() + db.get_session.return_value = {"id": "sess_1", "source": "tui"} + 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_missing_row_still_ended(self, mock_get_db): + """A session with no state.db row can't be gateway-owned — keep the + pre-existing reap behavior.""" + db = MagicMock() + db.get_session.return_value = None + 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") diff --git a/tui_gateway/server.py b/tui_gateway/server.py index fcd29723bd0..412c89aaf15 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -499,6 +499,42 @@ def _transfer_active_session_slot( return False +# Session sources the TUI/desktop backend must never end in state.db: the +# messaging gateway owns those sessions' lifecycle — the TUI is only a viewer +# (a resume of a Telegram/Discord/... session). Ending one creates the +# #60609 Groundhog Day routing loop (see _finalize_session). Sources the +# TUI backend itself creates ("tui", plus whatever a client passes as its +# own ``source``) and the CLI's own sessions are NOT gateway-owned. +_NON_GATEWAY_SOURCES = frozenset({ + "", "tui", "cli", "webui", "desktop", "cron", "subagent", "test", + "local", "acp", "webhook", "api_server", "msgraph_webhook", +}) + + +def _is_gateway_owned_source(source: str) -> bool: + """True when ``source`` names a messaging-gateway platform whose session + lifecycle belongs to the gateway, not to this TUI backend. + + Structural rather than a hardcoded platform list: any source that + resolves to a known gateway ``Platform`` (built-in enum member OR a + registered platform plugin, via ``Platform._missing_``) counts, so new + platforms are covered automatically. Local/self-owned sources are + excluded explicitly — ``local``/``webhook``/``api_server`` are Platform + members but their sessions are not owned by a remote chat surface that + routes by session_key, so reaping them is safe and keeps /resume clean. + """ + src = (source or "").strip().lower() + if src in _NON_GATEWAY_SOURCES: + return False + try: + from gateway.config import Platform + + Platform(src) # raises ValueError for arbitrary non-platform strings + return True + except Exception: + return False + + def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> None: """Best-effort finalize hook + memory commit for a session. @@ -586,22 +622,16 @@ def _finalize_session(session: dict | None, end_reason: str = "tui_close") -> No try: db = _get_db() if db is not None: - # Don't end gateway-originated sessions — the gateway owns their - # lifecycle. The TUI is a viewer, not the owner. 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) - _GATEWAY_SOURCES = frozenset({ - "bluebubbles", "telegram", "discord", "signal", - "whatsapp", "sms", "slack", "mattermost", - "matrix", "line", "wechat", "facebook", - "imessage", "googlechat", - }) + # Don't end gateway-originated sessions — the gateway owns + # their lifecycle. The TUI is a viewer, not the owner. + # 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) row = db.get_session(session_id) source = (row or {}).get("source", "") - if source not in _GATEWAY_SOURCES: + if not _is_gateway_owned_source(source): db.end_session(session_id, end_reason) except Exception: pass