Skip to content

fix(gateway): give each queue-mode text follow-up its own turn - #77892

Open
EauDoon wants to merge 7 commits into
NousResearch:mainfrom
EauDoon:fix/queue-mode-text-followup-merge
Open

EauDoon wants to merge 7 commits into
NousResearch:mainfrom
EauDoon:fix/queue-mode-text-followup-merge

Conversation

@EauDoon

@EauDoon EauDoon commented Aug 3, 2026

Copy link
Copy Markdown

What does this PR do?

With display.busy_input_mode: queue, every TEXT message sent while the agent is busy is newline-joined into a single pending event and answered as one turn. Message boundaries are destroyed, so the model receives an unlabelled blob and pairs answers to the wrong questions.

The path:

  1. run.py::_handle_active_session_busy_message declines plain TEXT in queue mode and returns False:
if (
    event.message_type == MessageType.TEXT
    and busy_text_mode == "queue"
    and effective_mode != "steer"
):
    return False
  1. Control falls through to the adapter's debounce, and _flush_text_debounce_now pushes the burst into the single pending slot via merge_pending_message_event(..., merge_text=True), which does:
existing.text = f"{existing.text}\n{event.text}"

That merge has no time bound. It applies to a message arriving 30 seconds later exactly as to one arriving 0.3 seconds later, so an entire conversation's worth of follow-ups collapses into one turn. The drain then pops one event and runs it as a single turn.

This is already recognised in-tree. The comment at the FIFO site in run.py describes the raw merge as destroying message boundaries "so two separate user messages sent while the agent was busy... arrived as one mashed-together turn", and routes through _enqueue_fifo to fix it. That fix reached interrupt mode, steer-fallback and /queue. The queue-mode text path returns False before ever getting there.

The fix: hand the flushed burst to the runner's _queue_or_replace_pending_event — the same FIFO entry point those other paths use — so each follow-up gets its own turn in arrival order.

Merging inside the debounce window (0.35s rolling / 1.0s hard cap) is deliberately preserved. A single thought split across two quick taps should stay one turn; that is what the debounce is for.

The runner is reached through the bound _busy_session_handler it already installs on the adapter, so this needs no wiring changes in run.py and stays a single-file change. Adapters with no runner attached (TUI, standalone, tests) fall back to the previous merge, keeping #28503's no-message-loss guarantee intact.

Related Issue

No exact issue exists, so I did not use Fixes #.

Type of Change

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

Changes Made

  • gateway/platforms/base.py_flush_text_debounce_now now routes the flushed burst to the runner's FIFO, falling back to the existing merge when no runner is attached or the enqueue raises.
  • tests/gateway/test_debounce_flush_fifo.py — new, 6 tests.

How to Test

Reproduction:

  1. Set display.busy_input_mode: queue in config.yaml.
  2. Send a message that starts a long turn.
  3. While it runs, send three more distinct questions, spaced a couple of seconds apart.
  4. Before: one reply that mixes all three, often pairing answers to the wrong questions. After: three turns, each answering its own question in arrival order.

Automated:

pytest tests/gateway/test_debounce_flush_fifo.py -q          # 6 passed
pytest tests/gateway/test_active_session_text_merge.py -q    # 7 passed, unchanged

The two behavioural tests fail on main and pass with the fix:

# with gateway/platforms/base.py reverted
FAILED test_successive_bursts_each_get_their_own_turn
FAILED test_third_burst_appends_in_arrival_order
2 failed, 4 passed

# with the fix
6 passed

The other two tests pin the fallback (no runner attached, and a raising FIFO) and pass either way by design.

Wider regression check across the related gateway suites — test_active_session_text_merge, test_queue_command, test_queue_consumption, test_pending_drain_no_recursion, test_pending_drain_race, test_pending_event_none, test_busy_session_ack, test_discord_pending_text_batch_shutdown, test_internal_event_never_interrupts_busy_session39 passed.

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
  • My PR contains only changes related to this fix
  • I've run the relevant suites and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Ubuntu 24.04 (Docker, live gateway, Telegram) and Windows 11 (unit tests)

Documentation & Housekeeping

  • I've updated relevant documentation — N/A, no user-facing config or API change
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A, no new keys
  • I've updated CONTRIBUTING.md or AGENTS.md — N/A
  • I've considered cross-platform impact — N/A, no file I/O, process or terminal handling; pure in-memory dispatch
  • I've updated tool descriptions/schemas — N/A

Notes for the reviewer

Two things worth a look:

The 32-message cap now applies here. Text follow-ups that previously merged now occupy FIFO slots, so _BUSY_QUEUE_MAX_PENDING (32) can be reached where it could not before. Far beyond any realistic conversation, but it drops with a logger.warning and nothing user-visible. Happy to add a user-facing notice if you'd like it in scope.

The fallback is deliberate but worth confirming. The FIFO lives on the runner, so an adapter with no runner has nothing to enqueue into. Falling back to the merge is the only option there that preserves #28503's guarantee. It does mean the existing tests in test_active_session_text_merge.py pass unchanged — they use _make_adapter(), which sets _busy_session_handler = None. That is why the new test file attaches a runner explicitly.

This was found and validated on a live deployment running v0.19.0 (2026.7.20) before being reproduced against main.

Relation to other open PRs

Happy to rebase or fold this in if maintainers would rather land it alongside either.

Fallback hardening

_queue_or_replace_pending_event can decline silently — it returns without queueing and without raising when the source resolves to no adapter, or when _BUSY_QUEUE_MAX_PENDING (32) is reached. Treating the call as success would drop the burst, and the cap was effectively unreachable before this change since the old merge collapsed every follow-up into one slot rather than one entry each.

The flush therefore confirms the queue actually grew, and falls back to the historical merge when it did not. Two cases take the historical path directly:

  • A media occupant in the pending slot. _queue_or_replace_pending_event caption-merges into it and does not grow the queue, which the depth check would misread as a decline and merge a second time.
  • A source that resolves to a different adapter. That adapter owns a different pending slot, and the drain that delivers this burst runs on ours.

Merging is lossy; dropping is worse. Covered by test_merge_preserved_when_fifo_declines_silently, test_merge_preserved_when_adapter_unresolvable, test_enqueue_failure_falls_back_to_merge and test_merge_preserved_when_no_runner_attached.

The debounce flush handed every burst to merge_pending_message_event with
merge_text=True, which newline-joins onto whatever already occupies the
single pending slot. That merge has no time bound, so all follow-ups sent
during a long turn collapsed into one turn and the model lost the message
boundaries it needs to answer each question separately. Users see answers
paired to the wrong question, or several questions answered as one blob.

Route the flushed burst through the runner's _queue_or_replace_pending_event
instead. That is the same FIFO entry point interrupt mode, steer-fallback
and /queue already use, so each follow-up now gets its own turn in arrival
order. Merging inside the debounce window is preserved on purpose: a single
thought split across two quick taps should stay one turn.

The runner is reached through the bound _busy_session_handler it already
installs on the adapter, so no wiring changes are needed in run.py. Adapters
with no runner attached (TUI, standalone, tests) fall back to the previous
merge, which keeps the no-message-loss guarantee from NousResearch#28503 intact.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EauDoon added 2 commits August 4, 2026 01:55
_queue_or_replace_pending_event returns early without queueing when it
cannot resolve an adapter for the event source. Delegating into that path
unconditionally would DROP the burst where the previous merge would have
kept it. Guard on the runner resolving the source back to this adapter and
fall back to the merge otherwise: merging is lossy, dropping is worse.
Give the stub runner an _adapter_for_source (the real runner has one) and
add a regression test asserting an unresolvable source falls back to the
merge instead of being dropped.
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P2 Medium — degraded but workaround exists labels Aug 3, 2026
EauDoon added 4 commits August 4, 2026 02:06
_queue_or_replace_pending_event returns WITHOUT queueing and WITHOUT
raising when the source resolves to no adapter or the per-session pending
cap is reached. Treating that as success dropped the burst, where the
historical merge would still have delivered it. The cap was also
effectively unreachable before, since the old merge collapsed every
follow-up into one slot instead of one entry each. Confirm the queue
actually grew and fall back to the merge when it did not.
…wning adapters

Two cases the depth check alone got wrong. A media occupant in the pending
slot is caption-merged by _queue_or_replace_pending_event WITHOUT growing
the queue, which read as a decline and merged a second time. And a source
that resolves to a different adapter owns a different pending slot, so the
burst would land where this adapter drain never looks. Both now take the
historical merge.
The flush now confirms the queue actually grew before treating the FIFO
handoff as success, so a stub without _queue_depth never exercises the
FIFO path at all and the suite silently stops proving the fix.
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:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants