[DX-790] Add support for Chat message updates and deletes

.claude/skills/ably-codebase-review/SKILL.md

@@ -103,13 +103,23 @@ Launch these agents **in parallel**. Each agent gets a focused mandate and uses
 5. **Read** command files and look for unguarded `this.log()` calls (not inside `if (!this.shouldOutputJson(flags))`)
 6. **Grep** for quoted resource names patterns like `"${` or `'${` near `channel`, `name`, `app` variables — should use `formatResource()`
 
+**Method (grep — structured output format):**
+7. **Grep** for box-drawing characters (`┌`, `┬`, `├`, `└`, `│`) in command files — non-JSON output must use multi-line labeled blocks, not ASCII tables or grids
+8. **Grep** for subscribe commands that call `getAll()` or equivalent before subscribing — subscribe commands must NOT fetch initial state (they should only listen for new events)
+9. For data-outputting commands, **read** both the JSON and non-JSON output paths and compare fields — non-JSON should expose the same fields as JSON mode (omitting only null/empty values)
+10. **Grep** for local `interface` definitions in `src/commands/` that duplicate SDK types (e.g., `interface CursorPosition`, `interface CursorData`, `interface PresenceMessage`) — these should import from `ably`, `@ably/spaces`, or `@ably/chat` instead. Display/output interfaces in `src/utils/` are intentional and fine.
+
 **Method (LSP — for completeness mapping):**
-7. Use `LSP findReferences` on `shouldOutputJson` to get the definitive list of all commands that check for JSON mode — cross-reference against the full command list to find commands missing JSON guards
+11. Use `LSP findReferences` on `shouldOutputJson` to get the definitive list of all commands that check for JSON mode — cross-reference against the full command list to find commands missing JSON guards
 
 **Reasoning guidance:**
 - List commands don't use `formatSuccess()` (no action to confirm) — this is correct, not a deviation
 - `chalk.red("✗")` / `chalk.green("✓")` as visual indicators in test/bench output is acceptable
 - `chalk.yellow("Warning: ...")` should use `formatWarning()` instead — `formatWarning` adds the `⚠` symbol automatically and "Warning:" prefix is unnecessary
+- ASCII tables/grids in non-JSON output are deviations — use multi-line labeled blocks with `formatLabel()` instead
+- Subscribe commands fetching initial state (via `getAll()`, `getSelf()`, etc.) before subscribing are deviations — subscribe should only listen for new events
+- Non-JSON output that hides fields available in JSON mode is a deviation — both modes should expose the same data
+- Local `interface` definitions in command files that duplicate SDK types are deviations — import from the SDK package instead. Display/output interfaces in `src/utils/` (e.g., `MemberOutput`, `MessageDisplayFields`) are intentional transformations, not duplicates.
 
 ### Agent 4: Flag Architecture Sweep
 
@@ -143,11 +153,14 @@ Launch these agents **in parallel**. Each agent gets a focused mandate and uses
 3. **Grep** for `shouldOutputJson` in command files to find all JSON-aware commands
 4. Cross-reference: every leaf command should appear in both the `logJsonResult`/`logJsonEvent` list and the `shouldOutputJson` list
 5. **Read** streaming commands to verify they use `logJsonEvent`, one-shot commands use `logJsonResult`
+6. **Read** each `logJsonResult`/`logJsonEvent` call and verify data is nested under a domain key — singular for events/single items (e.g., `{message: ...}`, `{cursor: ...}`), plural for collections (e.g., `{cursors: [...]}`, `{rules: [...]}`). Top-level envelope fields are `type`, `command`, `success` only. Metadata like `total`, `timestamp`, `appId` may sit alongside the domain key.
 
 **Reasoning guidance:**
 - Commands that ONLY have human output (no JSON path) are deviations
 - Direct `formatJsonRecord` usage in command files should use `logJsonResult`/`logJsonEvent` instead
 - Topic index commands (showing help) don't need JSON output
+- Data spread at the top level without a domain key is a deviation — nest under a singular or plural domain noun
+- Metadata fields (`total`, `timestamp`, `hasMore`, `appId`) alongside the domain key are acceptable — they describe the result, not the domain objects
 
 ### Agent 6: Test Pattern Sweep
 
@@ -184,15 +197,17 @@ 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
 
 **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
 

.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
@@ -176,14 +196,23 @@ if (!this.shouldOutputJson(flags)) {
   this.log(formatListening("Listening for messages."));
 }
 
-// JSON output — use logJsonResult for one-shot results:
+// JSON output — nest data under a domain key, not spread at top level.
+// Envelope provides top-level: type, command, success.
+// One-shot single result — singular domain key:
+if (this.shouldOutputJson(flags)) {
+  this.logJsonResult({ message: messageData }, flags);
+  // → {"type":"result","command":"...","success":true,"message":{...}}
+}
+
+// One-shot collection result — plural domain key + optional metadata:
 if (this.shouldOutputJson(flags)) {
-  this.logJsonResult({ channel: args.channel, message }, flags);
+  this.logJsonResult({ messages: items, total: items.length }, flags);
 }
 
-// Streaming events — use logJsonEvent:
+// Streaming events — singular domain key:
 if (this.shouldOutputJson(flags)) {
-  this.logJsonEvent({ eventType: event.type, message, channel: channelName }, flags);
+  this.logJsonEvent({ message: messageData }, flags);
+  // → {"type":"event","command":"...","message":{...}}
 }
 ```
 
@@ -389,8 +418,15 @@ See the "Keeping Skills Up to Date" section in `CLAUDE.md` for the full list of
 - [ ] All human output wrapped in `if (!this.shouldOutputJson(flags))`
 - [ ] Output helpers used correctly (`formatProgress`, `formatSuccess`, `formatWarning`, `formatListening`, `formatResource`, `formatTimestamp`, `formatClientId`, `formatEventType`, `formatLabel`, `formatHeading`, `formatIndex`)
 - [ ] `formatSuccess()` messages end with `.` (period)
+- [ ] Non-JSON data output uses multi-line labeled blocks (see `patterns.md` "Human-Readable Output Format"), not tables or custom grids
+- [ ] Non-JSON output exposes all available SDK fields (same data as JSON mode, omitting only null/empty values)
+- [ ] SDK types imported directly (`import type { CursorUpdate } from "@ably/spaces"`) — no local interface redefinitions of SDK types (display interfaces in `src/utils/` are fine)
+- [ ] Field coverage checked against SDK type definitions (`node_modules/ably/ably.d.ts`, `node_modules/@ably/spaces/dist/mjs/types.d.ts`)
+- [ ] Subscribe commands do NOT fetch initial state — they only listen for new events (use `get-all` for current state)
 - [ ] Resource names use `formatResource(name)`, never quoted
 - [ ] JSON output uses `logJsonResult()` (one-shot) or `logJsonEvent()` (streaming), not direct `formatJsonRecord()`
+- [ ] JSON data nested under a domain key (singular for events/single items, plural for collections) — not spread at top level (see `patterns.md` "JSON Data Nesting Convention")
+- [ ] `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"`)