fix(matrix): remove eyes reaction on processing complete - #6852
Closed
fxfitz wants to merge 3 commits into
Closed
Conversation
The on_processing_complete handler was never removing the eyes reaction because _send_reaction didn't return the reaction event_id. Fix: - _send_reaction returns Optional[str] event_id - on_processing_start stores it in _pending_reactions dict - on_processing_complete redacts the eyes reaction before adding completion emoji
_send_reaction now returns Optional[str] (event_id) instead of bool. Tests updated: - test_send_reaction: assert result == event_id string - test_send_reaction_no_client: assert result is None - test_on_processing_start_sends_eyes: _send_reaction returns event_id, now also asserts _pending_reactions is populated - test_on_processing_complete_sends_check: set up _pending_reactions and mock _redact_reaction, assert eyes reaction is redacted before sending check
Add debug logging when eyes reaction redaction fails, and add tests for the success=False path and the no-pending-reaction edge case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Merged via PR #7154. All 3 commits were cherry-picked with your authorship preserved. We rebased onto the new ProcessingOutcome enum API (from a concurrent PR) and resolved the conflicts. Thanks for the clean fix! |
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
2 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.
Problem
When Hermes starts processing a Matrix message, it adds the 👀 (eyes) reaction. When processing completes, the 👀 is never removed — both 👀 and ✅/❌ appear together instead of the eyes being replaced.
Root Cause
_send_reaction()returned only abooland discarded the reaction event ID from the Matrix server. Without that event ID,on_processing_completehad no way to call_redact_reaction(), so it just added the completion emoji alongside the eyes.Discord doesn't have this issue — its API lets you remove reactions by emoji name directly.
Fix
_send_reaction→Optional[str]: Now returns the reaction event ID fromRoomSendResponse.event_id._pending_reactionsdict: Tracks(room_id, message_event_id) → reaction_event_id.on_processing_start: Stores the eyes reaction event ID after sending.on_processing_complete: Redacts the eyes reaction before sending the completion emoji. Logs a debug message if redaction fails.Testing
✅ Fix has been running locally on this deployment. 👀 is now correctly removed and replaced with ✅/❌.
Test coverage includes:
success=Truepath (eyes redacted, ✅ sent)success=Falsepath (eyes redacted, ❌ sent)Note: Telegram (issue #6068) may have the same pattern.
Branch: fxfitz/hermes-agent@8cdda296 (based on upstream main
3b554bf8)Locally tested: ✅