Skip to content

fix(chat): preserve message order across history and live delivery - #760

Merged
mvschwarz merged 1 commit into
mvschwarz:mainfrom
rudycelekli:fix/chat-watch-buffer-order-20261004
Oct 7, 2026
Merged

mvschwarz merged 1 commit into
mvschwarz:mainfrom
rudycelekli:fix/chat-watch-buffer-order-20261004

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

What a user gets

Chat watchers receive messages in their persisted order while the initial history switches to live delivery. A message arriving during the awaited buffered drain now joins its tail rather than overtaking an older buffered message.

This affects ordinary rig chatroom watch and the existing /api/rigs/:rigId/chat/watch SSE surface. The patch only moves the existing live-mode transition after the pending queue is drained.

How you verified it

  • Base f53558d982ec413a76a20be3c08c2c7effc180cf.
  • Before: 1 failing ordering regression and 19 passing controls in the actual chat route suite. The stream emitted buffered A, newly arrived C, then older buffered B, unlike SQLite history.
  • After: 33 passing chat route/repository tests across two files. The new cases compare every streamed ID with persisted history and verify uniqueness and subscription release on disconnect.
  • Tests use actual Hono SSE/TransformStream writes, migrated native SQLite, ChatRepository and EventBus. A write observer schedules arrivals at the replay/drain boundaries; it calls the original writer and does not fabricate stream output. This is a deterministic model-free route fixture, not an authenticated provider or browser test.
  • Full build and lint both pass. All eight exact-head fork workflow jobs passed before submission.

Downstream effect / scenario coverage

Only the history-to-live ordering seam changes. Subscription setup, initial history count, message bodies, deduplication and disconnect cleanup remain in the existing path. The stub scenario runner has no chatroom send/watch action or streaming-order assertion; the real route/stream/SQLite comparison exercises this seam directly.

Anything you were unsure about

This does not add SSE reconnect/replay behavior or a new buffer policy. It does not claim live provider or browser testing.

Prepared with AI assistance under human direction. The source change and paired regression were reviewed before submission.

Exact-head verification

All eight unchanged fork Tests jobs passed at 7d7ac43f6bb024e095c3d43f78a12687227f3deb: workflow evidence. Upstream PR checks are observed separately after submission.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6970d3b0-149e-4810-b121-a2349536db0e
📥 Commits

Reviewing files that changed from the base of the PR and between bd82e19 and 7d7ac43.

📒 Files selected for processing (2)
  • packages/daemon/src/routes/chat.ts
  • packages/daemon/test/chat-routes.test.ts
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@mvschwarz

Copy link
Copy Markdown
Owner

Thanks, @rudycelekli, for keeping chat messages in order across history and live delivery. We've got it, and it's queued with your other PRs until the current release is cut. We'll reply here with the outcome.

@openrig-review openrig-review left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved at 7d7ac43. A chat watcher no longer receives a newer live message before an older buffered one: the existing phase transition now happens after the history drain. No new API, dependency, refusal, exit code, timeout or retry. Independent review MERGE-READY at this head; required CI passes.

— dev60-planner@v-openrig-build

@mvschwarz
mvschwarz merged commit 5dadaf0 into mvschwarz:main Oct 7, 2026
10 checks passed
@mvschwarz

Copy link
Copy Markdown
Owner

Merged. Thanks, @rudycelekli, for keeping chat messages in order across history and live delivery. It's on main now, not in a release yet.

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