diff --git a/.claude/skills/ably-codebase-review/SKILL.md b/.claude/skills/ably-codebase-review/SKILL.md index fc3603e82..35821348f 100644 --- a/.claude/skills/ably-codebase-review/SKILL.md +++ b/.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 diff --git a/.claude/skills/ably-new-command/SKILL.md b/.claude/skills/ably-new-command/SKILL.md index 5c40bcbe0..88bcb0940 100644 --- a/.claude/skills/ably-new-command/SKILL.md +++ b/.claude/skills/ably-new-command/SKILL.md @@ -176,14 +176,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({ channel: args.channel, message }, flags); + this.logJsonResult({ message: messageData }, flags); + // → {"type":"result","command":"...","success":true,"message":{...}} } -// Streaming events — use logJsonEvent: +// One-shot collection result — plural domain key + optional metadata: if (this.shouldOutputJson(flags)) { - this.logJsonEvent({ eventType: event.type, message, channel: channelName }, flags); + this.logJsonResult({ messages: items, total: items.length }, flags); +} + +// Streaming events — singular domain key: +if (this.shouldOutputJson(flags)) { + this.logJsonEvent({ message: messageData }, flags); + // → {"type":"event","command":"...","message":{...}} } ``` @@ -389,8 +398,14 @@ 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") - [ ] 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 c7dd25dd1..cb81af49d 100644 --- a/.claude/skills/ably-new-command/references/patterns.md +++ b/.claude/skills/ably-new-command/references/patterns.md @@ -59,17 +59,32 @@ async run(): Promise { sequenceCounter++; // Format and output the message if (this.shouldOutputJson(flags)) { - // Use "event" type for streaming records. IMPORTANT: don't use "type" as a + // Nest data under a singular domain key. IMPORTANT: don't use "type" as a // data key — it's reserved by the envelope. Use "eventType" instead. this.logJsonEvent({ - eventType: "message", // not "type" — that's reserved by the envelope - channel: args.channel, - data: message.data, - name: message.name, - timestamp: message.timestamp, + message: { + id: message.id, + timestamp: message.timestamp, + channel: args.channel, + event: message.name, + clientId: message.clientId, + serial: message.serial, + data: message.data, + }, }, flags); } else { - // Human-readable output with formatTimestamp, formatResource, chalk colors + // Human-readable output — multi-line labeled block per event + // Use shared formatters where available (e.g., formatMessagesOutput for channels) + const timestamp = formatMessageTimestamp(message.timestamp); + this.log(formatTimestamp(timestamp)); + if (message.id) this.log(`${formatLabel("ID")} ${message.id}`); + this.log(`${formatLabel("Timestamp")} ${timestamp}`); + this.log(`${formatLabel("Channel")} ${formatResource(args.channel)}`); + this.log(`${formatLabel("Event")} ${formatEventType(message.name || "(none)")}`); + if (message.clientId) this.log(`${formatLabel("Client ID")} ${formatClientId(message.clientId)}`); + if (message.serial) this.log(`${formatLabel("Serial")} ${message.serial}`); + this.log(`${formatLabel("Data")} ${String(message.data)}`); + this.log(""); // blank line between events } }); @@ -112,9 +127,9 @@ async run(): Promise { await channel.publish(message as Ably.Message); if (this.shouldOutputJson(flags)) { - // Use "result" type for one-shot results. Don't use "success" as a data key + // Nest data under a domain key. Don't use "success" as a data key // for batch summaries — it overrides the envelope's success field. Use "allSucceeded". - this.logJsonResult({ channel: args.channel }, flags); + this.logJsonResult({ message: { channel: args.channel, name: message.name, data: message.data } }, flags); } else { this.log(formatSuccess("Message published to channel: " + formatResource(args.channel) + ".")); } @@ -158,10 +173,11 @@ async run(): Promise { const messages = history.items; if (this.shouldOutputJson(flags)) { - this.logJsonResult({ messages }, flags); + // Plural domain key for collections, optional metadata alongside + this.logJsonResult({ messages, total: messages.length }, flags); } else { this.log(formatSuccess(`Found ${messages.length} messages.`)); - // Display each message + // Display each message using multi-line labeled blocks } } catch (error) { this.fail(error, flags, "history", { channel: args.channel }); @@ -195,11 +211,15 @@ async run(): Promise { const data = result.items?.[0] || {}; if (this.shouldOutputJson(flags)) { - this.logJsonResult({ resource: args.id, ...data }, flags); + // Singular domain key for single-item results + this.logJsonResult({ resource: data }, flags); } else { + // Multi-line labeled block — one field per line, using formatLabel for all labels this.log(`Details for ${formatResource(args.id)}:\n`); this.log(`${formatLabel("Field")} ${data.field}`); this.log(`${formatLabel("Status")} ${data.status}`); + if (data.clientId) this.log(`${formatLabel("Client ID")} ${formatClientId(data.clientId)}`); + // Omit null/undefined fields, show everything else } } catch (error) { this.fail(error, flags, "resourceGet", { resource: args.id }); @@ -318,6 +338,7 @@ async run(): Promise { const limited = flags.limit ? items.slice(0, flags.limit) : items; if (this.shouldOutputJson(flags)) { + // Plural domain key for collections, metadata alongside this.logJsonResult({ items: limited, total: limited.length, appId }, flags); } else { this.log(`Found ${limited.length} item${limited.length !== 1 ? "s" : ""}:\n`); @@ -355,13 +376,275 @@ async run(): Promise { const result = await controlApi.someMethod(appId, data); if (this.shouldOutputJson(flags)) { + // Singular domain key for single-item results this.logJsonResult({ resource: result }, flags); } else { this.log(formatSuccess("Resource created: " + formatResource(result.id) + ".")); - // Display additional fields + // Display additional fields using formatLabel + this.log(`${formatLabel("Status")} ${result.status}`); + this.log(`${formatLabel("Created")} ${new Date(result.createdAt).toISOString()}`); } } catch (error) { this.fail(error, flags, "createResource"); } } ``` + +--- + +## Human-Readable Output Format + +All non-JSON output for data records uses **multi-line labeled blocks**. Never use ASCII tables, box-drawing characters (`┌─┬─┐`, `│`), or custom grid layouts. + +### Streaming events (subscribe commands) + +Each event is a multi-line block with a timestamp header, then labeled fields. Separate events with a blank line. + +``` +[2024-01-15T10:30:00.000Z] +ID: msg-123 +Timestamp: 2024-01-15T10:30:00.000Z +Channel: my-channel +Event: message.created +Client ID: user-123 +Serial: 01H... +Data: {"key": "value"} +``` + +Code pattern: +```typescript +// In the event handler callback +const timestamp = formatMessageTimestamp(message.timestamp); +this.log(formatTimestamp(timestamp)); // dim [timestamp] header +if (message.id) this.log(`${formatLabel("ID")} ${message.id}`); +this.log(`${formatLabel("Timestamp")} ${timestamp}`); +this.log(`${formatLabel("Channel")} ${formatResource(channelName)}`); +this.log(`${formatLabel("Event")} ${formatEventType(message.name)}`); +if (message.clientId) this.log(`${formatLabel("Client ID")} ${formatClientId(message.clientId)}`); +this.log(`${formatLabel("Data")} ${String(message.data)}`); +this.log(""); // blank line separator +``` + +For domain-specific events, create shared formatting functions in the appropriate utils file (e.g., `src/utils/spaces-output.ts` for spaces, `src/utils/output.ts` for channels). + +### One-shot results with multiple records (get-all) + +Use `formatIndex()` for numbering and `formatCountLabel()` for the heading. Index goes on its own line, fields are indented below it. + +``` +Current cursors (3 cursors): + +[1] + Client ID: user-123 + Connection ID: conn-abc + Position X: 150 + Position Y: 300 + Data: {"color": "red"} + +[2] + Client ID: user-456 + Connection ID: conn-def + Position X: 200 + Position Y: 400 +``` + +Code pattern: +```typescript +this.log(`${formatHeading("Current cursors")} (${formatCountLabel(items.length, "cursor")}):\n`); +for (let i = 0; i < items.length; i++) { + const item = items[i]; + this.log(`${formatIndex(i + 1)}`); + this.log(` ${formatLabel("Client ID")} ${formatClientId(item.clientId)}`); + this.log(` ${formatLabel("Connection ID")} ${item.connectionId}`); + this.log(` ${formatLabel("Position X")} ${item.position.x}`); + this.log(` ${formatLabel("Position Y")} ${item.position.y}`); + if (item.data) this.log(` ${formatLabel("Data")} ${JSON.stringify(item.data)}`); + this.log(""); // blank line between records +} +``` + +### History results (time-ordered records) + +History commands combine `formatIndex()` and `formatTimestamp()` on the same line as a heading, since records are time-ordered. This is distinct from get-all which uses index alone. + +``` +[1] [2024-01-15T10:30:00.000Z] + Event: message.created + Channel: my-channel + Data: {"key": "value"} + +[2] [2024-01-15T10:29:55.000Z] + Event: message.created + Channel: my-channel + Data: {"other": "data"} +``` + +Code pattern: +```typescript +for (let i = 0; i < messages.length; i++) { + const msg = messages[i]; + const ts = formatMessageTimestamp(msg.timestamp); + this.log(`${formatIndex(i + 1)} ${formatTimestamp(ts)}`); // index + timestamp on same line + this.log(` ${formatLabel("Event")} ${formatEventType(msg.name || "(none)")}`); + this.log(` ${formatLabel("Channel")} ${formatResource(channelName)}`); + if (msg.clientId) this.log(` ${formatLabel("Client ID")} ${formatClientId(msg.clientId)}`); + this.log(` ${formatLabel("Data")} ${String(msg.data)}`); + this.log(""); +} +``` + +### Single record results (get, acquire, set) + +Same labeled format, no index needed: + +``` +Lock ID: my-lock +Status: locked +Timestamp: 2024-01-15T10:30:00.000Z +Member: + Client ID: user-123 + Connection ID: conn-abc +Attributes: {"priority": "high"} +``` + +### Field display rules + +**Use SDK type definitions as the source of truth** for which fields exist. Before implementing output for a command, read the relevant SDK type definition to ensure all important fields are covered: + +| SDK | Type file | Key types | +|-----|-----------|-----------| +| `ably` | `node_modules/ably/ably.d.ts` | `Message`, `PresenceMessage`, `ChannelStateChange`, `ConnectionStateChange` | +| `@ably/spaces` | `node_modules/@ably/spaces/dist/mjs/types.d.ts` | `SpaceMember`, `CursorUpdate`, `CursorPosition`, `CursorData`, `Lock`, `ProfileData` | +| `@ably/chat` | `node_modules/@ably/chat/dist/chat/core/*.d.ts` | `Message` (chat), `PresenceMember`, `OccupancyEvent`, `Reaction` | + +**Use SDK source code as the source of truth** for method behavior — whether a method requires prior state (e.g., `space.enter()`), what side effects it has, what it actually returns. When in doubt, read the implementation: + +| SDK | Source directory | Key files | +|-----|-----------------|-----------| +| `ably` | `node_modules/ably/` | Realtime, REST, channels, presence | +| `@ably/spaces` | `node_modules/@ably/spaces/dist/mjs/` | `Space.js`, `Members.js`, `Locations.js`, `Cursors.js`, `Locks.js` | +| `@ably/chat` | `node_modules/@ably/chat/dist/chat/core/` | Rooms, messages, presence, reactions | + +**Import SDK types directly** — never redefine SDK interfaces locally. If `@ably/spaces` exports `CursorPosition`, import it: +```typescript +// WRONG — local redefinition duplicates SDK type +interface CursorPosition { x: number; y: number; } + +// CORRECT — import from SDK +import type { CursorPosition, CursorData, CursorUpdate } from "@ably/spaces"; +``` + +**Display interfaces are fine** — types like `MemberOutput`, `MessageDisplayFields`, `CursorOutput` in `src/utils/` that transform SDK types for output are intentional. They're the right place to decide field naming, null handling, and which fields to include. The SDK type defines what's *available*; the display interface defines what to *present*. + +**Field coverage:** +- **Show all available fields** — non-JSON output should expose the same data as JSON mode +- **Omit null/undefined/empty fields** — skip fields with no value (don't show "Profile: null") + +**Formatting:** +- **Use `formatLabel("Name")`** for all field labels — it appends `:` and applies dim styling +- **Use type-appropriate formatters**: `formatClientId()` for client IDs, `formatResource()` for resource names, `formatEventType()` for actions, `formatTimestamp()` for timestamp headers +- **Nested objects**: display as `JSON.stringify(data)` on the same line, or indent with `formatMessageData()` for large/multi-line JSON +- **Nested records** (e.g., member inside lock): use 2-space indent for the sub-fields + +### Command behavior semantics + +Commands must behave strictly according to their documented purpose — no unintended side effects. + +**Subscribe commands** — passive observers: +- **Only** listen for new events — must NOT fetch initial state (use `get-all` for that) +- **NOT enter presence/space** — use `enterSpace: false`. The Spaces SDK's `subscribe()` methods do NOT require `space.enter()` +- Use the message pattern: `formatProgress("Subscribing to X")` → `formatListening("Listening for X.")` + +**Get-all / get commands** — one-shot queries: +- **NOT enter presence/space** — `getAll()`, `get()` do NOT require `space.enter()` +- **NOT subscribe** to events or poll — fetch once, output, exit + +**Set commands** — one-shot mutations: +- Enter space (required by SDK), set value, output, **exit** +- **NOT subscribe** after setting — that is what subscribe commands are for + +**Enter / acquire commands** — hold state until Ctrl+C / `--duration`: +- Enter space, output confirmation with all relevant fields, then `waitAndTrackCleanup` +- **NOT subscribe** to other events + +**Side-effect rules:** +- `space.enter()` only when SDK requires it (set, enter, acquire) +- Call `this.markAsEntered()` after every `space.enter()` (enables cleanup) +- `initializeSpace(enterSpace: true)` calls `markAsEntered()` automatically + +```typescript +// WRONG — subscribe enters the space +await this.initializeSpace(flags, spaceName, { enterSpace: true }); +await this.space!.members.subscribe("update", handler); + +// CORRECT — subscribe is passive +await this.initializeSpace(flags, spaceName, { enterSpace: false }); +await this.space!.members.subscribe("update", handler); + +// WRONG — get-all enters the space +await this.space!.enter(); +const data = await this.space!.locations.getAll(); + +// CORRECT — get-all just fetches +const data = await this.space!.locations.getAll(); + +// WRONG — set command subscribes after setting +await this.space!.locations.set(location); +this.space!.locations.subscribe("update", handler); // NO +await this.waitAndTrackCleanup(flags, "location"); // NO + +// CORRECT — set command exits after setting +await this.space!.locations.set(location); +// run() completes, finally() handles cleanup +``` + +--- + +## JSON Data Nesting Convention + +The JSON envelope provides three top-level fields (`type`, `command`, `success`). Domain data must be nested under a **domain key**, not spread at the top level. + +### Streaming events (logJsonEvent) — singular domain key + +```typescript +// CORRECT — nest event payload under a singular domain key +this.logJsonEvent({ message: messageData }, flags); // → {"type":"event","command":"channels:subscribe","message":{...}} +this.logJsonEvent({ cursor: cursorData }, flags); // spaces cursors +this.logJsonEvent({ member: memberData }, flags); // spaces members +this.logJsonEvent({ lock: lockData }, flags); // spaces locks +this.logJsonEvent({ location: locationData }, flags); // spaces locations +this.logJsonEvent({ annotation: annotationData }, flags); // channels annotations +this.logJsonEvent({ reaction: reactionData }, flags); // rooms reactions + +// WRONG — spreading fields loses the domain boundary +this.logJsonEvent({ clientId, position, data }, flags); +``` + +### One-shot single results (logJsonResult) — singular domain key + +```typescript +this.logJsonResult({ lock: lockData }, flags); // → {"type":"result","command":"spaces:locks:get","success":true,"lock":{...}} +this.logJsonResult({ key: keyData }, flags); // auth keys create +this.logJsonResult({ app: appData }, flags); // apps create +this.logJsonResult({ rule: ruleData }, flags); // apps rules create +``` + +### One-shot collection results (logJsonResult) — plural domain key + metadata + +```typescript +this.logJsonResult({ cursors: items }, flags); // → {"type":"result","command":"spaces:cursors:get-all","success":true,"cursors":[...]} +this.logJsonResult({ rules: items, appId, total }, flags); // rules list — metadata alongside collection +this.logJsonResult({ channels: items, total, hasMore }, flags); // channels list +``` + +Metadata fields (`total`, `timestamp`, `hasMore`, `appId`) may sit alongside the collection key since they describe the result, not the domain objects. + +### Choosing the domain key name + +| Scenario | Key | Example | +|----------|-----|---------| +| Single event | Singular noun matching the SDK type | `message`, `cursor`, `member`, `lock` | +| Single result (create/get) | Singular noun | `key`, `app`, `rule`, `lock` | +| Collection result (list/get-all) | Plural noun | `keys`, `apps`, `rules`, `cursors` | + +The key name should match the SDK/domain terminology, not be generic. Use `message` not `data`, `cursor` not `item`. diff --git a/.claude/skills/ably-review/SKILL.md b/.claude/skills/ably-review/SKILL.md index 3826fb17b..d7ba5f346 100644 --- a/.claude/skills/ably-review/SKILL.md +++ b/.claude/skills/ably-review/SKILL.md @@ -98,6 +98,14 @@ For each changed command file, run the relevant checks. Spawn agents for paralle 4. **Grep** for `formatSuccess(` and check lines end with `.` 5. **Read** the file and look for unguarded `this.log()` calls (not inside `if (!this.shouldOutputJson(flags))`) 6. Look for quoted resource names instead of `formatResource(name)` +7. **Grep** for box-drawing characters (`┌`, `┬`, `├`, `└`, `─.*─`, `│`) — non-JSON output must use multi-line labeled blocks, not ASCII tables or grids +8. **Read** the file and check that non-JSON data output uses `formatLabel()` for field labels in multi-line blocks, not inline or single-line formatting +9. **Check** subscribe commands do NOT fetch initial state (no `getAll()` or equivalent before subscribing) — subscribe should only listen for new events + +**Field completeness check (read — for data-outputting commands):** +1. **Read** the JSON output path and compare fields emitted vs the non-JSON output path — non-JSON should expose the same fields as JSON mode (omitting only null/empty values) +2. **Check** that available SDK fields (e.g., `connectionId`, `clientId`, `isConnected`, `profileData`, `lastEvent`) are shown in non-JSON output, not just in JSON mode +3. **Grep** for local `interface` definitions in command files that duplicate SDK types (e.g., `interface CursorPosition`, `interface CursorData`) — these should import from `ably`, `@ably/spaces`, or `@ably/chat` instead. Display/output interfaces in `src/utils/` are fine. **Flag architecture check (grep, with LSP for ambiguous cases):** 1. **Grep** for flag spreads (`productApiFlags`, `clientIdFlag`, `durationFlag`, `rewindFlag`, `timeRangeFlags`, `ControlBaseCommand.globalFlags`) @@ -110,6 +118,7 @@ For each changed command file, run the relevant checks. Spawn agents for paralle 2. **Grep** for `formatJsonRecord` — direct usage should be flagged as needing migration 3. **Grep** for `shouldOutputJson` — verify human output is guarded 4. **Read** the file to verify streaming commands use `logJsonEvent` and one-shot commands use `logJsonResult` +5. **Read** `logJsonResult`/`logJsonEvent` call sites and check data is nested under a domain key (singular for events/single items, plural for collections) — not spread at top level. Top-level envelope fields are `type`, `command`, `success` only. Metadata like `total`, `timestamp`, `appId` may sit alongside the domain key. **Control API helper check (grep — for Control API commands only):** 1. **Grep** for `resolveAppId` — should use `requireAppId` instead (encapsulates null check and `fail()`) diff --git a/AGENTS.md b/AGENTS.md index 7d3feedfa..608f65faa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -219,9 +219,34 @@ All output helpers use the `format` prefix and are exported from `src/utils/outp - **Count labels**: `formatCountLabel(n, "message")` — cyan count + pluralized label. - **Limit warnings**: `formatLimitWarning(count, limit, "items")` — yellow warning if results truncated. - **JSON guard**: All human-readable output (progress, success, listening messages) must be wrapped in `if (!this.shouldOutputJson(flags))` so it doesn't pollute `--json` output. Only JSON payloads should be emitted when `--json` is active. -- **JSON envelope**: Use `this.logJsonResult(data, flags)` for one-shot results and `this.logJsonEvent(data, flags)` for streaming events. These are shorthand for `this.log(this.formatJsonRecord("result"|"event", data, flags))`. The envelope wraps data in `{type, command, success?, ...data}`. Do NOT add ad-hoc `success: true/false` — the envelope handles it. `--json` produces compact single-line output (NDJSON for streaming). `--pretty-json` is unchanged. +- **JSON envelope**: Use `this.logJsonResult(data, flags)` for one-shot results and `this.logJsonEvent(data, flags)` for streaming events. The envelope adds three top-level fields (`type`, `command`, `success?`). Nest domain data under a **domain key** (see "JSON data nesting convention" below). Do NOT add ad-hoc `success: true/false` — the envelope handles it. `--json` produces compact single-line output (NDJSON for streaming). `--pretty-json` is unchanged. - **JSON errors**: Use `this.fail(error, flags, component, context?)` as the single error funnel in command `run()` methods. It logs the CLI event, preserves structured error data (Ably codes, HTTP status), emits JSON error envelope when `--json` is active, and calls `this.error()` for human-readable output. Returns `never` — no `return;` needed after calling it. Do NOT call `this.error()` directly — it is an internal implementation detail of `fail`. -- **History output**: Use `[index] timestamp` ordering: `` `${formatIndex(index + 1)} ${formatTimestamp(timestamp)}` ``. Consistent across all history commands (channels, logs, connection-lifecycle, push). +- **History output**: Use `[index] [timestamp]` on the same line as a heading: `` `${formatIndex(index + 1)} ${formatTimestamp(timestamp)}` ``, then fields indented below. This is distinct from **get-all output** which uses `[index]` alone on its own line. See `references/patterns.md` "History results" and "One-shot results" for both patterns. + +### Structured output format (non-JSON) + +All non-JSON output for data records must use **multi-line labeled blocks** — one block per record, separated by blank lines. Never use ASCII tables (`┌─┬─┐`, `│`, box-drawing characters) or custom grid layouts. Non-JSON output must expose the same fields as JSON output (omit only null/undefined/empty values). Use `formatLabel()` for field names, type-appropriate formatters for values (`formatClientId`, `formatResource`, `formatEventType`, `formatTimestamp`). Check SDK type definitions (see "Ably Knowledge" below) as the source of truth for available fields — import SDK types directly, never redefine them locally. See `references/patterns.md` "Human-Readable Output Format" in the `ably-new-command` skill for detailed examples. + +### JSON data nesting convention + +The envelope provides three top-level fields: `type`, `command`, and `success`. All domain data must be nested under a **domain key** — never spread raw data fields at the top level alongside envelope fields. + +- **Events and single results**: nest under a **singular** domain key (`message`, `cursor`, `lock`) +- **Collection results**: nest under a **plural** domain key (`cursors`, `rules`, `keys`) +- **Metadata** (`total`, `timestamp`, `hasMore`, `appId`) may sit alongside the domain key + +See `references/patterns.md` "JSON Data Nesting Convention" in the `ably-new-command` skill for detailed examples and domain key naming. + +### Command behavior semantics + +Each command type has strict rules about what side effects it may have. Remove unintended side effects (e.g., auto-entering presence) and support passive ("dumb") operations where applicable. Key principles: +- **Subscribe** = passive observer (no `space.enter()`, no fetching initial state) +- **Get-all / get** = one-shot query (no `space.enter()`, no subscribing) +- **Set** = one-shot mutation (enter, set, exit — no subscribing after) +- **Enter / acquire** = hold state until Ctrl+C / `--duration` +- Call `space.enter()` only when SDK requires it; always call `this.markAsEntered()` after + +See `references/patterns.md` "Command behavior semantics" in the `ably-new-command` skill for full rules, side-effect table, and code examples. ### Error handling architecture @@ -288,6 +313,7 @@ When adding COMMANDS sections in `src/help.ts`, use `chalk.bold()` for headers, - Platform: https://ably.com/docs/platform - The CLI uses Ably SDKs for all data plane commands. When an API exists in the data plane REST API but has no corresponding SDK method, use the Pub/Sub SDK's request method. - The Control API has no official SDK, so raw HTTP requests are used. +- **SDK packages (`node_modules/ably/`, `node_modules/@ably/spaces/`, `node_modules/@ably/chat/`) are the local source of truth** for types and method behavior. Type definitions (e.g., `ably.d.ts`, `types.d.ts`) tell you what fields exist; source code (e.g., `Space.js`, `Members.js`) tells you how methods behave (side effects, prerequisites like `space.enter()`). When in doubt, read the implementation — not just the types. See `references/patterns.md` "Field display rules" in the `ably-new-command` skill for the full path table and import conventions. ## Development Standards diff --git a/README.md b/README.md index 9c27fc824..d31488286 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ $ npm install -g @ably/cli $ ably COMMAND running command... $ ably (--version) -@ably/cli/0.17.0 darwin-arm64 node-v24.4.1 +@ably/cli/0.17.0 darwin-arm64 node-v25.3.0 $ ably --help [COMMAND] USAGE $ ably COMMAND @@ -177,10 +177,12 @@ $ ably-interactive * [`ably rooms typing keystroke ROOM`](#ably-rooms-typing-keystroke-room) * [`ably rooms typing subscribe ROOM`](#ably-rooms-typing-subscribe-room) * [`ably spaces`](#ably-spaces) +* [`ably spaces create SPACE`](#ably-spaces-create-space) * [`ably spaces cursors`](#ably-spaces-cursors) * [`ably spaces cursors get-all SPACE`](#ably-spaces-cursors-get-all-space) * [`ably spaces cursors set SPACE`](#ably-spaces-cursors-set-space) * [`ably spaces cursors subscribe SPACE`](#ably-spaces-cursors-subscribe-space) +* [`ably spaces get SPACE`](#ably-spaces-get-space) * [`ably spaces list`](#ably-spaces-list) * [`ably spaces locations`](#ably-spaces-locations) * [`ably spaces locations get-all SPACE`](#ably-spaces-locations-get-all-space) @@ -193,7 +195,9 @@ $ ably-interactive * [`ably spaces locks subscribe SPACE`](#ably-spaces-locks-subscribe-space) * [`ably spaces members`](#ably-spaces-members) * [`ably spaces members enter SPACE`](#ably-spaces-members-enter-space) +* [`ably spaces members get-all SPACE`](#ably-spaces-members-get-all-space) * [`ably spaces members subscribe SPACE`](#ably-spaces-members-subscribe-space) +* [`ably spaces subscribe SPACE`](#ably-spaces-subscribe-space) * [`ably stats`](#ably-stats) * [`ably stats account`](#ably-stats-account) * [`ably stats app [ID]`](#ably-stats-app-id) @@ -1421,8 +1425,8 @@ Delete an annotation from a channel message ``` USAGE - $ ably channels annotations delete CHANNEL SERIAL TYPE [-v] [--json | --pretty-json] [--client-id ] [--count ] - [-n ] + $ ably channels annotations delete CHANNEL SERIAL TYPE [-v] [--json | --pretty-json] [--client-id ] [-n + ] ARGUMENTS CHANNEL The channel name @@ -1434,7 +1438,6 @@ 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. - --count= The annotation count (for multiple.v1 types) --json Output in JSON format --pretty-json Output in colorized JSON format @@ -1444,7 +1447,7 @@ DESCRIPTION EXAMPLES $ ably channels annotations delete my-channel "01234567890:0" "reactions:flag.v1" --name thumbsup - $ ably channels annotations delete my-channel "01234567890:0" "reactions:multiple.v1" --name thumbsup --count 2 + $ ably channels annotations delete my-channel "01234567890:0" "reactions:multiple.v1" --name thumbsup $ ably channels annotations delete my-channel "01234567890:0" "reactions:flag.v1" --json @@ -1468,7 +1471,7 @@ ARGUMENTS FLAGS -v, --verbose Output verbose logs --json Output in JSON format - --limit= [default: 50] Maximum number of results to return (default: 50) + --limit= [default: 100] Maximum number of results to return (default: 100) --pretty-json Output in colorized JSON format DESCRIPTION @@ -3691,20 +3694,60 @@ DESCRIPTION EXAMPLES $ ably spaces list + $ ably spaces get my-space + + $ ably spaces create my-space + + $ ably spaces subscribe my-space + $ ably spaces members enter my-space $ ably spaces locations set my-space COMMANDS + ably spaces create Create a new space ably spaces cursors Commands for interacting with Cursors in Ably Spaces + ably spaces get Get the current state of a space ably spaces list List active spaces ably spaces locations Commands for location management in Ably Spaces ably spaces locks Commands for component locking in Ably Spaces ably spaces members Commands for managing members in Ably Spaces + ably spaces subscribe Subscribe to space update events ``` _See code: [src/commands/spaces/index.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/index.ts)_ +## `ably spaces create SPACE` + +Create a new space + +``` +USAGE + $ ably spaces create SPACE [-v] [--json | --pretty-json] [--client-id ] + +ARGUMENTS + SPACE Space to create + +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. + --json Output in JSON format + --pretty-json Output in colorized JSON format + +DESCRIPTION + Create a new space + +EXAMPLES + $ ably spaces create my-space + + $ ably spaces create my-space --json + + $ ably spaces create my-space --pretty-json +``` + +_See code: [src/commands/spaces/create.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/create.ts)_ + ## `ably spaces cursors` Commands for interacting with Cursors in Ably Spaces @@ -3840,6 +3883,35 @@ EXAMPLES _See code: [src/commands/spaces/cursors/subscribe.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/cursors/subscribe.ts)_ +## `ably spaces get SPACE` + +Get the current state of a space + +``` +USAGE + $ ably spaces get SPACE [-v] [--json | --pretty-json] + +ARGUMENTS + SPACE Space to get + +FLAGS + -v, --verbose Output verbose logs + --json Output in JSON format + --pretty-json Output in colorized JSON format + +DESCRIPTION + Get the current state of a space + +EXAMPLES + $ ably spaces get my-space + + $ ably spaces get my-space --json + + $ ably spaces get my-space --pretty-json +``` + +_See code: [src/commands/spaces/get.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/get.ts)_ + ## `ably spaces list` List active spaces @@ -3930,13 +4002,12 @@ Set your location in a space ``` USAGE - $ ably spaces locations set SPACE --location [-v] [--json | --pretty-json] [--client-id ] [-D ] + $ ably spaces locations set SPACE --location [-v] [--json | --pretty-json] [--client-id ] ARGUMENTS SPACE Space to set location in FLAGS - -D, --duration= Automatically exit after N seconds -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. @@ -4202,6 +4273,37 @@ EXAMPLES _See code: [src/commands/spaces/members/enter.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/members/enter.ts)_ +## `ably spaces members get-all SPACE` + +Get all members in a space + +``` +USAGE + $ ably spaces members get-all SPACE [-v] [--json | --pretty-json] [--client-id ] + +ARGUMENTS + SPACE Space to get members from + +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. + --json Output in JSON format + --pretty-json Output in colorized JSON format + +DESCRIPTION + Get all members in a space + +EXAMPLES + $ ably spaces members get-all my-space + + $ ably spaces members get-all my-space --json + + $ ably spaces members get-all my-space --pretty-json +``` + +_See code: [src/commands/spaces/members/get-all.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/members/get-all.ts)_ + ## `ably spaces members subscribe SPACE` Subscribe to member presence events in a space @@ -4236,6 +4338,40 @@ EXAMPLES _See code: [src/commands/spaces/members/subscribe.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/members/subscribe.ts)_ +## `ably spaces subscribe SPACE` + +Subscribe to space update events + +``` +USAGE + $ ably spaces subscribe SPACE [-v] [--json | --pretty-json] [--client-id ] [-D ] + +ARGUMENTS + SPACE Space to subscribe to + +FLAGS + -D, --duration= Automatically exit after N seconds + -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. + --json Output in JSON format + --pretty-json Output in colorized JSON format + +DESCRIPTION + Subscribe to space update events + +EXAMPLES + $ ably spaces subscribe my-space + + $ ably spaces subscribe my-space --json + + $ ably spaces subscribe my-space --pretty-json + + $ ably spaces subscribe my-space --duration 30 +``` + +_See code: [src/commands/spaces/subscribe.ts](https://github.com/ably/ably-cli/blob/v0.17.0/src/commands/spaces/subscribe.ts)_ + ## `ably stats` View statistics for your Ably account or apps diff --git a/src/commands/spaces/create.ts b/src/commands/spaces/create.ts new file mode 100644 index 000000000..d727374bf --- /dev/null +++ b/src/commands/spaces/create.ts @@ -0,0 +1,70 @@ +import { Args } from "@oclif/core"; + +import { productApiFlags, clientIdFlag } from "../../flags.js"; +import { SpacesBaseCommand } from "../../spaces-base-command.js"; +import { + formatProgress, + formatResource, + formatSuccess, +} from "../../utils/output.js"; + +export default class SpacesCreate extends SpacesBaseCommand { + static override args = { + space: Args.string({ + description: "Space to create", + required: true, + }), + }; + + static override description = "Create a new space"; + + static override examples = [ + "$ ably spaces create my-space", + "$ ably spaces create my-space --json", + "$ ably spaces create my-space --pretty-json", + ]; + + static override flags = { + ...productApiFlags, + ...clientIdFlag, + }; + + async run(): Promise { + const { args, flags } = await this.parse(SpacesCreate); + const { space: spaceName } = args; + + try { + if (!this.shouldOutputJson(flags)) { + this.log(formatProgress(`Creating space ${formatResource(spaceName)}`)); + } + + await this.initializeSpace(flags, spaceName, { + enterSpace: false, + setupConnectionLogging: false, + }); + + this.logCliEvent( + flags, + "space", + "created", + "Space created successfully", + { spaceName }, + ); + + if (this.shouldOutputJson(flags)) { + this.logJsonResult( + { + space: { + name: spaceName, + }, + }, + flags, + ); + } else { + this.log(formatSuccess(`Space ${formatResource(spaceName)} created.`)); + } + } catch (error) { + this.fail(error, flags, "spaceCreate", { spaceName }); + } + } +} diff --git a/src/commands/spaces/cursors/get-all.ts b/src/commands/spaces/cursors/get-all.ts index 0e2d1592f..419885803 100644 --- a/src/commands/spaces/cursors/get-all.ts +++ b/src/commands/spaces/cursors/get-all.ts @@ -1,27 +1,20 @@ +import { type CursorUpdate } from "@ably/spaces"; import { Args } from "@oclif/core"; -import chalk from "chalk"; import { productApiFlags, clientIdFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; -import isTestMode from "../../../utils/test-mode.js"; import { + formatCountLabel, + formatHeading, + formatIndex, formatProgress, - formatSuccess, formatResource, - formatClientId, + formatWarning, } from "../../../utils/output.js"; - -interface CursorPosition { - x: number; - y: number; -} - -interface CursorUpdate { - clientId?: string; - connectionId?: string; - data?: Record; - position: CursorPosition; -} +import { + formatCursorBlock, + formatCursorOutput, +} from "../../../utils/spaces-output.js"; export default class SpacesCursorsGetAll extends SpacesBaseCommand { static override args = { @@ -54,310 +47,39 @@ export default class SpacesCursorsGetAll extends SpacesBaseCommand { setupConnectionLogging: false, }); - // Get the space if (!this.shouldOutputJson(flags)) { this.log( - formatProgress(`Connecting to space: ${formatResource(spaceName)}`), + formatProgress( + `Fetching cursors for space ${formatResource(spaceName)}`, + ), ); } - // Enter the space - await this.space!.enter(); - - // Wait for space to be properly entered before fetching cursors - await new Promise((resolve, reject) => { - // Set a reasonable timeout to avoid hanging indefinitely - const timeout = setTimeout(() => { - reject(new Error("Timed out waiting for space connection")); - }, 5000); - - const checkSpaceStatus = () => { - try { - // Check realtime client state - if (this.realtimeClient!.connection.state === "connected") { - clearTimeout(timeout); - if (this.shouldOutputJson(flags)) { - this.logJsonResult( - { - connectionId: this.realtimeClient!.connection.id, - spaceName, - status: "connected", - }, - flags, - ); - } else { - this.log( - formatSuccess(`Entered space: ${formatResource(spaceName)}.`), - ); - } - - resolve(); - } else if ( - this.realtimeClient!.connection.state === "failed" || - this.realtimeClient!.connection.state === "closed" || - this.realtimeClient!.connection.state === "suspended" - ) { - clearTimeout(timeout); - reject( - new Error( - `Space connection failed with state: ${this.realtimeClient!.connection.state}`, - ), - ); - } else { - // Still connecting, check again shortly - setTimeout(checkSpaceStatus, 100); - } - } catch (error) { - clearTimeout(timeout); - reject(error); - } - }; - - checkSpaceStatus(); - }); - - // Subscribe to cursor updates to ensure we receive remote cursors - let cursorUpdateReceived = false; - const cursorMap = new Map(); - - // Show initial message - if (!this.shouldOutputJson(flags)) { - const waitSeconds = isTestMode() ? "0.5" : "5"; - this.log(`Collecting cursor positions for ${waitSeconds} seconds...`); - this.log(chalk.dim("─".repeat(60))); - } - - const cursorUpdateHandler = (cursor: CursorUpdate) => { - cursorUpdateReceived = true; - - // Update the cursor map - if (cursor.connectionId) { - cursorMap.set(cursor.connectionId, cursor); - - // Show live cursor position updates - if ( - !this.shouldOutputJson(flags) && - this.shouldUseTerminalUpdates() - ) { - const clientDisplay = cursor.clientId || "Unknown"; - const x = cursor.position.x; - const y = cursor.position.y; - - this.log( - `${chalk.gray("►")} ${formatClientId(clientDisplay)}: (${chalk.yellow(x)}, ${chalk.yellow(y)})`, - ); - } - } - }; - - try { - await this.space!.cursors.subscribe("update", cursorUpdateHandler); - } catch (error) { - // If subscription fails, continue anyway - if (!this.shouldOutputJson(flags)) { - this.debug(`Cursor subscription error: ${error}`); - } - } - - // Wait for 5 seconds (or shorter in test mode) - const waitTime = isTestMode() ? 500 : 5000; - await new Promise((resolve) => { - setTimeout(() => { - resolve(); - }, waitTime); - }); - - // Unsubscribe from cursor updates - this.space!.cursors.unsubscribe("update", cursorUpdateHandler); - - // Ensure connection is stable before calling getAll() - if (this.realtimeClient!.connection.state !== "connected") { - await new Promise((resolve, reject) => { - const timeout = setTimeout(() => { - reject(new Error("Timed out waiting for connection to stabilize")); - }, 5000); - - this.realtimeClient!.connection.once("connected", () => { - clearTimeout(timeout); - resolve(); - }); - - if (this.realtimeClient!.connection.state === "connected") { - clearTimeout(timeout); - resolve(); - } - }); - } - - // Now get all cursors (including locally cached ones) and merge with live updates - try { - const allCursors = await this.space!.cursors.getAll(); - - // Add any cached cursors that we didn't see in live updates - if (Array.isArray(allCursors)) { - allCursors.forEach((cursor) => { - if ( - cursor && - cursor.connectionId && - !cursorMap.has(cursor.connectionId) - ) { - cursorMap.set(cursor.connectionId, cursor as CursorUpdate); - } - }); - } else if (allCursors && typeof allCursors === "object") { - // Handle object return type - Object.values(allCursors).forEach((cursor) => { - if ( - cursor && - cursor.connectionId && - !cursorMap.has(cursor.connectionId) - ) { - cursorMap.set(cursor.connectionId, cursor as CursorUpdate); - } - }); - } - } catch { - // If getAll fails due to connection issues, use only the live updates we collected - if (!this.shouldOutputJson(flags)) { - this.log( - chalk.yellow( - "Warning: Could not fetch all cursors, showing only live updates", - ), - ); - } - } + const allCursors = await this.space!.cursors.getAll(); - const cursors = [...cursorMap.values()]; + const cursors: CursorUpdate[] = Object.values(allCursors).filter( + (cursor): cursor is CursorUpdate => cursor != null, + ); if (this.shouldOutputJson(flags)) { this.logJsonResult( { - cursors: cursors.map((cursor: CursorUpdate) => ({ - clientId: cursor.clientId, - connectionId: cursor.connectionId, - data: cursor.data, - position: cursor.position, - })), - spaceName, - cursorUpdateReceived, + cursors: cursors.map((cursor) => formatCursorOutput(cursor)), }, flags, ); + } else if (cursors.length === 0) { + this.log(formatWarning("No active cursors found in space.")); } else { - if (!cursorUpdateReceived && cursors.length === 0) { - this.log(chalk.dim("─".repeat(60))); - this.log( - chalk.yellow( - "No cursor updates are being sent in this space. Make sure other clients are actively setting cursor positions.", - ), - ); - return; - } - - if (cursors.length === 0) { - this.log(chalk.dim("─".repeat(60))); - this.log(chalk.yellow("No active cursors found in space.")); - return; - } - - // Show summary table - this.log(chalk.dim("─".repeat(60))); this.log( - chalk.bold( - `\nCursor Summary - ${cursors.length} cursor${cursors.length === 1 ? "" : "s"} found:\n`, - ), + `\n${formatHeading("Current cursors")} (${formatCountLabel(cursors.length, "cursor")}):\n`, ); - // Table header - const colWidths = { client: 20, x: 8, y: 8, connection: 20 }; - this.log( - chalk.gray( - "┌" + - "─".repeat(colWidths.client + 2) + - "┬" + - "─".repeat(colWidths.x + 2) + - "┬" + - "─".repeat(colWidths.y + 2) + - "┬" + - "─".repeat(colWidths.connection + 2) + - "┐", - ), - ); - this.log( - chalk.gray("│ ") + - chalk.bold("Client ID".padEnd(colWidths.client)) + - chalk.gray(" │ ") + - chalk.bold("X".padEnd(colWidths.x)) + - chalk.gray(" │ ") + - chalk.bold("Y".padEnd(colWidths.y)) + - chalk.gray(" │ ") + - chalk.bold("connection".padEnd(colWidths.connection)) + - chalk.gray(" │"), - ); - this.log( - chalk.gray( - "├" + - "─".repeat(colWidths.client + 2) + - "┼" + - "─".repeat(colWidths.x + 2) + - "┼" + - "─".repeat(colWidths.y + 2) + - "┼" + - "─".repeat(colWidths.connection + 2) + - "┤", - ), - ); - - // Table rows - cursors.forEach((cursor: CursorUpdate) => { - const clientId = (cursor.clientId || "Unknown").slice( - 0, - colWidths.client, - ); - const x = cursor.position.x.toString().slice(0, colWidths.x); - const y = cursor.position.y.toString().slice(0, colWidths.y); - const connectionId = (cursor.connectionId || "Unknown").slice( - 0, - colWidths.connection, - ); - - this.log( - chalk.gray("│ ") + - formatClientId(clientId.padEnd(colWidths.client)) + - chalk.gray(" │ ") + - chalk.yellow(x.padEnd(colWidths.x)) + - chalk.gray(" │ ") + - chalk.yellow(y.padEnd(colWidths.y)) + - chalk.gray(" │ ") + - chalk.dim(connectionId.padEnd(colWidths.connection)) + - chalk.gray(" │"), - ); + cursors.forEach((cursor: CursorUpdate, index: number) => { + this.log(`${formatIndex(index + 1)}`); + this.log(formatCursorBlock(cursor, { indent: " " })); + this.log(""); }); - - this.log( - chalk.gray( - "└" + - "─".repeat(colWidths.client + 2) + - "┴" + - "─".repeat(colWidths.x + 2) + - "┴" + - "─".repeat(colWidths.y + 2) + - "┴" + - "─".repeat(colWidths.connection + 2) + - "┘", - ), - ); - - // Show additional data if any cursor has it - const cursorsWithData = cursors.filter((c) => c.data); - if (cursorsWithData.length > 0) { - this.log(`\n${chalk.bold("Additional Data:")}`); - cursorsWithData.forEach((cursor: CursorUpdate) => { - this.log( - ` ${formatClientId(cursor.clientId || "Unknown")}: ${JSON.stringify(cursor.data)}`, - ); - }); - } } } catch (error) { this.fail(error, flags, "cursorGetAll", { spaceName }); diff --git a/src/commands/spaces/cursors/set.ts b/src/commands/spaces/cursors/set.ts index f8bcf3fc1..ce60f122c 100644 --- a/src/commands/spaces/cursors/set.ts +++ b/src/commands/spaces/cursors/set.ts @@ -1,6 +1,5 @@ +import type { CursorData, CursorPosition } from "@ably/spaces"; import { Args, Flags } from "@oclif/core"; -import chalk from "chalk"; - import { errorMessage } from "../../../utils/errors.js"; import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; @@ -12,18 +11,6 @@ import { formatLabel, } from "../../../utils/output.js"; -// Define cursor types based on Ably documentation -interface CursorPosition { - x: number; - y: number; -} - -interface CursorData { - [key: string]: unknown; -} - -// CursorUpdate interface no longer required in this file - export default class SpacesCursorsSet extends SpacesBaseCommand { static override args = { space: Args.string({ @@ -70,7 +57,6 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { private simulationIntervalId: NodeJS.Timeout | null = null; - // Override finally to ensure resources are cleaned up async finally(err: Error | undefined): Promise { if (this.simulationIntervalId) { clearInterval(this.simulationIntervalId); @@ -89,14 +75,12 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { let cursorData: Record; if (flags.simulate) { - // For simulate mode, use provided x/y or generate random starting position const startX = flags.x ?? Math.floor(Math.random() * 1000); const startY = flags.y ?? Math.floor(Math.random() * 1000); cursorData = { position: { x: startX, y: startY }, }; - // If --data is also provided with simulate, treat it as additional cursor data if (flags.data) { try { const additionalData = JSON.parse(flags.data); @@ -111,12 +95,10 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { } } } else if (flags.x !== undefined && flags.y !== undefined) { - // Use x & y flags cursorData = { position: { x: flags.x, y: flags.y }, }; - // If --data is also provided with x/y flags, treat it as additional cursor data if (flags.data) { try { const additionalData = JSON.parse(flags.data); @@ -131,7 +113,6 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { } } } else if (flags.data) { - // Use --data JSON format try { cursorData = JSON.parse(flags.data); } catch { @@ -143,7 +124,6 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { ); } - // Validate position when using --data if ( !cursorData.position || typeof (cursorData.position as Record).x !== @@ -191,30 +171,36 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { if (this.shouldOutputJson(flags)) { this.logJsonResult( { - cursor: cursorForOutput, - spaceName, + cursors: [ + { + clientId: this.realtimeClient!.auth.clientId, + connectionId: this.realtimeClient!.connection.id, + position: ( + cursorForOutput as { position: { x: number; y: number } } + ).position, + data: + (cursorForOutput as { data?: Record }) + .data ?? null, + }, + ], }, flags, ); } else { this.log( - formatSuccess( - `Set cursor in space ${formatResource(spaceName)} with data: ${chalk.blue(JSON.stringify(cursorForOutput))}.`, - ), + formatSuccess(`Set cursor in space ${formatResource(spaceName)}.`), ); + const lines: string[] = [ + `${formatLabel("Position X")} ${position.x}`, + `${formatLabel("Position Y")} ${position.y}`, + ]; + if (data) { + lines.push(`${formatLabel("Data")} ${JSON.stringify(data)}`); + } + this.log(lines.join("\n")); } - // Decide how long to remain connected - if (flags.duration === 0) { - // Give Ably a moment to propagate the cursor update before exiting so that - // subscribers in automated tests have a chance to receive the event. - await new Promise((resolve) => setTimeout(resolve, 600)); - - // In immediate exit mode, we don't keep the process alive beyond this. - this.exit(0); - } - - // Start simulation if requested + // In simulate mode, keep running with periodic cursor updates if (flags.simulate) { this.logCliEvent( flags, @@ -231,7 +217,6 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { this.simulationIntervalId = setInterval(async () => { try { - // Generate random position within reasonable bounds const simulatedX = Math.floor(Math.random() * 1000); const simulatedY = Math.floor(Math.random() * 800); @@ -266,29 +251,15 @@ export default class SpacesCursorsSet extends SpacesBaseCommand { ); } }, 250); - } - // Inform the user and wait until interrupted or timeout (if provided) - this.logCliEvent( - flags, - "cursor", - "waiting", - "Cursor set – waiting for further instructions", - { duration: flags.duration ?? "indefinite" }, - ); + if (!this.shouldOutputJson(flags)) { + this.log(formatListening("Simulating cursor movement.")); + } - if (!this.shouldOutputJson(flags)) { - this.log( - flags.duration - ? `Waiting ${flags.duration}s before exiting… Press Ctrl+C to exit sooner.` - : formatListening("Cursor set."), - ); + await this.waitAndTrackCleanup(flags, "cursor", flags.duration); } - await this.waitAndTrackCleanup(flags, "cursor", flags.duration); - - // After cleanup (handled in finally), ensure the process exits so user doesn't need multiple Ctrl-C - this.exit(0); + // Non-simulate mode: run() completes, finally() handles cleanup } catch (error) { this.fail(error, flags, "cursorSet", { spaceName }); } diff --git a/src/commands/spaces/cursors/subscribe.ts b/src/commands/spaces/cursors/subscribe.ts index b0aba2653..46a0be0a0 100644 --- a/src/commands/spaces/cursors/subscribe.ts +++ b/src/commands/spaces/cursors/subscribe.ts @@ -4,12 +4,13 @@ import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; import { formatListening, - formatResource, - formatSuccess, + formatProgress, formatTimestamp, - formatClientId, - formatLabel, } from "../../../utils/output.js"; +import { + formatCursorBlock, + formatCursorOutput, +} from "../../../utils/spaces-output.js"; export default class SpacesCursorsSubscribe extends SpacesBaseCommand { static override args = { @@ -41,9 +42,12 @@ export default class SpacesCursorsSubscribe extends SpacesBaseCommand { const { space: spaceName } = args; try { - await this.initializeSpace(flags, spaceName, { enterSpace: true }); + if (!this.shouldOutputJson(flags)) { + this.log(formatProgress("Subscribing to cursor updates")); + } + + await this.initializeSpace(flags, spaceName, { enterSpace: false }); - // Subscribe to cursor updates this.logCliEvent( flags, "cursor", @@ -52,39 +56,30 @@ export default class SpacesCursorsSubscribe extends SpacesBaseCommand { ); try { - // Define the listener function this.listener = (cursorUpdate: CursorUpdate) => { try { const timestamp = new Date().toISOString(); - const eventData = { - member: { - clientId: cursorUpdate.clientId, - connectionId: cursorUpdate.connectionId, - }, - position: cursorUpdate.position, - data: cursorUpdate.data, - spaceName, - timestamp, - eventType: "cursor_update", - }; this.logCliEvent( flags, "cursor", "updateReceived", "Cursor update received", - eventData, + { + clientId: cursorUpdate.clientId, + position: cursorUpdate.position, + timestamp, + }, ); if (this.shouldOutputJson(flags)) { - this.logJsonEvent(eventData, flags); - } else { - // Include data field in the output if present - const dataString = cursorUpdate.data - ? ` data: ${JSON.stringify(cursorUpdate.data)}` - : ""; - this.log( - `${formatTimestamp(timestamp)} ${formatClientId(cursorUpdate.clientId)} ${formatLabel("position")} ${JSON.stringify(cursorUpdate.position)}${dataString}`, + this.logJsonEvent( + { cursor: formatCursorOutput(cursorUpdate) }, + flags, ); + } else { + this.log(formatTimestamp(timestamp)); + this.log(formatCursorBlock(cursorUpdate)); + this.log(""); } } catch (error) { this.fail(error, flags, "cursorSubscribe", { @@ -93,10 +88,7 @@ export default class SpacesCursorsSubscribe extends SpacesBaseCommand { } }; - // Workaround for known SDK issue: cursors.subscribe() fails if the underlying ::$cursors channel is not attached await this.waitForCursorsChannelAttachment(flags); - - // Subscribe using the listener await this.space!.cursors.subscribe("update", this.listener); this.logCliEvent( @@ -111,32 +103,13 @@ export default class SpacesCursorsSubscribe extends SpacesBaseCommand { }); } - this.logCliEvent( - flags, - "cursor", - "listening", - "Listening for cursor updates...", - ); - - if (!this.shouldOutputJson(flags)) { - // Log the ready signal for E2E tests - this.log("Subscribing to cursor movements"); - } - - // Print success message if (!this.shouldOutputJson(flags)) { - this.log( - formatSuccess(`Subscribed to space: ${formatResource(spaceName)}.`), - ); - this.log(formatListening("Listening for cursor movements.")); + this.log(`\n${formatListening("Listening for cursor movements.")}\n`); } - // Wait until the user interrupts or the optional duration elapses await this.waitAndTrackCleanup(flags, "cursor", flags.duration); } catch (error) { this.fail(error, flags, "cursorSubscribe", { spaceName }); - } finally { - // Cleanup is now handled by base class finally() method } } } diff --git a/src/commands/spaces/get.ts b/src/commands/spaces/get.ts new file mode 100644 index 000000000..197905a14 --- /dev/null +++ b/src/commands/spaces/get.ts @@ -0,0 +1,147 @@ +import { Args } from "@oclif/core"; + +import { productApiFlags } from "../../flags.js"; +import { SpacesBaseCommand } from "../../spaces-base-command.js"; +import { + formatCountLabel, + formatHeading, + formatIndex, + formatProgress, + formatResource, +} from "../../utils/output.js"; +import { formatMemberBlock } from "../../utils/spaces-output.js"; +import type { MemberOutput } from "../../utils/spaces-output.js"; + +const SPACE_CHANNEL_TAG = "::$space"; + +export default class SpacesGet extends SpacesBaseCommand { + static override args = { + space: Args.string({ + description: "Space to get", + required: true, + }), + }; + + static override description = "Get the current state of a space"; + + static override examples = [ + "$ ably spaces get my-space", + "$ ably spaces get my-space --json", + "$ ably spaces get my-space --pretty-json", + ]; + + static override flags = { + ...productApiFlags, + }; + + async run(): Promise { + const { args, flags } = await this.parse(SpacesGet); + const { space: spaceName } = args; + + try { + if (!this.shouldOutputJson(flags)) { + this.log( + formatProgress( + `Fetching state for space ${formatResource(spaceName)}`, + ), + ); + } + + const rest = await this.createAblyRestClient(flags); + if (!rest) return; + + const channelName = `${spaceName}${SPACE_CHANNEL_TAG}`; + const presenceResponse = await rest.request( + "get", + `/channels/${encodeURIComponent(channelName)}/presence`, + 2, + {}, + ); + + if (presenceResponse.statusCode !== 200) { + this.fail( + `Failed to fetch space state: ${presenceResponse.statusCode}`, + flags, + "spaceGet", + { spaceName }, + ); + } + + const items = presenceResponse.items || []; + + if (items.length === 0) { + this.fail( + `Space ${spaceName} not found (no members currently present). Spaces only exist while members are present.`, + flags, + "spaceGet", + { spaceName }, + ); + } + + const members: MemberOutput[] = items.map( + (item: { + clientId: string; + connectionId: string; + action: string; + timestamp: number; + data?: { + profileUpdate?: { current?: Record }; + locationUpdate?: { current?: unknown }; + }; + }) => ({ + clientId: item.clientId, + connectionId: item.connectionId, + isConnected: item.action !== "leave" && item.action !== "absent", + profileData: item.data?.profileUpdate?.current ?? null, + location: item.data?.locationUpdate?.current ?? null, + lastEvent: { + name: item.action, + timestamp: item.timestamp, + }, + }), + ); + + if (this.shouldOutputJson(flags)) { + this.logJsonResult( + { + space: { + name: spaceName, + members, + }, + }, + flags, + ); + } else { + this.log(`\n${formatHeading("Space:")} ${formatResource(spaceName)}\n`); + this.log( + `${formatHeading("Members")} (${formatCountLabel(members.length, "member")}):\n`, + ); + + for (let i = 0; i < members.length; i++) { + const member = members[i]; + this.log(`${formatIndex(i + 1)}`); + // Use manual formatting since we have MemberOutput, not SpaceMember + this.log( + formatMemberBlock( + { + clientId: member.clientId, + connectionId: member.connectionId, + isConnected: member.isConnected, + profileData: member.profileData as Record, + location: member.location, + lastEvent: member.lastEvent as { + name: string; + timestamp: number; + }, + } as import("@ably/spaces").SpaceMember, + { indent: " " }, + ), + ); + this.log(""); + } + } + } catch (error) { + this.fail(error, flags, "spaceGet", { spaceName }); + } + } +} diff --git a/src/commands/spaces/index.ts b/src/commands/spaces/index.ts index 5acb0807b..b9b771dc9 100644 --- a/src/commands/spaces/index.ts +++ b/src/commands/spaces/index.ts @@ -8,6 +8,9 @@ export default class SpacesIndex extends BaseTopicCommand { static override examples = [ "<%= config.bin %> <%= command.id %> list", + "<%= config.bin %> <%= command.id %> get my-space", + "<%= config.bin %> <%= command.id %> create my-space", + "<%= config.bin %> <%= command.id %> subscribe my-space", "<%= config.bin %> <%= command.id %> members enter my-space", "<%= config.bin %> <%= command.id %> locations set my-space", ]; diff --git a/src/commands/spaces/list.ts b/src/commands/spaces/list.ts index 3c1b2e3ad..cf6201111 100644 --- a/src/commands/spaces/list.ts +++ b/src/commands/spaces/list.ts @@ -2,30 +2,14 @@ import { Flags } from "@oclif/core"; import { productApiFlags } from "../../flags.js"; import { - formatLabel, formatCountLabel, formatLimitWarning, formatResource, } from "../../utils/output.js"; import { SpacesBaseCommand } from "../../spaces-base-command.js"; -interface SpaceMetrics { - connections?: number; - presenceConnections?: number; - presenceMembers?: number; - publishers?: number; - subscribers?: number; -} - -interface SpaceStatus { - occupancy?: { - metrics?: SpaceMetrics; - }; -} - interface SpaceItem { spaceName: string; - status?: SpaceStatus; channelId?: string; [key: string]: unknown; } @@ -61,8 +45,6 @@ export default class SpacesList extends SpacesBaseCommand { const rest = await this.createAblyRestClient(flags); if (!rest) return; - // Build params for channel listing - // We request more channels than the limit to account for filtering interface ChannelParams { limit: number; prefix?: string; @@ -76,7 +58,6 @@ export default class SpacesList extends SpacesBaseCommand { params.prefix = flags.prefix; } - // Fetch channels const channelsResponse = await rest.request( "get", "/channels", @@ -92,54 +73,35 @@ export default class SpacesList extends SpacesBaseCommand { ); } - // Filter to only include space channels const allChannels = channelsResponse.items || []; - - // Map to store deduplicated spaces const spaces = new Map(); - // Filter for space channels and deduplicate for (const channel of allChannels) { const { channelId } = channel; - // Check if this is a space channel (has ::$space suffix) if (channelId.includes("::$space")) { - // Extract the base space name (everything before the first ::$space) - // We need to escape the $ in the regex pattern since it's a special character const spaceNameMatch = channelId.match(/^(.+?)::\$space.*$/); if (spaceNameMatch && spaceNameMatch[1]) { const spaceName = spaceNameMatch[1]; - // Only add if we haven't seen this space before if (!spaces.has(spaceName)) { - // Store the original channel data but with the simple space name - const spaceData: SpaceItem = { - ...channel, + spaces.set(spaceName, { channelId: spaceName, spaceName, - }; - spaces.set(spaceName, spaceData); + }); } } } } - // Convert map to array const spacesList = [...spaces.values()]; - - // Limit the results to the requested number const limitedSpaces = spacesList.slice(0, flags.limit); if (this.shouldOutputJson(flags)) { this.logJsonResult( { - hasMore: spacesList.length > flags.limit, - shown: limitedSpaces.length, spaces: limitedSpaces.map((space: SpaceItem) => ({ - metrics: space.status?.occupancy?.metrics || {}, spaceName: space.spaceName, })), - timestamp: new Date().toISOString(), - total: spacesList.length, }, flags, ); @@ -150,39 +112,11 @@ export default class SpacesList extends SpacesBaseCommand { } this.log( - `Found ${formatCountLabel(limitedSpaces.length, "active space")}:`, + `Found ${formatCountLabel(limitedSpaces.length, "active space")}:\n`, ); limitedSpaces.forEach((space: SpaceItem) => { this.log(`${formatResource(space.spaceName)}`); - - // Show occupancy if available - if (space.status?.occupancy?.metrics) { - const { metrics } = space.status.occupancy; - this.log( - ` ${formatLabel("Connections")} ${metrics.connections || 0}`, - ); - this.log( - ` ${formatLabel("Publishers")} ${metrics.publishers || 0}`, - ); - this.log( - ` ${formatLabel("Subscribers")} ${metrics.subscribers || 0}`, - ); - - if (metrics.presenceConnections !== undefined) { - this.log( - ` ${formatLabel("Presence Connections")} ${metrics.presenceConnections}`, - ); - } - - if (metrics.presenceMembers !== undefined) { - this.log( - ` ${formatLabel("Presence Members")} ${metrics.presenceMembers}`, - ); - } - } - - this.log(""); // Add a line break between spaces }); const warning = formatLimitWarning( @@ -190,7 +124,7 @@ export default class SpacesList extends SpacesBaseCommand { flags.limit, "spaces", ); - if (warning) this.log(warning); + if (warning) this.log(`\n${warning}`); } } catch (error) { this.fail(error, flags, "spaceList"); diff --git a/src/commands/spaces/locations/get-all.ts b/src/commands/spaces/locations/get-all.ts index 4149dbe12..4b74007eb 100644 --- a/src/commands/spaces/locations/get-all.ts +++ b/src/commands/spaces/locations/get-all.ts @@ -1,48 +1,17 @@ import { Args } from "@oclif/core"; -import chalk from "chalk"; -import { errorMessage } from "../../../utils/errors.js"; import { productApiFlags, clientIdFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; import { - formatClientId, + formatCountLabel, formatHeading, + formatIndex, formatLabel, formatProgress, formatResource, - formatSuccess, + formatWarning, } from "../../../utils/output.js"; - -interface LocationData { - [key: string]: unknown; -} - -interface Member { - clientId?: string; - memberId?: string; - isCurrentMember?: boolean; -} - -interface LocationWithCurrent { - current: { - member: Member; - }; - location?: LocationData; - data?: LocationData; - [key: string]: unknown; -} - -interface LocationItem { - [key: string]: unknown; - clientId?: string; - connectionId?: string; - data?: LocationData; - id?: string; - location?: LocationData; - member?: Member; - memberId?: string; - userId?: string; -} +import type { LocationEntry } from "../../../utils/spaces-output.js"; export default class SpacesLocationsGetAll extends SpacesBaseCommand { static override args = { @@ -75,55 +44,6 @@ export default class SpacesLocationsGetAll extends SpacesBaseCommand { setupConnectionLogging: false, }); - if (!this.shouldOutputJson(flags)) { - this.log( - formatProgress(`Connecting to space: ${formatResource(spaceName)}`), - ); - } - - await this.space!.enter(); - - await new Promise((resolve, reject) => { - const timeout = setTimeout(() => { - reject(new Error("Timed out waiting for space connection")); - }, 5000); - - const checkSpaceStatus = () => { - try { - if (this.realtimeClient!.connection.state === "connected") { - clearTimeout(timeout); - if (!this.shouldOutputJson(flags)) { - this.log( - formatSuccess( - `Connected to space: ${formatResource(spaceName)}.`, - ), - ); - } - - resolve(); - } else if ( - this.realtimeClient!.connection.state === "failed" || - this.realtimeClient!.connection.state === "closed" || - this.realtimeClient!.connection.state === "suspended" - ) { - clearTimeout(timeout); - reject( - new Error( - `Space connection failed with connection state: ${this.realtimeClient!.connection.state}`, - ), - ); - } else { - setTimeout(checkSpaceStatus, 100); - } - } catch (error) { - clearTimeout(timeout); - reject(error); - } - }; - - checkSpaceStatus(); - }); - if (!this.shouldOutputJson(flags)) { this.log( formatProgress( @@ -132,139 +52,47 @@ export default class SpacesLocationsGetAll extends SpacesBaseCommand { ); } - let locations: LocationItem[] = []; try { const locationsFromSpace = await this.space!.locations.getAll(); - if (locationsFromSpace && typeof locationsFromSpace === "object") { - if (Array.isArray(locationsFromSpace)) { - locations = locationsFromSpace as LocationItem[]; - } else if (Object.keys(locationsFromSpace).length > 0) { - locations = Object.entries(locationsFromSpace).map( - ([memberId, locationData]) => ({ - location: locationData, - memberId, - }), - ) as LocationItem[]; - } - } - - const knownMetaKeys = new Set([ - "clientId", - "connectionId", - "current", - "id", - "member", - "memberId", - "userId", - ]); - - const extractLocationData = (item: LocationItem): unknown => { - if (item.location !== undefined) return item.location; - if (item.data !== undefined) return item.data; - const rest: Record = {}; - for (const [key, value] of Object.entries(item)) { - if (!knownMetaKeys.has(key)) { - rest[key] = value; - } - } - return Object.keys(rest).length > 0 ? rest : null; - }; - - const validLocations = locations.filter((item: LocationItem) => { - if (item === null || item === undefined) return false; - - const locationData = extractLocationData(item); - - if (locationData === null || locationData === undefined) return false; - if ( - typeof locationData === "object" && - Object.keys(locationData as object).length === 0 + const entries: LocationEntry[] = Object.entries(locationsFromSpace) + .filter( + ([, loc]) => + loc != null && + !( + typeof loc === "object" && + Object.keys(loc as object).length === 0 + ), ) - return false; - - return true; - }); + .map(([connectionId, loc]) => ({ connectionId, location: loc })); if (this.shouldOutputJson(flags)) { this.logJsonResult( { - locations: validLocations.map((item: LocationItem) => { - const currentMember = - "current" in item && - item.current && - typeof item.current === "object" - ? (item.current as LocationWithCurrent["current"]).member - : undefined; - const member = item.member || currentMember; - const memberId = - item.memberId || - member?.memberId || - member?.clientId || - item.clientId || - item.id || - item.userId || - "Unknown"; - const locationData = extractLocationData(item); - return { - isCurrentMember: member?.isCurrentMember || false, - location: locationData, - memberId, - }; - }), - spaceName, - timestamp: new Date().toISOString(), + locations: entries.map((entry) => ({ + connectionId: entry.connectionId, + location: entry.location, + })), }, flags, ); - } else if (!validLocations || validLocations.length === 0) { + } else if (entries.length === 0) { this.log( - chalk.yellow("No locations are currently set in this space."), + formatWarning("No locations are currently set in this space."), ); } else { - const locationsCount = validLocations.length; this.log( - `\n${formatHeading("Current locations")} (${chalk.bold(String(locationsCount))}):\n`, + `\n${formatHeading("Current locations")} (${formatCountLabel(entries.length, "location")}):\n`, ); - for (const location of validLocations) { - // Check if location has 'current' property with expected structure - if ( - "current" in location && - typeof location.current === "object" && - location.current !== null && - "member" in location.current - ) { - const locationWithCurrent = location as LocationWithCurrent; - const { member } = locationWithCurrent.current; - this.log( - `Member ID: ${formatResource(member.memberId || member.clientId || "Unknown")}`, - ); - try { - const locationData = extractLocationData(location); - - this.log( - `- ${formatClientId(member.memberId || member.clientId || "Unknown")}:`, - ); - this.log( - ` ${formatLabel("Location")} ${JSON.stringify(locationData, null, 2)}`, - ); - - if (member.isCurrentMember) { - this.log(` ${chalk.dim("(Current member)")}`); - } - } catch (error) { - this.log( - `- ${chalk.red("Error displaying location item")}: ${errorMessage(error)}`, - ); - } - } else { - // Simpler display if location doesn't have expected structure - this.log(`- ${formatClientId("Member")}:`); - this.log( - ` ${formatLabel("Location")} ${JSON.stringify(location, null, 2)}`, - ); - } + for (let i = 0; i < entries.length; i++) { + const entry = entries[i]; + this.log(`${formatIndex(i + 1)}`); + this.log(` ${formatLabel("Connection ID")} ${entry.connectionId}`); + this.log( + ` ${formatLabel("Location")} ${JSON.stringify(entry.location)}`, + ); + this.log(""); } } } catch (error) { diff --git a/src/commands/spaces/locations/set.ts b/src/commands/spaces/locations/set.ts index caaa7c9ed..680fd3415 100644 --- a/src/commands/spaces/locations/set.ts +++ b/src/commands/spaces/locations/set.ts @@ -1,22 +1,8 @@ -import type { LocationsEvents } from "@ably/spaces"; import { Args, Flags } from "@oclif/core"; -import chalk from "chalk"; -import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; +import { productApiFlags, clientIdFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; -import { - formatSuccess, - formatListening, - formatResource, - formatTimestamp, - formatClientId, - formatLabel, -} from "../../../utils/output.js"; - -// Define the type for location subscription -interface LocationSubscription { - unsubscribe: () => void; -} +import { formatSuccess, formatResource } from "../../../utils/output.js"; export default class SpacesLocationsSet extends SpacesBaseCommand { static override args = { @@ -41,24 +27,11 @@ export default class SpacesLocationsSet extends SpacesBaseCommand { description: "Location data to set (JSON format)", required: true, }), - ...durationFlag, }; - private subscription: LocationSubscription | null = null; - private locationHandler: - | ((locationUpdate: LocationsEvents.UpdateEvent) => void) - | null = null; - private isE2EMode = false; // Track if we're in E2E mode to skip cleanup - - // Override finally to ensure resources are cleaned up async finally(err: Error | undefined): Promise { - // For E2E tests with duration=0, skip all cleanup to avoid hanging - if (this.isE2EMode) { - return; - } - // Clear location before leaving space - if (this.space) { + if (this.space && this.hasEnteredSpace) { try { await Promise.race([ this.space.locations.set(null), @@ -86,74 +59,6 @@ export default class SpacesLocationsSet extends SpacesBaseCommand { { location }, ); - // Check if we should exit immediately (optimized path for E2E tests) - const shouldExitImmediately = - typeof flags.duration === "number" && flags.duration === 0; - - if (shouldExitImmediately) { - // Set E2E mode flag to skip cleanup in finally block - this.isE2EMode = true; - - // For E2E mode, suppress unhandled promise rejections from Ably SDK cleanup - const originalHandler = process.listeners("unhandledRejection"); - process.removeAllListeners("unhandledRejection"); - process.on("unhandledRejection", (reason, promise) => { - // Ignore connection-related errors during E2E test cleanup - const reasonStr = String(reason); - if ( - reasonStr.includes("Connection closed") || - reasonStr.includes("80017") - ) { - // Silently ignore these errors in E2E mode - return; - } - // Re-emit other errors to original handlers - originalHandler.forEach((handler) => { - if (typeof handler === "function") { - handler(reason, promise); - } - }); - }); - - // Optimized path for E2E tests - minimal setup and cleanup - try { - const setupResult = await this.setupSpacesClient(flags, spaceName); - this.realtimeClient = setupResult.realtimeClient; - this.space = setupResult.space; - - // Enter the space and set location - await this.space.enter(); - this.logCliEvent(flags, "spaces", "entered", "Entered space", { - clientId: this.realtimeClient.auth.clientId, - }); - - await this.space.locations.set(location); - this.logCliEvent(flags, "location", "setSuccess", "Set location", { - location, - }); - - if (this.shouldOutputJson(flags)) { - this.logJsonResult({ location, spaceName }, flags); - } else { - this.log( - formatSuccess( - `Location set in space: ${formatResource(spaceName)}.`, - ), - ); - } - } catch { - // If an error occurs in E2E mode, just exit cleanly after showing what we can - if (this.shouldOutputJson(flags)) { - this.logJsonResult({ location, spaceName }, flags); - } - // Don't call this.error() in E2E mode as it sets exit code to 1 - } - - // For E2E tests, force immediate exit regardless of any errors - this.exit(0); - } - - // Original path for interactive use try { await this.initializeSpace(flags, spaceName, { enterSpace: true }); @@ -165,102 +70,16 @@ export default class SpacesLocationsSet extends SpacesBaseCommand { this.logCliEvent(flags, "location", "setSuccess", "Set location", { location, }); - if (!this.shouldOutputJson(flags)) { - this.log( - formatSuccess(`Location set in space: ${formatResource(spaceName)}.`), - ); - } - // Subscribe to location updates from other users - this.logCliEvent( - flags, - "location", - "subscribing", - "Watching for other location changes...", - ); - if (!this.shouldOutputJson(flags)) { + if (this.shouldOutputJson(flags)) { + this.logJsonResult({ location }, flags); + } else { this.log( - `\n${formatListening("Watching for other location changes.")}\n`, + formatSuccess(`Location set in space: ${formatResource(spaceName)}.`), ); } - - // Store subscription handlers - this.locationHandler = (locationUpdate: LocationsEvents.UpdateEvent) => { - const timestamp = new Date().toISOString(); - const { member } = locationUpdate; - const { currentLocation } = locationUpdate; // Use current location - const { connectionId } = member; - - // Skip self events - check connection ID - const selfConnectionId = this.realtimeClient!.connection.id; - if (connectionId === selfConnectionId) { - return; - } - - const eventData = { - action: "update", - location: currentLocation, - member: { - clientId: member.clientId, - connectionId: member.connectionId, - }, - timestamp, - }; - this.logCliEvent( - flags, - "location", - "updateReceived", - "Location update received", - eventData, - ); - - if (this.shouldOutputJson(flags)) { - this.logJsonEvent(eventData, flags); - } else { - // For locations, use yellow for updates - const actionColor = chalk.yellow; - const action = "update"; - - this.log( - `${formatTimestamp(timestamp)} ${formatClientId(member.clientId || "Unknown")} ${actionColor(action)}d location:`, - ); - this.log( - ` ${formatLabel("Location")} ${JSON.stringify(currentLocation, null, 2)}`, - ); - } - }; - - // Subscribe to updates - this.space!.locations.subscribe("update", this.locationHandler); - this.subscription = { - unsubscribe: () => { - if (this.locationHandler && this.space) { - this.space.locations.unsubscribe("update", this.locationHandler); - this.locationHandler = null; - } - }, - }; - - this.logCliEvent( - flags, - "location", - "subscribed", - "Subscribed to location updates", - ); - - this.logCliEvent( - flags, - "location", - "listening", - "Listening for location updates...", - ); - - // Wait until the user interrupts or the optional duration elapses - await this.waitAndTrackCleanup(flags, "location", flags.duration); } catch (error) { this.fail(error, flags, "locationSet"); - } finally { - // Cleanup is now handled by base class finally() method } } } diff --git a/src/commands/spaces/locations/subscribe.ts b/src/commands/spaces/locations/subscribe.ts index 9ff8b31e5..5dd7f8743 100644 --- a/src/commands/spaces/locations/subscribe.ts +++ b/src/commands/spaces/locations/subscribe.ts @@ -1,39 +1,14 @@ import type { LocationsEvents } from "@ably/spaces"; import { Args } from "@oclif/core"; -import chalk from "chalk"; import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; import { - formatClientId, - formatEventType, - formatHeading, formatListening, formatProgress, - formatResource, - formatSuccess, formatTimestamp, - formatLabel, } from "../../../utils/output.js"; - -// Define interfaces for location types -interface SpaceMember { - clientId: string; - connectionId: string; - isConnected: boolean; - profileData: Record | null; -} - -interface LocationData { - [key: string]: unknown; -} - -interface LocationItem { - location: LocationData; - member: SpaceMember; -} - -// Define type for subscription +import { formatLocationUpdateBlock } from "../../../utils/spaces-output.js"; export default class SpacesLocationsSubscribe extends SpacesBaseCommand { static override args = { @@ -62,121 +37,16 @@ export default class SpacesLocationsSubscribe extends SpacesBaseCommand { async run(): Promise { const { args, flags } = await this.parse(SpacesLocationsSubscribe); const { space: spaceName } = args; - this.logCliEvent( - flags, - "subscribe.run", - "start", - `Starting spaces locations subscribe for space: ${spaceName}`, - ); try { - // Always show the readiness signal first, before attempting auth if (!this.shouldOutputJson(flags)) { - this.log("Subscribing to location updates"); + this.log(formatProgress("Subscribing to location updates")); } - this.logCliEvent( - flags, - "subscribe.run", - "initialSignalLogged", - "Initial readiness signal logged.", - ); - await this.initializeSpace(flags, spaceName, { enterSpace: true }); + await this.initializeSpace(flags, spaceName, { enterSpace: false }); - // Get current locations - this.logCliEvent( - flags, - "location", - "gettingInitial", - `Fetching initial locations for space ${spaceName}`, - ); if (!this.shouldOutputJson(flags)) { - this.log( - formatProgress( - `Fetching current locations for space ${formatResource(spaceName)}`, - ), - ); - } - - let locations: LocationItem[] = []; - try { - const result = await this.space!.locations.getAll(); - this.logCliEvent( - flags, - "location", - "gotInitial", - `Fetched initial locations`, - { locations: result }, - ); - - if (result && typeof result === "object") { - if (Array.isArray(result)) { - // Unlikely based on current docs, but handle if API changes - // Need to map Array result to LocationItem[] if structure differs - this.logCliEvent( - flags, - "location", - "initialFormatWarning", - "Received array format for initial locations, expected object", - ); - // Assuming array elements match expected structure for now: - locations = result.map( - (item: { location: LocationData; member: SpaceMember }) => ({ - location: item.location, - member: item.member, - }), - ); - } else if (Object.keys(result).length > 0) { - // Standard case: result is an object { connectionId: locationData } - locations = Object.entries(result).map( - ([connectionId, locationData]) => ({ - location: locationData as LocationData, - member: { - // Construct a partial SpaceMember as SDK doesn't provide full details here - clientId: "unknown", // clientId not directly available in getAll response - connectionId, - isConnected: true, // Assume connected for initial state - profileData: null, - }, - }), - ); - } - } - - if (this.shouldOutputJson(flags)) { - this.logJsonResult( - { - locations: locations.map((item) => ({ - // Map to a simpler structure for output if needed - connectionId: item.member.connectionId, - location: item.location, - })), - spaceName, - eventType: "locations_snapshot", - }, - flags, - ); - } else if (locations.length === 0) { - this.log( - chalk.yellow("No locations are currently set in this space."), - ); - } else { - this.log( - `\n${formatHeading("Current locations")} (${chalk.bold(locations.length.toString())}):\n`, - ); - for (const item of locations) { - this.log( - `- Connection ID: ${chalk.blue(item.member.connectionId || "Unknown")}`, - ); // Use connectionId as key - this.log( - ` ${formatLabel("Location")} ${JSON.stringify(item.location)}`, - ); - } - } - } catch (error) { - this.fail(error, flags, "locationSubscribe", { - spaceName, - }); + this.log(`\n${formatListening("Listening for location updates.")}\n`); } this.logCliEvent( @@ -185,58 +55,42 @@ export default class SpacesLocationsSubscribe extends SpacesBaseCommand { "subscribing", "Subscribing to location updates", ); - if (!this.shouldOutputJson(flags)) { - this.log(formatListening("Subscribing to location updates.")); - } - this.logCliEvent( - flags, - "location.subscribe", - "readySignalLogged", - "Final readiness signal 'Subscribing to location updates' logged.", - ); try { - // Define the location update handler const locationHandler = (update: LocationsEvents.UpdateEvent) => { try { const timestamp = new Date().toISOString(); - const eventData = { - action: "update", - location: update.currentLocation, - member: { - clientId: update.member.clientId, - connectionId: update.member.connectionId, - }, - previousLocation: update.previousLocation, - timestamp, - }; this.logCliEvent( flags, "location", "updateReceived", "Location update received", - { spaceName, ...eventData }, + { + clientId: update.member.clientId, + connectionId: update.member.connectionId, + timestamp, + }, ); if (this.shouldOutputJson(flags)) { this.logJsonEvent( { - spaceName, - eventType: "location_update", - ...eventData, + location: { + member: { + clientId: update.member.clientId, + connectionId: update.member.connectionId, + }, + currentLocation: update.currentLocation, + previousLocation: update.previousLocation, + timestamp, + }, }, flags, ); } else { - this.log( - `${formatTimestamp(timestamp)} ${formatClientId(update.member.clientId)} ${formatEventType("updated")} location:`, - ); - this.log( - ` ${formatLabel("Current")} ${JSON.stringify(update.currentLocation)}`, - ); - this.log( - ` ${formatLabel("Previous")} ${JSON.stringify(update.previousLocation)}`, - ); + this.log(formatTimestamp(timestamp)); + this.log(formatLocationUpdateBlock(update)); + this.log(""); } } catch (error) { this.fail(error, flags, "locationSubscribe", { @@ -245,7 +99,6 @@ export default class SpacesLocationsSubscribe extends SpacesBaseCommand { } }; - // Subscribe to location updates this.space!.locations.subscribe("update", locationHandler); this.logCliEvent( @@ -260,28 +113,9 @@ export default class SpacesLocationsSubscribe extends SpacesBaseCommand { }); } - this.logCliEvent( - flags, - "location", - "listening", - "Listening for location updates...", - ); - - // Wait until the user interrupts or the optional duration elapses await this.waitAndTrackCleanup(flags, "location", flags.duration); } catch (error) { this.fail(error, flags, "locationSubscribe", { spaceName }); - } finally { - // Wrap all cleanup in a timeout to prevent hanging - if (!this.shouldOutputJson(flags || {})) { - if (this.cleanupInProgress) { - this.log(formatSuccess("Graceful shutdown complete.")); - } else { - this.log( - formatSuccess("Duration elapsed, command finished cleanly."), - ); - } - } } } } diff --git a/src/commands/spaces/locks/acquire.ts b/src/commands/spaces/locks/acquire.ts index 146b97759..f03040d4e 100644 --- a/src/commands/spaces/locks/acquire.ts +++ b/src/commands/spaces/locks/acquire.ts @@ -8,8 +8,11 @@ import { formatSuccess, formatListening, formatResource, - formatLabel, } from "../../../utils/output.js"; +import { + formatLockBlock, + formatLockOutput, +} from "../../../utils/spaces-output.js"; export default class SpacesLocksAcquire extends SpacesBaseCommand { static override args = { @@ -92,6 +95,7 @@ export default class SpacesLocksAcquire extends SpacesBaseCommand { // Enter the space first this.logCliEvent(flags, "spaces", "entering", "Entering space..."); await this.space!.enter(); + this.markAsEntered(); this.logCliEvent(flags, "spaces", "entered", "Entered space", { clientId: this.realtimeClient!.auth.clientId, }); @@ -109,33 +113,19 @@ export default class SpacesLocksAcquire extends SpacesBaseCommand { lockId, lockData as LockOptions, ); - const lockDetails = { - lockId: lock.id, - member: lock.member - ? { - clientId: lock.member.clientId, - connectionId: lock.member.connectionId, - } - : null, - reason: lock.reason, - status: lock.status, - timestamp: lock.timestamp, - }; this.logCliEvent( flags, "lock", "acquired", `Lock acquired: ${lockId}`, - lockDetails, + { lockId: lock.id, status: lock.status }, ); if (this.shouldOutputJson(flags)) { - this.logJsonResult({ lock: lockDetails }, flags); + this.logJsonResult({ locks: [formatLockOutput(lock)] }, flags); } else { this.log(formatSuccess(`Lock acquired: ${formatResource(lockId)}.`)); - this.log( - `${formatLabel("Lock details")} ${this.formatJsonOutput(lockDetails, { ...flags, "pretty-json": true })}`, - ); + this.log(formatLockBlock(lock)); this.log(`\n${formatListening("Holding lock.")}`); } } catch (error) { diff --git a/src/commands/spaces/locks/get-all.ts b/src/commands/spaces/locks/get-all.ts index 1f0ccd097..c7e3e87d7 100644 --- a/src/commands/spaces/locks/get-all.ts +++ b/src/commands/spaces/locks/get-all.ts @@ -1,25 +1,20 @@ +import type { Lock } from "@ably/spaces"; import { Args } from "@oclif/core"; -import chalk from "chalk"; -import { errorMessage } from "../../../utils/errors.js"; import { productApiFlags, clientIdFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; import { + formatCountLabel, formatHeading, - formatLabel, + formatIndex, formatProgress, formatResource, - formatSuccess, + formatWarning, } from "../../../utils/output.js"; - -interface LockItem { - attributes?: Record; - id: string; - member?: { - clientId?: string; - }; - status?: string; -} +import { + formatLockBlock, + formatLockOutput, +} from "../../../utils/spaces-output.js"; export default class SpacesLocksGetAll extends SpacesBaseCommand { static override args = { @@ -52,58 +47,6 @@ export default class SpacesLocksGetAll extends SpacesBaseCommand { setupConnectionLogging: false, }); - // Get the space - if (!this.shouldOutputJson(flags)) { - this.log( - formatProgress(`Connecting to space: ${formatResource(spaceName)}`), - ); - } - - await this.space!.enter(); - - // Wait for space to be properly entered before fetching locks - await new Promise((resolve, reject) => { - const timeout = setTimeout(() => { - reject(new Error("Timed out waiting for space connection")); - }, 5000); - - const checkSpaceStatus = () => { - try { - if (this.realtimeClient!.connection.state === "connected") { - clearTimeout(timeout); - if (!this.shouldOutputJson(flags)) { - this.log( - formatSuccess( - `Connected to space: ${formatResource(spaceName)}.`, - ), - ); - } - - resolve(); - } else if ( - this.realtimeClient!.connection.state === "failed" || - this.realtimeClient!.connection.state === "closed" || - this.realtimeClient!.connection.state === "suspended" - ) { - clearTimeout(timeout); - reject( - new Error( - `Space connection failed with connection state: ${this.realtimeClient!.connection.state}`, - ), - ); - } else { - setTimeout(checkSpaceStatus, 100); - } - } catch (error) { - clearTimeout(timeout); - reject(error); - } - }; - - checkSpaceStatus(); - }); - - // Get all locks if (!this.shouldOutputJson(flags)) { this.log( formatProgress( @@ -112,56 +55,27 @@ export default class SpacesLocksGetAll extends SpacesBaseCommand { ); } - let locks: LockItem[] = []; - const result = await this.space!.locks.getAll(); - locks = Array.isArray(result) ? result : []; - - const validLocks = locks.filter((lock: LockItem) => { - if (!lock || !lock.id) return false; - return true; - }); + const locks: Lock[] = await this.space!.locks.getAll(); if (this.shouldOutputJson(flags)) { this.logJsonResult( { - locks: validLocks.map((lock) => ({ - attributes: lock.attributes || {}, - holder: lock.member?.clientId || null, - id: lock.id, - status: lock.status || "unknown", - })), - spaceName, - timestamp: new Date().toISOString(), + locks: locks.map((lock) => formatLockOutput(lock)), }, flags, ); - } else if (!validLocks || validLocks.length === 0) { - this.log(chalk.yellow("No locks are currently active in this space.")); + } else if (locks.length === 0) { + this.log(formatWarning("No locks are currently active in this space.")); } else { - const lockCount = validLocks.length; this.log( - `\n${formatHeading("Current locks")} (${chalk.bold(String(lockCount))}):\n`, + `\n${formatHeading("Current locks")} (${formatCountLabel(locks.length, "lock")}):\n`, ); - validLocks.forEach((lock: LockItem) => { - try { - this.log(`- ${formatResource(lock.id)}:`); - this.log(` ${formatLabel("Status")} ${lock.status || "unknown"}`); - this.log( - ` ${formatLabel("Holder")} ${lock.member?.clientId || "None"}`, - ); - - if (lock.attributes && Object.keys(lock.attributes).length > 0) { - this.log( - ` ${formatLabel("Attributes")} ${JSON.stringify(lock.attributes, null, 2)}`, - ); - } - } catch (error) { - this.log( - `- ${chalk.red("Error displaying lock item")}: ${errorMessage(error)}`, - ); - } - }); + for (let i = 0; i < locks.length; i++) { + this.log(`${formatIndex(i + 1)}`); + this.log(formatLockBlock(locks[i])); + this.log(""); + } } } catch (error) { this.fail(error, flags, "lockGetAll", { spaceName }); diff --git a/src/commands/spaces/locks/get.ts b/src/commands/spaces/locks/get.ts index 5148ba181..3c6bd9ed8 100644 --- a/src/commands/spaces/locks/get.ts +++ b/src/commands/spaces/locks/get.ts @@ -1,13 +1,12 @@ import { Args } from "@oclif/core"; -import chalk from "chalk"; import { productApiFlags, clientIdFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; +import { formatResource, formatWarning } from "../../../utils/output.js"; import { - formatLabel, - formatResource, - formatSuccess, -} from "../../../utils/output.js"; + formatLockBlock, + formatLockOutput, +} from "../../../utils/spaces-output.js"; export default class SpacesLocksGet extends SpacesBaseCommand { static override args = { @@ -45,21 +44,16 @@ export default class SpacesLocksGet extends SpacesBaseCommand { setupConnectionLogging: false, }); - await this.space!.enter(); - if (!this.shouldOutputJson(flags)) { - this.log(formatSuccess(`Entered space: ${formatResource(spaceName)}.`)); - } - try { const lock = await this.space!.locks.get(lockId); if (!lock) { if (this.shouldOutputJson(flags)) { - this.logJsonResult({ found: false, lockId }, flags); + this.logJsonResult({ locks: [] }, flags); } else { this.log( - chalk.yellow( - `Lock ${formatResource(lockId)} not found in space ${formatResource(spaceName)}`, + formatWarning( + `Lock ${formatResource(lockId)} not found in space ${formatResource(spaceName)}.`, ), ); } @@ -68,14 +62,9 @@ export default class SpacesLocksGet extends SpacesBaseCommand { } if (this.shouldOutputJson(flags)) { - this.logJsonResult( - structuredClone(lock) as Record, - flags, - ); + this.logJsonResult({ locks: [formatLockOutput(lock)] }, flags); } else { - this.log( - `${formatLabel("Lock details")} ${this.formatJsonOutput(structuredClone(lock), flags)}`, - ); + this.log(formatLockBlock(lock)); } } catch (error) { this.fail(error, flags, "lockGet"); diff --git a/src/commands/spaces/locks/subscribe.ts b/src/commands/spaces/locks/subscribe.ts index ef3035b51..fbcbbb4cd 100644 --- a/src/commands/spaces/locks/subscribe.ts +++ b/src/commands/spaces/locks/subscribe.ts @@ -1,17 +1,17 @@ import { type Lock } from "@ably/spaces"; import { Args } from "@oclif/core"; -import chalk from "chalk"; import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; import { - formatHeading, - formatLabel, formatListening, formatProgress, - formatResource, formatTimestamp, } from "../../../utils/output.js"; +import { + formatLockBlock, + formatLockOutput, +} from "../../../utils/spaces-output.js"; export default class SpacesLocksSubscribe extends SpacesBaseCommand { static override args = { @@ -38,177 +38,41 @@ export default class SpacesLocksSubscribe extends SpacesBaseCommand { private listener: ((lock: Lock) => void) | null = null; - private displayLockDetails(lock: Lock): void { - this.log(` ${formatLabel("Status")} ${lock.status}`); - this.log( - ` ${formatLabel("Member")} ${lock.member?.clientId || "Unknown"}`, - ); - - if (lock.member?.connectionId) { - this.log(` ${formatLabel("Connection ID")} ${lock.member.connectionId}`); - } - - if (lock.timestamp) { - this.log( - ` ${formatLabel("Timestamp")} ${new Date(lock.timestamp).toISOString()}`, - ); - } - - if (lock.attributes) { - this.log( - ` ${formatLabel("Attributes")} ${JSON.stringify(lock.attributes)}`, - ); - } - - if (lock.reason) { - this.log( - ` ${formatLabel("Reason")} ${lock.reason.message || lock.reason.toString()}`, - ); - } - } - async run(): Promise { const { args, flags } = await this.parse(SpacesLocksSubscribe); const { space: spaceName } = args; - this.logCliEvent( - flags, - "subscribe.run", - "start", - `Starting spaces locks subscribe for space: ${spaceName}`, - ); try { - // Always show the readiness signal first, before attempting auth if (!this.shouldOutputJson(flags)) { - this.log("Subscribing to lock events"); + this.log(formatProgress("Subscribing to lock events")); } - this.logCliEvent( - flags, - "subscribe.run", - "initialSignalLogged", - "Initial readiness signal logged.", - ); - - await this.initializeSpace(flags, spaceName, { enterSpace: true }); - if (!this.shouldOutputJson(flags)) { - this.log( - formatProgress(`Connecting to space: ${formatResource(spaceName)}`), - ); - } + await this.initializeSpace(flags, spaceName, { enterSpace: false }); - // Get current locks - this.logCliEvent( - flags, - "lock", - "gettingInitial", - "Fetching initial locks", - ); - if (!this.shouldOutputJson(flags)) { - this.log( - formatProgress( - `Fetching current locks for space ${formatResource(spaceName)}`, - ), - ); - } - - const locks = await this.space!.locks.getAll(); - this.logCliEvent( - flags, - "lock", - "gotInitial", - `Fetched ${locks.length} initial locks`, - { count: locks.length, locks }, - ); - - // Output current locks - if (locks.length === 0) { - if (!this.shouldOutputJson(flags)) { - this.log( - chalk.yellow("No locks are currently active in this space."), - ); - } - } else if (this.shouldOutputJson(flags)) { - this.logJsonResult( - { - locks: locks.map((lock) => ({ - id: lock.id, - member: lock.member, - status: lock.status, - timestamp: lock.timestamp, - ...(lock.attributes && { attributes: lock.attributes }), - ...(lock.reason && { reason: lock.reason }), - })), - spaceName, - status: "connected", - }, - flags, - ); - } else { - this.log( - `\n${formatHeading("Current locks")} (${chalk.bold(locks.length.toString())}):\n`, - ); - - for (const lock of locks) { - this.log(`- Lock ID: ${formatResource(lock.id)}`); - this.displayLockDetails(lock); - } - } - - // Subscribe to lock events this.logCliEvent( flags, "lock", "subscribing", "Subscribing to lock events", ); - if (!this.shouldOutputJson(flags)) { - this.log(formatListening("Subscribing to lock events.")); - } - this.logCliEvent( - flags, - "lock.subscribe", - "readySignalLogged", - "Final readiness signal 'Subscribing to lock events' logged.", - ); - // Define the listener function this.listener = (lock: Lock) => { const timestamp = new Date().toISOString(); - const eventData = { - lock: { - id: lock.id, - member: lock.member, - status: lock.status, - timestamp: lock.timestamp, - ...(lock.attributes && { attributes: lock.attributes }), - ...(lock.reason && { reason: lock.reason }), - }, - spaceName, - timestamp, - eventType: "lock_event", - }; - - this.logCliEvent( - flags, - "lock", - "event-update", - "Lock event received", - eventData, - ); + this.logCliEvent(flags, "lock", "event-update", "Lock event received", { + lockId: lock.id, + status: lock.status, + }); if (this.shouldOutputJson(flags)) { - this.logJsonEvent(eventData, flags); + this.logJsonEvent({ lock: formatLockOutput(lock) }, flags); } else { - this.log( - `${formatTimestamp(timestamp)} Lock ${formatResource(lock.id)} updated`, - ); - this.displayLockDetails(lock); + this.log(formatTimestamp(timestamp)); + this.log(formatLockBlock(lock)); + this.log(""); } }; - // Subscribe using the stored listener await this.space!.locks.subscribe(this.listener); this.logCliEvent( @@ -218,19 +82,13 @@ export default class SpacesLocksSubscribe extends SpacesBaseCommand { "Successfully subscribed to lock events", ); - this.logCliEvent( - flags, - "lock", - "listening", - "Listening for lock events...", - ); + if (!this.shouldOutputJson(flags)) { + this.log(`\n${formatListening("Listening for lock events.")}\n`); + } - // Wait until the user interrupts or the optional duration elapses await this.waitAndTrackCleanup(flags, "lock", flags.duration); } catch (error) { this.fail(error, flags, "lockSubscribe"); - } finally { - // Cleanup is now handled by base class finally() method } } } diff --git a/src/commands/spaces/members/enter.ts b/src/commands/spaces/members/enter.ts index bdaaf44f5..a80d71b9f 100644 --- a/src/commands/spaces/members/enter.ts +++ b/src/commands/spaces/members/enter.ts @@ -1,4 +1,4 @@ -import type { ProfileData, SpaceMember } from "@ably/spaces"; +import type { ProfileData } from "@ably/spaces"; import { Args, Flags } from "@oclif/core"; import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; @@ -6,12 +6,12 @@ import { SpacesBaseCommand } from "../../../spaces-base-command.js"; import { formatSuccess, formatListening, + formatProgress, formatResource, - formatTimestamp, - formatPresenceAction, - formatClientId, formatLabel, + formatClientId, } from "../../../utils/output.js"; +import { formatMemberOutput } from "../../../utils/spaces-output.js"; export default class SpacesMembersEnter extends SpacesBaseCommand { static override args = { @@ -46,16 +46,9 @@ export default class SpacesMembersEnter extends SpacesBaseCommand { const { args, flags } = await this.parse(SpacesMembersEnter); const { space: spaceName } = args; - // Keep track of the last event we've seen for each client to avoid duplicates - const lastSeenEvents = new Map< - string, - { action: string; timestamp: number } - >(); - try { - // Always show the readiness signal first, before attempting auth if (!this.shouldOutputJson(flags)) { - this.log(formatListening("Entering space.")); + this.log(formatProgress("Entering space")); } await this.initializeSpace(flags, spaceName, { enterSpace: false }); @@ -83,202 +76,36 @@ export default class SpacesMembersEnter extends SpacesBaseCommand { { profileData }, ); await this.space!.enter(profileData); - const enteredEventData = { + this.markAsEntered(); + this.logCliEvent(flags, "member", "enteredSpace", "Entered space", { connectionId: this.realtimeClient!.connection.id, - profile: profileData, - spaceName, - status: "connected", - }; - this.logCliEvent( - flags, - "member", - "enteredSpace", - "Entered space", - enteredEventData, - ); + profileData, + }); if (this.shouldOutputJson(flags)) { - this.logJsonResult(enteredEventData, flags); + const self = await this.space!.members.getSelf(); + this.logJsonResult({ members: [formatMemberOutput(self!)] }, flags); } else { this.log(formatSuccess(`Entered space: ${formatResource(spaceName)}.`)); + this.log( + `${formatLabel("Client ID")} ${formatClientId(this.realtimeClient!.auth.clientId)}`, + ); + this.log( + `${formatLabel("Connection ID")} ${this.realtimeClient!.connection.id}`, + ); if (profileData) { - this.log( - `${formatLabel("Profile")} ${JSON.stringify(profileData, null, 2)}`, - ); - } else { - // No profile data provided - this.logCliEvent( - flags, - "member", - "noProfileData", - "No profile data provided", - ); + this.log(`${formatLabel("Profile")} ${JSON.stringify(profileData)}`); } } - // Subscribe to member presence events to show other members' activities - this.logCliEvent( - flags, - "member", - "subscribing", - "Subscribing to member updates", - ); if (!this.shouldOutputJson(flags)) { - this.log(`\n${formatListening("Watching for other members.")}\n`); + this.log(`\n${formatListening("Holding presence.")}\n`); } - // Define the listener function - const listener = (member: SpaceMember) => { - const timestamp = new Date().toISOString(); - const now = Date.now(); - - // Determine the action from the member's lastEvent - const action = member.lastEvent?.name || "unknown"; - const clientId = member.clientId || "Unknown"; - const connectionId = member.connectionId || "Unknown"; - - // Skip self events - check connection ID - const selfConnectionId = this.realtimeClient!.connection.id; - if (member.connectionId === selfConnectionId) { - return; - } - - // Create a unique key for this client+connection combination - const clientKey = `${clientId}:${connectionId}`; - - // Check if we've seen this exact event recently (within 500ms) - // This helps avoid duplicate enter/leave events that might come through - const lastEvent = lastSeenEvents.get(clientKey); - - if ( - lastEvent && - lastEvent.action === action && - now - lastEvent.timestamp < 500 - ) { - this.logCliEvent( - flags, - "member", - "duplicateEventSkipped", - `Skipping duplicate event '${action}' for ${clientId}`, - { action, clientId }, - ); - return; // Skip duplicate events within 500ms window - } - - // Update the last seen event for this client+connection - lastSeenEvents.set(clientKey, { - action, - timestamp: now, - }); - - const memberEventData = { - action, - member: { - clientId: member.clientId, - connectionId: member.connectionId, - isConnected: member.isConnected, - profileData: member.profileData, - }, - spaceName, - timestamp, - eventType: "member_update", - }; - this.logCliEvent( - flags, - "member", - `update-${action}`, - `Member event '${action}' received`, - memberEventData, - ); - - if (this.shouldOutputJson(flags)) { - this.logJsonEvent(memberEventData, flags); - } else { - const { symbol: actionSymbol, color: actionColor } = - formatPresenceAction(action); - - this.log( - `${formatTimestamp(timestamp)} ${actionColor(actionSymbol)} ${formatClientId(clientId)} ${actionColor(action)}`, - ); - - const hasProfileData = - member.profileData && Object.keys(member.profileData).length > 0; - - if (hasProfileData) { - this.log( - ` ${formatLabel("Profile")} ${JSON.stringify(member.profileData, null, 2)}`, - ); - } else { - // No profile data available - this.logCliEvent( - flags, - "member", - "noProfileDataForMember", - "No profile data available for member", - ); - } - - if (connectionId === "Unknown") { - // Connection ID is unknown - this.logCliEvent( - flags, - "member", - "unknownConnectionId", - "Connection ID is unknown for member", - ); - } else { - this.log(` ${formatLabel("Connection ID")} ${connectionId}`); - } - - if (member.isConnected === false) { - this.log(` ${formatLabel("Status")} Not connected`); - } else { - // Member is connected - this.logCliEvent( - flags, - "member", - "memberConnected", - "Member is connected", - ); - } - } - }; - - // Subscribe using the stored listener - await this.space!.members.subscribe("update", listener); - - this.logCliEvent( - flags, - "member", - "subscribed", - "Subscribed to member updates", - ); - - this.logCliEvent( - flags, - "member", - "listening", - "Listening for member updates...", - ); - // Wait until the user interrupts or the optional duration elapses await this.waitAndTrackCleanup(flags, "member", flags.duration); } catch (error) { this.fail(error, flags, "memberEnter"); - } finally { - if (!this.shouldOutputJson(flags || {})) { - if (this.cleanupInProgress) { - this.log(formatSuccess("Graceful shutdown complete.")); - } else { - // Normal completion without user interrupt - this.logCliEvent( - flags || {}, - "member", - "completedNormally", - "Command completed normally", - ); - } - } } } } diff --git a/src/commands/spaces/members/get-all.ts b/src/commands/spaces/members/get-all.ts new file mode 100644 index 000000000..2dcbd18ba --- /dev/null +++ b/src/commands/spaces/members/get-all.ts @@ -0,0 +1,89 @@ +import type { SpaceMember } from "@ably/spaces"; +import { Args } from "@oclif/core"; + +import { productApiFlags, clientIdFlag } from "../../../flags.js"; +import { SpacesBaseCommand } from "../../../spaces-base-command.js"; +import { + formatCountLabel, + formatHeading, + formatIndex, + formatProgress, + formatResource, + formatWarning, +} from "../../../utils/output.js"; +import { + formatMemberBlock, + formatMemberOutput, +} from "../../../utils/spaces-output.js"; + +export default class SpacesMembersGetAll extends SpacesBaseCommand { + static override args = { + space: Args.string({ + description: "Space to get members from", + required: true, + }), + }; + + static override description = "Get all members in a space"; + + static override examples = [ + "$ ably spaces members get-all my-space", + "$ ably spaces members get-all my-space --json", + "$ ably spaces members get-all my-space --pretty-json", + ]; + + static override flags = { + ...productApiFlags, + ...clientIdFlag, + }; + + async run(): Promise { + const { args, flags } = await this.parse(SpacesMembersGetAll); + const { space: spaceName } = args; + + try { + await this.initializeSpace(flags, spaceName, { + enterSpace: false, + setupConnectionLogging: false, + }); + + if (!this.shouldOutputJson(flags)) { + this.log( + formatProgress( + `Fetching members for space ${formatResource(spaceName)}`, + ), + ); + } + + try { + const members: SpaceMember[] = await this.space!.members.getAll(); + + if (this.shouldOutputJson(flags)) { + this.logJsonResult( + { + members: members.map((member) => formatMemberOutput(member)), + }, + flags, + ); + } else if (members.length === 0) { + this.log(formatWarning("No members currently in this space.")); + } else { + this.log( + `\n${formatHeading("Current members")} (${formatCountLabel(members.length, "member")}):\n`, + ); + + for (let i = 0; i < members.length; i++) { + const member = members[i]; + this.log(`${formatIndex(i + 1)}`); + this.log(formatMemberBlock(member, { indent: " " })); + this.log(""); + } + } + } catch (error) { + this.fail(error, flags, "memberGetAll", { spaceName }); + } + } catch (error) { + this.fail(error, flags, "memberGetAll", { spaceName }); + } + } +} diff --git a/src/commands/spaces/members/subscribe.ts b/src/commands/spaces/members/subscribe.ts index 03ec36088..16017bd9c 100644 --- a/src/commands/spaces/members/subscribe.ts +++ b/src/commands/spaces/members/subscribe.ts @@ -1,18 +1,17 @@ import type { SpaceMember } from "@ably/spaces"; import { Args } from "@oclif/core"; -import chalk from "chalk"; import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; import { - formatClientId, - formatHeading, formatListening, - formatPresenceAction, formatProgress, formatTimestamp, - formatLabel, } from "../../../utils/output.js"; +import { + formatMemberEventBlock, + formatMemberOutput, +} from "../../../utils/spaces-output.js"; export default class SpacesMembersSubscribe extends SpacesBaseCommand { static override args = { @@ -56,74 +55,7 @@ export default class SpacesMembersSubscribe extends SpacesBaseCommand { this.log(formatProgress("Subscribing to member updates")); } - await this.initializeSpace(flags, spaceName, { enterSpace: true }); - - // Get current members - this.logCliEvent( - flags, - "member", - "gettingInitial", - "Fetching initial members", - ); - const members = await this.space!.members.getAll(); - const initialMembers = members.map((member) => ({ - clientId: member.clientId, - connectionId: member.connectionId, - isConnected: member.isConnected, - profileData: member.profileData, - })); - this.logCliEvent( - flags, - "member", - "gotInitial", - `Fetched ${members.length} initial members`, - { count: members.length, members: initialMembers }, - ); - - // Output current members - if (members.length === 0) { - if (!this.shouldOutputJson(flags)) { - this.log( - chalk.yellow("No members are currently present in this space."), - ); - } - } else if (this.shouldOutputJson(flags)) { - this.logJsonResult( - { - members: initialMembers, - spaceName, - status: "connected", - }, - flags, - ); - } else { - this.log( - `\n${formatHeading("Current members")} (${chalk.bold(members.length.toString())}):\n`, - ); - - for (const member of members) { - this.log(`- ${formatClientId(member.clientId || "Unknown")}`); - - if ( - member.profileData && - Object.keys(member.profileData).length > 0 - ) { - this.log( - ` ${formatLabel("Profile")} ${JSON.stringify(member.profileData, null, 2)}`, - ); - } - - if (member.connectionId) { - this.log( - ` ${formatLabel("Connection ID")} ${member.connectionId}`, - ); - } - - if (member.isConnected === false) { - this.log(` ${formatLabel("Status")} Not connected`); - } - } - } + await this.initializeSpace(flags, spaceName, { enterSpace: false }); if (!this.shouldOutputJson(flags)) { this.log(`\n${formatListening("Listening for member events.")}\n`); @@ -179,52 +111,20 @@ export default class SpacesMembersSubscribe extends SpacesBaseCommand { timestamp: now, }); - const memberEventData = { - action, - member: { - clientId: member.clientId, - connectionId: member.connectionId, - isConnected: member.isConnected, - profileData: member.profileData, - }, - spaceName, - timestamp, - eventType: "member_update", - }; this.logCliEvent( flags, "member", `update-${action}`, `Member event '${action}' received`, - memberEventData, + { action, clientId, connectionId }, ); if (this.shouldOutputJson(flags)) { - this.logJsonEvent(memberEventData, flags); + this.logJsonEvent({ member: formatMemberOutput(member) }, flags); } else { - const { symbol: actionSymbol, color: actionColor } = - formatPresenceAction(action); - - this.log( - `${formatTimestamp(timestamp)} ${actionColor(actionSymbol)} ${formatClientId(clientId)} ${actionColor(action)}`, - ); - - if ( - member.profileData && - Object.keys(member.profileData).length > 0 - ) { - this.log( - ` ${formatLabel("Profile")} ${JSON.stringify(member.profileData, null, 2)}`, - ); - } - - if (connectionId !== "Unknown") { - this.log(` ${formatLabel("Connection ID")} ${connectionId}`); - } - - if (member.isConnected === false) { - this.log(` ${formatLabel("Status")} Not connected`); - } + this.log(formatTimestamp(timestamp)); + this.log(formatMemberEventBlock(member, action)); + this.log(""); } }; diff --git a/src/commands/spaces/subscribe.ts b/src/commands/spaces/subscribe.ts new file mode 100644 index 000000000..5f7e0c79b --- /dev/null +++ b/src/commands/spaces/subscribe.ts @@ -0,0 +1,105 @@ +import type { SpaceMember } from "@ably/spaces"; +import { Args } from "@oclif/core"; + +import { productApiFlags, clientIdFlag, durationFlag } from "../../flags.js"; +import { SpacesBaseCommand } from "../../spaces-base-command.js"; +import { + formatCountLabel, + formatListening, + formatProgress, + formatTimestamp, +} from "../../utils/output.js"; +import { + formatMemberBlock, + formatMemberOutput, +} from "../../utils/spaces-output.js"; + +export default class SpacesSubscribe extends SpacesBaseCommand { + static override args = { + space: Args.string({ + description: "Space to subscribe to", + required: true, + }), + }; + + static override description = "Subscribe to space update events"; + + static override examples = [ + "$ ably spaces subscribe my-space", + "$ ably spaces subscribe my-space --json", + "$ ably spaces subscribe my-space --pretty-json", + "$ ably spaces subscribe my-space --duration 30", + ]; + + static override flags = { + ...productApiFlags, + ...clientIdFlag, + ...durationFlag, + }; + + private listener: ((spaceState: { members: SpaceMember[] }) => void) | null = + null; + + async run(): Promise { + const { args, flags } = await this.parse(SpacesSubscribe); + const { space: spaceName } = args; + + try { + if (!this.shouldOutputJson(flags)) { + this.log(formatProgress("Subscribing to space updates")); + } + + await this.initializeSpace(flags, spaceName, { enterSpace: false }); + + if (!this.shouldOutputJson(flags)) { + this.log(`\n${formatListening("Listening for space updates.")}\n`); + } + + this.logCliEvent( + flags, + "space", + "subscribing", + "Subscribing to space updates", + ); + + this.listener = (spaceState: { members: SpaceMember[] }) => { + const timestamp = new Date().toISOString(); + const members = spaceState.members; + + this.logCliEvent(flags, "space", "update", "Space update received", { + memberCount: members.length, + }); + + if (this.shouldOutputJson(flags)) { + this.logJsonEvent( + { members: members.map((m) => formatMemberOutput(m)) }, + flags, + ); + } else { + this.log(formatTimestamp(timestamp)); + this.log( + `Space update (${formatCountLabel(members.length, "member")}):\n`, + ); + + for (const member of members) { + this.log(formatMemberBlock(member, { indent: " " })); + this.log(""); + } + } + }; + + this.space!.subscribe("update", this.listener); + + this.logCliEvent( + flags, + "space", + "subscribed", + "Subscribed to space updates", + ); + + await this.waitAndTrackCleanup(flags, "space", flags.duration); + } catch (error) { + this.fail(error, flags, "spaceSubscribe"); + } + } +} diff --git a/src/spaces-base-command.ts b/src/spaces-base-command.ts index dbaf06898..8ac47e591 100644 --- a/src/spaces-base-command.ts +++ b/src/spaces-base-command.ts @@ -31,69 +31,103 @@ export abstract class SpacesBaseCommand extends AblyBaseCommand { protected spaces: Spaces | null = null; protected realtimeClient: Ably.Realtime | null = null; protected parsedFlags: BaseFlags = {}; + protected hasEnteredSpace = false; + + protected markAsEntered(): void { + this.hasEnteredSpace = true; + } async finally(error: Error | undefined): Promise { - // Always clean up connections + // The Spaces SDK subscribes to channel.presence internally (in the Space + // constructor) but provides no dispose/cleanup method. When the connection + // closes, the SDK's internal handlers receive errors that surface as + // unhandled rejections crashing the process. We suppress these during + // cleanup, matching the ChatBaseCommand pattern of tolerating SDK errors + // during teardown. + const suppressedErrors: unknown[] = []; + const onUnhandledRejection = (reason: unknown) => { + suppressedErrors.push(reason); + this.debug(`Suppressed unhandled rejection during cleanup: ${reason}`); + }; + + process.on("unhandledRejection", onUnhandledRejection); + try { - // Unsubscribe from all namespace listeners if (this.space !== null) { + // Unsubscribe from all namespace listeners try { await this.space.members.unsubscribe(); await this.space.locks.unsubscribe(); this.space.locations.unsubscribe(); this.space.cursors.unsubscribe(); } catch (error) { - // Log but don't throw unsubscribe errors - if (!this.shouldOutputJson(this.parsedFlags)) { - this.debug(`Namespace unsubscribe error: ${error}`); + this.debug(`Namespace unsubscribe error: ${error}`); + } + + // Unsubscribe the SDK's internal presence handler on the space channel. + // This removes the Spaces SDK's listener but cannot fully prevent + // errors from the Ably SDK's own channel state transitions during close. + try { + const spaceChannel = ( + this.space as unknown as { channel: Ably.RealtimeChannel } + ).channel; + if (spaceChannel) { + spaceChannel.presence.unsubscribe(); } + } catch (error) { + this.debug(`Space channel presence unsubscribe error: ${error}`); } - await this.space!.leave(); - // Wait a bit after leaving space - await new Promise((resolve) => setTimeout(resolve, 200)); - - // Spaces maintains an internal map of members which have timeouts. This keeps node alive. - // This is a workaround to hold off until those timeouts are cleared by the client, as otherwise - // we'll get unhandled presence rejections as the connection closes. - await new Promise((resolve) => { - let intervalId: ReturnType; - const maxWaitMs = 10000; // 10 second timeout - const startTime = Date.now(); - const getAll = async () => { - // Avoid waiting forever - if (Date.now() - startTime > maxWaitMs) { - clearInterval(intervalId); - this.debug("Timed out waiting for space members to clear"); - resolve(); - return; - } - - const members = await this.space!.members.getAll(); - if (members.filter((member) => !member.isConnected).length === 0) { - clearInterval(intervalId); - this.debug("space members cleared"); - resolve(); - } else { - this.debug( - `waiting for spaces members to clear, ${members.length} remaining`, - ); - } - }; - - intervalId = setInterval(() => { - getAll(); - }, 1000); - }); + // Only leave and wait for member cleanup if we actually entered the space + if (this.hasEnteredSpace) { + await this.space!.leave(); + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Spaces maintains an internal map of members which have timeouts. This keeps node alive. + // This is a workaround to hold off until those timeouts are cleared by the client, as otherwise + // we'll get unhandled presence rejections as the connection closes. + await new Promise((resolve) => { + let intervalId: ReturnType; + const maxWaitMs = 10000; + const startTime = Date.now(); + const getAll = async () => { + if (Date.now() - startTime > maxWaitMs) { + clearInterval(intervalId); + this.debug("Timed out waiting for space members to clear"); + resolve(); + return; + } + + const members = await this.space!.members.getAll(); + if ( + members.filter((member) => !member.isConnected).length === 0 + ) { + clearInterval(intervalId); + this.debug("space members cleared"); + resolve(); + } else { + this.debug( + `waiting for spaces members to clear, ${members.length} remaining`, + ); + } + }; + + intervalId = setInterval(() => { + getAll(); + }, 1000); + }); + } } } catch (error) { - // Log but don't throw cleanup errors - if (!this.shouldOutputJson(this.parsedFlags)) { - this.debug(`Space leave error: ${error}`); - } + this.debug(`Space cleanup error: ${error}`); } await super.finally(error); + + // Allow a tick for any remaining SDK-internal rejections to fire + // before removing the suppression handler. + await new Promise((resolve) => setTimeout(resolve, 50)); + process.removeListener("unhandledRejection", onUnhandledRejection); } // Ensure we have the spaces client and its related authentication resources @@ -247,6 +281,7 @@ export abstract class SpacesBaseCommand extends AblyBaseCommand { if (enterSpace) { this.logCliEvent(flags, "spaces", "entering", "Entering space..."); await this.space!.enter(); + this.markAsEntered(); this.logCliEvent(flags, "spaces", "entered", "Entered space", { clientId: this.realtimeClient!.auth.clientId, }); diff --git a/src/utils/spaces-output.ts b/src/utils/spaces-output.ts new file mode 100644 index 000000000..c54a698d8 --- /dev/null +++ b/src/utils/spaces-output.ts @@ -0,0 +1,227 @@ +import type { CursorUpdate, Lock, SpaceMember } from "@ably/spaces"; + +import { + formatClientId, + formatEventType, + formatLabel, + formatMessageTimestamp, + formatResource, +} from "./output.js"; + +// --- JSON display interfaces (used by logJsonResult / logJsonEvent) --- + +export interface MemberOutput { + clientId: string; + connectionId: string; + isConnected: boolean; + profileData: Record | null; + location: unknown | null; + lastEvent: { name: string; timestamp: number }; +} + +export interface CursorOutput { + clientId: string; + connectionId: string; + position: { x: number; y: number }; + data: Record | null; +} + +export interface LockOutput { + id: string; + status: string; + member: MemberOutput; + timestamp: number; + attributes: Record | null; + reason: { message?: string; code?: number; statusCode?: number } | null; +} + +export interface LocationEntry { + connectionId: string; + location: unknown; +} + +// --- JSON formatters (SDK type → display interface) --- + +export function formatMemberOutput(member: SpaceMember): MemberOutput { + return { + clientId: member.clientId, + connectionId: member.connectionId, + isConnected: member.isConnected, + profileData: member.profileData ?? null, + location: member.location ?? null, + lastEvent: { + name: member.lastEvent.name, + timestamp: member.lastEvent.timestamp, + }, + }; +} + +export function formatCursorOutput(cursor: CursorUpdate): CursorOutput { + return { + clientId: cursor.clientId, + connectionId: cursor.connectionId, + position: cursor.position, + data: (cursor.data as Record) ?? null, + }; +} + +export function formatLockOutput(lock: Lock): LockOutput { + return { + id: lock.id, + status: lock.status, + member: formatMemberOutput(lock.member), + timestamp: lock.timestamp, + attributes: (lock.attributes as Record) ?? null, + reason: lock.reason + ? { + message: lock.reason.message, + code: lock.reason.code, + statusCode: lock.reason.statusCode, + } + : null, + }; +} + +// --- Human-readable block formatters (for non-JSON output) --- + +/** + * Format a SpaceMember as a multi-line labeled block. + * Used in members enter, members subscribe, and as nested output in locks. + */ +export function formatMemberBlock( + member: SpaceMember, + options?: { indent?: string }, +): string { + const indent = options?.indent ?? ""; + const lines: string[] = [ + `${indent}${formatLabel("Client ID")} ${formatClientId(member.clientId)}`, + `${indent}${formatLabel("Connection ID")} ${member.connectionId}`, + `${indent}${formatLabel("Connected")} ${member.isConnected}`, + ]; + + if (member.profileData && Object.keys(member.profileData).length > 0) { + lines.push( + `${indent}${formatLabel("Profile")} ${JSON.stringify(member.profileData)}`, + ); + } + + if (member.location != null) { + lines.push( + `${indent}${formatLabel("Location")} ${JSON.stringify(member.location)}`, + ); + } + + lines.push( + `${indent}${formatLabel("Last Event")} ${member.lastEvent.name} at ${formatMessageTimestamp(member.lastEvent.timestamp)}`, + ); + + return lines.join("\n"); +} + +/** + * Format a SpaceMember event as a multi-line labeled block with action header. + * Used in members subscribe and members enter for streaming events. + */ +export function formatMemberEventBlock( + member: SpaceMember, + action: string, +): string { + const lines: string[] = [ + `${formatLabel("Action")} ${formatEventType(action)}`, + `${formatLabel("Client ID")} ${formatClientId(member.clientId)}`, + `${formatLabel("Connection ID")} ${member.connectionId}`, + `${formatLabel("Connected")} ${member.isConnected}`, + ]; + + if (member.profileData && Object.keys(member.profileData).length > 0) { + lines.push( + `${formatLabel("Profile")} ${JSON.stringify(member.profileData)}`, + ); + } + + if (member.location != null) { + lines.push(`${formatLabel("Location")} ${JSON.stringify(member.location)}`); + } + + return lines.join("\n"); +} + +/** + * Format a CursorUpdate as a multi-line labeled block. + */ +export function formatCursorBlock( + cursor: CursorUpdate, + options?: { indent?: string }, +): string { + const indent = options?.indent ?? ""; + const lines: string[] = [ + `${indent}${formatLabel("Client ID")} ${formatClientId(cursor.clientId)}`, + `${indent}${formatLabel("Connection ID")} ${cursor.connectionId}`, + `${indent}${formatLabel("Position X")} ${cursor.position.x}`, + `${indent}${formatLabel("Position Y")} ${cursor.position.y}`, + ]; + + if ( + cursor.data && + Object.keys(cursor.data as Record).length > 0 + ) { + lines.push( + `${indent}${formatLabel("Data")} ${JSON.stringify(cursor.data)}`, + ); + } + + return lines.join("\n"); +} + +/** + * Format a Lock as a multi-line labeled block. + */ +export function formatLockBlock(lock: Lock): string { + const lines: string[] = [ + `${formatLabel("Lock ID")} ${formatResource(lock.id)}`, + `${formatLabel("Status")} ${formatEventType(lock.status)}`, + `${formatLabel("Timestamp")} ${formatMessageTimestamp(lock.timestamp)}`, + `${formatLabel("Member")}`, + formatMemberBlock(lock.member, { indent: " " }), + ]; + + if ( + lock.attributes && + Object.keys(lock.attributes as Record).length > 0 + ) { + lines.push( + `${formatLabel("Attributes")} ${JSON.stringify(lock.attributes)}`, + ); + } + + if (lock.reason) { + lines.push( + `${formatLabel("Reason")} ${lock.reason.message || lock.reason.toString()}`, + ); + } + + return lines.join("\n"); +} + +/** + * Format a location update event as a multi-line labeled block. + */ +export function formatLocationUpdateBlock(update: { + member: SpaceMember; + currentLocation: unknown; + previousLocation: unknown; +}): string { + const lines: string[] = [ + `${formatLabel("Client ID")} ${formatClientId(update.member.clientId)}`, + `${formatLabel("Connection ID")} ${update.member.connectionId}`, + `${formatLabel("Current Location")} ${JSON.stringify(update.currentLocation)}`, + ]; + + if (update.previousLocation != null) { + lines.push( + `${formatLabel("Previous Location")} ${JSON.stringify(update.previousLocation)}`, + ); + } + + return lines.join("\n"); +} diff --git a/test/e2e/spaces/spaces-e2e.test.ts b/test/e2e/spaces/spaces-e2e.test.ts index d48a0ae89..e1ae7015f 100644 --- a/test/e2e/spaces/spaces-e2e.test.ts +++ b/test/e2e/spaces/spaces-e2e.test.ts @@ -191,7 +191,7 @@ describe("Spaces E2E Tests", () => { `bin/run.js spaces locations subscribe ${testSpaceId} --client-id ${client1Id} --duration 20`, outputPath, { - readySignal: "Fetching current locations for space", + readySignal: "Subscribing to location updates", timeoutMs: process.env.CI ? 40000 : 30000, // Increased timeout retryCount: 2, }, @@ -212,7 +212,7 @@ describe("Spaces E2E Tests", () => { }; const setLocationResult = await runBackgroundProcessAndGetOutput( - `bin/run.js spaces locations set ${testSpaceId} --location '${JSON.stringify(locationData)}' --client-id ${client2Id} --duration 0`, + `bin/run.js spaces locations set ${testSpaceId} --location '${JSON.stringify(locationData)}' --client-id ${client2Id}`, process.env.CI ? 15000 : 15000, // Timeout for the command ); @@ -252,7 +252,7 @@ describe("Spaces E2E Tests", () => { }; const updateLocationResult = await runBackgroundProcessAndGetOutput( - `bin/run.js spaces locations set ${testSpaceId} --location '${JSON.stringify(newLocationData)}' --client-id ${client2Id} --duration 0`, + `bin/run.js spaces locations set ${testSpaceId} --location '${JSON.stringify(newLocationData)}' --client-id ${client2Id}`, process.env.CI ? 15000 : 15000, // Increased local timeout ); @@ -349,7 +349,7 @@ describe("Spaces E2E Tests", () => { `bin/run.js spaces cursors subscribe ${testSpaceId} --client-id ${client1Id} --duration 20`, outputPath, { - readySignal: "Subscribing to cursor movements", + readySignal: "Subscribing to cursor updates", timeoutMs: 60000, // Increased timeout significantly retryCount: 3, }, @@ -370,7 +370,7 @@ describe("Spaces E2E Tests", () => { let currentOutput = await readProcessOutput(outputPath); if ( !currentOutput.includes("Entered space:") && - !currentOutput.includes("Subscribing to cursor movements") + !currentOutput.includes("Subscribing to cursor updates") ) { // The cursor subscribe process might have failed, let's skip this test shouldSkipCursorTest = true; @@ -400,7 +400,7 @@ describe("Spaces E2E Tests", () => { if ( output.includes(client2Id) && - output.includes("position:") && + output.includes("Position:") && output.includes("TestUser2") && output.includes("#ff0000") ) { diff --git a/test/helpers/mock-ably-spaces.ts b/test/helpers/mock-ably-spaces.ts index 22641ca19..908117077 100644 --- a/test/helpers/mock-ably-spaces.ts +++ b/test/helpers/mock-ably-spaces.ts @@ -109,11 +109,17 @@ export interface MockSpace { name: string; enter: Mock; leave: Mock; + subscribe: Mock; + unsubscribe: Mock; updateProfileData: Mock; members: MockSpaceMembers; locations: MockSpaceLocations; locks: MockSpaceLocks; cursors: MockSpaceCursors; + // Internal emitter for simulating space-level events + _emitter: AblyEventEmitter; + // Helper to emit space events + _emit: (event: string, data: unknown) => void; } /** @@ -158,6 +164,8 @@ function createMockSpaceMembers(): MockSpaceMembers { connectionId: "mock-connection-id", isConnected: true, profileData: {}, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, }), _emitter: emitter, _emit: (member: SpaceMember) => { @@ -174,7 +182,7 @@ function createMockSpaceLocations(): MockSpaceLocations { return { set: vi.fn().mockImplementation(async () => {}), - getAll: vi.fn().mockResolvedValue([]), + getAll: vi.fn().mockResolvedValue({}), getSelf: vi.fn().mockResolvedValue(null), subscribe: vi.fn((eventOrCallback, callback?) => { const cb = callback ?? eventOrCallback; @@ -204,7 +212,21 @@ function createMockSpaceLocks(): MockSpaceLocks { const emitter = new EventEmitter(); return { - acquire: vi.fn().mockResolvedValue({ id: "mock-lock-id" }), + acquire: vi.fn().mockResolvedValue({ + id: "mock-lock-id", + status: "locked", + member: { + clientId: "mock-client-id", + connectionId: "mock-connection-id", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, + timestamp: Date.now(), + attributes: undefined, + reason: undefined, + }), release: vi.fn().mockImplementation(async () => {}), get: vi.fn().mockResolvedValue(null), getAll: vi.fn().mockResolvedValue([]), @@ -237,7 +259,7 @@ function createMockSpaceCursors(): MockSpaceCursors { return { set: vi.fn().mockImplementation(async () => {}), - getAll: vi.fn().mockResolvedValue([]), + getAll: vi.fn().mockResolvedValue({}), subscribe: vi.fn((eventOrCallback, callback?) => { const cb = callback ?? eventOrCallback; const event = callback ? eventOrCallback : null; @@ -268,15 +290,43 @@ function createMockSpaceCursors(): MockSpaceCursors { * Create a mock space object. */ function createMockSpace(name: string): MockSpace { + const emitter = new EventEmitter(); + return { name, enter: vi.fn().mockImplementation(async () => {}), leave: vi.fn().mockImplementation(async () => {}), + subscribe: vi.fn((eventOrCallback: unknown, callback?: unknown) => { + const cb = (callback ?? eventOrCallback) as (...args: unknown[]) => void; + const event = callback + ? (eventOrCallback as string) + : (null as unknown as string); + emitter.on(event, cb); + }), + unsubscribe: vi.fn((eventOrCallback?: unknown, callback?: unknown) => { + if (!eventOrCallback) { + emitter.off(); + } else if (typeof eventOrCallback === "function") { + emitter.off( + null as unknown as string, + eventOrCallback as (...args: unknown[]) => void, + ); + } else if (callback) { + emitter.off( + eventOrCallback as string, + callback as (...args: unknown[]) => void, + ); + } + }), updateProfileData: vi.fn().mockImplementation(async () => {}), members: createMockSpaceMembers(), locations: createMockSpaceLocations(), locks: createMockSpaceLocks(), cursors: createMockSpaceCursors(), + _emitter: emitter, + _emit: (event: string, data: unknown) => { + emitter.emit(event, data); + }, }; } diff --git a/test/unit/commands/spaces/create.test.ts b/test/unit/commands/spaces/create.test.ts new file mode 100644 index 000000000..6a7082519 --- /dev/null +++ b/test/unit/commands/spaces/create.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { runCommand } from "@oclif/test"; +import { getMockAblySpaces } from "../../../helpers/mock-ably-spaces.js"; +import { getMockAblyRealtime } from "../../../helpers/mock-ably-realtime.js"; +import { + standardHelpTests, + standardArgValidationTests, + standardFlagTests, +} from "../../../helpers/standard-tests.js"; + +describe("spaces:create command", () => { + beforeEach(() => { + getMockAblyRealtime(); + getMockAblySpaces(); + }); + + standardHelpTests("spaces:create", import.meta.url); + standardArgValidationTests("spaces:create", import.meta.url, { + requiredArgs: ["test-space"], + }); + standardFlagTests("spaces:create", import.meta.url, ["--json"]); + + describe("functionality", () => { + it("should create a space successfully", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + + const { stdout } = await runCommand( + ["spaces:create", "test-space"], + import.meta.url, + ); + + expect(space.enter).not.toHaveBeenCalled(); + expect(stdout).toContain("created"); + expect(stdout).toContain("test-space"); + }); + + it("should output JSON with correct envelope structure", async () => { + const { stdout } = await runCommand( + ["spaces:create", "test-space", "--json"], + import.meta.url, + ); + + const json = JSON.parse(stdout); + expect(json).toHaveProperty("type", "result"); + expect(json).toHaveProperty("success", true); + expect(json).toHaveProperty("space"); + expect(json.space).toHaveProperty("name", "test-space"); + }); + + it("should not enter the space when creating", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + + await runCommand(["spaces:create", "test-space"], import.meta.url); + + expect(space.enter).not.toHaveBeenCalled(); + }); + }); + + describe("error handling", () => { + it("should handle connection errors gracefully", async () => { + const spacesMock = getMockAblySpaces(); + spacesMock.get.mockRejectedValue(new Error("Connection failed")); + + const { error } = await runCommand( + ["spaces:create", "test-space"], + import.meta.url, + ); + expect(error).toBeDefined(); + }); + }); +}); diff --git a/test/unit/commands/spaces/cursors/get-all.test.ts b/test/unit/commands/spaces/cursors/get-all.test.ts index f57744606..919a6b885 100644 --- a/test/unit/commands/spaces/cursors/get-all.test.ts +++ b/test/unit/commands/spaces/cursors/get-all.test.ts @@ -26,42 +26,38 @@ describe("spaces:cursors:get-all command", () => { it("should get all cursors from a space", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([ - { + space.cursors.getAll.mockResolvedValue({ + "conn-1": { clientId: "user-1", connectionId: "conn-1", position: { x: 100, y: 200 }, data: { color: "red" }, }, - { + "conn-2": { clientId: "user-2", connectionId: "conn-2", position: { x: 300, y: 400 }, data: { color: "blue" }, }, - ]); + }); const { stdout } = await runCommand( ["spaces:cursors:get-all", "test-space", "--json"], import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); - expect(space.cursors.subscribe).toHaveBeenCalledWith( - "update", - expect.any(Function), - ); + expect(space.enter).not.toHaveBeenCalled(); expect(space.cursors.getAll).toHaveBeenCalled(); - // The command outputs multiple JSON lines - check the content contains expected data - expect(stdout).toContain("test-space"); + // The command outputs JSON with cursors array + expect(stdout).toContain("cursors"); expect(stdout).toContain("success"); }); it("should handle no cursors found", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([]); + space.cursors.getAll.mockResolvedValue({}); const { stdout } = await runCommand( ["spaces:cursors:get-all", "test-space", "--json"], @@ -98,14 +94,14 @@ describe("spaces:cursors:get-all command", () => { it("should output JSON envelope with type and command for cursor results", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([ - { + space.cursors.getAll.mockResolvedValue({ + "conn-1": { clientId: "user-1", connectionId: "conn-1", position: { x: 10, y: 20 }, data: null, }, - ]); + }); const { stdout } = await runCommand( ["spaces:cursors:get-all", "test-space", "--json"], @@ -120,7 +116,6 @@ describe("spaces:cursors:get-all command", () => { expect(resultRecord).toHaveProperty("type", "result"); expect(resultRecord).toHaveProperty("command"); expect(resultRecord).toHaveProperty("success", true); - expect(resultRecord).toHaveProperty("spaceName", "test-space"); expect(resultRecord!.cursors).toBeInstanceOf(Array); }); }); @@ -130,7 +125,7 @@ describe("spaces:cursors:get-all command", () => { const realtimeMock = getMockAblyRealtime(); const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([]); + space.cursors.getAll.mockResolvedValue({}); await runCommand( ["spaces:cursors:get-all", "test-space", "--json"], @@ -138,7 +133,6 @@ describe("spaces:cursors:get-all command", () => { ); // Verify cleanup was performed - expect(space.leave).toHaveBeenCalled(); expect(realtimeMock.close).toHaveBeenCalled(); }); }); diff --git a/test/unit/commands/spaces/cursors/set.test.ts b/test/unit/commands/spaces/cursors/set.test.ts index 82b6863ba..5076f7c7e 100644 --- a/test/unit/commands/spaces/cursors/set.test.ts +++ b/test/unit/commands/spaces/cursors/set.test.ts @@ -91,6 +91,10 @@ describe("spaces:cursors:set command", () => { ); expect(stdout).toContain("Set cursor"); expect(stdout).toContain("test-space"); + expect(stdout).toContain("Position X:"); + expect(stdout).toContain("100"); + expect(stdout).toContain("Position Y:"); + expect(stdout).toContain("200"); }); it("should set cursor from --data with position object", async () => { @@ -165,8 +169,12 @@ describe("spaces:cursors:set command", () => { expect(result).toHaveProperty("type", "result"); expect(result).toHaveProperty("command", "spaces:cursors:set"); expect(result).toHaveProperty("success", true); - expect(result).toHaveProperty("spaceName", "test-space"); - expect(result!.cursor.position).toEqual({ x: 100, y: 200 }); + expect(result).toHaveProperty("cursors"); + expect(result!.cursors).toHaveLength(1); + expect(result!.cursors[0]).toHaveProperty("position"); + expect(result!.cursors[0].position).toEqual({ x: 100, y: 200 }); + expect(result!.cursors[0]).toHaveProperty("clientId"); + expect(result!.cursors[0]).toHaveProperty("connectionId"); }); }); diff --git a/test/unit/commands/spaces/cursors/subscribe.test.ts b/test/unit/commands/spaces/cursors/subscribe.test.ts index c53854c08..16a6a05a9 100644 --- a/test/unit/commands/spaces/cursors/subscribe.test.ts +++ b/test/unit/commands/spaces/cursors/subscribe.test.ts @@ -26,14 +26,14 @@ describe("spaces:cursors:subscribe command", () => { it("should subscribe to cursor updates in a space", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([]); + space.cursors.getAll.mockResolvedValue({}); await runCommand( ["spaces:cursors:subscribe", "test-space"], import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); + expect(space.enter).not.toHaveBeenCalled(); expect(space.cursors.subscribe).toHaveBeenCalledWith( "update", expect.any(Function), @@ -43,15 +43,15 @@ describe("spaces:cursors:subscribe command", () => { it("should display initial subscription message", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([]); + space.cursors.getAll.mockResolvedValue({}); const { stdout } = await runCommand( ["spaces:cursors:subscribe", "test-space"], import.meta.url, ); - expect(stdout).toContain("Subscribing"); - expect(stdout).toContain("test-space"); + expect(stdout).toContain("Subscribing to cursor updates"); + expect(stdout).toContain("Listening for cursor movements"); }); }); @@ -60,7 +60,7 @@ describe("spaces:cursors:subscribe command", () => { const realtimeMock = getMockAblyRealtime(); const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([]); + space.cursors.getAll.mockResolvedValue({}); // Use SIGINT to exit @@ -78,7 +78,7 @@ describe("spaces:cursors:subscribe command", () => { it("should output JSON event with envelope when cursor update is received", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([]); + space.cursors.getAll.mockResolvedValue({}); // Fire a cursor event synchronously when subscribe is called space.cursors.subscribe.mockImplementation( @@ -100,13 +100,16 @@ describe("spaces:cursors:subscribe command", () => { const records = parseNdjsonLines(stdout); const eventRecords = records.filter( - (r) => r.type === "event" && r.eventType === "cursor_update", + (r) => r.type === "event" && r.cursor, ); expect(eventRecords.length).toBeGreaterThan(0); const event = eventRecords[0]; expect(event).toHaveProperty("command"); - expect(event).toHaveProperty("spaceName", "test-space"); - expect(event).toHaveProperty("position"); + expect(event).toHaveProperty("cursor"); + expect(event.cursor).toHaveProperty("clientId", "user-1"); + expect(event.cursor).toHaveProperty("connectionId", "conn-1"); + expect(event.cursor).toHaveProperty("position"); + expect(event.cursor.position).toEqual({ x: 50, y: 75 }); }); }); @@ -114,7 +117,7 @@ describe("spaces:cursors:subscribe command", () => { it("should wait for cursors channel to attach if not already attached", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.cursors.getAll.mockResolvedValue([]); + space.cursors.getAll.mockResolvedValue({}); // Mock channel as attaching space.cursors.channel.state = "attaching"; @@ -141,10 +144,10 @@ describe("spaces:cursors:subscribe command", () => { }); describe("error handling", () => { - it("should handle space entry failure", async () => { + it("should handle subscribe failure", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.enter.mockRejectedValue(new Error("Connection failed")); + space.cursors.subscribe.mockRejectedValue(new Error("Connection failed")); const { error } = await runCommand( ["spaces:cursors:subscribe", "test-space"], diff --git a/test/unit/commands/spaces/get.test.ts b/test/unit/commands/spaces/get.test.ts new file mode 100644 index 000000000..3a7e47161 --- /dev/null +++ b/test/unit/commands/spaces/get.test.ts @@ -0,0 +1,153 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { runCommand } from "@oclif/test"; +import { getMockAblyRest } from "../../../helpers/mock-ably-rest.js"; +import { + standardHelpTests, + standardArgValidationTests, + standardFlagTests, +} from "../../../helpers/standard-tests.js"; + +describe("spaces:get command", () => { + const mockPresenceResponse = { + statusCode: 200, + items: [ + { + clientId: "user-1", + connectionId: "conn-1", + action: "present", + timestamp: 1710000000000, + data: { + profileUpdate: { current: { name: "Alice" } }, + locationUpdate: { current: { slide: 1 } }, + }, + }, + { + clientId: "user-2", + connectionId: "conn-2", + action: "present", + timestamp: 1710000001000, + data: { + profileUpdate: { current: null }, + locationUpdate: { current: null }, + }, + }, + ], + }; + + beforeEach(() => { + const mock = getMockAblyRest(); + mock.request.mockClear(); + mock.request.mockResolvedValue(mockPresenceResponse); + }); + + standardHelpTests("spaces:get", import.meta.url); + standardArgValidationTests("spaces:get", import.meta.url, { + requiredArgs: ["test-space"], + }); + standardFlagTests("spaces:get", import.meta.url, ["--json"]); + + describe("functionality", () => { + it("should fetch space state via REST presence API", async () => { + const mock = getMockAblyRest(); + + const { stdout } = await runCommand( + ["spaces:get", "test-space"], + import.meta.url, + ); + + expect(mock.request).toHaveBeenCalledOnce(); + const callArgs = mock.request.mock.calls[0]; + expect(callArgs[0]).toBe("get"); + expect(callArgs[1]).toContain("test-space"); + expect(callArgs[1]).toContain("%3A%3A%24space"); + expect(stdout).toContain("test-space"); + }); + + it("should output JSON with correct envelope structure", async () => { + const { stdout } = await runCommand( + ["spaces:get", "test-space", "--json"], + import.meta.url, + ); + + const json = JSON.parse(stdout); + expect(json).toHaveProperty("type", "result"); + expect(json).toHaveProperty("success", true); + expect(json).toHaveProperty("space"); + expect(json.space).toHaveProperty("name", "test-space"); + expect(json.space).toHaveProperty("members"); + expect(json.space.members).toBeInstanceOf(Array); + expect(json.space.members.length).toBe(2); + expect(json.space.members[0]).toHaveProperty("clientId", "user-1"); + expect(json.space.members[0]).toHaveProperty("isConnected", true); + expect(json.space.members[0]).toHaveProperty("profileData"); + expect(json.space.members[0].profileData).toEqual({ name: "Alice" }); + }); + + it("should parse presence data fields correctly", async () => { + const { stdout } = await runCommand( + ["spaces:get", "test-space", "--json"], + import.meta.url, + ); + + const json = JSON.parse(stdout); + const member = json.space.members[0]; + expect(member.location).toEqual({ slide: 1 }); + expect(member.lastEvent).toEqual({ + name: "present", + timestamp: 1710000000000, + }); + }); + + it("should display members in human-readable format", async () => { + const { stdout } = await runCommand( + ["spaces:get", "test-space"], + import.meta.url, + ); + + expect(stdout).toContain("Space:"); + expect(stdout).toContain("test-space"); + expect(stdout).toContain("Members"); + expect(stdout).toContain("Client ID:"); + expect(stdout).toContain("user-1"); + expect(stdout).toContain("Connection ID:"); + expect(stdout).toContain("conn-1"); + }); + }); + + describe("error handling", () => { + it("should error when space has no members (not found)", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ statusCode: 200, items: [] }); + + const { error } = await runCommand( + ["spaces:get", "test-space"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("not found"); + }); + + it("should handle API errors gracefully", async () => { + const mock = getMockAblyRest(); + mock.request.mockRejectedValue(new Error("API error")); + + const { error } = await runCommand( + ["spaces:get", "test-space"], + import.meta.url, + ); + expect(error).toBeDefined(); + }); + + it("should handle non-200 status codes", async () => { + const mock = getMockAblyRest(); + mock.request.mockResolvedValue({ statusCode: 404, items: [] }); + + const { error } = await runCommand( + ["spaces:get", "test-space"], + import.meta.url, + ); + expect(error).toBeDefined(); + }); + }); +}); diff --git a/test/unit/commands/spaces/list.test.ts b/test/unit/commands/spaces/list.test.ts index 60919eb23..da253e5c7 100644 --- a/test/unit/commands/spaces/list.test.ts +++ b/test/unit/commands/spaces/list.test.ts @@ -107,9 +107,6 @@ describe("spaces:list command", () => { const json = JSON.parse(stdout); expect(json).toHaveProperty("spaces"); - expect(json).toHaveProperty("total"); - expect(json).toHaveProperty("shown"); - expect(json).toHaveProperty("hasMore"); expect(json).toHaveProperty("success", true); expect(json.spaces).toBeInstanceOf(Array); expect(json.spaces.length).toBe(2); diff --git a/test/unit/commands/spaces/locations/get-all.test.ts b/test/unit/commands/spaces/locations/get-all.test.ts index 7856093f6..5b4fd1919 100644 --- a/test/unit/commands/spaces/locations/get-all.test.ts +++ b/test/unit/commands/spaces/locations/get-all.test.ts @@ -26,34 +26,26 @@ describe("spaces:locations:get-all command", () => { it("should get all locations from a space", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue([ - { - member: { clientId: "user-1", connectionId: "conn-1" }, - currentLocation: { x: 100, y: 200 }, - previousLocation: null, - }, - ]); + space.locations.getAll.mockResolvedValue({ + "conn-1": { x: 100, y: 200 }, + }); const { stdout } = await runCommand( ["spaces:locations:get-all", "test-space", "--json"], import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); + expect(space.enter).not.toHaveBeenCalled(); expect(space.locations.getAll).toHaveBeenCalled(); - expect(stdout).toContain("test-space"); + expect(stdout).toContain("locations"); }); it("should output JSON envelope with type and command for location results", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue([ - { - member: { clientId: "user-1", connectionId: "conn-1" }, - currentLocation: { x: 100, y: 200 }, - previousLocation: null, - }, - ]); + space.locations.getAll.mockResolvedValue({ + "conn-1": { x: 100, y: 200 }, + }); const { stdout } = await runCommand( ["spaces:locations:get-all", "test-space", "--json"], @@ -68,14 +60,19 @@ describe("spaces:locations:get-all command", () => { expect(resultRecord).toHaveProperty("type", "result"); expect(resultRecord).toHaveProperty("command"); expect(resultRecord).toHaveProperty("success", true); - expect(resultRecord).toHaveProperty("spaceName", "test-space"); expect(resultRecord!.locations).toBeInstanceOf(Array); + expect(resultRecord!.locations.length).toBe(1); + expect(resultRecord!.locations[0]).toHaveProperty( + "connectionId", + "conn-1", + ); + expect(resultRecord!.locations[0]).toHaveProperty("location"); }); it("should handle no locations found", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue([]); + space.locations.getAll.mockResolvedValue({}); const { stdout } = await runCommand( ["spaces:locations:get-all", "test-space", "--json"], diff --git a/test/unit/commands/spaces/locations/set.test.ts b/test/unit/commands/spaces/locations/set.test.ts index 47148a6ea..6afd85a70 100644 --- a/test/unit/commands/spaces/locations/set.test.ts +++ b/test/unit/commands/spaces/locations/set.test.ts @@ -90,7 +90,7 @@ describe("spaces:locations:set command", () => { }); describe("JSON output", () => { - it("should output JSON on success with --duration 0", async () => { + it("should output JSON on success", async () => { const spacesMock = getMockAblySpaces(); spacesMock._getSpace("test-space"); @@ -103,8 +103,6 @@ describe("spaces:locations:set command", () => { "--location", JSON.stringify(location), "--json", - "--duration", - "0", ], import.meta.url, ); @@ -112,7 +110,6 @@ describe("spaces:locations:set command", () => { const result = JSON.parse(stdout); expect(result.success).toBe(true); expect(result.location).toEqual(location); - expect(result.spaceName).toBe("test-space"); }); it("should output JSON error on invalid location", async () => { diff --git a/test/unit/commands/spaces/locations/subscribe.test.ts b/test/unit/commands/spaces/locations/subscribe.test.ts index 10167d06c..20f931996 100644 --- a/test/unit/commands/spaces/locations/subscribe.test.ts +++ b/test/unit/commands/spaces/locations/subscribe.test.ts @@ -26,24 +26,22 @@ describe("spaces:locations:subscribe command", () => { it("should subscribe to location updates in a space", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue({}); await runCommand( ["spaces:locations:subscribe", "test-space"], import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); + expect(space.enter).not.toHaveBeenCalled(); expect(space.locations.subscribe).toHaveBeenCalledWith( "update", expect.any(Function), ); }); - it("should display initial subscription message", async () => { + it("should display initial subscription message without fetching current locations", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue({}); const { stdout } = await runCommand( ["spaces:locations:subscribe", "test-space"], @@ -51,61 +49,96 @@ describe("spaces:locations:subscribe command", () => { ); expect(stdout).toContain("Subscribing to location updates"); - expect(stdout).toContain("test-space"); + expect(space.locations.getAll).not.toHaveBeenCalled(); }); - it("should fetch and display current locations", async () => { + it("should output location updates in block format", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue({ - "conn-1": { room: "lobby", x: 100 }, - "conn-2": { room: "chat", x: 200 }, - }); - const { stdout } = await runCommand( + // Capture the subscribe handler and invoke it with a mock update + let locationHandler: ((update: unknown) => void) | undefined; + space.locations.subscribe.mockImplementation( + (_event: string, handler: (update: unknown) => void) => { + locationHandler = handler; + }, + ); + + const runPromise = runCommand( ["spaces:locations:subscribe", "test-space"], import.meta.url, ); - expect(space.locations.getAll).toHaveBeenCalled(); - expect(stdout).toContain("Current locations"); + // Wait a tick for the subscribe to be set up + await new Promise((resolve) => setTimeout(resolve, 50)); + + if (locationHandler) { + locationHandler({ + member: { + clientId: "user-1", + connectionId: "conn-1", + }, + currentLocation: { room: "lobby" }, + previousLocation: { room: "entrance" }, + }); + } + + const { stdout } = await runPromise; + + expect(stdout).toContain("Client ID:"); + expect(stdout).toContain("Connection ID:"); + expect(stdout).toContain("Current Location:"); + expect(stdout).toContain("Previous Location:"); }); }); describe("JSON output", () => { - it("should output JSON envelope with initial locations snapshot", async () => { + it("should output JSON event envelope for location updates", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue({ - "conn-1": { room: "lobby", x: 100 }, - }); - const { stdout } = await runCommand( + let locationHandler: ((update: unknown) => void) | undefined; + space.locations.subscribe.mockImplementation( + (_event: string, handler: (update: unknown) => void) => { + locationHandler = handler; + }, + ); + + const runPromise = runCommand( ["spaces:locations:subscribe", "test-space", "--json"], import.meta.url, ); + await new Promise((resolve) => setTimeout(resolve, 50)); + + if (locationHandler) { + locationHandler({ + member: { + clientId: "user-1", + connectionId: "conn-1", + }, + currentLocation: { room: "lobby" }, + previousLocation: null, + }); + } + + const { stdout } = await runPromise; + const records = parseNdjsonLines(stdout); - const resultRecord = records.find( - (r) => - r.type === "result" && - r.eventType === "locations_snapshot" && - Array.isArray(r.locations), - ); - expect(resultRecord).toBeDefined(); - expect(resultRecord).toHaveProperty("command"); - expect(resultRecord).toHaveProperty("success", true); - expect(resultRecord).toHaveProperty("spaceName", "test-space"); - expect(resultRecord!.locations).toBeInstanceOf(Array); + const eventRecord = records.find((r) => r.type === "event" && r.location); + expect(eventRecord).toBeDefined(); + expect(eventRecord).toHaveProperty("command"); + expect(eventRecord!.location).toHaveProperty("member"); + expect(eventRecord!.location.member).toHaveProperty("clientId", "user-1"); + expect(eventRecord!.location).toHaveProperty("currentLocation"); + expect(eventRecord!.location).toHaveProperty("previousLocation"); }); }); describe("cleanup behavior", () => { it("should close client on completion", async () => { const realtimeMock = getMockAblyRealtime(); - const spacesMock = getMockAblySpaces(); - const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockResolvedValue({}); + getMockAblySpaces(); // Use SIGINT to exit @@ -120,24 +153,20 @@ describe("spaces:locations:subscribe command", () => { }); describe("error handling", () => { - it("should handle getAll rejection gracefully", async () => { + it("should handle subscribe error gracefully", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locations.getAll.mockRejectedValue( - new Error("Failed to get locations"), - ); + space.locations.subscribe.mockImplementation(() => { + throw new Error("Failed to subscribe to locations"); + }); - // The command handles the error via fail and exits const { error } = await runCommand( ["spaces:locations:subscribe", "test-space"], import.meta.url, ); - // Command should report the error expect(error).toBeDefined(); - expect(error?.message).toContain("Failed to get locations"); - // Command should NOT continue to subscribe after getAll fails - expect(space.locations.subscribe).not.toHaveBeenCalled(); + expect(error?.message).toContain("Failed to subscribe to locations"); }); }); }); diff --git a/test/unit/commands/spaces/locks/acquire.test.ts b/test/unit/commands/spaces/locks/acquire.test.ts index 9be20aa3b..894705420 100644 --- a/test/unit/commands/spaces/locks/acquire.test.ts +++ b/test/unit/commands/spaces/locks/acquire.test.ts @@ -27,8 +27,16 @@ describe("spaces:locks:acquire command", () => { space.locks.acquire.mockResolvedValue({ id: "my-lock", status: "locked", - member: { clientId: "mock-client-id", connectionId: "conn-1" }, + member: { + clientId: "mock-client-id", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, timestamp: Date.now(), + attributes: undefined, reason: undefined, }); @@ -49,8 +57,17 @@ describe("spaces:locks:acquire command", () => { space.locks.acquire.mockResolvedValue({ id: "my-lock", status: "locked", - member: { clientId: "mock-client-id", connectionId: "conn-1" }, + member: { + clientId: "mock-client-id", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, timestamp: Date.now(), + attributes: undefined, + reason: undefined, }); const { stdout } = await runCommand( @@ -100,8 +117,17 @@ describe("spaces:locks:acquire command", () => { space.locks.acquire.mockResolvedValue({ id: "my-lock", status: "locked", - member: { clientId: "mock-client-id", connectionId: "conn-1" }, + member: { + clientId: "mock-client-id", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: 1700000000000 }, + }, timestamp: 1700000000000, + attributes: undefined, + reason: undefined, }); const { stdout } = await runCommand( @@ -111,9 +137,17 @@ describe("spaces:locks:acquire command", () => { const result = JSON.parse(stdout); expect(result).toHaveProperty("success", true); - expect(result).toHaveProperty("lock"); - expect(result.lock).toHaveProperty("lockId", "my-lock"); - expect(result.lock).toHaveProperty("status", "locked"); + expect(result).toHaveProperty("locks"); + expect(result.locks).toHaveLength(1); + expect(result.locks[0]).toHaveProperty("id", "my-lock"); + expect(result.locks[0]).toHaveProperty("status", "locked"); + expect(result.locks[0]).toHaveProperty("member"); + expect(result.locks[0].member).toHaveProperty( + "clientId", + "mock-client-id", + ); + expect(result.locks[0]).toHaveProperty("attributes", null); + expect(result.locks[0]).toHaveProperty("reason", null); }); }); diff --git a/test/unit/commands/spaces/locks/get-all.test.ts b/test/unit/commands/spaces/locks/get-all.test.ts index ea3d8391b..966cbe0ca 100644 --- a/test/unit/commands/spaces/locks/get-all.test.ts +++ b/test/unit/commands/spaces/locks/get-all.test.ts @@ -29,8 +29,18 @@ describe("spaces:locks:get-all command", () => { space.locks.getAll.mockResolvedValue([ { id: "lock-1", - member: { clientId: "user-1", connectionId: "conn-1" }, + member: { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, status: "locked", + timestamp: Date.now(), + attributes: undefined, + reason: undefined, }, ]); @@ -39,9 +49,9 @@ describe("spaces:locks:get-all command", () => { import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); + expect(space.enter).not.toHaveBeenCalled(); expect(space.locks.getAll).toHaveBeenCalled(); - expect(stdout).toContain("test-space"); + expect(stdout).toContain("locks"); }); it("should output JSON envelope with type and command for lock results", async () => { @@ -50,8 +60,18 @@ describe("spaces:locks:get-all command", () => { space.locks.getAll.mockResolvedValue([ { id: "lock-1", - member: { clientId: "user-1", connectionId: "conn-1" }, + member: { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, status: "locked", + timestamp: Date.now(), + attributes: undefined, + reason: undefined, }, ]); @@ -68,8 +88,13 @@ describe("spaces:locks:get-all command", () => { expect(resultRecord).toHaveProperty("type", "result"); expect(resultRecord).toHaveProperty("command"); expect(resultRecord).toHaveProperty("success", true); - expect(resultRecord).toHaveProperty("spaceName", "test-space"); expect(resultRecord!.locks).toBeInstanceOf(Array); + expect(resultRecord!.locks[0]).toHaveProperty("id", "lock-1"); + expect(resultRecord!.locks[0]).toHaveProperty("member"); + expect(resultRecord!.locks[0].member).toHaveProperty( + "clientId", + "user-1", + ); }); it("should handle no locks found", async () => { diff --git a/test/unit/commands/spaces/locks/get.test.ts b/test/unit/commands/spaces/locks/get.test.ts index c8054e5d6..ab0592182 100644 --- a/test/unit/commands/spaces/locks/get.test.ts +++ b/test/unit/commands/spaces/locks/get.test.ts @@ -53,8 +53,18 @@ describe("spaces:locks:get command", () => { const space = spacesMock._getSpace("test-space"); space.locks.get.mockResolvedValue({ id: "my-lock", - member: { clientId: "user-1", connectionId: "conn-1" }, + member: { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, status: "locked", + timestamp: Date.now(), + attributes: undefined, + reason: undefined, }); const { stdout } = await runCommand( @@ -62,7 +72,7 @@ describe("spaces:locks:get command", () => { import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); + expect(space.enter).not.toHaveBeenCalled(); expect(space.locks.get).toHaveBeenCalledWith("my-lock"); expect(stdout).toContain("my-lock"); }); @@ -72,8 +82,18 @@ describe("spaces:locks:get command", () => { const space = spacesMock._getSpace("test-space"); space.locks.get.mockResolvedValue({ id: "my-lock", - member: { clientId: "user-1", connectionId: "conn-1" }, + member: { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, status: "locked", + timestamp: Date.now(), + attributes: undefined, + reason: undefined, }); const { stdout } = await runCommand( @@ -83,13 +103,18 @@ describe("spaces:locks:get command", () => { const records = parseNdjsonLines(stdout); const resultRecord = records.find( - (r) => r.type === "result" && r.id === "my-lock", + (r) => r.type === "result" && Array.isArray(r.locks), ); expect(resultRecord).toBeDefined(); expect(resultRecord).toHaveProperty("type", "result"); expect(resultRecord).toHaveProperty("command", "spaces:locks:get"); expect(resultRecord).toHaveProperty("success", true); - expect(resultRecord).toHaveProperty("status", "locked"); + expect(resultRecord!.locks).toHaveLength(1); + expect(resultRecord!.locks[0]).toHaveProperty("id", "my-lock"); + expect(resultRecord!.locks[0]).toHaveProperty("status", "locked"); + expect(resultRecord!.locks[0]).toHaveProperty("member"); + expect(resultRecord!.locks[0]).toHaveProperty("attributes", null); + expect(resultRecord!.locks[0]).toHaveProperty("reason", null); }); it("should handle lock not found", async () => { @@ -103,7 +128,12 @@ describe("spaces:locks:get command", () => { ); expect(space.locks.get).toHaveBeenCalledWith("nonexistent-lock"); - expect(stdout).toBeDefined(); + const records = parseNdjsonLines(stdout); + const resultRecord = records.find( + (r) => r.type === "result" && Array.isArray(r.locks), + ); + expect(resultRecord).toBeDefined(); + expect(resultRecord!.locks).toEqual([]); }); }); diff --git a/test/unit/commands/spaces/locks/subscribe.test.ts b/test/unit/commands/spaces/locks/subscribe.test.ts index 77476dc5d..fcd22308e 100644 --- a/test/unit/commands/spaces/locks/subscribe.test.ts +++ b/test/unit/commands/spaces/locks/subscribe.test.ts @@ -26,21 +26,19 @@ describe("spaces:locks:subscribe command", () => { it("should subscribe to lock events in a space", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locks.getAll.mockResolvedValue([]); await runCommand( ["spaces:locks:subscribe", "test-space"], import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); + expect(space.enter).not.toHaveBeenCalled(); expect(space.locks.subscribe).toHaveBeenCalledWith(expect.any(Function)); }); - it("should display initial subscription message", async () => { + it("should display listening message without fetching initial locks", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locks.getAll.mockResolvedValue([]); const { stdout } = await runCommand( ["spaces:locks:subscribe", "test-space"], @@ -48,60 +46,75 @@ describe("spaces:locks:subscribe command", () => { ); expect(stdout).toContain("Subscribing to lock events"); - expect(stdout).toContain("test-space"); + expect(space.locks.getAll).not.toHaveBeenCalled(); }); - it("should fetch and display current locks", async () => { + it("should output lock events using block format", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locks.getAll.mockResolvedValue([ - { - id: "lock-1", - status: "locked", - member: { clientId: "user-1", connectionId: "conn-1" }, - }, - { - id: "lock-2", - status: "pending", - member: { clientId: "user-2", connectionId: "conn-2" }, - }, - ]); - const { stdout } = await runCommand( - ["spaces:locks:subscribe", "test-space"], - import.meta.url, + // Capture the subscribe callback and invoke it with a lock event + space.locks.subscribe.mockImplementation( + (callback: (lock: unknown) => void) => { + callback({ + id: "lock-1", + status: "locked", + member: { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, + timestamp: Date.now(), + attributes: undefined, + reason: undefined, + }); + return Promise.resolve(); + }, ); - expect(space.locks.getAll).toHaveBeenCalled(); - expect(stdout).toContain("Current locks"); - expect(stdout).toContain("lock-1"); - }); - - it("should show message when no locks exist", async () => { - const spacesMock = getMockAblySpaces(); - const space = spacesMock._getSpace("test-space"); - space.locks.getAll.mockResolvedValue([]); - const { stdout } = await runCommand( ["spaces:locks:subscribe", "test-space"], import.meta.url, ); - expect(stdout).toContain("No locks"); + expect(stdout).toContain("Lock ID:"); + expect(stdout).toContain("lock-1"); + expect(stdout).toContain("Status:"); + expect(stdout).toContain("locked"); + expect(stdout).toContain("Member:"); + expect(stdout).toContain("user-1"); }); }); describe("JSON output", () => { - it("should output JSON envelope with initial locks snapshot", async () => { + it("should output JSON event envelope for lock events", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locks.getAll.mockResolvedValue([ - { - id: "lock-1", - status: "locked", - member: { clientId: "user-1", connectionId: "conn-1" }, + + // Capture the subscribe callback and invoke it with a lock event + space.locks.subscribe.mockImplementation( + (callback: (lock: unknown) => void) => { + callback({ + id: "lock-1", + status: "locked", + member: { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, + timestamp: Date.now(), + attributes: undefined, + reason: undefined, + }); + return Promise.resolve(); }, - ]); + ); const { stdout } = await runCommand( ["spaces:locks:subscribe", "test-space", "--json"], @@ -109,26 +122,20 @@ describe("spaces:locks:subscribe command", () => { ); const records = parseNdjsonLines(stdout); - const resultRecord = records.find( - (r) => r.type === "result" && Array.isArray(r.locks), - ); - expect(resultRecord).toBeDefined(); - expect(resultRecord).toHaveProperty("type", "result"); - expect(resultRecord).toHaveProperty("command"); - expect(resultRecord).toHaveProperty("success", true); - expect(resultRecord).toHaveProperty("spaceName", "test-space"); - expect(resultRecord!.locks).toBeInstanceOf(Array); + const eventRecord = records.find((r) => r.type === "event" && r.lock); + expect(eventRecord).toBeDefined(); + expect(eventRecord).toHaveProperty("type", "event"); + expect(eventRecord).toHaveProperty("command"); + expect(eventRecord!.lock).toHaveProperty("id", "lock-1"); + expect(eventRecord!.lock).toHaveProperty("status", "locked"); + expect(eventRecord!.lock).toHaveProperty("member"); }); }); describe("cleanup behavior", () => { it("should close client on completion", async () => { const realtimeMock = getMockAblyRealtime(); - const spacesMock = getMockAblySpaces(); - const space = spacesMock._getSpace("test-space"); - space.locks.getAll.mockResolvedValue([]); - - // Use SIGINT to exit + getMockAblySpaces(); await runCommand( ["spaces:locks:subscribe", "test-space"], @@ -141,19 +148,20 @@ describe("spaces:locks:subscribe command", () => { }); describe("error handling", () => { - it("should handle getAll rejection gracefully", async () => { + it("should handle subscribe rejection gracefully", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.locks.getAll.mockRejectedValue(new Error("Failed to get locks")); + space.locks.subscribe.mockRejectedValue( + new Error("Failed to subscribe to locks"), + ); - // The command catches errors and continues - const { stdout } = await runCommand( + const { error } = await runCommand( ["spaces:locks:subscribe", "test-space"], import.meta.url, ); - // Command should have run (output should be present) - expect(stdout).toBeDefined(); + // Command should have attempted to run and reported the error + expect(error).toBeDefined(); }); }); }); diff --git a/test/unit/commands/spaces/members/enter.test.ts b/test/unit/commands/spaces/members/enter.test.ts index eccb28cd2..c90d229f5 100644 --- a/test/unit/commands/spaces/members/enter.test.ts +++ b/test/unit/commands/spaces/members/enter.test.ts @@ -62,20 +62,6 @@ describe("spaces:members:enter command", () => { }); }); - describe("member event handling", () => { - it("should subscribe to member update events", async () => { - const spacesMock = getMockAblySpaces(); - const space = spacesMock._getSpace("test-space"); - - await runCommand(["spaces:members:enter", "test-space"], import.meta.url); - - expect(space.members.subscribe).toHaveBeenCalledWith( - "update", - expect.any(Function), - ); - }); - }); - describe("JSON output", () => { it("should output JSON on success", async () => { const spacesMock = getMockAblySpaces(); @@ -88,8 +74,16 @@ describe("spaces:members:enter command", () => { const result = JSON.parse(stdout); expect(result.success).toBe(true); - expect(result.spaceName).toBe("test-space"); - expect(result.status).toBe("connected"); + expect(result.members).toBeDefined(); + expect(result.members).toHaveLength(1); + expect(result.members[0]).toHaveProperty("clientId", "mock-client-id"); + expect(result.members[0]).toHaveProperty( + "connectionId", + "mock-connection-id", + ); + expect(result.members[0]).toHaveProperty("isConnected", true); + expect(result.members[0]).toHaveProperty("location", null); + expect(result.members[0]).toHaveProperty("lastEvent"); }); it("should output JSON error on invalid profile", async () => { diff --git a/test/unit/commands/spaces/members/get-all.test.ts b/test/unit/commands/spaces/members/get-all.test.ts new file mode 100644 index 000000000..fd7a98364 --- /dev/null +++ b/test/unit/commands/spaces/members/get-all.test.ts @@ -0,0 +1,147 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { runCommand } from "@oclif/test"; +import { getMockAblySpaces } from "../../../../helpers/mock-ably-spaces.js"; +import { getMockAblyRealtime } from "../../../../helpers/mock-ably-realtime.js"; +import { parseNdjsonLines } from "../../../../helpers/ndjson.js"; +import { + standardHelpTests, + standardArgValidationTests, + standardFlagTests, +} from "../../../../helpers/standard-tests.js"; + +describe("spaces:members:get-all command", () => { + beforeEach(() => { + getMockAblyRealtime(); + getMockAblySpaces(); + }); + + standardHelpTests("spaces:members:get-all", import.meta.url); + standardArgValidationTests("spaces:members:get-all", import.meta.url, { + requiredArgs: ["test-space"], + }); + standardFlagTests("spaces:members:get-all", import.meta.url, ["--json"]); + + describe("functionality", () => { + it("should get all members from a space", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + space.members.getAll.mockResolvedValue([ + { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: { name: "Alice" }, + location: null, + lastEvent: { name: "enter", timestamp: 1710000000000 }, + }, + ]); + + const { stdout } = await runCommand( + ["spaces:members:get-all", "test-space", "--json"], + import.meta.url, + ); + + expect(space.enter).not.toHaveBeenCalled(); + expect(space.members.getAll).toHaveBeenCalled(); + expect(stdout).toContain("members"); + }); + + it("should output JSON envelope with type and command for member results", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + space.members.getAll.mockResolvedValue([ + { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: { name: "Alice" }, + location: { slide: 1 }, + lastEvent: { name: "enter", timestamp: 1710000000000 }, + }, + { + clientId: "user-2", + connectionId: "conn-2", + isConnected: true, + profileData: null, + location: null, + lastEvent: { name: "enter", timestamp: 1710000001000 }, + }, + ]); + + const { stdout } = await runCommand( + ["spaces:members:get-all", "test-space", "--json"], + import.meta.url, + ); + + const records = parseNdjsonLines(stdout); + const resultRecord = records.find( + (r) => r.type === "result" && Array.isArray(r.members), + ); + expect(resultRecord).toBeDefined(); + expect(resultRecord).toHaveProperty("type", "result"); + expect(resultRecord).toHaveProperty("command"); + expect(resultRecord).toHaveProperty("success", true); + expect(resultRecord!.members).toBeInstanceOf(Array); + expect((resultRecord!.members as unknown[]).length).toBe(2); + expect( + (resultRecord!.members as Record[])[0], + ).toHaveProperty("clientId", "user-1"); + }); + + it("should handle no members found", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + space.members.getAll.mockResolvedValue([]); + + const { stdout } = await runCommand( + ["spaces:members:get-all", "test-space"], + import.meta.url, + ); + + expect(stdout).toContain("No members currently in this space"); + }); + + it("should display members in human-readable format", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + space.members.getAll.mockResolvedValue([ + { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: { name: "Alice" }, + location: null, + lastEvent: { name: "enter", timestamp: 1710000000000 }, + }, + ]); + + const { stdout } = await runCommand( + ["spaces:members:get-all", "test-space"], + import.meta.url, + ); + + expect(stdout).toContain("Current members"); + expect(stdout).toContain("Client ID:"); + expect(stdout).toContain("user-1"); + expect(stdout).toContain("Connection ID:"); + expect(stdout).toContain("conn-1"); + expect(stdout).toContain("Connected:"); + }); + }); + + describe("error handling", () => { + it("should handle errors gracefully", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + space.members.getAll.mockRejectedValue( + new Error("Failed to get members"), + ); + + const { error } = await runCommand( + ["spaces:members:get-all", "test-space"], + import.meta.url, + ); + expect(error).toBeDefined(); + }); + }); +}); diff --git a/test/unit/commands/spaces/members/subscribe.test.ts b/test/unit/commands/spaces/members/subscribe.test.ts index 11ccbbf54..602ace389 100644 --- a/test/unit/commands/spaces/members/subscribe.test.ts +++ b/test/unit/commands/spaces/members/subscribe.test.ts @@ -21,67 +21,40 @@ describe("spaces:members:subscribe command", () => { standardFlagTests("spaces:members:subscribe", import.meta.url, ["--json"]); describe("functionality", () => { - it("should display current members from getAll()", async () => { + it("should subscribe to member events and output in block format", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.members.getAll.mockResolvedValue([ - { - clientId: "user-1", - connectionId: "conn-1", - isConnected: true, - profileData: {}, - }, - { - clientId: "user-2", - connectionId: "conn-2", - isConnected: true, - profileData: {}, - }, - ]); - - const { stdout } = await runCommand( - ["spaces:members:subscribe", "test-space"], - import.meta.url, - ); - - expect(space.members.getAll).toHaveBeenCalled(); - expect(stdout).toContain("Current members"); - expect(stdout).toContain("user-1"); - expect(stdout).toContain("user-2"); - }); - it("should show profile data for members", async () => { - const spacesMock = getMockAblySpaces(); - const space = spacesMock._getSpace("test-space"); - space.members.getAll.mockResolvedValue([ - { - clientId: "user-1", - connectionId: "conn-1", - isConnected: true, - profileData: { name: "Alice", role: "admin" }, + // Emit a member event after subscription is set up + space.members.subscribe.mockImplementation( + (event: string, cb: (member: unknown) => void) => { + // Fire the callback asynchronously to simulate an incoming event + setTimeout(() => { + cb({ + clientId: "user-1", + connectionId: "other-conn-1", + isConnected: true, + profileData: { name: "Alice" }, + location: null, + lastEvent: { name: "update", timestamp: Date.now() }, + }); + }, 10); + return Promise.resolve(); }, - ]); - - const { stdout } = await runCommand( - ["spaces:members:subscribe", "test-space"], - import.meta.url, ); - expect(stdout).toContain("Alice"); - expect(stdout).toContain("admin"); - }); - - it("should show message when no members are present", async () => { - const spacesMock = getMockAblySpaces(); - const space = spacesMock._getSpace("test-space"); - space.members.getAll.mockResolvedValue([]); - const { stdout } = await runCommand( ["spaces:members:subscribe", "test-space"], import.meta.url, ); - expect(stdout).toContain("No members are currently present"); + expect(stdout).toContain("Action:"); + expect(stdout).toContain("update"); + expect(stdout).toContain("Client ID:"); + expect(stdout).toContain("user-1"); + expect(stdout).toContain("Connection ID:"); + expect(stdout).toContain("other-conn-1"); + expect(stdout).toContain("Connected:"); }); }); @@ -89,14 +62,13 @@ describe("spaces:members:subscribe command", () => { it("should subscribe to member update events", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.members.getAll.mockResolvedValue([]); await runCommand( ["spaces:members:subscribe", "test-space"], import.meta.url, ); - expect(space.enter).toHaveBeenCalled(); + expect(space.enter).not.toHaveBeenCalled(); expect(space.members.subscribe).toHaveBeenCalledWith( "update", expect.any(Function), @@ -105,17 +77,25 @@ describe("spaces:members:subscribe command", () => { }); describe("JSON output", () => { - it("should output JSON for initial members", async () => { + it("should output JSON event for member updates", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.members.getAll.mockResolvedValue([ - { - clientId: "user-1", - connectionId: "conn-1", - isConnected: true, - profileData: { name: "Alice" }, + + space.members.subscribe.mockImplementation( + (event: string, cb: (member: unknown) => void) => { + setTimeout(() => { + cb({ + clientId: "user-1", + connectionId: "other-conn-1", + isConnected: true, + profileData: { name: "Alice" }, + location: null, + lastEvent: { name: "update", timestamp: Date.now() }, + }); + }, 10); + return Promise.resolve(); }, - ]); + ); const { stdout } = await runCommand( ["spaces:members:subscribe", "test-space", "--json"], @@ -123,9 +103,9 @@ describe("spaces:members:subscribe command", () => { ); const result = JSON.parse(stdout); - expect(result.success).toBe(true); - expect(result.members).toHaveLength(1); - expect(result.members[0].clientId).toBe("user-1"); + expect(result.type).toBe("event"); + expect(result.member).toBeDefined(); + expect(result.member.clientId).toBe("user-1"); }); }); @@ -133,7 +113,7 @@ describe("spaces:members:subscribe command", () => { it("should handle errors gracefully", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); - space.enter.mockRejectedValue(new Error("Connection failed")); + space.members.subscribe.mockRejectedValue(new Error("Connection failed")); const { error } = await runCommand( ["spaces:members:subscribe", "test-space"], diff --git a/test/unit/commands/spaces/subscribe.test.ts b/test/unit/commands/spaces/subscribe.test.ts new file mode 100644 index 000000000..88d219334 --- /dev/null +++ b/test/unit/commands/spaces/subscribe.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { runCommand } from "@oclif/test"; +import { getMockAblySpaces } from "../../../helpers/mock-ably-spaces.js"; +import { getMockAblyRealtime } from "../../../helpers/mock-ably-realtime.js"; +import { + standardHelpTests, + standardArgValidationTests, + standardFlagTests, +} from "../../../helpers/standard-tests.js"; + +describe("spaces:subscribe command", () => { + beforeEach(() => { + getMockAblyRealtime(); + getMockAblySpaces(); + }); + + standardHelpTests("spaces:subscribe", import.meta.url); + standardArgValidationTests("spaces:subscribe", import.meta.url, { + requiredArgs: ["test-space"], + }); + standardFlagTests("spaces:subscribe", import.meta.url, ["--json"]); + + describe("functionality", () => { + it("should subscribe to space update events and output in block format", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + + space.subscribe.mockImplementation( + (event: string, cb: (state: unknown) => void) => { + setTimeout(() => { + cb({ + members: [ + { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: { name: "Alice" }, + location: null, + lastEvent: { name: "enter", timestamp: Date.now() }, + }, + ], + }); + }, 10); + }, + ); + + const { stdout } = await runCommand( + ["spaces:subscribe", "test-space"], + import.meta.url, + ); + + expect(stdout).toContain("Space update"); + expect(stdout).toContain("Client ID:"); + expect(stdout).toContain("user-1"); + expect(stdout).toContain("Connection ID:"); + expect(stdout).toContain("conn-1"); + }); + + it("should subscribe to update events without entering space", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + + await runCommand(["spaces:subscribe", "test-space"], import.meta.url); + + expect(space.enter).not.toHaveBeenCalled(); + expect(space.subscribe).toHaveBeenCalledWith( + "update", + expect.any(Function), + ); + }); + }); + + describe("JSON output", () => { + it("should output JSON event for space updates", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + + space.subscribe.mockImplementation( + (event: string, cb: (state: unknown) => void) => { + setTimeout(() => { + cb({ + members: [ + { + clientId: "user-1", + connectionId: "conn-1", + isConnected: true, + profileData: { name: "Alice" }, + location: null, + lastEvent: { name: "enter", timestamp: 1710000000000 }, + }, + ], + }); + }, 10); + }, + ); + + const { stdout } = await runCommand( + ["spaces:subscribe", "test-space", "--json"], + import.meta.url, + ); + + const result = JSON.parse(stdout); + expect(result.type).toBe("event"); + expect(result.members).toBeDefined(); + expect(result.members).toBeInstanceOf(Array); + expect(result.members[0].clientId).toBe("user-1"); + }); + }); + + describe("error handling", () => { + it("should handle subscription errors gracefully", async () => { + const spacesMock = getMockAblySpaces(); + const space = spacesMock._getSpace("test-space"); + space.subscribe.mockImplementation(() => { + throw new Error("Subscription failed"); + }); + + const { error } = await runCommand( + ["spaces:subscribe", "test-space"], + import.meta.url, + ); + expect(error).toBeDefined(); + expect(error?.message).toContain("Subscription failed"); + }); + }); +});