[AIT-285] Fix clearing buffered object operations during sync sequence by VeskeR · Pull Request #2150 · ably/ably-js

VeskeR · 2026-01-16T11:59:28Z

Buffered object operations are now only cleared on non-resume ATTACHED, not when a new OBJECT_SYNC sequence starts. This is because the realtime server caches object state at the moment of channel attachment, so all operations received since attachment must be preserved and applied.

For server-initiated resync, the server is expected to send an ATTACHED message with RESUMED=false before the new OBJECT_SYNC sequence. However, if an OBJECT_SYNC is received after a completed sync without preceding ATTACHED, the client handles this as best-effort by buffering operations from that point. Since the buffer is cleared at sync completion, this works implicitly.

Resolves AIT-285

Summary by CodeRabbit

Bug Fixes
- Improved LiveObjects attach behavior: during non-resumed attachments buffered object operations are now cleared to ensure correct re-synchronization and hydration.
- Better handling of buffering and discard semantics between attach and sync sequences.
Tests
- Expanded and consolidated tests for ATTACHED scenarios, validating resumed vs non-resumed behavior and buffer semantics.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-16T11:59:41Z

Walkthrough

Renames LiveObjects' attach handler to onNonResumeAttached and moves clearing of buffered object operations from sync start to non-resumed ATTACHED handling; tests updated with a shared ATTACHED injector and expanded coverage for resumed vs non-resumed attachment sequences.

Changes

Cohort / File(s)	Summary
Channel → Object attach handling `src/common/lib/client/realtimechannel.ts`	`processMessage` now invokes `RealtimeObject.onNonResumeAttached(hasObjects)` for non-resumed ATTACHED messages (one-line call site change).
LiveObjects: attach & sync logic `src/plugins/liveobjects/realtimeobject.ts`	Renamed `onAttached(...)` → `onNonResumeAttached(...)`; moved `_bufferedObjectOperations` clearing from `_startNewSync()` into `onNonResumeAttached()` and updated related logs/comments and sync-start behavior.
Tests: ATTACHED injection & scenarios `test/realtime/liveobjects.test.js`	Added `injectAttachedMessage(helper, channel, flags)` helper and updated many tests to use it; expanded scenarios to distinguish ATTACHED with/without RESUMED and HAS_OBJECTS flags and validate buffering/hydration semantics.

Sequence Diagram(s)

sequenceDiagram
    participant Server
    participant Transport
    participant Channel
    participant RealtimeObject

    Note over Server,Transport: Non-resumed ATTACHED (HAS_OBJECTS maybe set)
    Server->>Transport: ATTACHED (flags, maybe HAS_OBJECTS)
    Transport->>Channel: protocol message
    Channel->>RealtimeObject: processMessage -> onNonResumeAttached(hasObjects)
    RealtimeObject->>RealtimeObject: clear _bufferedObjectOperations
    RealtimeObject->>RealtimeObject: start new sync (OBJECT_SYNC flow)
    Server->>Transport: OBJECT_SYNC... (sync messages)
    Transport->>Channel: sync messages
    Channel->>RealtimeObject: handleObjectSyncMessages...
    RealtimeObject->>RealtimeObject: after sync end -> flush buffered ops

sequenceDiagram
    participant Server
    participant Transport
    participant Channel
    participant RealtimeObject

    Note over Server,Transport: Resumed ATTACHED (RESUMED flag set)
    Server->>Transport: ATTACHED (RESUMED | flags)
    Transport->>Channel: protocol message
    Channel->>RealtimeObject: processMessage -> (resumed attach path)
    RealtimeObject->>RealtimeObject: do not clear buffered ops on attach
    Server->>Transport: OBJECT_SYNC... (if any)
    Transport->>Channel: sync messages
    Channel->>RealtimeObject: handleObjectSyncMessages...
    RealtimeObject->>RealtimeObject: flush/apply buffered ops when appropriate

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰
I hopped in when the channel clicked "attach",
Cleared my crumbs, then watched the sync dispatch,
Buffered carrots kept safe till the end,
Now every hop and nibble finds its friend. 🥕✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing the clearing of buffered object operations during sync sequences, which is the core objective of this PR.
Linked Issues check	✅ Passed	The PR implements all required behavior from AIT-285: clearing buffered ops on non-resume ATTACHED, preserving them during OBJECT_SYNC starts, and applying them on sync completion.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue objectives: renaming onAttached to onNonResumeAttached, moving buffer clearing to attachment time, and expanding test coverage for the new semantics.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch AIT-285/fix-clearing-buffered-ops-during-sync

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e95e32e and 06b746a.

