From 6f285efb8058ee5bd1b91e4e0ba9187ec8b183e8 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 12 May 2026 17:02:29 -0700 Subject: [PATCH] fix(telegram): clear in-progress reaction on cancelled processing (#24628) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user runs /stop or a session is interrupted mid-flight, the 👀 in-progress reaction lingered on the user's message indefinitely. Without another agent run to swap it for 👍/👎, the eyes stayed there forever — visually misleading (looks like the agent is still working). Fix: on ProcessingOutcome.CANCELLED, call set_message_reaction with reaction=None to clear all reactions on the message. Documented Bot API semantics (equivalent to Bot API 10.0's deleteMessageReaction, but works on PTB 22.6 already without the version bump). Test changes: - Renamed test_on_processing_complete_cancelled_keeps_existing_reaction → test_on_processing_complete_cancelled_clears_reaction; updated assertion to expect set_message_reaction(reaction=None). - Added test_on_processing_complete_cancelled_skipped_when_disabled (TELEGRAM_REACTIONS=false short-circuits). - Added test_clear_reactions_handles_api_error_gracefully and test_clear_reactions_returns_false_without_bot to cover the new _clear_reactions helper. --- gateway/platforms/telegram.py | 34 +++++++++++++++- tests/gateway/test_telegram_reactions.py | 49 +++++++++++++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index a821160cfc8..415ddb5608b 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -4761,6 +4761,27 @@ class TelegramAdapter(BasePlatformAdapter): logger.debug("[%s] set_message_reaction failed (%s): %s", self.name, emoji, e) return False + async def _clear_reactions(self, chat_id: str, message_id: str) -> bool: + """Clear all reactions from a Telegram message. + + Calling ``set_message_reaction`` with ``reaction=None`` (or an empty + sequence) is the documented Bot API way to remove all bot-set + reactions on a message — equivalent to Bot API 10.0's + ``deleteMessageReaction`` but supported in PTB 22.6 already. + """ + if not self._bot: + return False + try: + await self._bot.set_message_reaction( + chat_id=int(chat_id), + message_id=int(message_id), + reaction=None, + ) + return True + except Exception as e: + logger.debug("[%s] clear reactions failed: %s", self.name, e) + return False + async def on_processing_start(self, event: MessageEvent) -> None: """Add an in-progress reaction when message processing begins.""" if not self._reactions_enabled(): @@ -4775,12 +4796,23 @@ class TelegramAdapter(BasePlatformAdapter): Unlike Discord (additive reactions), Telegram's set_message_reaction replaces all existing reactions in one call — no remove step needed. + + On CANCELLED outcomes (e.g. the user runs ``/stop``, or a session is + interrupted mid-flight), we explicitly clear the 👀 in-progress + reaction so it doesn't linger on the user's message indefinitely. + Without this clear, the only way to remove the 👀 was to wait for + another agent run to swap it to 👍/👎 — which never happens if the + cancellation was the last activity in the chat. """ if not self._reactions_enabled(): return chat_id = getattr(event.source, "chat_id", None) message_id = getattr(event, "message_id", None) - if chat_id and message_id and outcome != ProcessingOutcome.CANCELLED: + if not (chat_id and message_id): + return + if outcome == ProcessingOutcome.CANCELLED: + await self._clear_reactions(chat_id, message_id) + else: await self._set_reaction( chat_id, message_id, diff --git a/tests/gateway/test_telegram_reactions.py b/tests/gateway/test_telegram_reactions.py index 143161e9b71..8b3b0686bb4 100644 --- a/tests/gateway/test_telegram_reactions.py +++ b/tests/gateway/test_telegram_reactions.py @@ -218,17 +218,62 @@ async def test_on_processing_complete_skipped_when_disabled(monkeypatch): @pytest.mark.asyncio -async def test_on_processing_complete_cancelled_keeps_existing_reaction(monkeypatch): - """Expected cancellation should not replace the in-progress reaction.""" +async def test_on_processing_complete_cancelled_clears_reaction(monkeypatch): + """Cancelled processing should clear the in-progress reaction. + + Without this clear, the 👀 reaction lingers on the user's message + indefinitely (until another agent run swaps it for 👍/👎). On a + ``/stop`` that ends a session, that reaction never gets cleaned up. + """ monkeypatch.setenv("TELEGRAM_REACTIONS", "true") adapter = _make_adapter() event = _make_event() await adapter.on_processing_complete(event, ProcessingOutcome.CANCELLED) + # set_message_reaction with reaction=None clears all reactions on the + # message (Bot API documented semantics; equivalent to Bot API 10.0's + # deleteMessageReaction but works on PTB 22.6 already). + adapter._bot.set_message_reaction.assert_awaited_once_with( + chat_id=123, + message_id=456, + reaction=None, + ) + + +@pytest.mark.asyncio +async def test_on_processing_complete_cancelled_skipped_when_disabled(monkeypatch): + """Cancelled processing should not call the API when reactions are off.""" + monkeypatch.delenv("TELEGRAM_REACTIONS", raising=False) + adapter = _make_adapter() + event = _make_event() + + await adapter.on_processing_complete(event, ProcessingOutcome.CANCELLED) + adapter._bot.set_message_reaction.assert_not_awaited() +@pytest.mark.asyncio +async def test_clear_reactions_handles_api_error_gracefully(monkeypatch): + """API errors during clear should not propagate.""" + monkeypatch.setenv("TELEGRAM_REACTIONS", "true") + adapter = _make_adapter() + adapter._bot.set_message_reaction = AsyncMock(side_effect=RuntimeError("no perms")) + + result = await adapter._clear_reactions("123", "456") + assert result is False + + +@pytest.mark.asyncio +async def test_clear_reactions_returns_false_without_bot(monkeypatch): + """_clear_reactions should return False when bot is not available.""" + adapter = _make_adapter() + adapter._bot = None + + result = await adapter._clear_reactions("123", "456") + assert result is False + + # ── config.py bridging ───────────────────────────────────────────────