feat(telegram): fire telegram:reaction hook when user reacts to bot messages - #53814
Open
fsaad1984 wants to merge 10 commits into
Open
feat(telegram): fire telegram:reaction hook when user reacts to bot messages#53814fsaad1984 wants to merge 10 commits into
fsaad1984 wants to merge 10 commits into
Conversation
Anthropic returns HTTP 400 when a tool_use block is not immediately followed by its tool_result. Two root causes exist: 1. Context compression inserts messages between the pair. _strip_orphaned_tool_blocks (PR NousResearch#52145) already fixes the *wire payload*, but it mutates api_messages — a shallow copy of the canonical messages list. The canonical list is unchanged, so the *next* API call rebuilds the same broken payload and hits the same 400 again. 2. A cron/subagent session is interrupted before the tool_result is appended. Concrete reproduction: the approval guard blocks execute_code inside a cron job (no user present), the tool handler returns an error JSON which the tool_executor normally wraps in a tool_result message. But in this case the gateway reloaded the session transcript from disk AFTER the interruption, finding disk=0 messages vs memory=37. The live (correct) history was preserved, but a prior interrupted turn had left a bare tool_use as the last assistant block with no following user/tool_result turn. _strip_orphaned_tool_blocks never ran against the canonical list, so the next API call sent the broken transcript verbatim. Fix — three-file change: * agent/error_classifier.py: new FailoverReason.orphaned_tool_use + detection pattern in _classify_400. The Anthropic error message always contains both 'tool_use' and 'tool_result', which is distinctive enough for a safe substring match. retryable=True so the retry loop continues rather than aborting. * agent/turn_retry_state.py: orphaned_tool_use_retry_attempted flag so the recovery branch fires at most once per turn (prevents an infinite strip-and-retry loop if stripping somehow fails to fix the issue). * agent/conversation_loop.py: recovery branch that runs _strip_orphaned_tool_blocks against the canonical messages list (not just the wire payload) so the cleaned transcript is persisted and the retry sees a valid conversation. Reproduction: long gateway session → tool call → execute_code blocked by cron approval guard → gateway reload from disk finds stale/empty transcript → live history preserved but contains orphaned tool_use → HTTP 400 crash-loop.
…nd OpenAI-style canonical messages The canonical messages list uses OpenAI-style role=tool/tool_calls, not the Anthropic wire format that _strip_orphaned_tool_blocks expects. The original fix stripped 0 entries because it passed the wrong list. Now: (1) detect orphaned IDs from api_messages (Anthropic format), (2) strip api_messages via _strip_orphaned_tool_blocks, (3) also clean the canonical messages list by removing orphaned tool_calls entries and their matching role=tool messages so the next api_messages rebuild produces a valid transcript.
…ssages api_messages at error-handler time is pre-conversion; the Anthropic adapter converts tool_calls→tool_use internally. Detect orphaned IDs from canonical messages (role=tool / tool_calls) instead of api_messages.
…l messages The canonical messages pair IS present but adjacency breaks during Anthropic adapter conversion (context compaction injects synthetic user messages). Parse the IDs directly from the Anthropic 400 error string. Also fix: used 'classified_err' (undefined) instead of 'api_error'.
…essages - Add MessageReactionHandler to TelegramAdapter — registers with PTB 22.6+ - _handle_reaction() looks up message via rich_sent_store to confirm it's a bot message, then fires the _reaction_callback with a structured payload (chat_id, message_id, user_id, user_name, new/old_reactions, message_text) - Add set_reaction_callback() to BasePlatformAdapter so the runner can install the callback without knowing the concrete adapter type - GatewayRunner._handle_telegram_reaction() emits telegram:reaction hook event via self.hooks, wired up in all three adapter-setup paths - All three adapter-setup paths in run.py updated (initial connect, reconnect loop, multi-profile path) The hook fires only for reactions on messages the bot sent (verified via rich_sent_store lookup), ignoring reactions on user messages entirely.
alt-glitch
added
type/feature
…re API call When context compression fires, the newly-created compaction message (role=assistant, content starting with '[CONTEXT COMPACTION — REFERENCE ONLY]') can inherit the tool_calls from the last assistant turn that was archived. Those tool_calls have no matching tool results in the active message list (the results were soft-archived with the pre-compaction transcript), causing Anthropic HTTP 400: 'tool_use ids found without tool_result blocks immediately after'. The existing runtime-recovery path (_orphaned_tool_use_retry) catches this and strips the IDs — but only after already exhausting 3 retries, and only for the outermost orphan set. When multiple compaction layers accumulate, the one-shot retry still fails on the second set of orphaned IDs. Fix: in sanitize_api_messages() (which runs before every API call), detect assistant messages whose text content is a compaction summary and strip any tool_calls from them proactively. This prevents the 400 entirely without touching the stored conversation history.
The strip_orphaned_tool_blocks function checks result[i+1] for adjacency, but was running before _merge_consecutive_roles. This meant that two consecutive user messages (one plain, one with tool_result) would falsely look non-adjacent, causing valid tool_use blocks to be stripped. Fix: swap the call order — merge first, then strip orphans. Add two regression tests: - test_strips_non_adjacent_tool_use: verifies that a tool_use whose tool_result is separated by an intervening assistant turn is stripped - test_consecutive_user_messages_merged_before_adjacency_check: verifies that a valid pair is preserved when merge makes the result adjacent Addresses reviewer feedback on PR NousResearch#52145.
Contributor
|
Thanks for adding a narrow PTB reaction entry point; current main does not register Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-security-boundary
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support for Telegram message reaction events on messages sent by the bot.
What changed
Payload
chat_id, message_id, user_id, user_name, new_reactions (list[str]), old_reactions (list[str]), message_text
Notes