Fix thread detail subscription race#3174
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Approved Straightforward bug fix for a race condition in thread subscriptions. The change introduces event buffering before snapshot loading using a standard pattern, is limited in scope, and includes a comprehensive test validating the fix. You can customize Macroscope's approvability policy. Learn more. |
Summary
Fixes a race in the thread-detail WebSocket subscription path where live thread events could be emitted while the server was still loading the initial thread snapshot. The subscription now starts observing live thread-detail events before snapshot loading, buffers them, then emits only events newer than the snapshot sequence after the snapshot has been sent.
What Changed
streamThreadDetailEventsAfterSnapshothelper that filters live orchestration domain events to the requested thread, thread-detail event types, and events newer than a given snapshot sequence.subscribeThreadto buffer matching live events whilegetThreadDetailByIdandgetSnapshotSequenceresolve, preventing events that occur during snapshot loading from being missed.thread.message-sentevent.Why
This addresses an intermittent first-prompt rendering bug: when sending the first prompt, the first user message could briefly appear in the conversation and then disappear, leaving only the rest of the conversation. In that failure mode, the first visible conversation item could become the
Working for...label instead of the user’s submitted message.Validation
pnpm exec vp checkpassed; it reported existing lint warnings outside the touched server files.pnpm exec vp run typecheckpassed.pnpm exec vp test run apps/server/src/server.test.ts -t "buffers thread detail events emitted while loading the initial thread snapshot"passed.Proof
Note
Medium Risk
Changes real-time WebSocket subscription ordering for thread detail, which affects client conversation state; scope is localized to subscribeThread with a targeted test.
Overview
Fixes a race in
subscribeThreadwhere thread-detail domain events could fire while the initial snapshot was still loading, so clients could miss the first message (e.g. first user prompt briefly vanishing).The handler now starts listening to live thread-detail events immediately (via a scoped fork into an unbounded queue), loads the thread detail and snapshot sequence, then streams snapshot first followed by buffered/live events whose sequence is greater than the loaded snapshot sequence. A new
streamThreadDetailEventsAfterSnapshothelper centralizes filtering by thread, detail event types, and sequence.Adds a server regression test that delays the snapshot, emits a user message during the gap, and asserts the client gets snapshot then the buffered
thread.message-sentevent.Reviewed by Cursor Bugbot for commit e0e6596. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix race condition in
subscribeThreadthat dropped events emitted during snapshot loadingliveEventQueuein thesubscribeThreadhandler that begins buffering thread detail events (from sequence 0) immediately, before the snapshot is fetched.streamThreadDetailEventsAfterSnapshothelper in ws.ts to encapsulate the filtered event stream logic.Macroscope summarized e0e6596.