📒 Files selected for processing (3)

src/common/lib/client/realtimechannel.ts
src/plugins/liveobjects/realtimeobject.ts
test/realtime/liveobjects.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

src/plugins/liveobjects/realtimeobject.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: test-npm-package
GitHub Check: test-browser (webkit)
GitHub Check: test-node (20.x)
GitHub Check: test-browser (firefox)
GitHub Check: test-node (16.x)
GitHub Check: test-browser (chromium)
GitHub Check: test-node (18.x)

🔇 Additional comments (8)

src/common/lib/client/realtimechannel.ts (1)

566-568: LGTM — correct non-resume hook for LiveObjects.
Switching to onNonResumeAttached matches the updated resync behavior and keeps object buffering semantics intact.

test/realtime/liveobjects.test.js (7)

183-197: Centralized ATTACHED injection helper is a good consolidation.
Routing ATTACHED through the transport keeps protocol flow realistic and reduces duplication across tests.

2501-2549: Non‑resume ATTACHED buffer discard scenario is well covered.
The test cleanly separates pre‑ATTACHED vs post‑ATTACHED ops and validates the intended discard/apply behavior.

2552-2600: RESUMED ATTACHED path preserves buffered ops as intended.
Good coverage that ops on both sides of a resumed ATTACHED are applied after sync.

2602-2653: New OBJECT_SYNC sequence test aligns with buffer‑retention rules.
This captures the regression case nicely and asserts both sequences’ ops are retained.

2655-2698: Best‑effort resync without ATTACHED is nicely exercised.
The scenario documents and validates the fallback buffering behavior.

2824-2826: Description update clarifies post‑sync behavior.
Nice small wording improvement for readability.

8515-8516: Using injectAttachedMessage keeps sync‑event tests consistent.
This refactor improves test cohesion and mirrors real transport handling.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.5)

test/realtime/liveobjects.test.js

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.}

Buffered object operations are now only cleared on non-resume ATTACHED, not when a new OBJECT_SYNC sequence starts. This is because the realtime server caches object state at the moment of channel attachment, so all operations received since attachment must be preserved and applied. For server-initiated resync, the server is expected to send an ATTACHED message with RESUMED=false before the new OBJECT_SYNC sequence. However, if an OBJECT_SYNC is received after a completed sync without preceding ATTACHED, the client handles this as best-effort by buffering operations from that point. Since the buffer is cleared at sync completion, this works implicitly. Resolves AIT-285

mschristensen

LGTM, just a small comment

mschristensen · 2026-01-20T12:41:45Z

src/plugins/liveobjects/realtimeobject.ts

+    // The realtime server caches the object state at the moment of channel attachment,
+    // so we must start fresh and buffer all incoming OBJECT messages from this point.


I think a more accurate description would be something like: "The realtime server will deliver a sync sequence following a non-resume attached, and guarantees that the objects sent in that sequence will at least include all operations up to the point of attachment."

lawrence-forooghian · 2026-01-21T18:42:59Z

