fix(server): start thread live subscribe immediately - #8420
Conversation
subscribeShell already starts its live buffer fork now. subscribeThread did not. Effect 4 runs forkScoped on a later tick unless startImmediately is set, so an event published while the thread snapshot loads can miss the buffer. The existing buffer test published after a 25ms sleep, which hid that gap. It now publishes during the snapshot load, same as the shell test.
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused server bug fix that starts the existing thread live-event buffer before snapshot or replay work, preventing events from being missed during that window. The accompanying test removes an artificial delay and directly verifies delivery during snapshot loading. You can add or adjust custom eligibility rules. Learn more. |
What Changed
subscribeThreadnow starts its live event buffer fork withstartImmediately: true, the same assubscribeShell.The test that covers events published while the thread snapshot loads no longer waits 25ms before publishing. That wait let the late fork still catch the event, so a lost event would not fail the test. It now publishes during the snapshot load.
Why
The comment on
subscribeThreadalready says live delivery must attach before the snapshot loads, or an event can be lost. Effect 4 runsforkScopedon a later tick unless you passstartImmediately. Shell already passed it. Thread did not.A message or activity that lands in that gap never reaches the client. Reconnect cannot put it back if catch-up already used a snapshot taken before that event.
UI Changes
None.
Checklist
Note
Low Risk
Small, targeted WebSocket subscription fix aligned with existing shell behavior; test change only tightens coverage.
Overview
Thread WebSocket subscriptions now fork the live event buffer with
startImmediately: true, matchingsubscribeShell. Under Effect 4,forkScopedruns on a later tick by default, so events published while the thread snapshot or catch-up replay was still loading could miss the live stream and never reach the client.The integration test for “message sent during snapshot load” publishes the event immediately during snapshot fetch instead of sleeping 25ms first, so a regression in that ordering would fail instead of being masked by delayed scheduling.
Reviewed by Cursor Bugbot for commit 70be50a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Start WebSocket live stream subscription fiber immediately in
makeWsRpcLayerPasses
{ startImmediately: true }to theEffect.forkScopedcall in ws.ts so the live delivery fiber starts on the current tick, before snapshot or replay reads begin. This closes a window where events published during snapshot loading could be lost. The test stub in server.test.ts removes a 25 ms sleep and now publishes a live event during snapshot load to exercise this timing.Macroscope summarized 70be50a.