Skip to content

fix(gateway): prevent third-sender message drop in queue-mode debounce - #31496

Open
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/queue-debounce-three-sender-drop
Open

fix(gateway): prevent third-sender message drop in queue-mode debounce#31496
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/queue-debounce-three-sender-drop

Conversation

@AhmetArif0

Copy link
Copy Markdown
Contributor

Summary

  • Bug: In busy_text_mode=queue (the default), messages from a third sender in a group chat are permanently and silently dropped when two prior senders have created a debounce/pending collision.
  • Root cause: _queue_text_debounce hit an early return without saving the incoming event anywhere when (a) sender B's debounce state is stuck in the store (task=None, timer fired but couldn't flush because pending belongs to sender A) and (b) the new message from sender C can't merge with B's stuck state OR with pending-A.
  • Fix: Remove the _can_merge(existing_pending, event) guard from the inner conflict branch and unconditionally call merge_pending_message_event — C's text is appended to the pending slot (or placed there directly if empty), matching the pre-debounce behaviour that always queued follow-ups regardless of sender.

Affected path

gateway/platforms/base.py_queue_text_debounce(), the inner if state is not None and not self._can_merge_text_debounce_events(state.event, event): branch introduced in #31341.

Reproduction scenario

  1. Agent is processing a session (active session lock held).
  2. Sender A's message is debounced, timer fires, flushed to _pending_messages.
  3. Sender B's message is debounced; timer fires but _flush_text_debounce_now returns False because pending-A is a different sender → B stuck in store with task=None, no retry scheduled.
  4. Sender C (third distinct sender) arrives → _queue_text_debounce tries to flush B, fails again → hits returnC is never written to store, pending, or anywhere else.

Only affects multi-user group chats (Matrix, Slack channels, Discord servers, WeChat groups) with 3+ participants messaging simultaneously during an active session.

Changes

File Change
gateway/platforms/base.py Remove sender-check guard; always call merge_pending_message_event in the stuck-debounce fallback
tests/gateway/test_active_session_text_merge.py Add test_third_sender_not_dropped_when_debounce_store_is_stuck regression test

Test plan

  • New regression test test_third_sender_not_dropped_when_debounce_store_is_stuck pins the exact 3-sender scenario (A in pending, B stuck in store with task=None, C from third sender → C survives in _pending_messages)
  • All 16 tests in test_active_session_text_merge.py pass
  • test_busy_session_ack.py (16 tests), test_base_topic_sessions.py (10 tests), test_wecom.py (44 tests) — all pass, no regressions

When busy_text_mode=queue and a debounce state for sender B is stuck in
the store (timer fired but the pending slot belongs to sender A, a
different sender), any message from a third sender C hit an early
`return` without being saved to the debounce store, pending_messages, or
anywhere else — permanently and silently dropped.

Remove the `_can_merge(existing_pending, event)` guard from the inner
conflict branch in `_queue_text_debounce` and unconditionally call
`merge_pending_message_event` instead. C's text is appended to the
pending slot (or placed there directly if empty), matching the
pre-debounce behaviour that always queued follow-ups regardless of
sender. Sender attribution is best-effort when 3+ senders collide
simultaneously; losing the message entirely is worse.

Adds a regression test that pins the 3-sender scenario: A in pending,
B stuck in the debounce store with task=None, C from a third sender —
C must survive in _pending_messages and B must remain in the store for
_drain_pending_after_session_command to recover.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels May 24, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the shared-session collision; current main still has the early return at gateway/platforms/base.py:4262-4277, so the delivery-loss premise is valid for queue-mode shared sessions.

Problems

  • The new unconditional merge at gateway/platforms/base.py:2830 appends C's text to the existing A event. merge_pending_message_event(..., merge_text=True) only updates existing.text (gateway/platforms/base.py:2137-2144), so C is processed under A's source. That conflicts with the existing separate-sender contract in tests/gateway/test_active_session_text_merge.py:245-271.
  • The new test only checks that C text appears in the pending slot. It does not verify original event/source attribution or eventual separate delivery for B and C.

Suggested changes

  • Preserve the events as separate queue entries instead of merging different senders. The runner's existing head-slot/overflow FIFO pattern is documented at gateway/run.py:4365-4425.
  • Add a drain-path regression asserting A, B, and C are processed separately with their original sources.

Automated hermes-sweeper review.

Comment thread gateway/platforms/base.py
# same safety net as the pre-debounce path that always queued
# follow-ups. Sender attribution is best-effort when 3+ senders
# collide simultaneously; losing the message entirely is worse.
merge_pending_message_event(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This merges C into A's existing MessageEvent. With merge_text=True, the helper only mutates existing.text and retains A's source (gateway/platforms/base.py:2137-2144), so C is misattributed. Please enqueue a distinct event instead of merging different senders.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
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 P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants