Skip to content

fix(gateway): preserve queued turns at recursion cap - #78015

Open
smicbee wants to merge 2 commits into
NousResearch:mainfrom
smicbee:fix/queue-recursion-fifo-loss
Open

smicbee wants to merge 2 commits into
NousResearch:mainfrom
smicbee:fix/queue-recursion-fifo-loss

Conversation

@smicbee

@smicbee smicbee commented Aug 3, 2026

Copy link
Copy Markdown

What does this PR do?

Prevents accepted /queue turns from being silently dropped when the gateway reaches its interrupt recursion cap.

The failure occurs after the current FIFO head has been drained and _promote_queued_event() has already staged the next item in the adapter's single pending slot. The recursion-cap path then requeues the current event by merging it into that same slot, overwriting the staged successor.

Example state transition before this fix:

initial:             slot=Q1, overflow=[Q2, Q3]
after drain/promote: pending=Q1, slot=Q2, overflow=[Q3]
recursion-cap requeue (old): slot=Q1, overflow=[Q3]  # Q2 lost

This PR restores the unprocessed current event at the FIFO head while first moving any staged successor back to the front of overflow:

recursion-cap requeue (new): slot=Q1, overflow=[Q2, Q3]

Each accepted queue item therefore remains a separate full turn in original arrival order.

Related Issue / PR overlap

No dedicated issue found.

The recursion-cap behavior also appears inside broader platform-specific PRs #72674 (Email boundaries) and #76818 (Slack recovery). This PR intentionally isolates the generic message-loss fix and regression test on current main, without taking either PR's unrelated Email or Slack changes.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/run.py
    • Add _requeue_event_at_front() to restore the drained event without overwriting an already promoted successor.
    • Use the helper at the interrupt recursion cap instead of merge_pending_message_event().
  • tests/gateway/test_queue_consumption.py
    • Add a Q1 → Q2 → Q3 regression that reproduces drain, promotion, recursion-cap requeue, and complete FIFO consumption.

How to Test

scripts/run_tests.sh \
  tests/gateway/test_queue_consumption.py \
  tests/gateway/test_queue_command.py \
  tests/gateway/test_pending_drain_no_recursion.py \
  tests/gateway/test_pending_drain_race.py \
  tests/gateway/test_active_session_text_merge.py \
  tests/gateway/test_steer_fifo_overwrite.py \
  tests/gateway/test_busy_session_ack.py \
  tests/gateway/test_discord_pending_text_batch_shutdown.py \
  -q

python -m ruff check gateway/run.py tests/gateway/test_queue_consumption.py
git diff --check origin/main...HEAD

Local result on Linux:

  • 35 passed, 0 failed across the 8 affected Gateway files
  • Ruff: passed
  • git diff --check: passed

The full repository suite was started successfully but not completed locally because it currently contains more than 20,000 tests; the focused Gateway regression set above was run through the canonical wrapper.

Checklist

Code

  • I've read the Contributing Guide
  • My commit message follows Conventional Commits
  • I searched existing issues and PRs and documented the overlapping broader PRs above
  • This PR contains only the generic queue-loss fix and its regression test
  • Full repository test suite completed locally
  • Added behavioral regression coverage
  • Tested on Linux

Documentation & Housekeeping

  • Documentation update: N/A; no user-facing behavior or configuration is added
  • cli-config.yaml.example: N/A; no config keys changed
  • CONTRIBUTING.md / AGENTS.md: N/A; existing FIFO architecture is preserved
  • Cross-platform impact considered; change uses platform-independent Python data structures
  • Tool descriptions/schemas: N/A

@alt-glitch alt-glitch added type/bug Something isn't working 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 labels Aug 3, 2026
@Adolanium

Copy link
Copy Markdown

