diff --git a/.claude/skills/ably-codebase-review/SKILL.md b/.claude/skills/ably-codebase-review/SKILL.md index fc3603e8..603ac16c 100644 --- a/.claude/skills/ably-codebase-review/SKILL.md +++ b/.claude/skills/ably-codebase-review/SKILL.md @@ -184,8 +184,9 @@ Launch these agents **in parallel**. Each agent gets a focused mandate and uses **Method (grep/read — text patterns):** 1. **Grep** for `waitUntilInterruptedOrTimeout` in command files — should use `this.waitAndTrackCleanup()` instead 2. **Grep** for `setupChannelStateLogging` in subscribe commands (rooms/*, spaces/*) — flag those that don't call it -3. **Read** command files and check `static examples` arrays for `--json` or `--pretty-json` examples — flag leaf commands that have examples but no JSON variant -4. **Compare** skill templates (`patterns.md`, `SKILL.md`, `testing.md` — already read in Step 1) against actual codebase method names/imports — flag any outdated patterns +3. **Grep** for `room.attach()` or `space.enter()` in all rooms/* and spaces/* commands — verify it's only called for commands that need a realtime connection. In the Chat SDK, methods using `this._chatApi.*` are REST (no attach needed), while methods using `this._channel.publish()` or `this._channel.presence.*` need realtime attachment. REST-only: messages send/update/delete/history, occupancy get. Needs attach: presence enter/get/subscribe, typing keystroke/stop, reactions send/subscribe, occupancy subscribe, messages subscribe. Unnecessary attachment adds latency and creates an unneeded realtime connection. +4. **Read** command files and check `static examples` arrays for `--json` or `--pretty-json` examples — flag leaf commands that have examples but no JSON variant +5. **Compare** skill templates (`patterns.md`, `SKILL.md`, `testing.md` — already read in Step 1) against actual codebase method names/imports — flag any outdated patterns **Method (LSP — for base class verification):** 5. If a subscribe command doesn't call `setupChannelStateLogging` directly, use `LSP goToDefinition` on the base class to check if it's handled there @@ -193,6 +194,7 @@ Launch these agents **in parallel**. Each agent gets a focused mandate and uses **Reasoning guidance:** - `waitUntilInterruptedOrTimeout` in bench commands may be acceptable (different lifecycle) - Missing `setupChannelStateLogging` in rooms/spaces may be handled by `ChatBaseCommand`/`SpacesBaseCommand` — check the base class +- `room.attach()` in REST-based commands is a deviation. Chat SDK methods using `this._chatApi.*` (messages send/update/delete/history, occupancy get) are pure REST calls. Methods using `this._channel.publish()` or `this._channel.presence.*` (reactions send, typing keystroke, presence enter/get/subscribe, occupancy subscribe, messages subscribe) DO need attachment. - Topic index commands and `help.ts` don't need `--json` examples - Skill template accuracy issues are low-effort, high-value fixes diff --git a/.claude/skills/ably-new-command/SKILL.md b/.claude/skills/ably-new-command/SKILL.md index 5c40bcbe..ea4f8692 100644 --- a/.claude/skills/ably-new-command/SKILL.md +++ b/.claude/skills/ably-new-command/SKILL.md @@ -19,6 +19,7 @@ Every command in this CLI falls into one of these patterns. Pick the right one b | **Get** | One-shot query for current state | `AblyBaseCommand` | REST | `channels occupancy get`, `rooms occupancy get` | | **List** | Enumerate resources via REST API | `AblyBaseCommand` | REST | `channels list`, `rooms list` | | **Enter** | Join presence/space then optionally listen | `AblyBaseCommand` | Realtime | `channels presence enter`, `spaces members enter` | +| **REST Mutation** | One-shot REST mutation (no subscription) | `AblyBaseCommand` | REST | `rooms messages update`, `rooms messages delete` | | **CRUD** | Create/read/update/delete via Control API | `ControlBaseCommand` | Control API (HTTP) | `integrations create`, `queues delete` | **Specialized base classes** — some command groups have dedicated base classes that handle common setup (client lifecycle, cleanup, shared flags): @@ -31,6 +32,25 @@ Every command in this CLI falls into one of these patterns. Pick the right one b If your command falls into one of these groups, extend the specialized base class instead of `AblyBaseCommand` or `ControlBaseCommand` directly. **Exception:** if your command only needs a REST client (e.g., history queries that don't enter a space or join a room), you may use `AblyBaseCommand` directly — the specialized base class is most valuable when the command needs realtime connections, cleanup lifecycle, or shared setup like `room.attach()` / `space.enter()`. +### When to call `room.attach()` / `space.enter()` + +**Not every Chat/Spaces command needs `room.attach()` or `space.enter()`.** Before adding attachment, check whether the SDK method you're calling requires an active realtime connection or is a pure REST call: + +| Needs `room.attach()` | Does NOT need `room.attach()` | +|------------------------|-------------------------------| +| Subscribe (realtime listener) | Send (REST via `chatApi.sendMessage`) | +| Presence enter/get/update/leave | Update (REST via `chatApi.updateMessage`) | +| Occupancy subscribe | Delete (REST via `chatApi.deleteMessage`) | +| Typing keystroke/stop | Annotate/append (REST mutation) | +| Reactions send (realtime publish) | History queries (REST via `chatApi.history`) | +| Reactions subscribe | Occupancy get (REST via `chatApi.getOccupancy`) | + +**How it works in the SDK:** Methods that go through `this._chatApi.*` are REST calls and don't need attachment. Methods that use `this._channel.publish()`, `this._channel.presence.*`, or subscribe to channel events require the realtime channel to be attached. Room-level reactions use `this._channel.publish()` (realtime), but messages send/update/delete use `this._chatApi.*` (REST). + +**Rule of thumb:** If the SDK method is a one-shot REST call (returns a Promise with a result, no callback/listener), you do NOT need `room.attach()`. Just call `chatClient.rooms.get(roomId)` to get the room handle and invoke the method directly. Attaching unnecessarily adds latency and creates a realtime connection that isn't needed. + +**How to verify:** Check the Chat SDK source or typedoc — methods that are REST-based don't require the room to be in an `attached` state. When in doubt, test without `room.attach()` — if the SDK method works, attachment isn't needed. + ## Step 2: Create the command file ### File location @@ -391,6 +411,7 @@ See the "Keeping Skills Up to Date" section in `CLAUDE.md` for the full list of - [ ] `formatSuccess()` messages end with `.` (period) - [ ] Resource names use `formatResource(name)`, never quoted - [ ] JSON output uses `logJsonResult()` (one-shot) or `logJsonEvent()` (streaming), not direct `formatJsonRecord()` +- [ ] `room.attach()` / `space.enter()` only called when the SDK method requires a realtime connection (subscribe, send, presence) — NOT for REST mutations (update, delete, annotate, history) - [ ] Subscribe/enter commands use `this.waitAndTrackCleanup(flags, component, flags.duration)` (not `waitUntilInterruptedOrTimeout`) - [ ] Error handling uses `this.fail()` exclusively, not `this.error()` or `this.log(chalk.red(...))` - [ ] Component strings are camelCase: single-word lowercase (`"room"`, `"auth"`), multi-word camelCase (`"channelPublish"`, `"roomPresenceSubscribe"`) diff --git a/.claude/skills/ably-new-command/references/patterns.md b/.claude/skills/ably-new-command/references/patterns.md index c7dd25dd..8e520e52 100644 --- a/.claude/skills/ably-new-command/references/patterns.md +++ b/.claude/skills/ably-new-command/references/patterns.md @@ -5,6 +5,7 @@ Pick the pattern that matches your command from Step 1 of the skill, then follow ## Table of Contents - [Subscribe Pattern](#subscribe-pattern) - [Publish/Send Pattern](#publishsend-pattern) +- [REST Mutation Pattern](#rest-mutation-pattern) - [History Pattern](#history-pattern) - [Get Pattern](#get-pattern) - [Enter/Presence Pattern](#enterpresence-pattern) @@ -135,6 +136,57 @@ For single-shot publish, REST is preferred (simpler, no connection overhead). Se --- +## REST Mutation Pattern + +For one-shot SDK operations that are pure REST calls (send, update, delete, annotate, history, occupancy get). These do **NOT** need `room.attach()` or `space.enter()` — they only need a room/space handle. In the Chat SDK, methods that go through `this._chatApi.*` are REST-based, while methods that use `this._channel.publish()` or `this._channel.presence.*` require realtime attachment. + +Flags for REST mutation commands: +```typescript +static override flags = { + ...productApiFlags, + ...clientIdFlag, // Users may want to test "can client B update client A's message?" + // command-specific flags here +}; +``` + +```typescript +async run(): Promise { + const { args, flags } = await this.parse(MyMutationCommand); + + try { + const chatClient = await this.createChatClient(flags); + if (!chatClient) { + return this.fail("Failed to create Chat client", flags, "roomMessageUpdate"); + } + + this.setupConnectionStateLogging(chatClient.realtime, flags); + + // NO room.attach() — update/delete/annotate are REST calls + const room = await chatClient.rooms.get(args.room); + + if (!this.shouldOutputJson(flags)) { + this.log(formatProgress("Updating message " + formatResource(args.serial) + " in room " + formatResource(args.room))); + } + + const result = await room.messages.update(args.serial, updateParams, details); + + if (this.shouldOutputJson(flags)) { + this.logJsonResult({ room: args.room, serial: args.serial, versionSerial: result.version.serial }, flags); + } else { + this.log(formatSuccess(`Message ${formatResource(args.serial)} updated in room ${formatResource(args.room)}.`)); + } + } catch (error) { + this.fail(error, flags, "roomMessageUpdate", { room: args.room, serial: args.serial }); + } +} +``` + +**Key difference from Subscribe/Send:** No `room.attach()`, no `durationFlag`, no `rewindFlag`, no `waitAndTrackCleanup`. The command creates the client, gets the room handle, calls the REST method, and exits. + +See `src/commands/rooms/messages/update.ts` and `src/commands/rooms/messages/delete.ts` as references. + +--- + ## History Pattern ```typescript diff --git a/.claude/skills/ably-review/SKILL.md b/.claude/skills/ably-review/SKILL.md index 3826fb17..8fc7c24a 100644 --- a/.claude/skills/ably-review/SKILL.md +++ b/.claude/skills/ably-review/SKILL.md @@ -117,8 +117,9 @@ For each changed command file, run the relevant checks. Spawn agents for paralle **Lifecycle check (grep/read):** 1. **Grep** for `waitUntilInterruptedOrTimeout` — should use `this.waitAndTrackCleanup()` instead -2. **Read** `static examples` and check for `--json` or `--pretty-json` variant -3. **Read** the command description — verify imperative mood, sentence case, no trailing period +2. **Grep** for `room.attach()` or `space.enter()` — verify it's only called for commands that need a realtime connection. In the Chat SDK, methods using `this._chatApi.*` are REST (no attach needed), while methods using `this._channel.publish()` or `this._channel.presence.*` need realtime attachment. REST-only: messages send/update/delete/history, occupancy get. Needs attach: presence enter/get/subscribe, typing keystroke/stop, reactions send/subscribe, occupancy subscribe, messages subscribe. Unnecessary attachment adds latency and creates an unneeded realtime connection. +3. **Read** `static examples` and check for `--json` or `--pretty-json` variant +4. **Read** the command description — verify imperative mood, sentence case, no trailing period ### For changed test files (`test/unit/commands/**/*.ts`) diff --git a/README.md b/README.md index 8afb4c46..ec5b71f7 100644 --- a/README.md +++ b/README.md @@ -177,6 +177,7 @@ $ ably-interactive * [`ably rooms`](#ably-rooms) * [`ably rooms list`](#ably-rooms-list) * [`ably rooms messages`](#ably-rooms-messages) +* [`ably rooms messages delete ROOM SERIAL`](#ably-rooms-messages-delete-room-serial) * [`ably rooms messages history ROOM`](#ably-rooms-messages-history-room) * [`ably rooms messages reactions`](#ably-rooms-messages-reactions) * [`ably rooms messages reactions remove ROOM MESSAGESERIAL REACTION`](#ably-rooms-messages-reactions-remove-room-messageserial-reaction) @@ -184,6 +185,7 @@ $ ably-interactive * [`ably rooms messages reactions subscribe ROOM`](#ably-rooms-messages-reactions-subscribe-room) * [`ably rooms messages send ROOM TEXT`](#ably-rooms-messages-send-room-text) * [`ably rooms messages subscribe ROOMS`](#ably-rooms-messages-subscribe-rooms) +* [`ably rooms messages update ROOM SERIAL TEXT`](#ably-rooms-messages-update-room-serial-text) * [`ably rooms occupancy`](#ably-rooms-occupancy) * [`ably rooms occupancy get ROOM`](#ably-rooms-occupancy-get-room) * [`ably rooms occupancy subscribe ROOM`](#ably-rooms-occupancy-subscribe-room) @@ -3667,10 +3669,47 @@ EXAMPLES $ ably rooms messages subscribe my-room $ ably rooms messages history my-room + + $ ably rooms messages update my-room "serial" "Updated text" + + $ ably rooms messages delete my-room "serial" ``` _See code: [src/commands/rooms/messages/index.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/rooms/messages/index.ts)_ +## `ably rooms messages delete ROOM SERIAL` + +Delete a message in an Ably Chat room + +``` +USAGE + $ ably rooms messages delete ROOM SERIAL [-v] [--json | --pretty-json] [--client-id ] [--description ] + +ARGUMENTS + ROOM The room containing the message to delete + SERIAL The serial of the message to delete + +FLAGS + -v, --verbose Output verbose logs + --client-id= Overrides any default client ID when using API authentication. Use "none" to explicitly set + no client ID. Not applicable when using token authentication. + --description= Description of the delete operation + --json Output in JSON format + --pretty-json Output in colorized JSON format + +DESCRIPTION + Delete a message in an Ably Chat room + +EXAMPLES + $ ably rooms messages delete my-room "serial-001" + + $ ably rooms messages delete my-room "serial-001" --description "spam removal" + + $ ably rooms messages delete my-room "serial-001" --json +``` + +_See code: [src/commands/rooms/messages/delete.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/rooms/messages/delete.ts)_ + ## `ably rooms messages history ROOM` Get historical messages from an Ably Chat room @@ -3942,6 +3981,47 @@ EXAMPLES _See code: [src/commands/rooms/messages/subscribe.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/rooms/messages/subscribe.ts)_ +## `ably rooms messages update ROOM SERIAL TEXT` + +Update a message in an Ably Chat room + +``` +USAGE + $ ably rooms messages update ROOM SERIAL TEXT [-v] [--json | --pretty-json] [--client-id ] [--description ] + [--headers ] [--metadata ] + +ARGUMENTS + ROOM The room containing the message to update + SERIAL The serial of the message to update + TEXT The new message text + +FLAGS + -v, --verbose Output verbose logs + --client-id= Overrides any default client ID when using API authentication. Use "none" to explicitly set + no client ID. Not applicable when using token authentication. + --description= Description of the update operation + --headers= Additional headers for the message (JSON format) + --json Output in JSON format + --metadata= Additional metadata for the message (JSON format) + --pretty-json Output in colorized JSON format + +DESCRIPTION + Update a message in an Ably Chat room + +EXAMPLES + $ ably rooms messages update my-room "serial-001" "Updated text" + + $ ably rooms messages update my-room "serial-001" "Updated text" --description "typo fix" + + $ ably rooms messages update my-room "serial-001" "Updated text" --metadata '{"edited":true}' + + $ ably rooms messages update my-room "serial-001" "Updated text" --headers '{"source":"cli"}' + + $ ably rooms messages update my-room "serial-001" "Updated text" --json +``` + +_See code: [src/commands/rooms/messages/update.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/rooms/messages/update.ts)_ + ## `ably rooms occupancy` Commands for monitoring room occupancy diff --git a/src/commands/rooms/messages/delete.ts b/src/commands/rooms/messages/delete.ts new file mode 100644 index 00000000..9dda89ef --- /dev/null +++ b/src/commands/rooms/messages/delete.ts @@ -0,0 +1,116 @@ +import { Args, Flags } from "@oclif/core"; +import type { OperationDetails } from "@ably/chat"; + +import { productApiFlags, clientIdFlag } from "../../../flags.js"; +import { ChatBaseCommand } from "../../../chat-base-command.js"; +import { + formatProgress, + formatSuccess, + formatResource, +} from "../../../utils/output.js"; + +export default class MessagesDelete extends ChatBaseCommand { + static override args = { + room: Args.string({ + description: "The room containing the message to delete", + required: true, + }), + serial: Args.string({ + description: "The serial of the message to delete", + required: true, + }), + }; + + static override description = "Delete a message in an Ably Chat room"; + + static override examples = [ + '$ ably rooms messages delete my-room "serial-001"', + '$ ably rooms messages delete my-room "serial-001" --description "spam removal"', + '$ ably rooms messages delete my-room "serial-001" --json', + ]; + + static override flags = { + ...productApiFlags, + ...clientIdFlag, + description: Flags.string({ + description: "Description of the delete operation", + }), + }; + + async run(): Promise { + const { args, flags } = await this.parse(MessagesDelete); + + try { + const chatClient = await this.createChatClient(flags); + + if (!chatClient) { + return this.fail( + "Failed to create Chat client", + flags, + "roomMessageDelete", + ); + } + + this.setupConnectionStateLogging(chatClient.realtime, flags); + + const room = await chatClient.rooms.get(args.room); + + if (!this.shouldOutputJson(flags)) { + this.log( + formatProgress( + "Deleting message " + + formatResource(args.serial) + + " in room " + + formatResource(args.room), + ), + ); + } + + // Build operation details + const details: OperationDetails | undefined = flags.description + ? { description: flags.description } + : undefined; + + this.logCliEvent( + flags, + "roomMessageDelete", + "deleting", + `Deleting message ${args.serial} from room ${args.room}`, + { room: args.room, serial: args.serial }, + ); + + const result = await room.messages.delete(args.serial, details); + + this.logCliEvent( + flags, + "roomMessageDelete", + "messageDeleted", + `Message ${args.serial} deleted from room ${args.room}`, + { room: args.room, serial: args.serial }, + ); + + if (this.shouldOutputJson(flags)) { + this.logJsonResult( + { + room: args.room, + serial: args.serial, + versionSerial: result.version.serial, + }, + flags, + ); + } else { + this.log( + formatSuccess( + `Message ${formatResource(args.serial)} deleted from room ${formatResource(args.room)}.`, + ), + ); + this.log(` Version serial: ${formatResource(result.version.serial)}`); + } + } catch (error) { + this.fail(error, flags, "roomMessageDelete", { + room: args.room, + serial: args.serial, + }); + } + } +} diff --git a/src/commands/rooms/messages/history.ts b/src/commands/rooms/messages/history.ts index 13f9c7cf..1dffce7e 100644 --- a/src/commands/rooms/messages/history.ts +++ b/src/commands/rooms/messages/history.ts @@ -66,15 +66,15 @@ export default class MessagesHistory extends ChatBaseCommand { const chatClient = await this.createChatClient(flags); if (!chatClient) { - this.fail("Failed to create Chat client", flags, "roomMessageHistory"); + return this.fail( + "Failed to create Chat client", + flags, + "roomMessageHistory", + ); } - // Get the room const room = await chatClient.rooms.get(args.room); - // Attach to the room - await room.attach(); - if (!this.shouldSuppressOutput(flags)) { if (this.shouldOutputJson(flags)) { this.logJsonEvent( @@ -122,7 +122,7 @@ export default class MessagesHistory extends ChatBaseCommand { historyParams.end !== undefined && historyParams.start > historyParams.end ) { - this.fail( + return this.fail( "--start must be earlier than or equal to --end", flags, "roomMessageHistory", diff --git a/src/commands/rooms/messages/index.ts b/src/commands/rooms/messages/index.ts index 90511770..fd985db8 100644 --- a/src/commands/rooms/messages/index.ts +++ b/src/commands/rooms/messages/index.ts @@ -11,5 +11,7 @@ export default class MessagesIndex extends BaseTopicCommand { '<%= config.bin %> <%= command.id %> send my-room "Hello world!"', "<%= config.bin %> <%= command.id %> subscribe my-room", "<%= config.bin %> <%= command.id %> history my-room", + '<%= config.bin %> <%= command.id %> update my-room "serial" "Updated text"', + '<%= config.bin %> <%= command.id %> delete my-room "serial"', ]; } diff --git a/src/commands/rooms/messages/reactions/remove.ts b/src/commands/rooms/messages/reactions/remove.ts index 53fb358f..8b119ad0 100644 --- a/src/commands/rooms/messages/reactions/remove.ts +++ b/src/commands/rooms/messages/reactions/remove.ts @@ -50,7 +50,7 @@ export default class MessagesReactionsRemove extends ChatBaseCommand { const chatClient = await this.createChatClient(flags); if (!chatClient) { - this.fail( + return this.fail( "Failed to create Chat client", flags, "roomMessageReactionRemove", diff --git a/src/commands/rooms/messages/reactions/send.ts b/src/commands/rooms/messages/reactions/send.ts index fe3af2da..73cb05d1 100644 --- a/src/commands/rooms/messages/reactions/send.ts +++ b/src/commands/rooms/messages/reactions/send.ts @@ -59,7 +59,7 @@ export default class MessagesReactionsSend extends ChatBaseCommand { flags.count !== undefined && flags.count <= 0 ) { - this.fail( + return this.fail( "Count must be a positive integer for Multiple type reactions", flags, "roomMessageReactionSend", @@ -71,7 +71,7 @@ export default class MessagesReactionsSend extends ChatBaseCommand { this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail( + return this.fail( "Failed to create Chat client", flags, "roomMessageReactionSend", diff --git a/src/commands/rooms/messages/reactions/subscribe.ts b/src/commands/rooms/messages/reactions/subscribe.ts index e79d3173..c487f341 100644 --- a/src/commands/rooms/messages/reactions/subscribe.ts +++ b/src/commands/rooms/messages/reactions/subscribe.ts @@ -60,7 +60,7 @@ export default class MessagesReactionsSubscribe extends ChatBaseCommand { this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail( + return this.fail( "Failed to initialize clients", flags, "roomMessageReactionSubscribe", diff --git a/src/commands/rooms/messages/send.ts b/src/commands/rooms/messages/send.ts index c68514e6..79dcdc0e 100644 --- a/src/commands/rooms/messages/send.ts +++ b/src/commands/rooms/messages/send.ts @@ -100,62 +100,44 @@ export default class MessagesSend extends ChatBaseCommand { this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail("Failed to create Chat client", flags, "roomMessageSend"); + return this.fail( + "Failed to create Chat client", + flags, + "roomMessageSend", + ); } // Set up connection state logging this.setupConnectionStateLogging(this.chatClient.realtime, flags); // Parse metadata if provided - let metadata; + let metadata: JsonObject | undefined; if (flags.metadata) { - try { - metadata = JSON.parse(flags.metadata); - this.logCliEvent( - flags, - "message", - "metadataParsed", - "Message metadata parsed", - { metadata }, - ); - } catch (error) { - this.fail( - `Invalid metadata JSON: ${errorMessage(error)}`, - flags, - "roomMessageSend", - ); + const parsedMetadata = this.parseJsonFlag( + flags.metadata, + "metadata", + flags, + ); + if ( + typeof parsedMetadata !== "object" || + parsedMetadata === null || + Array.isArray(parsedMetadata) + ) { + this.fail("Metadata must be a JSON object", flags, "roomMessageSend"); } + + metadata = parsedMetadata as JsonObject; + + this.logCliEvent( + flags, + "message", + "metadataParsed", + "Message metadata parsed", + { metadata }, + ); } - // Get the room with default options - this.logCliEvent( - flags, - "room", - "gettingRoom", - `Getting room handle for ${args.room}`, - ); const room = await this.chatClient.rooms.get(args.room); - this.logCliEvent( - flags, - "room", - "gotRoom", - `Got room handle for ${args.room}`, - ); - - // Attach to the room - this.logCliEvent( - flags, - "room", - "attaching", - `Attaching to room ${args.room}`, - ); - await room.attach(); - this.logCliEvent( - flags, - "room", - "attached", - `Attached to room ${args.room}`, - ); // Validate count and delay const count = Math.max(1, flags.count); diff --git a/src/commands/rooms/messages/update.ts b/src/commands/rooms/messages/update.ts new file mode 100644 index 00000000..b5882daf --- /dev/null +++ b/src/commands/rooms/messages/update.ts @@ -0,0 +1,207 @@ +import { Args, Flags } from "@oclif/core"; +import type { + Headers, + JsonObject, + OperationDetails, + UpdateMessageParams, +} from "@ably/chat"; + +import { productApiFlags, clientIdFlag } from "../../../flags.js"; +import { ChatBaseCommand } from "../../../chat-base-command.js"; +import { + formatProgress, + formatSuccess, + formatResource, +} from "../../../utils/output.js"; + +export default class MessagesUpdate extends ChatBaseCommand { + static override args = { + room: Args.string({ + description: "The room containing the message to update", + required: true, + }), + serial: Args.string({ + description: "The serial of the message to update", + required: true, + }), + text: Args.string({ + description: "The new message text", + required: true, + }), + }; + + static override description = "Update a message in an Ably Chat room"; + + static override examples = [ + '$ ably rooms messages update my-room "serial-001" "Updated text"', + '$ ably rooms messages update my-room "serial-001" "Updated text" --description "typo fix"', + '$ ably rooms messages update my-room "serial-001" "Updated text" --metadata \'{"edited":true}\'', + '$ ably rooms messages update my-room "serial-001" "Updated text" --headers \'{"source":"cli"}\'', + '$ ably rooms messages update my-room "serial-001" "Updated text" --json', + ]; + + static override flags = { + ...productApiFlags, + ...clientIdFlag, + description: Flags.string({ + description: "Description of the update operation", + }), + headers: Flags.string({ + description: "Additional headers for the message (JSON format)", + }), + metadata: Flags.string({ + description: "Additional metadata for the message (JSON format)", + }), + }; + + async run(): Promise { + const { args, flags } = await this.parse(MessagesUpdate); + + try { + // Parse and validate metadata before any client setup + let metadata: JsonObject | undefined; + if (flags.metadata !== undefined) { + const parsedMetadata = this.parseJsonFlag( + flags.metadata, + "metadata", + flags, + ); + if ( + typeof parsedMetadata !== "object" || + parsedMetadata === null || + Array.isArray(parsedMetadata) + ) { + this.fail( + "Metadata must be a JSON object", + flags, + "roomMessageUpdate", + ); + } + + metadata = parsedMetadata as JsonObject; + + this.logCliEvent( + flags, + "roomMessageUpdate", + "metadataParsed", + "Message metadata parsed", + { metadata }, + ); + } + + // Parse and validate headers before any client setup + let headers: Headers | undefined; + if (flags.headers !== undefined) { + const parsedHeaders = this.parseJsonFlag( + flags.headers, + "headers", + flags, + ); + if ( + typeof parsedHeaders !== "object" || + parsedHeaders === null || + Array.isArray(parsedHeaders) + ) { + this.fail( + "Headers must be a JSON object", + flags, + "roomMessageUpdate", + ); + } + + headers = parsedHeaders as Headers; + + this.logCliEvent( + flags, + "roomMessageUpdate", + "headersParsed", + "Message headers parsed", + { headers }, + ); + } + + const chatClient = await this.createChatClient(flags); + + if (!chatClient) { + return this.fail( + "Failed to create Chat client", + flags, + "roomMessageUpdate", + ); + } + + this.setupConnectionStateLogging(chatClient.realtime, flags); + + const room = await chatClient.rooms.get(args.room); + + if (!this.shouldOutputJson(flags)) { + this.log( + formatProgress( + "Updating message " + + formatResource(args.serial) + + " in room " + + formatResource(args.room), + ), + ); + } + + // Build update params + const updateParams: UpdateMessageParams = { + text: args.text, + ...(metadata && { metadata }), + ...(headers && { headers }), + }; + + // Build operation details + const details: OperationDetails | undefined = flags.description + ? { description: flags.description } + : undefined; + + this.logCliEvent( + flags, + "roomMessageUpdate", + "updating", + `Updating message ${args.serial} in room ${args.room}`, + { room: args.room, serial: args.serial }, + ); + + const result = await room.messages.update( + args.serial, + updateParams, + details, + ); + + this.logCliEvent( + flags, + "roomMessageUpdate", + "messageUpdated", + `Message ${args.serial} updated in room ${args.room}`, + { room: args.room, serial: args.serial }, + ); + + if (this.shouldOutputJson(flags)) { + this.logJsonResult( + { + room: args.room, + serial: args.serial, + updatedText: result.text, + versionSerial: result.version.serial, + }, + flags, + ); + } else { + this.log( + formatSuccess( + `Message ${formatResource(args.serial)} updated in room ${formatResource(args.room)}.`, + ), + ); + this.log(` Version serial: ${formatResource(result.version.serial)}`); + } + } catch (error) { + this.fail(error, flags, "roomMessageUpdate", { + room: args.room, + serial: args.serial, + }); + } + } +} diff --git a/src/commands/rooms/occupancy/get.ts b/src/commands/rooms/occupancy/get.ts index 4f1ff210..3f39a8ec 100644 --- a/src/commands/rooms/occupancy/get.ts +++ b/src/commands/rooms/occupancy/get.ts @@ -1,5 +1,5 @@ import { Args } from "@oclif/core"; -import { ChatClient, Room, OccupancyData } from "@ably/chat"; +import { ChatClient, Room } from "@ably/chat"; import { ChatBaseCommand } from "../../../chat-base-command.js"; import { clientIdFlag, productApiFlags } from "../../../flags.js"; import { formatResource } from "../../../utils/output.js"; @@ -37,40 +37,18 @@ export default class RoomsOccupancyGet extends ChatBaseCommand { this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail("Failed to create Chat client", flags, "roomOccupancyGet"); + return this.fail( + "Failed to create Chat client", + flags, + "roomOccupancyGet", + ); } const { room: roomName } = args; - // Get the room with occupancy enabled this.room = await this.chatClient.rooms.get(roomName); - // Attach to the room to access occupancy with timeout - let attachTimeout; - await Promise.race([ - this.room.attach(), - new Promise((_, reject) => { - attachTimeout = setTimeout( - () => reject(new Error("Room attach timeout")), - 10000, - ); - }), - ]); - - clearTimeout(attachTimeout); - - // Get occupancy metrics using the Chat SDK's occupancy API - let occupancyTimeout; - const occupancyMetrics = await Promise.race([ - this.room.occupancy.get(), - new Promise((_, reject) => { - occupancyTimeout = setTimeout( - () => reject(new Error("Occupancy get timeout")), - 5000, - ); - }), - ]); - clearTimeout(occupancyTimeout); + const occupancyMetrics = await this.room.occupancy.get(); // Output the occupancy metrics based on format if (this.shouldOutputJson(flags)) { diff --git a/src/commands/rooms/occupancy/subscribe.ts b/src/commands/rooms/occupancy/subscribe.ts index 4c65b315..3cbc77e4 100644 --- a/src/commands/rooms/occupancy/subscribe.ts +++ b/src/commands/rooms/occupancy/subscribe.ts @@ -61,7 +61,7 @@ export default class RoomsOccupancySubscribe extends ChatBaseCommand { this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail( + return this.fail( "Failed to create Chat client", flags, "roomOccupancySubscribe", diff --git a/src/commands/rooms/reactions/send.ts b/src/commands/rooms/reactions/send.ts index 16a43b32..6878b756 100644 --- a/src/commands/rooms/reactions/send.ts +++ b/src/commands/rooms/reactions/send.ts @@ -70,9 +70,14 @@ export default class RoomsReactionsSend extends ChatBaseCommand { this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail("Failed to create Chat client", flags, "roomReactionSend", { - room: roomName, - }); + return this.fail( + "Failed to create Chat client", + flags, + "roomReactionSend", + { + room: roomName, + }, + ); } // Set up connection state logging diff --git a/src/commands/rooms/reactions/subscribe.ts b/src/commands/rooms/reactions/subscribe.ts index e2f5bbe1..97f5d709 100644 --- a/src/commands/rooms/reactions/subscribe.ts +++ b/src/commands/rooms/reactions/subscribe.ts @@ -44,7 +44,7 @@ export default class RoomsReactionsSubscribe extends ChatBaseCommand { this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail( + return this.fail( "Failed to initialize clients", flags, "roomReactionSubscribe", diff --git a/src/commands/rooms/typing/keystroke.ts b/src/commands/rooms/typing/keystroke.ts index dba4ada5..d297c4d3 100644 --- a/src/commands/rooms/typing/keystroke.ts +++ b/src/commands/rooms/typing/keystroke.ts @@ -67,7 +67,11 @@ export default class TypingKeystroke extends ChatBaseCommand { // Create Chat client this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail("Failed to initialize clients", flags, "roomTypingKeystroke"); + return this.fail( + "Failed to initialize clients", + flags, + "roomTypingKeystroke", + ); } const { room: roomName } = args; diff --git a/src/commands/rooms/typing/subscribe.ts b/src/commands/rooms/typing/subscribe.ts index 82c49168..9d9a66c1 100644 --- a/src/commands/rooms/typing/subscribe.ts +++ b/src/commands/rooms/typing/subscribe.ts @@ -39,7 +39,11 @@ export default class TypingSubscribe extends ChatBaseCommand { // Create Chat client this.chatClient = await this.createChatClient(flags); if (!this.chatClient) { - this.fail("Failed to initialize clients", flags, "roomTypingSubscribe"); + return this.fail( + "Failed to initialize clients", + flags, + "roomTypingSubscribe", + ); } const { room: roomName } = args; diff --git a/test/helpers/mock-ably-chat.ts b/test/helpers/mock-ably-chat.ts index 803b753a..b4993f2f 100644 --- a/test/helpers/mock-ably-chat.ts +++ b/test/helpers/mock-ably-chat.ts @@ -55,6 +55,8 @@ export interface MockRoomMessages { subscribe: Mock; send: Mock; get: Mock; + update: Mock; + delete: Mock; reactions: MockMessageReactions; // Internal emitter for simulating events _emitter: AblyEventEmitter; @@ -236,6 +238,20 @@ function createMockRoomMessages(): MockRoomMessages { createdAt: Date.now(), }), get: vi.fn().mockResolvedValue({ items: [] }), + update: vi.fn().mockResolvedValue({ + serial: "mock-serial", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "mock-version-serial", timestamp: new Date() }, + }), + delete: vi.fn().mockResolvedValue({ + serial: "mock-serial", + clientId: "mock-client-id", + text: "", + timestamp: new Date(), + version: { serial: "mock-version-serial", timestamp: new Date() }, + }), reactions: createMockMessageReactions(), _emitter: emitter, _emit: (message: Message) => { diff --git a/test/unit/commands/rooms/features.test.ts b/test/unit/commands/rooms/features.test.ts index 54aee47b..bc390651 100644 --- a/test/unit/commands/rooms/features.test.ts +++ b/test/unit/commands/rooms/features.test.ts @@ -22,7 +22,6 @@ describe("rooms feature commands", function () { import.meta.url, ); - expect(room.attach).toHaveBeenCalled(); expect(room.occupancy.get).toHaveBeenCalled(); expect(stdout).toContain("5"); }); @@ -187,7 +186,7 @@ describe("rooms feature commands", function () { const chatMock = getMockAblyChat(); const room = chatMock.rooms._getRoom("test-room"); - room.attach.mockRejectedValue(new Error("Connection failed")); + room.occupancy.get.mockRejectedValue(new Error("Connection failed")); const { error } = await runCommand( ["rooms:occupancy:get", "test-room"], diff --git a/test/unit/commands/rooms/messages.test.ts b/test/unit/commands/rooms/messages.test.ts index 6445aff3..9f23079e 100644 --- a/test/unit/commands/rooms/messages.test.ts +++ b/test/unit/commands/rooms/messages.test.ts @@ -23,7 +23,6 @@ describe("rooms messages commands", function () { import.meta.url, ); - expect(room.attach).toHaveBeenCalled(); expect(room.messages.send).toHaveBeenCalledWith({ text: "HelloWorld", }); @@ -382,7 +381,6 @@ describe("rooms messages commands", function () { import.meta.url, ); - expect(room.attach).toHaveBeenCalled(); expect(room.messages.history).toHaveBeenCalled(); expect(stdout).toContain("Historical message 1"); expect(stdout).toContain("Historical message 2"); diff --git a/test/unit/commands/rooms/messages/delete.test.ts b/test/unit/commands/rooms/messages/delete.test.ts new file mode 100644 index 00000000..e010a576 --- /dev/null +++ b/test/unit/commands/rooms/messages/delete.test.ts @@ -0,0 +1,171 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { runCommand } from "@oclif/test"; +import { getMockAblyChat } from "../../../../helpers/mock-ably-chat.js"; +import { captureJsonLogs } from "../../../../helpers/ndjson.js"; +import { + standardHelpTests, + standardArgValidationTests, + standardFlagTests, +} from "../../../../helpers/standard-tests.js"; + +describe("rooms:messages:delete command", () => { + beforeEach(() => { + getMockAblyChat(); + }); + + standardHelpTests("rooms:messages:delete", import.meta.url); + standardArgValidationTests("rooms:messages:delete", import.meta.url, { + requiredArgs: ["test-room", "serial-001"], + }); + standardFlagTests("rooms:messages:delete", import.meta.url, [ + "--json", + "--description", + ]); + + describe("functionality", () => { + it("should delete a message successfully", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.delete.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + const { stdout } = await runCommand( + ["rooms:messages:delete", "test-room", "serial-001"], + import.meta.url, + ); + + expect(room.messages.delete).toHaveBeenCalledWith( + "serial-001", + undefined, + ); + expect(stdout).toContain("deleted"); + expect(stdout).toContain("test-room"); + }); + + it("should pass description as OperationDetails", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.delete.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + await runCommand( + [ + "rooms:messages:delete", + "test-room", + "serial-001", + "--description", + "spam-removal", + ], + import.meta.url, + ); + + expect(room.messages.delete).toHaveBeenCalledWith("serial-001", { + description: "spam-removal", + }); + }); + + it("should not pass details when no description", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.delete.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + await runCommand( + ["rooms:messages:delete", "test-room", "serial-001"], + import.meta.url, + ); + + expect(room.messages.delete).toHaveBeenCalledWith( + "serial-001", + undefined, + ); + }); + + it("should emit JSON envelope with --json", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.delete.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + const records = await captureJsonLogs(async () => { + await runCommand( + ["rooms:messages:delete", "test-room", "serial-001", "--json"], + import.meta.url, + ); + }); + + const results = records.filter( + (r) => r.type === "result" && r.room === "test-room", + ); + expect(results.length).toBeGreaterThan(0); + const record = results[0]; + expect(record).toHaveProperty("type", "result"); + expect(record).toHaveProperty("command", "rooms:messages:delete"); + expect(record).toHaveProperty("success", true); + expect(record).toHaveProperty("room", "test-room"); + expect(record).toHaveProperty("serial", "serial-001"); + expect(record).toHaveProperty("versionSerial", "version-serial-001"); + }); + + it("should display version serial in human output", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.delete.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + const { stdout } = await runCommand( + ["rooms:messages:delete", "test-room", "serial-001"], + import.meta.url, + ); + + expect(stdout).toContain("version-serial-001"); + }); + }); + + describe("error handling", () => { + it("should handle API error", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.delete.mockRejectedValue(new Error("Delete failed")); + + const { error } = await runCommand( + ["rooms:messages:delete", "test-room", "serial-001"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Delete failed"); + }); + }); +}); diff --git a/test/unit/commands/rooms/messages/history.test.ts b/test/unit/commands/rooms/messages/history.test.ts index b4e61d81..ed3893d4 100644 --- a/test/unit/commands/rooms/messages/history.test.ts +++ b/test/unit/commands/rooms/messages/history.test.ts @@ -53,7 +53,6 @@ describe("rooms:messages:history command", () => { import.meta.url, ); - expect(room.attach).toHaveBeenCalled(); expect(room.messages.history).toHaveBeenCalled(); expect(stdout).toContain("Historical message 1"); expect(stdout).toContain("Historical message 2"); diff --git a/test/unit/commands/rooms/messages/send.test.ts b/test/unit/commands/rooms/messages/send.test.ts index e27edb85..bb288e36 100644 --- a/test/unit/commands/rooms/messages/send.test.ts +++ b/test/unit/commands/rooms/messages/send.test.ts @@ -39,7 +39,6 @@ describe("rooms:messages:send command", () => { import.meta.url, ); - expect(room.attach).toHaveBeenCalled(); expect(room.messages.send).toHaveBeenCalledWith({ text: "HelloWorld", }); diff --git a/test/unit/commands/rooms/messages/update.test.ts b/test/unit/commands/rooms/messages/update.test.ts new file mode 100644 index 00000000..85fa333e --- /dev/null +++ b/test/unit/commands/rooms/messages/update.test.ts @@ -0,0 +1,348 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { runCommand } from "@oclif/test"; +import { getMockAblyChat } from "../../../../helpers/mock-ably-chat.js"; +import { captureJsonLogs } from "../../../../helpers/ndjson.js"; +import { + standardHelpTests, + standardArgValidationTests, + standardFlagTests, +} from "../../../../helpers/standard-tests.js"; + +describe("rooms:messages:update command", () => { + beforeEach(() => { + getMockAblyChat(); + }); + + standardHelpTests("rooms:messages:update", import.meta.url); + standardArgValidationTests("rooms:messages:update", import.meta.url, { + requiredArgs: ["test-room", "serial-001", "updated-text"], + }); + standardFlagTests("rooms:messages:update", import.meta.url, [ + "--json", + "--metadata", + "--headers", + "--description", + ]); + + describe("functionality", () => { + it("should update a message successfully", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + const { stdout } = await runCommand( + ["rooms:messages:update", "test-room", "serial-001", "updated-text"], + import.meta.url, + ); + + expect(room.messages.update).toHaveBeenCalledWith( + "serial-001", + { text: "updated-text" }, + undefined, + ); + expect(stdout).toContain("updated"); + expect(stdout).toContain("test-room"); + }); + + it("should pass metadata when --metadata provided", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--metadata", + '{"edited":true}', + ], + import.meta.url, + ); + + expect(room.messages.update).toHaveBeenCalledWith( + "serial-001", + { text: "updated-text", metadata: { edited: true } }, + undefined, + ); + }); + + it("should pass headers when --headers provided", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--headers", + '{"source":"cli"}', + ], + import.meta.url, + ); + + expect(room.messages.update).toHaveBeenCalledWith( + "serial-001", + { text: "updated-text", headers: { source: "cli" } }, + undefined, + ); + }); + + it("should pass description as OperationDetails", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--description", + "typo-fix", + ], + import.meta.url, + ); + + expect(room.messages.update).toHaveBeenCalledWith( + "serial-001", + { text: "updated-text" }, + { description: "typo-fix" }, + ); + }); + + it("should not pass details when no description", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + await runCommand( + ["rooms:messages:update", "test-room", "serial-001", "updated-text"], + import.meta.url, + ); + + expect(room.messages.update).toHaveBeenCalledWith( + "serial-001", + { text: "updated-text" }, + undefined, + ); + }); + + it("should emit JSON envelope with --json", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + const records = await captureJsonLogs(async () => { + await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--json", + ], + import.meta.url, + ); + }); + + const results = records.filter( + (r) => r.type === "result" && r.room === "test-room", + ); + expect(results.length).toBeGreaterThan(0); + const record = results[0]; + expect(record).toHaveProperty("type", "result"); + expect(record).toHaveProperty("command", "rooms:messages:update"); + expect(record).toHaveProperty("success", true); + expect(record).toHaveProperty("room", "test-room"); + expect(record).toHaveProperty("serial", "serial-001"); + expect(record).toHaveProperty("versionSerial", "version-serial-001"); + }); + + it("should display version serial in human output", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockResolvedValue({ + serial: "serial-001", + clientId: "mock-client-id", + text: "updated-text", + timestamp: new Date(), + version: { serial: "version-serial-001", timestamp: new Date() }, + }); + + const { stdout } = await runCommand( + ["rooms:messages:update", "test-room", "serial-001", "updated-text"], + import.meta.url, + ); + + expect(stdout).toContain("version-serial-001"); + }); + }); + + describe("error handling", () => { + it("should handle invalid metadata JSON", async () => { + const { error } = await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--metadata", + "invalid-json", + ], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toMatch(/Invalid metadata JSON/i); + }); + + it("should reject non-object metadata", async () => { + const { error } = await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--metadata", + "42", + ], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toMatch(/Metadata must be a JSON object/i); + }); + + it("should reject array metadata", async () => { + const { error } = await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--metadata", + "[1,2,3]", + ], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toMatch(/Metadata must be a JSON object/i); + }); + + it("should handle invalid headers JSON", async () => { + const { error } = await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--headers", + "invalid-json", + ], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toMatch(/Invalid headers JSON/i); + }); + + it("should reject non-object headers", async () => { + const { error } = await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--headers", + "42", + ], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toMatch(/Headers must be a JSON object/i); + }); + + it("should reject array headers", async () => { + const { error } = await runCommand( + [ + "rooms:messages:update", + "test-room", + "serial-001", + "updated-text", + "--headers", + "[1,2,3]", + ], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toMatch(/Headers must be a JSON object/i); + }); + + it("should handle API error", async () => { + const chatMock = getMockAblyChat(); + const room = chatMock.rooms._getRoom("test-room"); + + room.messages.update.mockRejectedValue(new Error("Update failed")); + + const { error } = await runCommand( + ["rooms:messages:update", "test-room", "serial-001", "updated-text"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Update failed"); + }); + }); +}); diff --git a/test/unit/commands/rooms/occupancy/get.test.ts b/test/unit/commands/rooms/occupancy/get.test.ts index f3d456e0..f26027dc 100644 --- a/test/unit/commands/rooms/occupancy/get.test.ts +++ b/test/unit/commands/rooms/occupancy/get.test.ts @@ -32,7 +32,6 @@ describe("rooms:occupancy:get command", () => { import.meta.url, ); - expect(room.attach).toHaveBeenCalled(); expect(room.occupancy.get).toHaveBeenCalled(); expect(stdout).toContain("Connections: 5"); expect(stdout).toContain("Presence Members: 3"); @@ -79,9 +78,7 @@ describe("rooms:occupancy:get command", () => { it("should output JSON error on failure", async () => { const chatMock = getMockAblyChat(); const room = chatMock.rooms._getRoom("test-room"); - room.attach.mockImplementation(async () => { - throw new Error("Room attach timeout"); - }); + room.occupancy.get.mockRejectedValue(new Error("Service unavailable")); const { stdout } = await runCommand( ["rooms:occupancy:get", "test-room", "--json"],