mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(telegram): clear in-progress reaction on cancelled processing (#24628)
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.
This commit is contained in:
parent
413990c945
commit
6f285efb80
2 changed files with 80 additions and 3 deletions
|
|
@ -4761,6 +4761,27 @@ class TelegramAdapter(BasePlatformAdapter):
|
||||||
logger.debug("[%s] set_message_reaction failed (%s): %s", self.name, emoji, e)
|
logger.debug("[%s] set_message_reaction failed (%s): %s", self.name, emoji, e)
|
||||||
return False
|
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:
|
async def on_processing_start(self, event: MessageEvent) -> None:
|
||||||
"""Add an in-progress reaction when message processing begins."""
|
"""Add an in-progress reaction when message processing begins."""
|
||||||
if not self._reactions_enabled():
|
if not self._reactions_enabled():
|
||||||
|
|
@ -4775,12 +4796,23 @@ class TelegramAdapter(BasePlatformAdapter):
|
||||||
|
|
||||||
Unlike Discord (additive reactions), Telegram's set_message_reaction
|
Unlike Discord (additive reactions), Telegram's set_message_reaction
|
||||||
replaces all existing reactions in one call — no remove step needed.
|
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():
|
if not self._reactions_enabled():
|
||||||
return
|
return
|
||||||
chat_id = getattr(event.source, "chat_id", None)
|
chat_id = getattr(event.source, "chat_id", None)
|
||||||
message_id = getattr(event, "message_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(
|
await self._set_reaction(
|
||||||
chat_id,
|
chat_id,
|
||||||
message_id,
|
message_id,
|
||||||
|
|
|
||||||
|
|
@ -218,17 +218,62 @@ async def test_on_processing_complete_skipped_when_disabled(monkeypatch):
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_on_processing_complete_cancelled_keeps_existing_reaction(monkeypatch):
|
async def test_on_processing_complete_cancelled_clears_reaction(monkeypatch):
|
||||||
"""Expected cancellation should not replace the in-progress reaction."""
|
"""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")
|
monkeypatch.setenv("TELEGRAM_REACTIONS", "true")
|
||||||
adapter = _make_adapter()
|
adapter = _make_adapter()
|
||||||
event = _make_event()
|
event = _make_event()
|
||||||
|
|
||||||
await adapter.on_processing_complete(event, ProcessingOutcome.CANCELLED)
|
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()
|
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 ───────────────────────────────────────────────
|
# ── config.py bridging ───────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue