[AIT-280] Apply LiveObjects operations on ACK by lawrence-forooghian · Pull Request #419 · ably/specification

lawrence-forooghian · 2026-01-23T14:17:54Z

Written by Claude based on LODR-054. JS implementation in ably/ably-js#2155.

There are a couple of places here where I'd appreciate input from reviewers; please see the unresolved comments towards the end of the PR.

textile/objects-features.textile

Based on [1] at c3a4917. Implementation and tests are Claude-generated from the spec; I've 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). [1] ably/specification#419

Based on [1] at c3a4917. Implementation and tests are Claude-generated from the spec; I've 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). TODO: - test for batch [1] ably/specification#419

Based on [1] 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 [1] ably/specification#419

This is the code that a LiveObjects mutation operation will fail with if the channel enters DETACHED, SUSPENDED, or FAILED whilst waiting for the objects sync state to become SYNCED. See [1]. [1] ably/specification#419

lawrence-forooghian · 2026-02-16T15:49:21Z

Added 57f4449 (new spec point RTO20e1) to address ably/ably-js#2155 (comment) (will squash everything before merge)

…c, RTO20d1) Replace throwing errors with logging when publishAndApply cannot apply operations on ACK. RTO20c: log error and bail if siteCode or serials array unavailable (unexpected server behaviour). Split into RTO20c1 and RTO20c2. RTO20d1: log debug and skip individual operations with null serial (conflation — not currently expected but handled gracefully). This is the outcome of discussion [1] plus a conversation that I had with Paddy. [1] #419 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ttypic

Like it

VeskeR · 2026-02-18T14:43:26Z

textile/objects-features.textile

+**** @(RTO20d2b)@ @ObjectMessage.siteCode@ to the "CD2j":../features#CD2j @ConnectionDetails.siteCode@
+*** @(RTO20d3)@ Add the synthetic @ObjectMessage@ to the list
+** @(RTO20e)@ If the "RTO17":#RTO17 sync state is not @SYNCED@, wait for the sync state to transition to @SYNCED@
+*** @(RTO20e1)@ If the channel enters the @DETACHED@, @SUSPENDED@, or @FAILED@ state while waiting for the sync state to transition to @SYNCED@, the @publishAndApply@ operation must fail with an @ErrorInfo@ error with @code@ @92008@, a @statusCode@ of @400@, a @message@ stating that the objects sync could not be completed due to the channel entering the respective state, and @cause@ set to the @RealtimeChannel.errorReason@ if it is set


Seeing the outcome of the siteCode and serials discussion in #419 (comment) - where we decided not to throw an error - I'm curious whether this spec was intentionally kept as a throw. As we are still in the post-successful-ACK branch, the same rationale from RTO20c and RTO20d1 seems applicable here: log an error that we couldn't apply the operation locally rather than throwing.

Also, the suggested log message - 'stating that the objects sync could not be completed due to the channel entering the respective state' - doesn't feel right in this context. A user is performing a mutation, so receiving an error about objects sync not completing is confusing IMO. It might be clearer to follow the phrasing pattern from RTO20c and RTO20d1 and say something like: 'the operation will not be applied locally because objects sync could not be completed due to the channel entering the respective state'.

I think that both of those are fairly edge-case things at the moment — RTO20c is completely for handling server behaviour that should never occur, and RTO20d1 is for handling server behaviour that currently doesn't occur but may in the future (and I think that we may still end up revisiting the apply-on-ACK behaviour in such a case). I think that the general expectation remains that we make a best effort to apply on ACK, and I think that we shouldn't fail to do so silently in the case where we can't apply on ACK because we failed to sync.

I agree re the error message though, have changed; now reads "the operation could not be applied locally due to the channel entering the {state} state whilst waiting for objects sync to complete"

I think that both of those are fairly edge-case things at the moment — RTO20c is completely for handling server behaviour that should never occur, and RTO20d1 is for handling server behaviour that currently doesn't occur but may in the future (and I think that we may still end up revisiting the apply-on-ACK behaviour in such a case). I think that the general expectation remains that we make a best effort to apply on ACK, and I think that we shouldn't fail to do so silently in the case where we can't apply on ACK because we failed to sync.

If you'd prefer to leave as throw then sure, just wanted to point out the difference with two other cases and to ensure this is an intentional behavior. Also in this ably-js PR comment, it seems that Paddy recognizes this "failed to apply because of sync" case as separate to two other mentioned above and is fine with leaving it as throw. Fine by me then.

I agree re the error message though, have changed; now reads "the operation could not be applied locally due to the channel entering the {state} state whilst waiting for objects sync to complete"

Thanks

Going to leave this unresolved for visibility

Based on [1] 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 [1] ably/specification#419

Address #419 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Address ably/specification#419 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Based on [1] 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 [1] ably/specification#419

Address ably/specification#419 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

This reflects the functionality implemented in [1], based on the DR [2] and spec [3]. Written by Claude. [1] ably/ably-js#2155 [2] https://ably.atlassian.net/wiki/spaces/LOB/pages/4671832082/LODR-054+SDK+apply+operations+on+ACK) [3] ably/specification#419

Written by Claude based on LODR-054 [1]. Additional behaviours not in the DR, decided during spec review: > Fail publishAndApply when channel state prevents sync completion > (RTO20e1): > > The RTO20e wait for the objects sync to reach SYNCED has no failure > condition, which means publishAndApply can hang indefinitely if the > channel never completes the sync — for example, during a disconnection > loop where publish succeeds (ACK received) but the OBJECT_SYNC sequence > never finishes. > > This adds RTO20e1 to fail the operation when the channel enters > DETACHED, SUSPENDED, or FAILED. The objects sync requires the channel to > be ATTACHED to receive the OBJECT_SYNC sequence (RTO4c), so waiting for > sync is essentially waiting for the channel to become attached. The > decision of when to give up on that is already established by RTL11: > queued presence messages — which are also waiting for the channel to > become ATTACHED (RTP16b) — are failed when the channel enters DETACHED, > SUSPENDED, or FAILED. We apply the same rule here. > > See conversation [2]. > Log a message (without throwing an error) when we do not have all the > data needed to apply locally > > In the first version of this spec, we threw an error in these cases, but > after discussion with Paddy ([3] plus a call) he suggested this > behaviour instead. > > RTO20c: log error and bail if siteCode or serials array unavailable > (unexpected server behaviour). > > RTO20d1: log debug and skip individual operations with null serial > (conflation — not currently expected but handled gracefully). [1] https://ably.atlassian.net/wiki/spaces/LOB/pages/4671832082/LODR-054+SDK+apply+operations+on+ACK [2] ably/ably-js#2155 (comment) [3] #419 (comment)

Based on [1] at 56a0bba. 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 [1] ably/specification#419

github-actions bot temporarily deployed to staging/pull/419 January 23, 2026 14:18 Inactive

lawrence-forooghian changed the base branch from main to clarify-BufferedObjectsOperations-scope January 23, 2026 14:18

lawrence-forooghian force-pushed the AIT-280-LiveObjects-apply-on-ACK branch from f54eee0 to 09a0dad Compare January 23, 2026 14:21

github-actions bot temporarily deployed to staging/pull/419 January 23, 2026 14:21 Inactive

Base automatically changed from clarify-BufferedObjectsOperations-scope to main January 23, 2026 14:24

lawrence-forooghian commented Jan 23, 2026

View reviewed changes