Skip to content

Fix out-of-order notification processing#18

Closed
agentcooper wants to merge 2 commits intocoder:mainfrom
agentcooper:agentcooper/acp-client-fix-out-of-order
Closed

Fix out-of-order notification processing#18
agentcooper wants to merge 2 commits intocoder:mainfrom
agentcooper:agentcooper/acp-client-fix-out-of-order

Conversation

@agentcooper
Copy link
Contributor

I've noticed that the ACP client sometimes receives text chunk updates in the incorrect order.

I've added a failing integration test where the client connects to the server that sends numbers from 1 to 100 and the client verifies the order.

The fix changes notification (messages like session/update) processing from concurrent goroutine spawning to a serial queue.

@agentcooper agentcooper changed the title agentcooper/acp client fix out of order Fix out-of-order notification processing Jan 30, 2026
agentcooper added a commit to agentcooper/acp-go-sdk that referenced this pull request Feb 5, 2026
@inercia
Copy link

inercia commented Feb 6, 2026

I'm also seeing this problem.

@ThomasK33
Copy link
Member

Thanks for working on this and for adding the integration coverage around ordering.

I think this is effectively a duplicate of #8, and it also reintroduces the same class of queue-handling issues we discussed in the earlier iterations there:

  • notificationQueue is bounded (make(chan *anyMessage, 1000)), but enqueue in the read loop is blocking (c.notificationQueue <- &msgCopy).
  • If that queue fills, receive() can block and stop reading further frames, including responses, which can cause in-flight requests to hang.
  • There is no explicit overflow policy (and if we add “fallback to concurrent handling” to avoid blocking, that would again break ordering guarantees).

Given that #8 already has the full history, discussion, and related fixes, I’d prefer that we consolidate there and close this as a duplicate.

@ThomasK33 ThomasK33 closed this Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants