Skip to content

feat(push): support realtime message data in batch-publish channel items#358

Merged
maratal merged 1 commit intomainfrom
fix/345-channel-batch-publish
Apr 20, 2026
Merged

feat(push): support realtime message data in batch-publish channel items#358
maratal merged 1 commit intomainfrom
fix/345-channel-batch-publish

Conversation

@maratal
Copy link
Copy Markdown
Contributor

@maratal maratal commented Apr 20, 2026

Summary

Supersedes #355.

  • Channel-based batch items may include an optional "message" field whose value is published as the realtime message alongside the push notification in extras.push
  • Uses the existing prepareMessageFromInput from message.ts (via JSON.stringify) — no new utility needed
  • The message field follows the same shape as --message in push publish (fix(push publish): parse --message as a full Ably message shape #352): name and data are extracted; if only name is present, remaining fields become data

Example

[
  {
    "channels": ["my-channel"],
    "payload": {"notification": {"title": "Hello", "body": "World"}},
    "message": {"name": "alert", "data": "Server down"}
  }
]

Test plan

  • pnpm test:unit passes (21 tests in batch-publish, covering string message, object with name+data, object with name-only, no message)
  • pnpm exec eslint . clean

🤖 Generated with Claude Code

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cli-web-cli Ready Ready Preview, Comment Apr 20, 2026 7:48pm

Request Review

@claude-code-ably-assistant
Copy link
Copy Markdown

Walkthrough

This PR adds support for an optional message field in channel-based batch-publish items, allowing a realtime message to be published alongside the push notification in a single batch request. The field accepts a string or JSON object and is parsed through the existing prepareMessageFromInput utility (reusing the same shape as --message in push publish). Recipient-based items silently ignored any message field before; they now emit a warning and strip it.

Changes

Area Files Summary
Commands src/commands/push/batch-publish.ts Add message field support to channel batch items: parse via prepareMessageFromInput, merge extras, guard against extras.push conflict; warn and strip message on recipient items
Tests test/unit/commands/push/batch-publish.test.ts 8 new unit tests covering string message, name+data extraction, name-only → remaining fields as data, non-push extras merging, extras.push conflict error, plain object → data, recipient warning, and no-message baseline

Review Notes

  • Behavioral change (channel items): Channel batch specs now emit extras: { push: ..., ...otherExtras } instead of a bare extras: { push: ... }. Items without a message field are unaffected structurally — data/name are only added when present.
  • Behavioral change (recipient items): Previously, a message field on a recipient item was silently forwarded to the API. It is now stripped and a logWarning is emitted. This could surface as a visible warning for existing callers who were (incorrectly) passing message on recipient items.
  • Error path added: If message.extras.push is set on a channel item, the command calls this.fail() immediately — no request is made. This is a new hard failure where previously there was no validation.
  • No new dependencies: prepareMessageFromInput is reused from src/utils/message.ts; import added to batch-publish.ts.
  • Test coverage: All new code paths are covered by the 8 added tests.

@maratal maratal requested review from sacOO7 and umair-ably April 20, 2026 15:59
@maratal
Copy link
Copy Markdown
Contributor Author

maratal commented Apr 20, 2026

I'm not running copilot on this since it did all the job in the closed PR.

Copy link
Copy Markdown

@claude-code-ably-assistant claude-code-ably-assistant bot left a comment

Choose a reason for hiding this comment

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

Summary

File Status Issues
src/commands/push/batch-publish.ts ✅ OK
test/unit/commands/push/batch-publish.test.ts ✅ OK

The implementation is correct. A few things I checked specifically:

Error handling inside .map()this.fail() is called inside the channelItems.map() callback for the extras.push validation check. This is safe: fail() throws an oclif error, it propagates through .map() to the outer catch, and the outer catch's this.fail() re-throws it immediately (guarded by if (error instanceof Error && "oclif" in error) throw error). No double output. This pattern is explicitly supported by the base class.

Validation is pre-requestchannelBatchSpecs is fully computed (and validated) before any HTTP call is made, so a failing item prevents the channel POST from being sent at all. The test confirms this.

Round-trip JSON.stringify(entry.message)entry.message is already a parsed JS value (from the outer JSON.parse), so stringifying then parsing again inside prepareMessageFromInput is a round-trip. Works correctly for all input shapes (string, object, array, primitive).

Test coverage — all the interesting cases are exercised: string message, name+data extraction, name-only (remaining fields → data), non-push extras merging, extras.push rejection, recipient-item stripping, and no-message baseline. No --duration in test calls, no auth flags, no it.skip.

Ready to merge.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for including an optional realtime message (name/data/extras) on channel-based items in push:batch-publish, publishing it via /messages alongside the push payload in extras.push, while keeping recipient-based items routed to /push/batch/publish (and ignoring message there).

Changes:

  • Accept optional message field on channel items and convert it to an Ably message shape via prepareMessageFromInput(JSON.stringify(...)).
  • Merge user message.extras with CLI-owned extras.push, and fail if message.extras.push is provided.
  • Add unit tests covering string/object message shapes, extras merging, collision failure, and recipient-item sanitization/warning.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/commands/push/batch-publish.ts Implements message support for channel-routed batch items; sanitizes recipient items by stripping message and warning.
test/unit/commands/push/batch-publish.test.ts Adds unit coverage for message parsing/merging behavior and error cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Collaborator

@umair-ably umair-ably left a comment

Choose a reason for hiding this comment

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

LGTM

@umair-ably
Copy link
Copy Markdown
Collaborator

this fixes the flaky e2e test #360

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@maratal maratal force-pushed the fix/345-channel-batch-publish branch from 032c036 to 72a01c5 Compare April 20, 2026 19:47
@maratal maratal enabled auto-merge April 20, 2026 19:48
@maratal maratal merged commit 48c657a into main Apr 20, 2026
11 checks passed
@maratal maratal deleted the fix/345-channel-batch-publish branch April 20, 2026 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants