[AIT-280] Apply operations on ACK

[lawrence-forooghian]

Based on ably/specification#419 at d809334. Implementation and tests are Claude-generated from the spec; I've reviewed them and given plenty of feedback, but largely resisted the temptation to tweak things that aren't quite how I'd write them but which are still correct.

The only behaviour here that's not in the spec is to also apply-on-ACK for batch operations (the batch API isn't in the spec yet).

Summary of decisions re modifications to existing tests (written by Claude):

• Removed redundant waitFor* calls after SDK operations (map.set(), counter.increment(), etc.) - with apply-on-ACK, values are available immediately after the operation promise resolves
• Kept waitFor* calls after REST operations (objectsHelper.operationRequest(), objectsHelper.createAndSetOnMap()) - these still require waiting for the echo to arrive over Realtime
• Added explanatory comment to applyOperationsScenarios noting that those tests cover operations received over Realtime (via REST), and pointing to the new "Apply on ACK" section for tests of locally-applied operations

Docs PR: ably/docs#3161

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Corrected Live Objects operations to apply locally upon server acknowledgment instead of waiting for echo responses. This improves responsiveness for counter increments, map updates, and batch operations.

• Documentation

  • Updated documentation to reflect corrected operation sequencing and batching semantics.

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

[coderabbitai]

Walkthrough

Implements apply-on-ACK semantics for LiveObjects operations. Operations published via publishAndApply() are sent to Ably and immediately applied locally upon ACK receipt. The applyOperation() method signature changes to accept an operation source parameter and return a boolean. A new enum ObjectsOperationSource distinguishes local vs. channel-sourced operations.

Changes

Cohort / File(s)	Summary
Documentation Updates liveobjects.d.ts	Batch operation documentation updated to describe that operations are sent to Ably and applied locally after acceptance, replacing prior echo-back model language.
Method Signature Changes src/common/lib/client/realtimechannel.ts	sendState() return type changed from Promise<void> to Promise<API.PublishResult | undefined> to propagate publish results.
LiveCounter Operation Handling src/plugins/liveobjects/livecounter.ts	Added ObjectsOperationSource import; increment() now uses publishAndApply(); applyOperation() signature updated to accept source parameter and return boolean; _throwNoPayloadError() return type changed to never.
LiveMap Operation Handling src/plugins/liveobjects/livemap.ts	Added ObjectsOperationSource import; set() and remove() now use publishAndApply(); applyOperation() and _applyOperation() signatures updated to accept source parameter and return boolean; error method return type changed to never.
LiveObject Base Class src/plugins/liveobjects/liveobject.ts	Abstract applyOperation() signature updated to accept source: ObjectsOperationSource parameter and return boolean instead of void.
Core RealtimeObject Architecture src/plugins/liveobjects/realtimeobject.ts	Added ObjectsOperationSource enum; new publishAndApply() method; publish() return type changed to Promise<API.PublishResult>; new buffering mechanism with _appliedOnAckSerials and _bufferedAcks; _applyObjectMessages() now accepts source parameter with logic to differentiate local vs. channel-sourced operations.
Batch Message Handling src/plugins/liveobjects/rootbatchcontext.ts	flush() method updated to use publishAndApply() instead of publish() for sending batched messages.
Test Infrastructure test/common/modules/private_api_recorder.js	Added 'read.ProtocolMessage.state' to allowlist of recognized private API identifiers.
LiveObjects Test Suite test/realtime/liveobjects.test.js	Added echo and ACK interceptor utilities; comprehensive new "Apply on ACK" test section covering local application on ACK, no double-apply validation, buffering during OBJECT_SYNC, and subscription behavior across LiveMap, LiveCounter, and batch operations.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant LO as RealtimeObject
    participant RC as Realtime Channel
    participant Server as Ably Server

    Client->>LO: publishAndApply(messages)
    LO->>RC: publish(messages)
    RC->>Server: Send OBJECT messages
    
    par Local Application
        LO->>LO: Buffer messages in _bufferedAcks
    and Server Processing
        Server->>RC: ACK received
    end
    
    RC->>LO: onAck callback triggered
    LO->>LO: Apply buffered messages locally<br/>(source: ObjectsOperationSource.local)
    LO->>LO: Update _appliedOnAckSerials
    LO->>Client: Promise resolves
    
    Note over LO,Server: When channel OBJECT messages arrive later,<br/>serials in _appliedOnAckSerials are skipped<br/>to prevent duplicate application

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• [DTP-1076, DTP-1077] Handle isolated create ops and site timeserials vector on the StateObject #1924 — Modifies same LiveObject classes and applyOperation signatures with related message handling changes.
• [DTP-1033] Add LiveObjects edit API #1948 — Modifies liveobjects stack including sendState and publish APIs with overlapping method signature changes.
• [DTP-955] Buffer and flush state operations during a STATE_SYNC sequence #1909 — Touches same liveobjects core files and changes applyOperation signatures with different buffering scope (STATE_SYNC vs. ACK).

Suggested reviewers

• mschristensen
• owenpearson

Poem

🐰 A hop and a bound through the ACK,
Operations apply—no looking back!
When server says yes with a gentle chime,
Local and channel sync in perfect time! ✨

🚥 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 pull request title 'AIT-280 Apply operations on ACK' clearly and concisely describes the main change: implementing apply-on-ACK behavior for LiveObjects operations, which aligns with the core change throughout the codebase.
Linked Issues check	✅ Passed	The code changes comprehensively implement apply-on-ACK behavior for LiveObjects operations as required by AIT-280, including new enum ObjectsOperationSource, publishAndApply method, modified applyOperation signatures, and extensive test coverage.
Out of Scope Changes check	✅ Passed	The PR includes one intentional out-of-scope enhancement: applying apply-on-ACK behavior to batch operations (not yet in the spec). All other changes directly support the primary AIT-280 objective of implementing apply-on-ACK.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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-280-apply-on-ACK

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

• X
• Mastodon
• Reddit
• LinkedIn

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