Repository navigation
Deliver SSE terminal frames after a mailbox overflow - #338
Merged
Merged
Conversation
sse_mailbox_push returned false for force_deliver once overflowed was set, so a full mailbox that rejected one more live event also dropped the subsequent terminal envelope. Request replay and A2A resubscribe then closed as slow_consumer without the real done/status. Honor force_deliver after overflow and clear the flag so the drain loop can write the terminal frame. Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
Keep both Unreleased changelog entries. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fe8d9707-329b-43bb-8e35-a32e167dbd70) |
Keep both Unreleased changelog entries. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep both Unreleased changelog entries. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
sse_mailbox_pushreturns false as soon asoverflowedis set.force_deliveris supposed to evict oldest events so a terminal SSE envelope (env.terminalon request replay and A2A resubscribe) still lands and the stream can close cleanly.The overflow check ran first. A full mailbox that rejected one more live event therefore also dropped the subsequent
done/ task-status frame. The subscriber drain loop inhandle_request_events/ A2A resubscribe then wroteslow_consumerand exited without the real terminal. The existing unit test only covered force-deliver while the mailbox was full but not yet overflowed.Fix
Honor
force_deliverafter overflow: evict oldest items if still at cap, push the terminal envelope, and clearoverflowedso the drain loop writes that frame instead of aborting.Non-terminal pushes after overflow still reject.
Tests
unit_sse_mailbox— overflow then force-deliver: terminal is at the back,overflowedis cleared, dropped live event never queued. All 3 cases / 6163 assertions locally (including ASan). Consumer-loop probe matching the API drain also delivereddoneinstead ofslow_consumer.Independently mergeable against
main(0456350). Code hunks (include/sse_mailbox.h,tests/test_sse_mailbox.cpp) do not overlap #321–#337.git merge-tree --write-treeauto-merges those siblings, including CHANGELOG[Unreleased].Note
Low Risk
Small, targeted change to bounded SSE buffering with a new regression test; affects slow-client stream teardown only, not normal event delivery.
Overview
Fixes a bug where slow SSE subscribers could miss the real stream ending after the mailbox had already overflowed.
sse_mailbox_pushno longer rejects every push onceoverflowedis set. Withforce_deliver=true(terminaldone/ task-status envelopes), it still evicts oldest buffered events if needed, enqueues the terminal frame, and clearsoverflowedso the drain loop emits that frame instead of onlyslow_consumer. Non-terminal pushes after overflow still fail as before.Adds a unit test for overflow-then-force-deliver and documents the fix in CHANGELOG.
Reviewed by Cursor Bugbot for commit 46172fa. Bugbot is set up for automated code reviews on this repo. Configure here.