Skip to content

refactor(matrix): remove delayed reaction redaction scheduling - #22090

Closed
Phoenix1819 wants to merge 1 commit into
NousResearch:mainfrom
Phoenix1819:matrix-reaction-cleanup
Closed

refactor(matrix): remove delayed reaction redaction scheduling#22090
Phoenix1819 wants to merge 1 commit into
NousResearch:mainfrom
Phoenix1819:matrix-reaction-cleanup

Conversation

@Phoenix1819

Copy link
Copy Markdown

Summary

This PR removes the delayed async reaction redaction logic from the Matrix gateway and replaces it with immediate redaction.

Problem

The Matrix adapter scheduled a background asyncio.Task for every reaction redaction, sleeping for 5 seconds first. This was intended to give homeservers time to deliver the final message event before the reaction disappeared, avoiding "missing event" errors in some clients.

In practice, the delay caused several issues:

  • Task leaks: if the bot disconnected while tasks were pending, cancellation was best-effort
  • Stale reactions: under load, multiple 5-second delays meant checkmark/cross reactions appeared alongside the old eyes reaction
  • Unnecessary complexity: a non-configurable constant, a task set, and cancellation logic for a client-side edge case

Solution

Redact reactions immediately via direct await:

  • on_processing_complete()_redact_reaction() directly
  • _redact_bot_approval_reactions()redact_message() directly

Both call sites log at debug level on failure, so errors are visible but non-fatal.

Removed

  • _schedule_reaction_redaction() method
  • _reaction_redaction_delay_seconds constant
  • _reaction_redaction_tasks: Set[asyncio.Task]
  • Task cancellation loop in disconnect()
  • Stale comment about the delay rationale

Backwards Compatibility

  • No configuration changes
  • No API changes
  • Behavior change: reactions disappear immediately instead of after ~5s — this is arguably the expected UX

Testing

  • Verified eyes reaction is replaced by checkmark/cross without lingering
  • Verified approval reactions are cleaned up immediately after resolution
  • Verified no exceptions propagate when redaction fails

Reactions are now redacted immediately instead of after a 5-second
async delay. The old logic scheduled a background task that slept
before calling _redact_reaction(), which added complexity:

- A Set[asyncio.Task] to track pending redactions
- Cancellation logic in disconnect()
- A non-configurable 5-second delay constant
- Risk of task leaks if disconnect raced with the scheduler

The delay was originally added to avoid "missing event" errors in
some Matrix clients, but in practice the issue is homeserver-specific
and better handled by the server-side event ordering. Immediate
redaction is simpler, more predictable, and avoids stale reactions
lingering when the bot is under load.

Changes:
- Removed _schedule_reaction_redaction() method
- Removed _reaction_redaction_delay_seconds and _reaction_redaction_tasks
- Removed task-cancellation cleanup in disconnect()
- on_processing_complete() now calls _redact_reaction() directly
- _redact_bot_approval_reactions() now calls redact_message() directly

All call sites handle failure gracefully with logger.debug.
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have platform/matrix Matrix adapter (E2EE) comp/gateway Gateway runner, session dispatch, delivery labels May 8, 2026
Phoenix1819 pushed a commit to Phoenix1819/hermes-agent that referenced this pull request May 10, 2026
…d tasks

Reaction redactions previously slept for 5 seconds before executing.
This blocked the processing loop unnecessarily. Now they fire immediately
as background tasks, with proper cleanup on disconnect.

Closes NousResearch#22090
@Phoenix1819

Copy link
Copy Markdown
Author

Withdrawing — the 5-second delay was a workaround for a real race condition. Removing it without data that the underlying issue is resolved is a behavioral change, not a clear improvement.

@Phoenix1819
Phoenix1819 deleted the matrix-reaction-cleanup branch May 16, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/matrix Matrix adapter (E2EE) type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants