Skip to content

Fix thread detail subscription race#3174

Open
Quicksaver wants to merge 6 commits into
pingdotgg:mainfrom
Quicksaver:fix/thread-detail-subscription-race
Open

Fix thread detail subscription race#3174
Quicksaver wants to merge 6 commits into
pingdotgg:mainfrom
Quicksaver:fix/thread-detail-subscription-race

Conversation

@Quicksaver

@Quicksaver Quicksaver commented Jun 19, 2026

Copy link
Copy Markdown

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

  • Added a streamThreadDetailEventsAfterSnapshot helper that filters live orchestration domain events to the requested thread, thread-detail event types, and events newer than a given snapshot sequence.
  • Updated subscribeThread to buffer matching live events while getThreadDetailById and getSnapshotSequence resolve, preventing events that occur during snapshot loading from being missed.
  • Preserved the snapshot-first stream contract by concatenating the snapshot event before buffered/live events and filtering the buffer against the loaded snapshot sequence.
  • Added a server regression test that delays the initial thread snapshot, emits a first user message during that delay, and verifies the subscriber receives the snapshot followed by the buffered thread.message-sent event.

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 check passed; it reported existing lint warnings outside the touched server files.
  • pnpm exec vp run typecheck passed.
  • 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

  • No proof files are attached.

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 subscribeThread where 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 streamThreadDetailEventsAfterSnapshot helper 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-sent event.

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 subscribeThread that dropped events emitted during snapshot loading

  • Before this fix, thread detail events emitted while the initial snapshot was being fetched were silently dropped, as the stream only started filtering after the snapshot returned.
  • Introduces a liveEventQueue in the subscribeThread handler that begins buffering thread detail events (from sequence 0) immediately, before the snapshot is fetched.
  • After the snapshot loads, buffered events with a sequence greater than the snapshot's sequence are replayed, then live events continue from the queue.
  • Adds a streamThreadDetailEventsAfterSnapshot helper in ws.ts to encapsulate the filtered event stream logic.
  • A new test in server.test.ts verifies the fix by blocking the snapshot query behind a gate and asserting that a pre-gate event appears in the subscription output.

Macroscope summarized e0e6596.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 53dad657-4716-4dc2-87ed-55c206caca18

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jun 19, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant