Skip to content

Fix thread detail subscription race#3174

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

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

Conversation

@Quicksaver

@Quicksaver Quicksaver commented Jun 19, 2026

Copy link
Copy Markdown

ID: 3174
URL: #3174
Title: Fix thread detail subscription race

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 start a scoped live listener before getThreadDetailById and getSnapshotSequence resolve, queueing matching live events that arrive during snapshot loading.
  • Preserved the snapshot-first stream contract by emitting the loaded snapshot first, then draining queued/live events whose sequence is greater than 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 test run apps/server/src/server.test.ts -t "buffers thread detail events emitted while loading the initial thread snapshot" --reporter=dot passed on head 1e7fd5d.
  • pnpm exec vp check passed on head 1e7fd5d, with existing lint warnings outside the PR diff.
  • pnpm exec vp run typecheck passed on head 1e7fd5d.
  • GitHub reports the PR is mergeable and approved; CodeRabbit, Cursor Bugbot, Macroscope, PR Size, PR Vouch, and Vercel checks are successful or skipped where appropriate for head 1e7fd5d.

Proof

  • No proof files are attached.

Note

Medium Risk
Changes real-time WebSocket subscription ordering for thread detail; behavior is intentional but affects all thread subscribers and relies on event sequence vs snapshot sequence correctness.

Overview
Fixes a race where `subscribeThread` could drop live thread-detail events that arrived while the server was still loading the initial snapshot—clients could briefly show the first user message and then lose it.

`subscribeThread` now forks a scoped listener on the domain event stream before `getThreadDetailById` / `getSnapshotSequence` finish, pushing matching events into an unbounded queue. A new `streamThreadDetailEventsAfterSnapshot` helper filters by thread, thread-detail event types, and `sequence > snapshotSequence`. The stream still sends the snapshot first, then queued/live events whose sequence is greater than the loaded snapshot sequence (no duplicates from events already in the snapshot).

Adds a server regression test that delays the snapshot, emits a `thread.message-sent` during the delay, and asserts the client receives snapshot then the buffered event.

Reviewed by Cursor Bugbot for commit 1e7fd5d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix race condition in `subscribeThread` that drops events emitted during snapshot loading

  • Before fetching the thread snapshot, `subscribeThread` in ws.ts now forks a stream that buffers live thread detail events into an unbounded queue.
  • After the snapshot resolves, the buffered stream filters out events with `sequence <= snapshotSequence`, ensuring no events are duplicated or dropped.
  • A new test in server.test.ts verifies that events emitted before the snapshot resolves are still delivered to the client in order.
  • Behavioral Change: clients now always receive events that occurred during snapshot loading, whereas previously those events were silently lost.

Macroscope summarized 1e7fd5d.

@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: 3eb230f9-043c-47ad-ae21-e981051e3db8

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.

@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 race condition fix that buffers events during snapshot loading to prevent them from being dropped. Limited scope with a clear test demonstrating the fix.

You can customize Macroscope's approvability policy. Learn more.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 19, 2026
@Quicksaver

Copy link
Copy Markdown
Author

Comment audit complete: I refreshed the tracked PR comments and found no actionable review feedback. Current comments are informational only: CodeRabbit skipped automated review because reviews are disabled, and Macroscope marked the PR approved. There are no inline review threads to resolve.

@Quicksaver

Copy link
Copy Markdown
Author

Comment audit refreshed: I rechecked the PR issue comments, review comments, and review threads. There is still no actionable review feedback to address. Current comments remain informational only: CodeRabbit skipped automated review because repository reviews are disabled, Macroscope marked the PR approved, and there are no inline review threads to resolve.

@macroscopeapp macroscopeapp Bot dismissed their stale review June 21, 2026 13:02

Dismissing prior approval to re-evaluate 7a1c76d

@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Jun 21, 2026

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service review: one convention violation found in the new desktop backend advertisement error. See inline comment.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/desktop/src/backend/DesktopBackendManager.ts Outdated
Comment thread apps/vscode-extension/src/backendManager.ts Outdated
Comment thread apps/vscode-extension/src/extension.ts Outdated
Comment thread apps/web/src/session-logic.ts Outdated
Comment thread apps/web/src/session-logic.ts Outdated
Comment thread apps/vscode-extension/src/webview.ts Outdated
Comment thread apps/web/src/components/ChatView.tsx

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service convention review found 3 issues. See inline comments.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/config.ts Outdated
Comment thread apps/server/src/git/GitWorkflowService.ts Outdated
Comment thread apps/server/src/auth/http.ts Outdated
Comment thread apps/web/src/components/chat/ChatComposer.tsx Outdated
Comment thread apps/web/src/components/chat/MessagesTimeline.tsx
@Quicksaver Quicksaver force-pushed the fix/thread-detail-subscription-race branch from 9e8cc6b to 58463a9 Compare June 26, 2026 06:34
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:XXL 1,000+ changed lines (additions + deletions). labels Jun 26, 2026
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