Implement message append, protocol v5 with publish/update return values, and update/delete over realtime

[SimonWoolf]

AIT-94

Summary by CodeRabbit

• New Features

  • Publish now returns per-message serials; update/delete/append return version info.
  • Message append operation added.
  • Default protocol baseline bumped to v5.

• Behavior / Bug Fixes

  • Acknowledgements now deliver per-message publish responses to callers.
  • Publish/update/delete/append APIs consistently return result objects with serial/version details.

• Tests

  • Added realtime and REST tests for publish/update/delete/append (serial/version validation).
  • Removed an old bundling test.

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

[coderabbitai]

Walkthrough

Adds per-message publish serials and versioned update/delete/append results to public types and channel APIs; threads publish/update responses through transport and queue callbacks; updates REST and Realtime channels to return these results; bumps protocol to v5; and adds tests for serials, updates, deletes, and appends.

Changes

Cohort / File(s)	Summary
Type Declarations & Public API ably.d.ts	Added PublishResult and UpdateDeleteResult; BatchPublishSuccessResult now includes serials. Added MESSAGE_APPEND and expanded MessageAction. Channel publish/update/delete/append signatures updated to return PublishResult / UpdateDeleteResult.
Realtime channel & annotations src/common/lib/client/realtimechannel.ts, src/common/lib/client/realtimeannotations.ts	Realtime publish/sendMessage now return PublishResult; update/delete now return UpdateDeleteResult; added appendMessage and private sendUpdate helper that validates serial, builds/sends update/delete/append protocol messages, and extracts versionSerial. realtimeannotations now await send calls.
REST channel & mixin src/common/lib/client/restchannel.ts, src/common/lib/client/restchannelmixin.ts	Rest publish/_publish return PublishResult. updateMessage/deleteMessage return UpdateDeleteResult; added appendMessage. updateDeleteMessage refactored to accept explicit action (`message.update
Transport & Queue src/common/lib/transport/protocol.ts, src/common/lib/transport/connectionmanager.ts, src/common/lib/transport/messagequeue.ts, src/common/lib/transport/transport.ts	Introduced PublishCallback / PublishResult usage. PendingMessage callbacks now use PublishCallback. ACK emits optional res?: PublishResult[] and is propagated into messageQueue.completeMessages, which forwards per-message PublishResult to callbacks. Removed queue bundling/merge logic.
Protocol message types src/common/lib/types/protocolmessage.ts	ProtocolMessage gains optional res?: API.PublishResult[].
Message utilities src/common/lib/types/message.ts	Exported stringifyAction with API.MessageAction return type, added encodeAction(action: API.MessageAction): number, and expanded actions to include 'message.append'.
Defaults src/common/lib/util/defaults.ts	Bumped Defaults.protocolVersion from 4 to 5.
Tests & test adjustments test/realtime/message.test.js, test/realtime/updates-deletes.test.js, test/rest/updates-deletes.test.js, test/realtime/init.test.js, test/rest/http.test.js, test/browser/modular.test.js	Removed skipped bundling test; added realtime and REST tests covering publish serials, update/delete/append operations, version metadata, error cases for missing serial, append behavior and reattach/rewind verification. Updated tests to expect protocol v5 and X-Ably-Version header '5'. Adjusted one test to import ErrorInfo.
Hooks src/platform/react-hooks/src/hooks/useChannelAttach.ts	Extended useEffect dependency array to include ably.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Channel as Channel (Rest|Realtime)
    participant Protocol as Transport/Protocol
    participant Queue as MessageQueue
    participant Backend as Server

    rect rgb(235,248,255)
    Note over App,Backend: Publish (single or batch)
    App->>Channel: publish(messages)
    Channel->>Protocol: send(ProtocolMessage with body)
    Protocol->>Queue: queue(PendingMessage with PublishCallback)
    Queue->>Backend: network send
    Backend-->>Protocol: ACK(serial, count, res[])
    Protocol->>Queue: completeMessages(serial, count, null, res[])
    Queue->>Queue: invoke each callback(err, publishResponse)
    Channel-->>App: resolve Promise<PublishResult> { serials: [...] }
    end

    rect rgb(255,245,235)
    Note over App,Backend: Update / Delete / Append
    App->>Channel: updateMessage(...) / deleteMessage(...) / appendMessage(...)
    Channel->>Channel: sendUpdate(message, action, op)
    Channel->>Protocol: send(ProtocolMessage action=message.update|.delete|.append)
    Protocol->>Queue: queue(PendingMessage with PublishCallback)
    Queue->>Backend: network send
    Backend-->>Protocol: ACK(serial, count, res[] with version)
    Protocol->>Queue: completeMessages(serial, count, null, res[])
    Queue->>Queue: invoke callback(err, publishResponse)
    Channel-->>App: resolve Promise<UpdateDeleteResult> { versionSerial: "..." }
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Poem

🐇 I hop through queues with serials in paw,
I nudge, append, update — then listen for awe.
Versions like breadcrumbs along the byte track,
Callbacks return answers and put data back.
A rabbit's small patch, tidy, swift — here's the knack!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 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 three main changes: message append functionality, protocol v5 upgrade with return values, and update/delete over realtime operations.
Linked Issues check	✅ Passed	The PR implements message append, protocol v5, publish/update return values, and realtime update/delete operations as required by AIT-94.
Out of Scope Changes check	✅ Passed	All changes align with the PR objectives: type definitions for PublishResult and UpdateDeleteResult, API method signatures updated to return these types, protocol message handling, test coverage, and a unrelated browser test fix.

✨ 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 protocol-v5

---

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

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

ably.d.ts (1)

1451-1465: Public typings for publish/update/delete/append and new MESSAGE_APPEND action look consistent with implementation

• Adding serials: (string | null)[] to BatchPublishSuccessResult and introducing PublishResult / UpdateDeleteResult matches the new protocol responses and REST/realtime implementations.
• Updating Channel.publish and RealtimeChannel.publish overloads to return Promise<PublishResult> (and corresponding doc changes) aligns with RestChannel.publish and RealtimeChannel.publish now returning PublishResponse.
• Exposing updateMessage / deleteMessage / appendMessage as returning Promise<UpdateDeleteResult> fits the new REST and realtime flows, which always return an object with a version field.
• Adding MessageActions.MESSAGE_APPEND and extending MessageAction with MESSAGE_APPEND dovetails with the new 'message.append' operation used in both REST and realtime paths.

One consideration: the public types narrow serials from optional (in internal PublishResponse) to required (in public PublishResult). The realtime implementation defensively resolves publishResponse || {} when the callback fires, and update/delete operations access serials with optional chaining serials?.[0]. This assumes protocol v5+ always populates serials for successful publish operations—tests confirm it currently does, but if backwards compatibility with older protocol versions or edge cases becomes relevant, the required-type assumption may need revisiting.

🧹 Nitpick comments (3)

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

11-12: Update/delete/append over REST are wired cleanly

The refactor to build a fresh requestMessage, encode it, and PATCH /messages/{serial} while returning a decoded UpdateDeleteResponse (or { version: null }) is sound and keeps the caller’s Message immutable. Only minor nit: the error string “cannot be updated” now also covers delete/append – consider slightly more generic wording if you touch this again.

Also applies to: 93-139

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

2-6: Realtime publish now returns protocol publish metadata as intended

Changing publish(...args) and sendMessage to return Promise<PublishResponse> cleanly exposes per-message serials from the transport layer. The publishResponse || {} fallback is safe given that PublishResponse.serials is optional, and keeps callers from having to handle undefined.

If you want stricter guarantees in the future (to better align with the public PublishResult.serials always being present), you could default to { serials: [] } rather than {} when no response is provided.

-        } else {
-          resolve(publishResponse || {});
-        }
+        } else {
+          resolve(publishResponse || { serials: [] });
+        }

Also applies to: 244-287, 493-503

---

1030-1088: sendUpdate flow is solid but could mirror REST’s request shape more closely

The realtime updateMessage/deleteMessage/appendMessage helpers correctly:

• Require message.serial.
• Check channel/connection are publishable.
• Encode a single MESSAGE with the appropriate action.
• Return UpdateDeleteResponse using publishResponse.serials?.[0] ?? null.

One small improvement would be to align the constructed Message with the REST path and avoid spreading the entire original message, to reduce risk of sending unintended fields:

-    const updateDeleteMsg = Message.fromValues({
-      ...message,
-      action: action,
-    });
-    if (operation) {
-      updateDeleteMsg.version = operation;
-    }
+    const updateDeleteMsg = Message.fromValues({
+      action,
+      serial: message.serial,
+      name: message.name,
+      data: message.data,
+      extras: message.extras,
+    });
+    if (operation) {
+      updateDeleteMsg.version = operation;
+    }

This would match RestChannelMixin.updateDeleteMessage and keep the wire payload minimal.

📜 Review details

Configuration used: CodeRabbit 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 194aed7 and 6600755.

📒 Files selected for processing (15)

• ably.d.ts (8 hunks)
• src/common/lib/client/realtimeannotations.ts (1 hunks)
• src/common/lib/client/realtimechannel.ts (5 hunks)
• src/common/lib/client/restchannel.ts (4 hunks)
• src/common/lib/client/restchannelmixin.ts (3 hunks)
• src/common/lib/transport/connectionmanager.ts (3 hunks)
• src/common/lib/transport/messagequeue.ts (3 hunks)
• src/common/lib/transport/protocol.ts (2 hunks)
• src/common/lib/transport/transport.ts (1 hunks)
• src/common/lib/types/message.ts (1 hunks)
• src/common/lib/types/protocolmessage.ts (2 hunks)
• src/common/lib/util/defaults.ts (1 hunks)
• test/realtime/message.test.js (0 hunks)
• test/realtime/updates-deletes.test.js (1 hunks)
• test/rest/updates-deletes.test.js (8 hunks)

💤 Files with no reviewable changes (1)

• test/realtime/message.test.js

🧰 Additional context used

🧬 Code graph analysis (10)

test/rest/updates-deletes.test.js (2)

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

• updateMessage (1030-1037)
• appendMessage (1048-1055)

src/common/lib/client/restchannel.ts (2)

• updateMessage (172-179)
• appendMessage (190-197)

src/common/lib/types/message.ts (2)

src/common/lib/types/protocolmessagecommon.ts (1)

• actions (5-28)

ably.d.ts (1)

• MessageAction (3572-3578)

src/common/lib/transport/messagequeue.ts (1)

src/common/lib/types/protocolmessage.ts (1)

• PublishResponse (152-154)

src/common/lib/transport/protocol.ts (1)

src/common/lib/types/protocolmessage.ts (1)

• PublishResponse (152-154)

src/common/lib/client/realtimeannotations.ts (2)

ably.d.ts (2)

• Message (3311-3369)
• Annotation (3374-3429)

src/common/lib/util/utils.ts (1)

• Properties (479-479)

test/realtime/updates-deletes.test.js (2)

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

• updateMessage (1030-1037)
• appendMessage (1048-1055)

src/common/lib/client/restchannel.ts (2)

• updateMessage (172-179)
• appendMessage (190-197)

src/common/lib/transport/connectionmanager.ts (1)

src/common/lib/transport/protocol.ts (2)

• PublishCallback (11-11)
• PendingMessage (13-30)

src/common/lib/client/restchannel.ts (2)

src/common/lib/types/protocolmessage.ts (2)

• PublishResponse (152-154)
• UpdateDeleteResponse (156-158)

src/common/types/http.ts (1)

• RequestBody (26-29)

src/common/lib/client/restchannelmixin.ts (2)

ably.d.ts (2)

• Message (3311-3369)
• MessageOperation (3494-3507)

src/common/lib/types/protocolmessage.ts (1)

• UpdateDeleteResponse (156-158)

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

src/common/lib/types/protocolmessage.ts (2)

• PublishResponse (152-154)
• UpdateDeleteResponse (156-158)

🪛 GitHub Actions: Lint

src/common/lib/client/realtimechannel.ts

[error] 1033-1033: ESLint: '_params' is defined but never used. (@typescript-eslint/no-unused-vars)

🪛 GitHub Check: lint

src/common/lib/transport/connectionmanager.ts

[failure] 5-5:
'PublishResponse' is defined but never used. Allowed unused vars must match /^_/u

---

[failure] 1807-1807:
'lastQueued' is assigned a value but never used. Allowed unused vars must match /^_/u

src/common/lib/client/realtimechannel.ts

[failure] 1051-1051:
'_params' is defined but never used

---

[failure] 1042-1042:
'_params' is defined but never used

---

[failure] 1033-1033:
'_params' is defined but never used

⏰ 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-node (20.x)
• GitHub Check: test-node (18.x)
• GitHub Check: test-node (16.x)
• GitHub Check: test-npm-package
• GitHub Check: test-browser (firefox)
• GitHub Check: test-browser (chromium)
• GitHub Check: test-browser (webkit)

🔇 Additional comments (21)

src/common/lib/util/defaults.ts (1)

94-94: Protocol version upgrade to v5 is correctly implemented.

The change properly updates the protocol version across all layers: REST headers via defaultGetHeaders/defaultPostHeaders, realtime connections via connectionmanager.ts, and connection parameters. No hardcoded v4 references remain in the codebase. The implementation allows version override via the request() method parameter, enabling flexibility for testing.

src/common/lib/types/message.ts (3)

23-30: LGTM! Actions array correctly updated.

The addition of 'message.append' to the actions array aligns with the protocol v5 upgrade and the MessageAction type definition in the public API.

---

32-34: LGTM! Improved type safety.

The updated return type API.MessageAction provides better type safety while remaining backward compatible.

---

36-38: LGTM! Useful encoding helper.

The encodeAction function provides the inverse operation to stringifyAction, enabling conversion from MessageAction strings to their numeric indices. The default to 'message.create' is appropriate.

src/common/lib/transport/transport.ts (1)

164-164: LGTM! ACK handler correctly propagates response data.

The addition of message.res as a third argument to the 'ack' event emission enables the transport layer to propagate PublishResponse data, supporting the protocol v5 upgrade for publish/update return values.

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

48-48: LGTM! Explicit await improves error propagation.

The change from return to await for async operations ensures errors are caught and propagated in the current execution context, improving error handling consistency across publish/update/delete flows.

Also applies to: 53-53

src/common/lib/types/protocolmessage.ts (2)

152-158: LGTM! New response types support protocol v5.

The PublishResponse and UpdateDeleteResponse types provide the structure for returning publish/update/delete operation results, enabling per-message serials and version metadata.

---

184-184: LGTM! ProtocolMessage extended with response field.

The addition of res?: PublishResponse[] enables the protocol message to carry response data for publish operations, supporting the new return value semantics in protocol v5.

test/rest/updates-deletes.test.js (4)

23-55: LGTM! Comprehensive publish serial tests.

The tests correctly validate that publish operations return serials for both single and batch messages, confirming the protocol v5 enhancement.

---

62-93: LGTM! getMessage tests validate serial-based retrieval.

The tests properly verify getMessage functionality with both string serials and Message objects, aligning with the new serial-based retrieval pattern.

---

99-181: LGTM! Update and delete operations thoroughly tested.

The tests validate operation metadata (clientId, description, metadata) and version tracking for both update and delete operations. The use of waitFor appropriately handles eventual consistency.

---

286-341: LGTM! Append operation test validates message concatenation.

The test properly verifies that appendMessage appends data, maintains operation metadata, and that the action is correctly set to 'message.update' after the append completes. The serial-based verification approach is consistent with other tests.

test/realtime/updates-deletes.test.js (3)

23-67: LGTM! Realtime publish serial tests.

The tests correctly validate that publish operations over realtime return serials for both single and batch messages, with proper resource cleanup using realtime.close().

---

72-171: LGTM! Realtime update and delete operations well-tested.

The tests properly use channel subscriptions to capture update and delete events, validating all aspects of the operation including version metadata, clientId, description, and metadata fields. The promise-based approach correctly handles asynchronous event delivery.

---

212-280: LGTM! Append test includes reattach verification.

The test thoroughly validates the append operation by first capturing the append event, then reattaching with rewind to verify the full concatenated message. This confirms both the immediate append event and the eventual consistency of the message state.

src/common/lib/transport/messagequeue.ts (1)

43-65: LGTM! Message completion updated to propagate PublishResponse.

The completeMessages method now correctly propagates PublishResponse data to per-message callbacks. The iteration properly maps each response to its corresponding message using the array index.

Note: This changes the callback signature from (err) to (err, publishResponse). Existing callbacks that only expect an error parameter will continue to work due to JavaScript's flexible argument handling.

src/common/lib/transport/connectionmanager.ts (1)

1751-1751: LGTM! Callback signature updated for PublishResponse support.

The send method signature correctly updated to use PublishCallback instead of ErrCallback, enabling the propagation of PublishResponse data through the messaging pipeline.

src/common/lib/transport/protocol.ts (2)

2-3: Publish callback type change is coherent with new response shape

Using PublishCallback = StandardCallback<PublishResponse> and updating PendingMessage.callback keeps the callback contract clear while enabling response metadata. ackRequired gating remains unchanged and appropriate.

Also applies to: 9-12, 15-29

---

36-42: Confirmed: ack emitter signature properly wired across all implementations

Transport base class correctly emits ack events with (message.msgSerial, message.count, message.res). Protocol listener receives this as (serial, count, res?: PublishResponse[]) and passes res to MessageQueue.completeMessages(), which has the signature (serial, count, err?, res?) and properly uses it at the callback site. Both WebSocketTransport and CometTransport inherit this behavior without override. The nack path correctly omits res since NACK messages don't carry publish responses.

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

19-26: REST publish and update/delete/append result plumbing looks correct

Normalizing the POST response into RestPublishResponse and then returning a bare PublishResponse from _publish is tidy, and the new updateMessage / deleteMessage / appendMessage wrappers cleanly delegate to channelMixin.updateDeleteMessage with the appropriate action and return UpdateDeleteResponse. No issues spotted.

Also applies to: 81-161, 172-197

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

505-512: sendPresence/sendState use sendMessage consistently

Presence and objects state now go through sendMessage and await its completion, so they’ll correctly propagate errors from the connection manager while ignoring the publish metadata they don’t need. This keeps all messaging paths uniform.

Also applies to: 514-521

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/common/lib/client/restchannelmixin.ts (2)

93-99: Consider renaming method to reflect append capability.

The method name updateDeleteMessage no longer accurately describes its functionality now that it also handles 'message.append'. Consider renaming to something like updateDeleteAppendMessage or a more generic modifyMessage for clarity.

This is a minor inconsistency and can be deferred if backwards compatibility is a concern.

---

126-127: Simplify: remove unnecessary variable assignment.

method is assigned but never conditionally changed. Call Resource.patch directly.

-    let method = Resource.patch;
-    const { body, unpacked } = await method<UpdateDeleteResponse>(
+    const { body, unpacked } = await Resource.patch<UpdateDeleteResponse>(

📜 Review details

Configuration used: CodeRabbit 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 b28b6c0 and ff35286.

📒 Files selected for processing (8)

• src/common/lib/client/realtimechannel.ts (5 hunks)
• src/common/lib/client/restchannel.ts (4 hunks)
• src/common/lib/client/restchannelmixin.ts (3 hunks)
• src/common/lib/transport/connectionmanager.ts (3 hunks)
• src/common/lib/types/protocolmessage.ts (2 hunks)
• src/platform/react-hooks/src/hooks/useChannelAttach.ts (1 hunks)
• test/realtime/message.test.js (0 hunks)
• test/realtime/updates-deletes.test.js (1 hunks)

💤 Files with no reviewable changes (1)

• test/realtime/message.test.js

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

• test/realtime/updates-deletes.test.js

🔇 Additional comments (13)

src/platform/react-hooks/src/hooks/useChannelAttach.ts (1)

42-42: LGTM! Proper React hooks compliance.

Adding ably to the dependency array is correct since it's referenced inside the effect (line 39). This ensures the effect re-runs when the Ably instance changes, triggering a re-attachment with the new instance.

src/common/lib/types/protocolmessage.ts (2)

152-158: LGTM! Protocol v5 response types are well-defined.

The new PublishResponse and UpdateDeleteResponse types clearly define the response shapes for publish/update/delete/append operations. The nullable types (string | null and (string | null)[]) appropriately handle cases where values may be absent in protocol responses.

---

184-184: LGTM! PublishResponse array field appropriately added.

The optional res field correctly allows a ProtocolMessage to carry multiple publish responses, consistent with the protocol v5 upgrade.

src/common/lib/transport/connectionmanager.ts (3)

7-7: LGTM! Callback type import updated for protocol v5.

The import change from ErrCallback to PublishCallback aligns with the protocol v5 upgrade that introduces publish/update/delete return values.

---

1747-1747: LGTM! Send method signature updated correctly.

The send method callback parameter type correctly changed to PublishCallback, consistent with the protocol v5 changes that enable returning publish responses to callers.

---

1801-1803: LGTM! Queue method updated and past issue resolved.

The queue method callback parameter correctly changed to PublishCallback. The previously flagged unused variables (lastQueued and maxSize) have been removed, leaving a clean implementation that simply logs and queues the message.

src/common/lib/client/restchannelmixin.ts (2)

11-11: LGTM!

Import aligns with the new return type for updateDeleteMessage.

---

113-121: Good practice: constructing new Message to avoid mutation.

Creating a new Message via fromValues() instead of mutating the input is the right approach. This prevents unexpected side effects for callers.

src/common/lib/client/restchannel.ts (2)

157-160: Delete operations on potentially missing keys are safe but worth noting.

The code handles both unpacked and non-unpacked responses correctly, defaulting to an empty object. The delete operations on channel and messageId are safe even if these keys don't exist, which is appropriate here since these fields are being intentionally stripped from the RestPublishResponse before returning a PublishResponse.

---

172-197: LGTM! Clean delegation pattern for update/delete/append operations.

All three methods (updateMessage, deleteMessage, appendMessage) follow a consistent delegation pattern to updateDeleteMessage, appropriately differentiating operations via the action parameter. The return type changes to Promise<UpdateDeleteResponse> align with the PR objectives.

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

493-521: LGTM! sendMessage propagates PublishResponse correctly.

The sendMessage method now correctly returns Promise<PublishResponse> (line 493) and defaults to an empty object if the response is undefined (line 499). The sendPresence and sendState methods appropriately consume the response via await without exposing it to their callers, maintaining their Promise<void> return types.

---

1030-1052: LGTM! Lint issue resolved and clean delegation pattern.

The previously reported @typescript-eslint/no-unused-vars issue with the _params parameter has been resolved by removing it from the signatures. All three methods (updateMessage, deleteMessage, appendMessage) now follow a consistent pattern, delegating to the new sendUpdate helper with appropriate action strings.

---

1054-1085: Implementation is sound.

Verified that PublishResponse.serials is correctly defined as an optional array of (string | null)[] in the codebase (src/common/lib/types/protocolmessage.ts:153). The code correctly:

• Uses optional chaining ?. to handle undefined serials
• Accesses [0] appropriately for the single message being sent
• Uses nullish coalescing ?? null to handle null elements (which occur when messages are discarded due to conflation rules)

The implementation matches the documented behavior that serials is an array with 1:1 correspondence to published messages.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

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

121-121: Use const instead of let for method.

The method variable is never reassigned, so it should be declared with const for better code clarity and to prevent accidental reassignment.

Apply this diff:

-    let method = Resource.patch;
+    const method = Resource.patch;

📜 Review details

Configuration used: CodeRabbit 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 ff35286 and 704a9ad.

📒 Files selected for processing (1)

• src/common/lib/client/restchannelmixin.ts (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/common/lib/client/restchannelmixin.ts (3)

ably.d.ts (1)

• Message (3311-3369)

src/common/lib/types/protocolmessage.ts (1)

• UpdateDeleteResponse (156-158)

src/common/lib/types/basemessage.ts (1)

• CipherOptions (13-24)

⏰ 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-browser (webkit)
• GitHub Check: test-node (18.x)
• GitHub Check: test-node (16.x)
• GitHub Check: test-browser (chromium)
• GitHub Check: test-browser (firefox)
• GitHub Check: test-node (20.x)
• GitHub Check: test-npm-package

🔇 Additional comments (4)

src/common/lib/client/restchannelmixin.ts (4)

11-11: LGTM!

The import of UpdateDeleteResponse correctly supports the new return type for the updateDeleteMessage method.

---

113-119: LGTM!

Creating a new Message via Message.fromValues instead of mutating the input message is good practice. The encoding flow is clean and consistent.

---

132-134: LGTM!

The response handling with a fallback to { version: null } is appropriate defensive coding, ensuring the method always returns a valid UpdateDeleteResponse even if the server response body is empty or null.

---

93-99: No type compatibility issues exist. The method signature change improves clarity by using explicit action parameter and explicit return type. The action parameter correctly maps to API.MessageAction, and the MessageOperation argument is structurally compatible with the Message.version property of type API.MessageVersion.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

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

120-121: Consider inlining the method variable.

The method variable is assigned but only used once immediately after. Since all update/delete/append operations now use PATCH, this could be simplified.

🔎 Suggested simplification:

-    let method = Resource.patch;
-    const { body, unpacked } = await method<API.UpdateDeleteResult>(
+    const { body, unpacked } = await Resource.patch<API.UpdateDeleteResult>(

📜 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 704a9ad and 1067c51.

📒 Files selected for processing (8)

• ably.d.ts (8 hunks)
• src/common/lib/client/realtimechannel.ts (5 hunks)
• src/common/lib/client/restchannel.ts (4 hunks)
• src/common/lib/client/restchannelmixin.ts (2 hunks)
• src/common/lib/transport/messagequeue.ts (3 hunks)
• src/common/lib/transport/protocol.ts (2 hunks)
• src/common/lib/types/protocolmessage.ts (1 hunks)
• test/realtime/updates-deletes.test.js (1 hunks)

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

• test/realtime/updates-deletes.test.js
• src/common/lib/types/protocolmessage.ts

🧰 Additional context used

🧬 Code graph analysis (3)

src/common/lib/transport/messagequeue.ts (1)

ably.d.ts (1)

• PublishResult (3512-3518)

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

src/common/lib/types/basemessage.ts (1)

• CipherOptions (13-24)

src/common/lib/client/restchannel.ts (2)

ably.d.ts (3)

• PublishResult (3512-3518)
• Message (3311-3369)
• MessageOperation (3494-3507)

src/common/types/http.ts (1)

• RequestBody (26-29)

⏰ 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-node (18.x)
• GitHub Check: test-node (20.x)
• GitHub Check: test-node (16.x)
• GitHub Check: test-browser (firefox)
• GitHub Check: test-browser (webkit)
• GitHub Check: test-browser (chromium)
• GitHub Check: test-npm-package

🔇 Additional comments (19)

src/common/lib/transport/messagequeue.ts (1)

43-65: LGTM! The indexed iteration correctly correlates publish responses with messages.

The refactored loop properly passes publishResponse (if available) from the res array to each message's callback by index. This aligns with the PublishCallback type signature in protocol.ts.

One consideration: the code assumes res array (when present) has at least as many elements as completeMessages. If the server ever returns fewer elements, res?.[i] would be undefined, which is safely handled by the optional chaining. This appears intentional given the PublishResult.serials can contain null entries per the type definition.

src/common/lib/transport/protocol.ts (2)

9-12: LGTM! Clean type definition for PublishCallback.

The PublishCallback type alias correctly wraps API.PublishResult in StandardCallback, providing proper typing for the callback chain. The import of both StandardCallback and ErrCallback is appropriate since ErrCallback is still used in onceIdle.

---

41-51: LGTM! Consistent propagation of publish results through the ack flow.

The changes correctly thread the res parameter from the transport's ack event through onAck to completeMessages. The optional typing (res?: API.PublishResult[]) appropriately handles cases where the response may not be present.

src/common/lib/client/restchannelmixin.ts (2)

112-118: LGTM! Clean message construction avoiding mutation of user's input.

Creating a new Message via fromValues and then assigning action and version properties correctly avoids mutating the original message object passed by the user. This is a good practice for API design.

---

131-132: LGTM! Safe fallback for missing response body.

The fallback { versionSerial: null } correctly handles cases where the response body might be empty or undefined, matching the UpdateDeleteResult interface contract where versionSerial can be null when a message was superseded.

src/common/lib/client/restchannel.ts (3)

24-25: LGTM! Clean type extension for internal response handling.

The RestPublishResponse type correctly extends API.PublishResult with optional internal fields (channel, messageId) that are stripped before returning to the caller.

---

140-161: LGTM! Response handling correctly decodes and sanitizes the publish result.

The implementation properly:

1. Decodes the response body based on format (binary/JSON)
2. Falls back to an empty object if body is undefined
3. Strips internal-only fields (channel, messageId) before returning

The delete operations mutate the decoded object, but since this is a freshly decoded object (not user-provided), this is acceptable.

---

191-198: LGTM! New appendMessage method follows the established pattern.

The appendMessage implementation correctly delegates to updateDeleteMessage with the 'message.append' action, maintaining consistency with updateMessage and deleteMessage.

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

240-283: LGTM! Publish flow correctly returns PublishResult.

The updated publish method properly:

1. Returns Promise<API.PublishResult>
2. Propagates the response from sendMessage
3. Provides a safe fallback { serials: [] } when response is undefined

---

490-500: LGTM! sendMessage correctly typed for optional PublishResult.

The return type Promise<API.PublishResult | undefined> correctly reflects that not all protocol messages (e.g., ATTACH, DETACH) return a publish result. The callback parameter publishResponse is properly propagated.

---

1027-1040: LGTM! Update/delete/append methods correctly delegate to sendUpdate.

The refactored methods are cleaner than the previous implementation:

• Removed the unused _params parameter (addressing the past lint issue)
• All three methods consistently delegate to sendUpdate with the appropriate action
• Return type Promise<API.UpdateDeleteResult> matches the API contract

---

1042-1073: LGTM! sendUpdate implements realtime update/delete/append correctly.

The private sendUpdate method properly:

1. Validates message serial presence with a clear error message
2. Checks publishable state before proceeding
3. Constructs a new Message to avoid mutating the input
4. Encodes and wraps in a protocol message
5. Extracts versionSerial from the first serial in the response

Minor observation: Line 1072 uses optional chaining (publishResponse?.serials?.[0]) which safely handles edge cases where the response or serials array might be missing.

ably.d.ts (7)

1460-1464: LGTM: Well-documented serials field addition

The addition of the serials array to BatchPublishSuccessResult is consistent with the new PublishResult interface and properly documents the 1:1 mapping to published messages, including the null case for conflated messages.

---

3509-3529: LGTM: Clean and well-documented result interfaces

The new PublishResult and UpdateDeleteResult interfaces provide clear return types for the protocol v5 publish and update/delete operations. The nullable fields are appropriately documented for edge cases (conflation and superseded updates).

---

3561-3578: LGTM: MESSAGE_APPEND properly integrated

The new MESSAGE_APPEND action type is properly added to both the MessageActions namespace and the MessageAction union type, following the existing pattern. The documentation clearly explains the append semantics.

---

2807-2826: Breaking change: Publish methods now return PublishResult

The publish method overloads now return Promise<PublishResult> instead of Promise<void>. This is a breaking change to the public API that allows callers to access the serials of published messages.

This appears intentional for the protocol v5 upgrade, but please confirm:

1. Is this a documented breaking change for v5?
2. Are there migration notes for users upgrading from v4?

---

2848-2878: LGTM: Well-designed message mutation API

The new updateMessage, deleteMessage, and appendMessage methods provide a consistent API for message mutations. The documentation clearly explains:

• Patch semantics for update/delete (non-null fields replace)
• Append semantics (data appended, other fields replace)
• Return type includes versionSerial for tracking

The API design is clean and consistent across all three operations.

---

3079-3095: LGTM: Realtime publish methods consistent with REST

The RealtimeChannel.publish overloads are updated consistently with the Channel interface, all returning Promise<PublishResult>. This maintains consistency between REST and Realtime APIs.

---

3117-3147: LGTM: Realtime message mutations match REST interface

The updateMessage, deleteMessage, and appendMessage methods on RealtimeChannel are perfectly consistent with the Channel interface, maintaining API parity between REST and Realtime clients.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (5)

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

92-133: updateDeleteMessage flow looks sound; consider broadening the error wording

The new updateDeleteMessage correctly:

• Requires a serial,
• Clones the input Message to avoid mutation,
• Sets action and version (from operation),
• Encodes and PATCHes /messages/{serial},
• Normalizes the response to an UpdateDeleteResult with a { versionSerial: null } fallback.

Only nit: the error text "cannot be updated" is now also used for delete/append operations; if you want it to read naturally for all three, you might rephrase (e.g. “cannot be updated, appended, or deleted”). Behavior-wise this is fine as-is.

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

22-25: REST publish → PublishResult wiring is correct; optionally harden the fallback path

The new publish / _publish path:

• Correctly serializes encoded messages,
• Calls Resource.post<RestPublishResponse>,
• Decodes either unpacked or encoded responses, and
• Strips channel / messageId to return a clean PublishResult.

One minor robustness point: in unexpected cases where the response body is empty, decoded becomes {} (cast to RestPublishResponse), so serials would be undefined at runtime even though the public type requires it. If you want to keep the type contract rock-solid, you could normalize this:

const decoded =
  (unpacked ? body : Utils.decodeBody<RestPublishResponse>(body, client._MsgPack, format)) ||
  ({} as RestPublishResponse);

const serials = decoded.serials ?? [];
delete decoded.channel;
delete decoded.messageId;

return { serials };

This keeps behavior identical in the normal case while guaranteeing serials is always an array.

Also applies to: 80-138, 140-162

ably.d.ts (2)

3023-3043: PublishResult / UpdateDeleteResult types align with the new protocol; consider mentioning append in the docs

Exported PublishResult and UpdateDeleteResult accurately capture:

• Per-message serials for publish, and
• The single new-version serial for an update/delete/append operation.

One small doc tweak: UpdateDeleteResult’s comment currently reads “updated or deleted message”, but this type is also used for appendMessage. Consider updating the wording (e.g. “updated, appended, or deleted message”) to avoid confusion for consumers reading just the d.ts.

---

2321-2345: Channel / RealtimeChannel publish overloads: types are right, wording could be slightly clearer

Both REST Channel.publish(...) and RealtimeChannel.publish(...) now correctly return Promise<PublishResult> across:

• publish(messages: Message[], ...)
• publish(message: Message, ...)
• publish(name: string, data: any, ...)

The implementation elsewhere returns a PublishResult whose serials array is 1:1 with the messages. For the single-message overload JSDoc, you currently say “containing the serial of the published message”; to be strictly accurate you might say “containing the serials of the published messages (a single-element array in this overload)”.

Also applies to: 2591-2603

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

1027-1073: Align sendUpdate’s version handling with the REST path to avoid leaking old version metadata

The new realtime update/delete/append APIs are wired sensibly:

• Public methods log and delegate to sendUpdate with the appropriate action literal.
• sendUpdate enforces presence of message.serial, checks publishability, builds a new Message, sends it via the MESSAGE action, and maps the first returned serial to versionSerial.

One behavioral difference from the REST implementation: here you build the update message via:

const updateDeleteMsg = Message.fromValues({
  ...message,
  action,
});
if (operation) {
  updateDeleteMsg.version = operation;
}

Whereas the REST mixin does:

const requestMessage = Message.fromValues(message);
requestMessage.action = action;
requestMessage.version = operation;

So for realtime updates with no operation argument, any existing message.version (from a prior receive) is forwarded back to the server; in the REST path it is always cleared/overridden. To keep the wire payloads minimal and semantics aligned between REST and Realtime, you may want to mirror the REST pattern:

Suggested alignment with REST version handling

-    const updateDeleteMsg = Message.fromValues({
-      ...message,
-      action: action,
-    });
-    if (operation) {
-      updateDeleteMsg.version = operation;
-    }
+    const updateDeleteMsg = Message.fromValues(message);
+    updateDeleteMsg.action = action;
+    // Always override any existing version metadata (even if operation is undefined),
+    // matching the REST path and avoiding resending server-generated version fields.
+    updateDeleteMsg.version = operation;

Also, as in the REST mixin, the error string still says “cannot be updated” even when performing delete/append; adjust wording if you’d like it to read naturally for all three operations.

📜 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 1067c51 and 7ece4a5.

📒 Files selected for processing (16)

• ably.d.ts
• src/common/lib/client/realtimeannotations.ts
• src/common/lib/client/realtimechannel.ts
• src/common/lib/client/restchannel.ts
• src/common/lib/client/restchannelmixin.ts
• src/common/lib/transport/connectionmanager.ts
• src/common/lib/transport/messagequeue.ts
• src/common/lib/transport/protocol.ts
• src/common/lib/transport/transport.ts
• src/common/lib/types/message.ts
• src/common/lib/types/protocolmessage.ts
• src/common/lib/util/defaults.ts
• src/platform/react-hooks/src/hooks/useChannelAttach.ts
• test/realtime/message.test.js
• test/realtime/updates-deletes.test.js
• test/rest/updates-deletes.test.js

💤 Files with no reviewable changes (1)

• test/realtime/message.test.js

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

• src/common/lib/util/defaults.ts
• src/common/lib/client/realtimeannotations.ts

🧰 Additional context used

🧬 Code graph analysis (9)

src/common/lib/types/protocolmessage.ts (1)

ably.d.ts (1)

• PublishResult (3026-3032)

src/common/lib/transport/protocol.ts (1)

ably.d.ts (1)

• PublishResult (3026-3032)

src/common/lib/transport/messagequeue.ts (1)

ably.d.ts (1)

• PublishResult (3026-3032)

src/common/lib/transport/connectionmanager.ts (1)

src/common/lib/transport/protocol.ts (2)

• PublishCallback (12-12)
• PendingMessage (14-31)

test/realtime/updates-deletes.test.js (2)

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

• updateMessage (1027-1030)
• appendMessage (1037-1040)

src/common/lib/client/restchannel.ts (2)

• updateMessage (173-180)
• appendMessage (191-198)

src/common/lib/client/restchannel.ts (2)

ably.d.ts (4)

• PublishResult (3026-3032)
• Message (2825-2883)
• MessageOperation (3008-3021)
• UpdateDeleteResult (3037-3043)

src/common/types/http.ts (1)

• RequestBody (26-29)

test/rest/updates-deletes.test.js (2)

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

• updateMessage (1027-1030)
• appendMessage (1037-1040)

src/common/lib/client/restchannel.ts (2)

• updateMessage (173-180)
• appendMessage (191-198)

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

ably.d.ts (2)

• PublishResult (3026-3032)
• UpdateDeleteResult (3037-3043)

src/common/lib/types/errorinfo.ts (1)

• ErrorInfo (35-66)

src/common/lib/client/restchannelmixin.ts (2)

ably.d.ts (3)

• Message (2825-2883)
• MessageOperation (3008-3021)
• UpdateDeleteResult (3037-3043)

src/common/lib/types/basemessage.ts (1)

• CipherOptions (13-24)

⏰ 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-node (18.x)
• 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-browser (webkit)

🔇 Additional comments (19)

src/platform/react-hooks/src/hooks/useChannelAttach.ts (1)

42-42: LGTM! Correctly adds missing dependency.

The ably instance is used within the effect (line 39), so including it in the dependency array follows React's exhaustive-deps rule. This ensures the effect re-runs when the Ably instance changes, which is the correct behavior.

src/common/lib/types/message.ts (2)

23-30: LGTM!

The addition of 'message.append' to the actions array is correct and aligns with the protocol v5 changes described in the PR objectives.

---

36-38: This concern is invalid due to TypeScript type safety.

The encodeAction function parameter type API.MessageAction is a union type of exactly the 6 string literals that exist in the actions array. TypeScript's type system guarantees that only these valid values can be passed to the function, so indexOf will never return -1 for properly typed inputs. No validation is needed; the code is already type-safe at compile time.

Note: This function is exported but never actually called in the codebase; Message.encode() inlines the same logic instead.

Likely an incorrect or invalid review comment.

src/common/lib/types/protocolmessage.ts (1)

178-178: LGTM!

The addition of the res field to support PublishResult[] is correct and aligns with the protocol v5 changes for returning publish serials.

src/common/lib/transport/transport.ts (1)

164-164: LGTM!

The ACK event now correctly passes the res parameter to propagate publish results. This is consistent with the protocol v5 changes and the updated messagequeue completion flow.

test/rest/updates-deletes.test.js (1)

1-342: LGTM!

This comprehensive test suite provides excellent coverage for REST message operations including:

• Publish serial returns (single and batch)
• Message retrieval, updates, deletes, and appends
• Operation metadata propagation
• Error handling for missing serials

The tests are well-structured and align with the protocol v5 changes.

src/common/lib/transport/messagequeue.ts (1)

43-69: LGTM!

The refactoring from forEach to an index-based loop is necessary and correct for correlating each message with its corresponding PublishResult. The use of optional chaining (res?.[i]) safely handles cases where res is undefined, and the callback correctly receives both error and publish response.

src/common/lib/transport/protocol.ts (3)

9-12: LGTM!

The introduction of the PublishCallback type alias correctly extends the callback signature to include PublishResult in addition to errors, supporting the protocol v5 publish result flow.

---

14-31: LGTM!

The PendingMessage class correctly updates the callback type to PublishCallback, maintaining type safety throughout the message queuing and acknowledgment flow.

---

41-52: LGTM!

The ACK handling correctly threads the res parameter from the transport event through to completeMessages, enabling per-message publish results to be returned to callers.

test/realtime/updates-deletes.test.js (1)

1-281: Excellent test coverage for realtime operations.

This comprehensive test suite provides excellent coverage for realtime message operations including:

• Publish serial returns (single and batch)
• Update/delete/append operations over realtime with event subscriptions
• Operation metadata verification
• Error handling for missing serials
• Reattach with rewind to verify message concatenation after append

The tests effectively validate the protocol v5 realtime flow.

src/common/lib/transport/connectionmanager.ts (2)

1747-1747: LGTM!

The send method signature correctly updated to use PublishCallback, maintaining type safety and consistency with the protocol v5 changes.

---

1801-1804: LGTM!

The queue method has been correctly simplified and the unused variables mentioned in the past review comment (lastQueued and maxSize) have been removed. The method now cleanly logs and enqueues the message.

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

173-198: REST update/delete/append delegations look consistent

updateMessage, deleteMessage, and appendMessage now all:

• Log with distinct method names, and
• Delegate to restchannelmixin.updateDeleteMessage with the appropriate action literal.

This keeps the REST surface thin and correctly aligned with the shared implementation in the mixin.

ably.d.ts (3)

1461-1475: Batch publish serials extension looks correct

Adding serials: (string | null)[] to BatchPublishSuccessResult with the documented 1:1 mapping to messages, and allowing nulls for conflated messages, is consistent with PublishResult and the per-message serial semantics elsewhere in this PR.

---

2364-2396: updateMessage / deleteMessage / appendMessage typings and docs are consistent

For both REST Channel and RealtimeChannel:

• updateMessage, deleteMessage, and appendMessage all return Promise<UpdateDeleteResult>.
• The docs clearly describe the patch semantics (for update/delete) and append semantics, and reference the result as containing the serial of the new version.

This matches the implementation strategy in the client code and the shape of UpdateDeleteResult.

Also applies to: 2629-2661

---

3075-3092: Message append action wiring looks correct

Introducing MessageActions.MESSAGE_APPEND = 'message.append' and extending MessageAction with it (alongside MESSAGE_SUMMARY) cleanly exposes append operations at the type level. The accompanying doc correctly describes how serial and data behave for appended messages.

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

240-284: Realtime publish now correctly exposes PublishResult

The updated publish:

• Normalizes input into an array of Messages,
• Encodes and size-checks against maxMessageSize,
• Uses throwIfUnpublishableState() for connection/channel gating, and
• Sends a MESSAGE protocol frame, returning the PublishResult (or { serials: [] } as a defensive fallback).

sendMessage wrapping connectionManager.send and returning the optional publishResponse is a straightforward way to surface the new per-publish result without disturbing existing call sites that ignore it.

Also applies to: 490-500

---

502-509: sendPresence / sendState correctly reuse sendMessage

Both sendPresence and sendState now:

• Build the appropriate ProtocolMessage (PRESENCE / OBJECT),
• Delegate to sendMessage, and
• Ignore any returned PublishResult.

This shares the transport path with publish without altering semantics for callers of these methods.

Also applies to: 511-518

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

test/realtime/updates-deletes.test.js (1)

211-275: Avoid potential race between rewind options and implicit attach in append test

In the append test, after await channel.detach(); you:

• call channel.subscribe(...) (which can implicitly start an attach using the current options),
• then await channel.setOptions({ params: { rewind: '1' } });,
• then await channel.attach();.

There’s a small risk that the implicit attach started by subscribe races ahead of setOptions, so the attach that delivers the rewound message might not include rewind=1.

Reordering to apply options before any operation that can attach makes the test more robust, for example:

Suggested reorder to eliminate the race

-        // now reattach with rewind to get the full concatenated message
-        await channel.detach();
-        const updatePromise = new Promise((resolve) => {
-          channel.subscribe((msg) => {
-            resolve(msg);
-          });
-        });
-
-        await channel.setOptions({ params: { rewind: '1' } });
-        await channel.attach();
+        // now reattach with rewind to get the full concatenated message
+        await channel.detach();
+        await channel.setOptions({ params: { rewind: '1' } });
+        const updatePromise = new Promise((resolve) => {
+          channel.subscribe((msg) => {
+            resolve(msg);
+          });
+        });
+        await channel.attach();

Functionally the same, but guarantees the attach (implicit or explicit) uses the rewound options.

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

92-133: REST update/delete/append flow is consistent and avoids mutating caller input

The new updateDeleteMessage shape (explicit action union, UpdateDeleteResult return, fresh requestMessage via Message.fromValues, PATCH to /messages/{serial} and decoding the typed result) looks correct and matches the new API surface without mutating the caller’s message.

One tiny nit: the error text "cannot be updated" now also covers delete and append operations; if you want the message to be fully accurate, you could widen it to “updated, deleted, or appended”.

ably.d.ts (1)

3023-3043: New PublishResult/UpdateDeleteResult and MESSAGE_APPEND action fit the protocol changes

• PublishResult and UpdateDeleteResult definitions match how results are used across REST and Realtime APIs.
• Adding MessageActions.MESSAGE_APPEND and including it in MessageAction reflects the new append operation.
• Minor documentation nit: UpdateDeleteResult is also used for append operations; you might want to mention “update, delete, or append” in its description for clarity.

Overall the typings and comments line up well with the new message operation features.

Also applies to: 3045-3093

📜 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 7ece4a5 and 05db75c.

📒 Files selected for processing (11)

• ably.d.ts
• src/common/lib/client/realtimechannel.ts
• src/common/lib/client/restchannel.ts
• src/common/lib/client/restchannelmixin.ts
• src/common/lib/transport/messagequeue.ts
• src/common/lib/transport/protocol.ts
• src/common/lib/types/protocolmessage.ts
• test/realtime/init.test.js
• test/realtime/updates-deletes.test.js
• test/rest/http.test.js
• test/rest/updates-deletes.test.js

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

• src/common/lib/types/protocolmessage.ts
• test/rest/updates-deletes.test.js

🧰 Additional context used

🧬 Code graph analysis (6)

src/common/lib/transport/protocol.ts (1)

ably.d.ts (1)

• PublishResult (3026-3032)

src/common/lib/client/restchannel.ts (2)

ably.d.ts (4)

• PublishResult (3026-3032)
• Message (2825-2883)
• MessageOperation (3008-3021)
• UpdateDeleteResult (3037-3043)

src/common/types/http.ts (1)

• RequestBody (26-29)

test/realtime/updates-deletes.test.js (4)

test/rest/updates-deletes.test.js (37)

• expect (4-4)
• helper (10-10)
• helper (24-24)
• helper (39-39)
• helper (63-63)
• helper (82-82)
• helper (100-100)
• helper (146-146)
• helper (189-189)
• helper (221-221)
• helper (237-237)
• helper (253-253)
• helper (269-269)
• helper (287-287)
• helper (331-331)
• channel (26-26)
• channel (41-41)
• channel (65-65)
• channel (67-67)
• channel (84-84)
• channel (86-86)
• channel (102-102)
• channel (104-104)
• channel (148-148)
• channel (150-150)
• channel (191-191)
• channel (193-193)
• channel (223-223)
• channel (239-239)
• channel (255-255)
• channel (271-271)
• updateMessage (108-111)
• updateMessage (196-199)
• operation (113-117)
• operation (154-158)
• operation (300-304)
• appendMessage (295-298)

test/realtime/init.test.js (3)

• helper (9-9)
• helper (28-28)
• realtime (279-279)

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

• updateMessage (1027-1030)
• appendMessage (1037-1040)

src/common/lib/client/restchannel.ts (2)

• updateMessage (173-180)
• appendMessage (191-198)

src/common/lib/transport/messagequeue.ts (1)

ably.d.ts (1)

• PublishResult (3026-3032)

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

ably.d.ts (3)

• PublishResult (3026-3032)
• Message (2825-2883)
• UpdateDeleteResult (3037-3043)

src/common/lib/types/errorinfo.ts (1)

• ErrorInfo (35-66)

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

src/common/lib/types/basemessage.ts (1)

• CipherOptions (13-24)

⏰ 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-node (18.x)
• GitHub Check: test-node (16.x)
• GitHub Check: test-npm-package
• GitHub Check: test-node (20.x)
• GitHub Check: test-browser (webkit)
• GitHub Check: test-browser (chromium)
• GitHub Check: test-browser (firefox)

🔇 Additional comments (14)

src/common/lib/transport/messagequeue.ts (1)

5-5: Ack response propagation into per-message callbacks looks correct

The additional res?: API.PublishResult[] parameter and the per-index mapping inside completeMessages align with the existing serial/count logic and safely handle missing results (res undefined or shorter than count). No issues spotted with this wiring.

Also applies to: 43-65

test/rest/http.test.js (1)

35-40: Header version assertion correctly updated for protocol v5

Updating the X-Ably-Version expectation to '5' matches the protocol bump; the rest of the header checks remain consistent.

test/realtime/init.test.js (1)

46-47: Transport URI version check matches protocol v5

The connectUri assertion now expecting 'v=5' is consistent with the new default protocol version and the REST header test.

src/common/lib/transport/protocol.ts (1)

9-13: PublishResult-aware ack handling is wired cleanly end-to-end

Using PublishCallback = StandardCallback<API.PublishResult> for PendingMessage.callback and threading the optional res?: API.PublishResult[] from the transport into onAck and messageQueue.completeMessages keeps the existing callback contract while adding result data. The types and control flow remain coherent.

Also applies to: 16-22, 41-43, 49-52

test/realtime/updates-deletes.test.js (1)

23-170: Realtime publish/update/delete tests give solid coverage

These tests exercise the new realtime behaviours well: single and batch serials from publish, plus end-to-end verification that updateMessage and deleteMessage propagate serial, versionSerial, clientId, description, metadata, action, name, and payload as expected. No issues here.

Also applies to: 175-206

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

24-25: REST publish and message mutation APIs now line up cleanly with the new result types

• publish/_publish now correctly surface an API.PublishResult, decoding the HTTP response and stripping internal channel/messageId fields defined in RestPublishResponse.
• The new/updated updateMessage, deleteMessage, and appendMessage methods returning API.UpdateDeleteResult match the mixin implementation and the public typings.

Behaviour and types look consistent with the new protocol semantics.

Also applies to: 80-81, 137-162, 173-198

ably.d.ts (3)

1462-1475: BatchPublishSuccessResult now exposes per-message serials

Adding serials: (string | null)[] here, with docs matching PublishResult.serials, usefully exposes the per-message serials for successful batch publishes per channel and is consistent with the new publish result model.

---

2321-2396: REST Channel publish/update/delete/append typings match the new behaviour

The updated Channel.publish overloads returning Promise<PublishResult> and the updateMessage / deleteMessage / appendMessage methods returning Promise<UpdateDeleteResult> mirror the REST implementation and the new tests. The doc comments clearly explain the result objects and patch/append semantics.

---

2593-2661: RealtimeChannel publish and message mutation typings are consistent with the realtime API

RealtimeChannel’s publish overloads now returning Promise<PublishResult>, and updateMessage / deleteMessage / appendMessage returning Promise<UpdateDeleteResult>, accurately describe what the realtime layer now returns and align with the new realtime tests exercising serials and versionSerials.

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

240-284: LGTM: Publish flow correctly returns PublishResult.

The updated publish method properly awaits and returns the PublishResult from sendMessage, with a sensible fallback to { serials: [] } when the response is undefined. This aligns with the protocol v5 changes and the PublishResult interface definition.

---

490-500: LGTM: sendMessage properly propagates PublishResult.

The method correctly threads the publishResponse from the connectionManager.send callback through to the promise resolution, returning undefined on the error path.

---

502-518: LGTM: sendPresence and sendState correctly updated.

Both methods appropriately converted to async/await style. They correctly ignore the PublishResult return value since presence and state messages don't need to track serials.

---

1027-1040: LGTM: Update/delete/append methods follow consistent pattern.

All three methods correctly delegate to the sendUpdate helper with appropriate actions. The previously flagged unused _params parameter has been removed, resolving the lint errors.

✅ Addressed previous review feedback about unused parameters.

---

1042-1071: LGTM: sendUpdate implementation is robust and well-structured.

The helper method correctly:

• Validates message.serial presence with a clear, actionable error message
• Reuses throwIfUnpublishableState() for consistent state checking
• Encodes and sends the update/delete/append via the publish flow
• Safely extracts versionSerial from the response with proper null-handling for all edge cases

The extraction logic publishResponse?.serials?.[0] ?? null correctly handles undefined responses, empty arrays, and null serials, matching the UpdateDeleteResult interface specification.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/browser/modular.test.js (1)

613-619: Excellent fix: Spy now delegates to original function.

This correctly resolves the issue described in the commit message. The spy now captures the Content-Type header for the target request while always delegating to and returning the result of originalDo, ensuring the publish operation completes successfully.

Optional: Use strict equality for method comparison

Consider using strict equality (===) instead of loose equality (==) on line 615 for consistency with JavaScript best practices:

-            if (method == 'post' && path == `/channels/${channelName}/messages`) {
+            if (method === 'post' && path === `/channels/${channelName}/messages`) {

📜 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 05db75c and faf7497.

📒 Files selected for processing (1)

• test/browser/modular.test.js

⏰ 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-browser (firefox)
• GitHub Check: test-browser (chromium)
• GitHub Check: test-browser (webkit)
• GitHub Check: test-npm-package
• GitHub Check: test-node (18.x)
• GitHub Check: test-node (16.x)
• GitHub Check: test-node (20.x)

🔇 Additional comments (1)

test/browser/modular.test.js (1)

25-25: LGTM: ErrorInfo export added to modular bundle.

The addition of ErrorInfo to the modular bundle exports is appropriate and aligns with enhanced error handling capabilities in the updated protocol.

[lawrence-forooghian]

The realtime variants of these methods still accept a params although they don't in the spec and they are presumably ignored.

cc @ttypic is the removal of these params from the Realtime variant a breaking change in Java? in ably-cocoa we can introduce an overload that doesn't take params and then document that the variant that takes params will ignore them in the realtime case (not great). We haven't marked these APIs as experimental or anything so can't really just remove the params-accepting variant

[SimonWoolf]

in ably-cocoa we can introduce an overload that doesn't take params and then document that the variant that takes params will ignore them in the realtime case

What'd be the point?

Like: right now there are no supported per-message params. So there is no user-visible behaviour difference between doing something with the params, in either rest or realtime apis, and not. But if at some point we discover a need for a per-message param, it's pretty likely we'll want to support it in both rest and realtime apis, no?

Right now there's no spec'd standard for conveying params in a realtime publish, only a REST publish, but the asymmetry is obviously weird and annoying. I'm tempted to say that we should just have params in the api and put in the spec that if the user provides params the sdk should encode them in a ProtocolMessage.params map<string, string>. We don't currently do anything with that field serverside for a MESSAGE protocolmessage, but if we need to we can just.. start, with no protocol change

[lawrence-forooghian]

Sure, my preference is to avoid any sort of breaking change to the realtime API — in that case can we revert the API change in the spec PR and reintroduce the params (I don't hugely care what we do with them — spec can just say to ignore them for now if we like)?