src/common/lib/client/realtimechannel.ts

            // the Objects tree needs to be re-synced
            if (this._object) {
-              this._object.onAttached(hasObjects);
+              this._object.onNonResumeAttached(hasObjects);


Why is all of the on-ATTACHED behaviour now gated behind the absence of the RESUMED flag? The RTO4 behaviours in general are not dependent on whether it's RESUMED or not.

Hmm, looking at it again I see that it was always gated behind that flag. But it's not consistent with what the spec says, so we need to decide the correct behaviour and ensure spec and JS are consistent.

Thought about this a bit more. Was trying to understand how we populate the local objects state when we resume after connection recovery (internal Slack thread). Simon said:

for presence, in protocol v2 we always do a full sync after resume. maybe we might change that at some point, but right now that's what we do. I assume liveobjects does similarly

I think that if this is the case for LiveObjects, then the JS behaviour of not becoming SYNCING when resumed == true is probably not correct, since you could have a scenario like the following (I'm no longer specifically thinking about connection recovery here; in this scenario the client is already SYNCED):

i.e. this appears to be a scenario in which the assumption of RTO5f in ably/specification#416 (specifically, that the preceding ATTACHED has resumed == false) is not true.

@mschristensen are you able to clarify the Realtime behaviour here please?

The current behaviour in realtime is the same as presence, i.e. we send a sync sequence unconditionally after a resume https://github.com/ably/realtime/blob/5a7663451577fea09446ffec626aef4f0dfc65d5/go/realtime/lib/channel/attachment.go#L916

So it looks like we should change this in realtime so that the client can know whether or not to buffer ops post ATTACHED. Ticket: https://ably.atlassian.net/browse/AIT-326

If we were to make that change, how would a client populate its local objects state after connection recovery (i.e. it resumes an attachment but without having any local objects data yet)?

We discussed this in standup today; conclusions:

currently, if the HAS_OBJECTS flag is set on ATTACHED, this indicates that Realtime will send a sync sequence; thus, the LiveObjects client's condition for entering SYNCING and buffering ops should be to do so when HAS_OBJECTS is set, as opposed to checking RESUMED

note that spec points RTO4 and RTO4a do actually already mention the unconditional nature of the state sync

we'd like to consider introducing a mechanism for Realtime to conditionally send a sync sequence (for efficiency); have captured this in AIT-330

That is, do it when we get a discontinuity, not when a new sync sequence changes, per spec changes in [1]. Integration tests ported from JS in [2] at 06b746a. All written by Claude. [1] ably/specification#416 [2] ably/ably-js#2150

That is, do it when we get a discontinuity, not when a new sync sequence starts, per spec changes in [1]. Integration tests ported from JS in [2] at 06b746a. All written by Claude. [1] ably/specification#416 [2] ably/ably-js#2150

VeskeR requested review from lawrence-forooghian and mschristensen January 16, 2026 11:59

VeskeR changed the title ~~Fix clearing buffered object operations during sync sequence~~ [AIT-285] Fix clearing buffered object operations during sync sequence Jan 16, 2026

github-actions bot temporarily deployed to staging/pull/2150/bundle-report January 16, 2026 12:00 Inactive

github-actions bot temporarily deployed to staging/pull/2150/features January 16, 2026 12:00 Inactive

github-actions bot temporarily deployed to staging/pull/2150/typedoc January 16, 2026 12:00 Inactive

VeskeR force-pushed the AIT-285/fix-clearing-buffered-ops-during-sync branch from e95e32e to 06b746a Compare January 16, 2026 12:28

github-actions bot deployed to staging/pull/2150/bundle-report January 16, 2026 12:29 View deployment

github-actions bot deployed to staging/pull/2150/features January 16, 2026 12:29 View deployment

github-actions bot deployed to staging/pull/2150/typedoc January 16, 2026 12:29 View deployment

VeskeR mentioned this pull request Jan 16, 2026

[AIT-286] Fix BufferedObjectOperations clearing rules for LiveObjects sync ably/specification#416

Open

mschristensen approved these changes Jan 20, 2026

View reviewed changes

lawrence-forooghian reviewed Jan 21, 2026

View reviewed changes

lawrence-forooghian requested a review from ttypic January 22, 2026 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIT-285] Fix clearing buffered object operations during sync sequence#2150

[AIT-285] Fix clearing buffered object operations during sync sequence#2150
VeskeR wants to merge 1 commit intomainfrom
AIT-285/fix-clearing-buffered-ops-during-sync

VeskeR commented Jan 16, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Jan 16, 2026 •

edited

Loading

Uh oh!

mschristensen left a comment

Uh oh!

mschristensen Jan 20, 2026

Uh oh!

lawrence-forooghian Jan 21, 2026

Uh oh!

lawrence-forooghian Jan 21, 2026

Uh oh!

lawrence-forooghian Jan 22, 2026 •

edited

Loading

Uh oh!

mschristensen Jan 27, 2026

Uh oh!

lawrence-forooghian Jan 27, 2026

Uh oh!

lawrence-forooghian Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		// The realtime server caches the object state at the moment of channel attachment,
		// so we must start fresh and buffer all incoming OBJECT messages from this point.

Conversation

VeskeR commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Uh oh!

mschristensen left a comment

Choose a reason for hiding this comment

Uh oh!

mschristensen Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

lawrence-forooghian Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

lawrence-forooghian Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

lawrence-forooghian Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mschristensen Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

lawrence-forooghian Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

lawrence-forooghian Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

VeskeR commented Jan 16, 2026 •

edited

Loading

coderabbitai bot commented Jan 16, 2026 •

edited

Loading

lawrence-forooghian Jan 22, 2026 •

edited

Loading