Skip to content

fix(gateway): let a serial reader return once a busy burst input is held - #140539

Closed
dokterdok wants to merge 1 commit into
NousResearch:mainfrom
dokterdok:fix/serial-reader-busy-burst
Closed

dokterdok wants to merge 1 commit into
NousResearch:mainfrom
dokterdok:fix/serial-reader-busy-burst

Conversation

@dokterdok

Copy link
Copy Markdown

What does this PR do?

The problem. On Signal, IRC and the other platforms whose receive loop handles one message at a time, photos sent one by one, or a text split over two messages, while a turn is running are no longer merged into one follow-up turn. Each runs as its own turn, where Discord or Slack (and every platform before the unified runtime) gets one. The receive loop also stops reading for about 0.38 s on each such message, for every chat on that account.

Why.

  • dispatch_busy_burst (gateway/session_ingress_burst.py) holds a busy photo or queue-mode text for main's burst window (0.35 s debounce, 1 s cap), then commits the merged input once. Every caller returns only after that commit.
  • Signal's _sse_listener and IRC's _receive_loop await handle_message before they read the next message. So the burst's next input can only arrive after the burst was flushed and committed alone, and it starts a new burst. The wait is also the stall.
  • The existing burst test drives concurrent callers (asyncio.create_task), which a serial loop never is.

The fix.

Invariant: a serial reader returns as soon as a busy input is held, so the next input of the same burst can join it, and the reader never waits out the hold window. It still never runs ahead of what it returned on: a command, and the chat's next burst, wait for those inputs to commit, as main's reader did.

  • An adapter declares serial_receive_loop = True when it delivers a chat's next input only after handle_message returns, and returning triggers no acknowledgement.
  • For such an adapter, dispatch_busy_burst returns once the input is held, as main returned once the input was merged into the pending slot. The burst still commits once and in order, and later input still queues behind it (holds_busy_burst). With no caller waiting, a failed burst commit is logged instead of raised.
  • A command waits for earlier input. handle_message now runs burst_gate first. On a serial reader, a command flushes every held burst and waits until every pending burst commit has landed, then runs. /stop, /new and /reset still drop their own chat's held burst, as main dropped its debounce buffer. So /restart can't close admission under a held text, and /new can't miss a photo that is still capturing.
  • One uncommitted burst per chat. While a chat's burst is still committing, its next burst waits for that commit before it is held. The reader then waits, as main's reader waited at its first commit, so uncommitted input stays bounded.
  • Every other adapter keeps commit-before-return, with its acceptance copied from the commit, as today.

The flag is set on every receive path of that shape I found, each with a one-line comment naming its loop: Signal, IRC, email (the IMAP poll loop), Matrix (one sync at a time), Mattermost, ntfy, Photon, SimpleX, WeCom (WebSocket), WhatsApp (the bridge poll loop, for media) and Feishu (its per-chat lock).

Design choice.

  • Opt-in per adapter, not a new default. Returning early is only safe where returning triggers no acknowledgement. Where it does (an HTTP 200, a saved cursor), commit-before-return is what makes the ACK durable, so the default stays as it is. An adapter missing the flag behaves exactly as today.
  • Order, not a new fence. Commands wait for the reader's own earlier commits at entry. That reproduces main's serial order exactly, needs no new state in the authority, and leaves committed rows to the authority's existing /stop and /new cancellation. A fence carried through admission would duplicate fix(gateway): keep a chat's media and quick text in arrival order #140454's route fence.
  • Every command waits, on the active and the idle path: /approve and /deny, /queue, skill, plugin, quick and unknown commands alike. So a turn a command queues still lands after the held input. A commit never waits on the running turn, so a command sent to unblock that turn can't deadlock behind it.
  • Not flagged on purpose:
    • Buzz has a serial loop, but it saves its cursor only after the batch's handle_message calls return, so that return is an ACK (see Known limits). LINE and WhatsApp Cloud await a POST's messages in order, but answer HTTP 200 only after them.
    • Yuanbao's group consumer also waits for the turn to end after each dispatch, so the flag wouldn't free it.
    • Telegram reads per chat in order, but its photos go through batch tasks and its text through the text batcher, so the burst never holds its reader.
  • Not in BasePlatformAdapter: gateway/platforms/base.py is over its line cap, so the default is read with getattr(adapter, 'serial_receive_loop', False), and the rule is in the module docstring. handle_message's old _shared_burst_pending check moved into burst_gate. Two adapter files over the cap stay line-neutral (the flag replaces a blank line).

Before / after (session authority on, a turn running):

