Fix annotations publish/delete to reflect all fields in output

[umair-ably]

Summary

• JSON mode was missing count, data, and encoding from the response, and always included name (even when undefined). Now all fields are conditionally included only when provided.
• Non-JSON mode gave a generic success message with no indication of what was actually published. Now shows the annotation type, name, and count as labeled fields below the success line.

Applies to both annotations publish and annotations delete.

Before (JSON)

{"annotation":{"channel":"...","serial":"...","type":"metrics:total.v1","name":undefined}}

After (JSON)

{"annotation":{"channel":"...","serial":"...","type":"metrics:total.v1"}}
{"annotation":{"channel":"...","serial":"...","type":"rating:multiple.v1","name":"stars","count":4}}

Before (non-JSON)

✓ Annotation published on message <serial> in channel <channel>.

After (non-JSON)

✓ Annotation published on message <serial> in channel <channel>.
  Type: rating:multiple.v1
  Name: stars
  Count: 4

Test plan

• pnpm exec eslint . — 0 errors
• pnpm test test/unit/commands/channels/annotations/ — 68 tests pass
• Manual CLI testing against live Ably app with mutable messages enabled

🤖 Generated with Claude Code

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
cli-web-cli	Ready	Preview, Comment	Mar 31, 2026 3:31pm

[claude-code-ably-assistant]

Walkthrough

This PR fixes the output of annotations publish and annotations delete to correctly reflect all annotation fields. JSON mode was always including name even when undefined (and omitting count, data, encoding); non-JSON mode showed only a generic success line with no detail. Both modes now conditionally include fields only when present.

Changes

Area	Files	Summary
Commands	src/commands/channels/annotations/publish.ts	JSON envelope now conditionally spreads name, count, data, encoding; non-JSON output adds labeled Type, Name, Count fields below success line
Commands	src/commands/channels/annotations/delete.ts	JSON envelope now conditionally spreads name; non-JSON output adds labeled Type and Name fields below success line

Review Notes

• Behavioral change: Non-JSON output is now more verbose — callers or scripts parsing stdout (not --json) will see additional lines after the success message. Low risk but worth noting.
• JSON schema change: name is no longer always present in the annotation envelope; count, data, and encoding are now included when provided. Consumers relying on annotation.name always existing will see it absent for non-named annotation types.
• No new dependencies.
• Test coverage: PR description states 68 unit tests pass under test/unit/commands/channels/annotations/ — no new tests added, but the fix is covered by existing suite.
• formatResource() is used for count (a number) by converting it to string — consistent with existing patterns in the codebase.

[claude-code-ably-assistant]

Review: Fix annotations publish/delete to reflect all fields in output

The JSON fix (conditional spread to avoid undefined fields) is correct and the new labeled non-JSON output is a good improvement. Two issues worth addressing:

---

Issue 1 — publish.ts: data and encoding missing from non-JSON output (field parity violation)

The JSON path now conditionally includes data and encoding when provided, but the non-JSON else block only shows Type, Name, and Count. Per project conventions, non-JSON output must expose the same fields as JSON output (omitting only null/undefined values).

If a user runs:

ably channels annotations publish my-channel serial123 reactions:flag.v1 --data '{"foo":"bar"}' --encoding utf-8

The --json output will include data and encoding, but the human-readable output silently drops them. This makes the two modes inconsistent and hides the fact that those fields were actually sent.

Fix: Add conditional this.log blocks for data and encoding in the non-JSON path, mirroring the pattern used for count.

---

Issue 2 — Both files: logCliEvent still passes name: flags.name unconditionally

The PR correctly fixed logJsonResult to avoid name: undefined, but logCliEvent in both files still does:

{ channel: channelName, serial, type, name: flags.name }

This passes name: undefined into the verbose log context when no --name was given. While this is lower-severity than the JSON output bug (verbose logs are not user-facing in normal use), it's inconsistent with the stated goal of the PR.

---

Issue 3 — No regression tests for the specific bug fix

The test for JSON output in publish.test.ts only asserts channel and serial are present. It does not verify:

• name is absent when not provided (the bug this PR fixes)
• count, data, encoding are present when provided

If this regresses, no test would catch it. Worth adding a test case like "should not include undefined fields in JSON output" that publishes without --name and asserts result.annotation does not have a name key.

---

Summary

File	Issues
src/commands/channels/annotations/publish.ts	data/encoding missing from non-JSON output; logCliEvent passes name: undefined
src/commands/channels/annotations/delete.ts	logCliEvent passes name: undefined
test/unit/commands/channels/annotations/publish.test.ts	No test for absent-field correctness in JSON output

Issue 1 is the most impactful as it causes a visible inconsistency between --json and human output. Issues 2 and 3 are lower priority but worth fixing while you're in this area.

[umair-ably]

@claude addressed your feedback in b69e6b2

[claude-code-ably-assistant]

Review: Fix annotations publish/delete to reflect all fields in output

This is a clean, focused fix. The JSON output bug (name: undefined included unconditionally) is correctly resolved, and the new labeled non-JSON output is structured properly. A couple of minor things worth checking:

---

src/commands/channels/annotations/publish.ts — data field parity

In JSON mode, annotation.data is emitted (the parsed value — e.g., {"foo":"bar"} becomes {foo: "bar"}). In non-JSON mode, flags.data is shown (the raw input string). These diverge when the user passes a JSON payload:

$ ably channels annotations publish … --data '{"foo":"bar"}'

• JSON output: "data": {"foo": "bar"} (object)
• Non-JSON output: Data: {"foo":"bar"} (string)

For a human-readable display, showing the raw input isn't wrong, but it can surprise users who are cross-checking the two modes. Consider using JSON.stringify(annotation.data, null, 2) (or similar) in the non-JSON path so both surfaces reflect the same interpretation.

---

test/unit/commands/channels/annotations/delete.test.ts — no coverage for new non-JSON output

The PR adds tests for the JSON fixes (conditional name field) but doesn't test the new non-JSON labels (Type, Name) added to delete. The publish.test.ts at least has one non-JSON test for Data/Encoding. delete.test.ts has none for the new lines.

Not a blocker, but a quick expect(stdout).toContain("Type") test in the "functionality" block would give confidence the non-JSON path works.

---

Everything else looks good

• logCliEvent and logJsonResult both correctly fixed in both files — no stray name: flags.name remaining
• All new this.log() calls are inside the else branch (properly guarded)
• formatLabel / formatResource used correctly, no raw chalk.cyan() or quoted strings
• Flag architecture unchanged and correct (productApiFlags, clientIdFlag)
• Error handling unchanged and correct (this.fail(), no this.error())
• Test describe blocks follow standard naming

The data-parity note is a quality improvement, not a blocker. The missing delete non-JSON test is low risk given the simplicity of the change. Ready to merge if the team is comfortable with the data display divergence.

[claude-code-ably-assistant]

LGTM with two minor notes (see comments above). The JSON output bug fix is correct and well-tested.

[sacOO7]

Missing fields encoding, messageSerial and extras can be added to AnnotationDisplayFields interface in src/utils/output.ts. This is used by annotation.subscribe and annotation.get method

export interface AnnotationDisplayFields {
  id?: string;
  timestamp: number;
  channel: string;
  type: string;
  action?: string;
  name?: string;
  clientId?: string;
  count?: number;
  serial?: string;
  data?: unknown;
+ encoding?: string;
+ messageSerial?: string;
+ extras?: unknown;
  indexPrefix?: string;
}

[sacOO7]

Thanks for addressing feedback : )
LGTM