fix: messages out of order

[krulsaidme0w]

Fix notification processing order

Notifications were being processed concurrently, causing them to be handled out of order. This fix serializes notification processing through a dedicated queue and processing goroutine, ensuring notifications are handled in the exact order they are received.

Problem

When multiple notifications arrived in quick succession (e.g., streaming session updates), each notification spawned its own goroutine for processing. Due to goroutine scheduling, notifications could complete in a different order than they were received, breaking applications that depend on ordered message processing.

Solution

• Added notificationQueue channel to serialize notification processing
• Created processNotifications() goroutine that processes notifications sequentially
• Modified receive() loop to queue notifications instead of processing them concurrently

Changes

• Connection struct: Added notificationQueue chan *anyMessage field
• NewConnection(): Starts processNotifications() goroutine
• receive(): Queues notifications instead of spawning goroutines
• processNotifications(): New method that sequentially processes queued notifications

Testing

• All existing tests pass (except TestPromptCancellationSendsCancelAndAllowsNewSession which has a pre-existing timing issue)
• Verified with real-world usage where message ordering is now correct

Backward Compatibility

Fully backward compatible - public API unchanged, only internal processing order modified.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[krulsaidme0w]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[krulsaidme0w]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[krulsaidme0w]

@codex review pls

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[krulsaidme0w]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[ThomasK33]

The implementation looks solid.

One open question about the effectiveness of this PR, along with some style-related changes.

Side note: your commit messages have typos (fix: gracefull conection has spelling errors "graceful", "connection"), please squash these into a single well-written commit describing the change, e.g.: fix: serialize notification processing to maintain message order

[krulsaidme0w]

@codex review

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.

[krulsaidme0w]

@ThomasK33 I’ve updated the PR and fixed the issues you raised. This one is quite important, so I’d appreciate another look when you have time, thank you!

[williamfzc]

This one is quite important 👍

[krulsaidme0w]

Hi @ThomasK33 - hope you’re doing well. Quick ping to see if you could take another look at the updated PR when you have a chance. I’ve fixed the issues you mentioned. Thank you!

[ThomasK33]

Thanks again for the update, and apologies for the delay on my side. I appreciate your patience.

The direction and tests look good overall. I’d like to request one more round of changes, focused on queue safety (details are in the review comments):

• Use a bounded notification queue instead of an unbounded one.
• On overflow, fail fast with explicit connection cancellation and cause (no fallback to concurrent handling).
• Ensure notificationWg remains balanced if enqueue fails, and define the queue size via a named constant.

Once that’s addressed, I’m happy to re-review quickly.

[krulsaidme0w]

Implemented the queue-safety changes requested in review:

• Switched from unbounded queue to a bounded buffered queue (notificationQueue chan *anyMessage) with named capacity constant defaultMaxQueuedNotifications.
• Added explicit overflow handling: enqueue is non-blocking (select with default), and on full queue we fail fast by canceling/closing the connection with explicit cause errNotificationQueueOverflow.
• Ensured notificationWg stays balanced on enqueue failure by calling Done() immediately after failed enqueue.
• Kept ordering guarantees: notifications are still processed sequentially by processNotifications().
• Removed unbounded queue implementation files.
• Added/updated tests, including deterministic overflow coverage (TestConnectionFailsFastOnNotificationQueueOverflow), and verified with go test ./...

[krulsaidme0w]

@ThomasK33 thanks for the re-review and the detailed queue-safety notes.

I’ve implemented the bounded queue + fail-fast overflow behavior as requested (named capacity constant, explicit overflow error/cause, and ensured notificationWg stays balanced on enqueue failure). Ordering remains serialized via the single processing goroutine, and I added a deterministic overflow test to lock in the behavior.

On the queue limit specifically: I’m happy to discuss what a sensible default should be for this SDK (and whether we should consider making it configurable in the future). My current thinking was to pick a conservative fixed bound to avoid unbounded memory growth while still tolerating short bursts, but I’m very open to adjusting based on expected notification rates / consumer behavior.

If you have a preferred bound (or guidance on how you’d like to size it-e.g., based on typical streaming update bursts), I can update the constant accordingly.

Thanks again - once you’re happy with the queue limit choice, I think we should be in good shape to merge.

[ThomasK33]

Thanks for sticking with this. I re-checked the latest updates, and this looks good to me now.

I'll look into getting CI running in another branch/PR and will rebase this before merging.