Case Before After
Signal: two photos sent one by one 2 turns; reader held ~0.38 s each 1 turn with both photos; returns at once
IRC, busy_text_mode: queue: two lines 2 turns; reader held each time 1 turn, first half\nsecond half
A text, then an authorized /restart inside the window the text is queued first unchanged: /restart waits for that commit
/new while a photo is still capturing the photo commits, then /new cancels it unchanged
/stop or /new while the chat's burst is still held couldn't arrive before the commit drops the held burst, as main dropped its pending slot
The chat's last burst is still committing the reader waits unchanged: the next burst waits for it
Discord, Slack, webhooks and other concurrent or ACKing adapters merged; returns after commit unchanged

Related Issue

Fixes #140538

Refs #134889

Overlap:

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/session_ingress_burst.py:
    • _accepted returns at once for an adapter with serial_receive_loop; _commit logs a failed commit for such an adapter instead of raising it;
    • new burst_gate: the entry check handle_message runs first. It says whether a burst owns the route, and on a serial reader it makes a command wait for every earlier input;
    • dispatch_busy_burst waits for the chat's committing burst before it holds the next one, on a serial reader;
    • the module docstring states the rule.
  • gateway/platforms/base.py: handle_message awaits burst_gate before routing, on the active and idle paths (net −2 lines; _shared_burst_pending is gone).
  • plugins/platforms/{signal,irc,email,matrix,mattermost,ntfy,photon,simplex,wecom,whatsapp,feishu}/adapter.py: serial_receive_loop = True, with the loop named.
  • tests/gateway/test_busy_burst_serial_reader.py (new), with a real GatewayRunner, SessionAuthority and drain over SQLite:
    • test_signal_busy_photos_merge_without_holding_the_reader: two busy photos through SignalAdapter.handle_message, each awaited before the next, as _sse_listener does. Each call must return while its input is still held, and the follow-up must be one turn with both photos;
    • test_irc_split_text_merges_through_the_receive_loop: two lines fed to IRC's real _receive_loop in queue mode must run as one turn;
    • test_restart_after_a_held_text_waits_for_its_commit: a held text, then an authorized /restart (its restart stubbed to close admission). The text must be queued before admission closes;
    • test_new_waits_for_a_burst_still_committing: /new arrives while a photo's media capture is held. The photo must commit first and be cancelled with the old conversation, not run in the new one;
    • test_next_burst_waits_while_the_chats_last_one_commits: with the chat's first burst held in capture, the next photo's handle_message must not return until that commit lands.

How to Test

scripts/run_tests.sh tests/gateway/test_busy_burst_serial_reader.py
  1. Fail first. On main at 868b9f244f, the two merge tests fail: Signal's first call returns only after its burst committed ('the reader waited out the hold window'), and IRC runs first half and second half as two turns. The other three pass on main and pin its order: a reader that returns early without the gate and the per-chat wait fails them (the text is lost under /restart; the photo runs in the new conversation; the reader goes on with a burst still committing).
  2. On this branch: 5 passed. The touched modules' suites, 0 failed:
    • 154 passed over 15 files: burst merge, serial busy receive, durable acceptance, marker handoff, Signal, IRC, stop and cancel, the producer matrix, warning delivery, text merge, busy ACK, multiplex busy input;
    • 198 passed, 22 skipped over 15 files on handle_message's callers: the platform base, command bypass, busy controls, /busy, /queue, /steer, the reset deadlock, internal events, plaintext approval, guard release, restart drain, command dispatch, unknown commands, Signal formatting;
    • 412 passed, 7 skipped over the flagged adapters' suites (email, Matrix, Mattermost, ntfy, Photon, SimpleX, WeCom, WhatsApp, Feishu) and Yuanbao's pipeline.
  3. scripts/check --base 868b9f244f: 12 checks ok; code health 0 blocking, 0 advisory.

Independent review: one review round, then a final review-and-fix pass.

  • The round: one reviewer found no P1/P2 across the 12 receive paths first flagged. The other found one P1 and two P2s, all from input still held in memory after the reader returned: /restart closing admission under a held text, /stop and /new missing a burst already flushed or reached on the idle path, and an unbounded backlog behind a blocked commit. Each is fixed above and has a regression test here; the idle-path /new variant was checked with a separate probe (it drops the held text). Yuanbao was dropped from the flag list.
  • The final pass re-checked those fixes: the gate on every command path, deadlock and stall paths (a slow or failing commit, a command sent to unblock the turn), whether a held text can run after /stop, the one-burst limit across senders and chats, the teardown limit against main, and composition with fix(gateway): keep a chat's media and quick text in arrival order #140454. It found no P1/P2, so the code is unchanged. Probes run on both main and this branch confirmed that a held text never runs after /stop, that a failing burst commit doesn't hold the next command, and that main loses a held input to a teardown or a drain just as this branch does. Its remaining findings are listed under Known limits.

