feat(telegram): inbound reaction routing behind TELEGRAM_INBOUND_REACTIONS env toggle - #13992
feat(telegram): inbound reaction routing behind TELEGRAM_INBOUND_REACTIONS env toggle#13992giwaov wants to merge 2 commits into
Conversation
|
Hey @giwaov — thanks for picking up #13942, appreciate it. A few gaps I noticed that would be worth tightening before this merges: 1. The cache is never populated in practice. 2. Only 1 of 10 bot-authored send paths is hooked. 3. 4. Synthetic text format diverges from precedent. Feishu (merged) emits 5. No tests. Test plan is listed but unchecked, and the diff only touches I have an alternative implementation sitting on a fork that covers all of the above — populates the cache from all 10 send paths with Happy to either:
Whatever lands the cleanest implementation for the repo works for me — flagging the issues upfront so #13942 actually gets solved rather than half-solved. |
Register MessageReactionHandler when TELEGRAM_INBOUND_REACTIONS=1. Allowlisted emoji (👍 ✅ 👎 ❌) are translated to synthetic MessageEvents correlated to the originating conversation via a bounded LRU cache that maps bot message IDs to chat/thread context. Fixes NousResearch#13942
When TELEGRAM_INBOUND_REACTIONS=true, reactions a user places on bot-authored messages (👍 ✅ 👎 ❌) are routed through the normal message pipeline as synthetic "reaction:added:EMOJI" / "reaction:removed:EMOJI" events — mirroring the Feishu precedent in gateway/platforms/feishu.py. The bot can observe 👍 on its own question as a lightweight confirmation signal instead of needing a follow-up "yes" message. Implementation: - Register MessageReactionHandler when the feature is on (PTB's polling/webhook already allows Update.ALL_TYPES). - Cache recent bot-authored outbound messages in a bounded, TTL-pruned OrderedDict keyed by (chat_id, message_id) so reactions on third-party messages and stale messages are ignored. Send paths are hooked to record each outbound with a kind tag (regular / approval / model_picker). - Drop reactions from the bot's own account to avoid feedback loops with lifecycle reactions. - Only route allowlisted emoji (👍 ✅ 👎 ❌) in v1; custom premium emoji and anonymous aggregated counts are ignored. Config: new TELEGRAM_INBOUND_REACTIONS env var (default false), with YAML bridge telegram.inbound_reactions → env var alongside the existing telegram.reactions bridge. Docs: adds an "Inbound reactions (experimental)" section in the Telegram user guide and a row in the environment variables reference. Tests: adds 14 unit tests covering env toggle, cache insert/evict/TTL, unknown-message drop, added/removed/swap routing, bot-self filter, unsupported emoji, anonymous admin, custom-emoji skip, and two config-bridge tests.
b7d3139 to
1cd342e
Compare
|
Addressed the review and force-pushed an updated branch on top of current This version now:
Validation run locally:
Thanks for the detailed review and for pointing to the more complete approach. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for incorporating the prior review's cache, add/remove-delta, docs, and test coverage. The feature is still needed: current main requests Update.ALL_TYPES at plugins/platforms/telegram/adapter.py:1989-1993, but its registrations at :3173-3190 do not include a reaction handler.
Problems
- This branch targets the legacy
gateway/platforms/telegram.py; current main moved Telegram toplugins/platforms/telegram/adapter.pyin5600105478ffde29d7566b45421b100eaa29c4ef, including platform-specific YAML translation. The implementation and config bridge need a plugin-surface port. gateway/platforms/telegram.py:3261accepts a cache entry without checking its TTL. Expiry runs only on a later insert (:3220-3226), so an otherwise idle adapter can route a stale reaction.website/docs/user-guide/messaging/telegram.md:565promises a 👍/✅ acts asyes, while the handler emitsreaction:added:<emoji>events (gateway/platforms/telegram.py:3312).
Suggested changes
- Port the feature to the plugin adapter/config bridge, expire entries at lookup, and add the idle-after-TTL test.
- Describe the synthetic event behavior without guaranteeing yes-equivalence.
Automated hermes-sweeper review.
|
|
||
| # 1. Must be a message we sent | ||
| async with self._bot_message_cache_lock: | ||
| cached = self._bot_message_cache.get((chat_id, message_id)) |
There was a problem hiding this comment.
This lookup does not enforce the TTL. Entries are pruned only by a later _remember_bot_message() insert, so an idle adapter can route a reaction after the advertised expiry. Validate/delete the timestamp under this lock and add a no-later-send expiry test.
|
|
||
| ### Inbound reactions (experimental) | ||
|
|
||
| Hermes can also observe when *you* react to one of its messages and forward that as a lightweight confirmation signal. Reacting with 👍 or ✅ on a bot question has the same effect as replying "yes". Disabled by default. |
There was a problem hiding this comment.
The handler emits reaction:added:<emoji> / reaction:removed:<emoji>, not yes; please describe this as a synthetic reaction event rather than guaranteeing equivalent confirmation semantics.
Summary
Telegram reactions (👍 ✅ 👎 ❌) were silently ignored. This adds opt-in routing of reactions as synthetic messages.
MessageReactionHandlerwhenTELEGRAM_INBOUND_REACTIONS=1; wrapped intry/exceptfor library compatibilityMessageEventtext (👍→"👍", etc.)OrderedDict, max 512 entries, 1 h TTL) correlates bot message IDs to originating chat/thread contextsend()callTest plan
TELEGRAM_INBOUND_REACTIONS=1: react with 👍 → agent receives"👍"message in correct conversationFixes #13942