I checked this on current main and the bug is still there. This PR is the right fix, but it no longer applies because gateway/run.py was split into siblings. What is needed to land it:

  1. Move the cap site. The recursion cap is now in gateway/run_turn.py (around line 3814). It still calls merge_pending_message_event(adapter._pending_messages, session_key, pending_event), which replaces the staged next item. Swap that call for _requeue_event_at_front(...).
  2. Move the helper. _promote_queued_event, _enqueue_fifo and _queue_depth now live in gateway/run_busy.py, so _requeue_event_at_front belongs next to them. The overflow list is self._session_state(session_key).conversation.queued_events (see _overflow_queue in the same file), which matches what the helper already uses.
  3. Drop the queue_message branch right after the cap call. No adapter defines queue_message any more, so that branch is dead.

The Q1, Q2, Q3 FIFO test is a good invariant test and can move over as is, next to the other tests in tests/gateway/test_queue_consumption.py.

After those changes it is a small, focused diff. I have not opened a competing PR, so this one can carry the fix.

@driptothebone

Copy link
Copy Markdown

I reproduced the recursion-cap overwrite on current main: putting the unrun head back into the adapter slot replaces the next FIFO item that was already staged. I have a private port of your fix to the split gateway modules, with real runner/adapter tests that follow ten distinct messages through the depth cap and check separate text/photo/document payloads. Would you welcome a port and those tests on this PR? I would build on your work rather than open a competing PR.

@smicbee
smicbee force-pushed the fix/queue-recursion-fifo-loss branch from 58a2cd5 to bea3d26 Compare October 1, 2026 12:04
@smicbee

smicbee commented Oct 1, 2026

Copy link
Copy Markdown
Author

@Adolanium thanks, that was exactly the right pointer. I ported the fix to the split modules and force-pushed the branch (rebased on current main, single commit bea3d26):

  • _requeue_event_at_front() now lives in gateway/run_busy.py next to _promote_queued_event / _enqueue_fifo, using the queued_events overflow list.
  • The cap site in gateway/run_turn.py calls it instead of merge_pending_message_event(); the dead queue_message branch and the now-unused import are removed.
  • The Q1→Q2→Q3 FIFO test is carried over unchanged in tests/gateway/test_queue_consumption.py.

Local run of the 8 affected gateway test files: 37 passed, 0 failed. Diff is 3 files, +64/-4.

@driptothebone thanks for reproducing this and for the offer. I've just finished the port myself, but your real runner/adapter tests (ten messages through the depth cap, separate text/photo/document payloads) would be a valuable addition. Feel free to open a PR against my branch smicbee:fix/queue-recursion-fifo-loss or paste them here, and I'll fold them in.

@alt-glitch alt-glitch added P1 High — major feature broken, no workaround and removed P2 Medium — degraded but workaround exists labels Oct 1, 2026
@smicbee

smicbee commented Oct 1, 2026

Copy link
Copy Markdown
Author

@Adolanium thanks again for the pointer. The port to the split modules is in (bea3d26), and @driptothebone's real runner/adapter tests are now merged into this branch as well (head 6147b53): ten messages through the depth cap, plus separate TEXT/PHOTO/DOCUMENT payloads with and without an overflow tail. With the fix, 22 tests pass in the two affected files. With gateway/ reverted to the pre-fix state, exactly the 7 new tests fail. This should be ready for another look.

@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists and removed P1 High — major feature broken, no workaround labels Oct 1, 2026

kvnloo commented Oct 1, 2026

Copy link
Copy Markdown

FIFO/requeue review: the updated split-module port preserves event identity instead of feeding the recursion-cap item back through merge semantics. _requeue_event_at_front() first moves the already-promoted successor back to overflow, then restores the unprocessed head in the adapter slot, which gives Q1,Q2,Q3 back exactly. The real-runner tests also exercise ten turns across depth resets and TEXT/PHOTO/DOCUMENT payloads, so this is no longer only a helper-level invariant. I don't see a blocker in the current port.

This branch has not been deployed

No deployments
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.

6 participants