Known limits:

  • Buzz keeps commit-before-return, so its busy bursts still don't merge and its loop still waits, as on main. Merging there needs its cursor saved behind the commit, a separate change.
  • A held burst lives in memory for up to 1 s before it commits. An external stop (a signal, hermes gateway stop, idle exit), a profile retirement or an adapter teardown in that window loses it, as main loses an input its reader is still holding in the same debounce wait. Returning doesn't trigger an additional acknowledgement on the flagged platforms, but email, Matrix, WhatsApp and Feishu already consume or acknowledge before handle_message, as on main. An in-band command can no longer overtake it.
  • A photo burst followed by a text from the same sender still runs as two turns in order (photos, then text), the burst rule every platform has today. Main merged the text into the photo turn.
  • A command waits for every pending burst commit of its adapter, not only its own chat's. Signal, IRC and the other single-loop readers waited for those anyway. On Feishu, whose chats otherwise run in parallel, a command can now wait for another chat's commit (not its hold window: held bursts are flushed at once).
  • A bare yes answering an approval is held like any queue-mode text. So a second line from the same sender within 0.35 s merges with it and is no longer read as the answer, as on Discord and Slack today.
  • Where an adapter also calls handle_message from a second task (Matrix within one sync, the text batchers), that task can run while a command or the next burst waits on a commit. Two inputs of one chat in the same sync or batch flush can then land out of order (a non-mergeable input ahead of a burst held just before it), or a photo right after a /stop can be dropped with the held burst.
  • The flag list comes from reading each adapter's receive path. Teams' SDK dispatch couldn't be read from the repo, so Teams is left unflagged (as today).

Still needs a live retest:

  • Signal (signal-cli): send a message, then two photos one by one while it answers. Expect one follow-up turn with both. Then a text followed at once by /restart: the text runs after the restart;
  • IRC with busy_text_mode: queue: two lines while a turn runs;
  • Matrix: two photos sent within about 0.3 s that land in separate syncs;
  • one of email, Mattermost or WhatsApp media with two photo messages while busy.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate. Related PRs are listed under Overlap above
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass. Not ticked: I ran the related suites above, not the whole tree; CI is the full run
  • python scripts/check passes
  • I've added tests for my changes
  • I've tested on my platform: macOS (Python 3.14)

Documentation & Housekeeping

  • Documentation: N/A; the rule is in the session_ingress_burst docstring and each adapter's flag comment
  • cli-config.yaml.example: N/A
  • CONTRIBUTING.md / AGENTS.md: N/A
  • Cross-platform: N/A (no file or process handling)
  • Tool descriptions/schemas: N/A

Signal and IRC await handle_message before they read their next input.
A busy photo or queue-mode text was held for the burst window and the
call returned only after the durable commit, so the burst's next input
could arrive only once the burst had committed alone: two turns where
main merged one, and the reader stalled about 0.38 s per busy input.

An adapter now declares serial_receive_loop when it delivers a chat's
next input only after handle_message returns and returning triggers no
acknowledgement. For it, dispatch_busy_burst returns as soon as the
input is held, as main returned once the input was merged into the
pending slot. The burst still commits once and in order, and a failed
commit is logged. Every other adapter keeps commit-before-return.

The reader still never runs ahead of what it returned on:
- a command first waits until every earlier input of that reader has
  committed (/stop, /new and /reset drop their own chat's held burst),
  so /restart cannot close admission under a held input and /new
  cannot miss one still committing;
- a chat holds at most one uncommitted burst: the next one waits for
  that commit, as main's reader waited at its first commit.

Flagged: Signal, IRC, email, Matrix, Mattermost, ntfy, Photon, SimpleX,
WeCom, WhatsApp and Feishu. Buzz is left out: it saves its cursor only
after the call returns.

Refs NousResearch#134889
Fixes NousResearch#140538
@dokterdok

Copy link
Copy Markdown
Author

Landed via #140973 (merged as 54809183676), salvaged with my commits kept as authored. Closing.

@dokterdok dokterdok closed this Oct 18, 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 comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists platform/email Email (IMAP/SMTP) adapter platform/feishu Feishu / Lark adapter platform/matrix Matrix adapter (E2EE) platform/signal Signal CLI adapter platform/whatsapp WhatsApp Business adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Signal, IRC and other one-at-a-time platforms: busy photos and split texts run as separate turns, and the reader stalls on each

2 participants