Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .claude/skills/ably-codebase-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,19 @@ Launch these agents **in parallel**. Each agent gets a focused mandate and uses
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.
7. **Check** hold commands (set, enter, acquire) emit `logJsonStatus("holding", ...)` after `logJsonResult` — this signals to JSON consumers that the command is alive and waiting for Ctrl+C / `--duration`
7. **Check** hold commands (set, enter, acquire) emit `logJsonStatus(JsonStatusType.Holding, ...)` after `logJsonResult`
8. **Check** subscribe commands emit `logJsonStatus(JsonStatusType.Subscribing, ...)` when connecting and `logJsonStatus(JsonStatusType.Listening, ...)` when attached. Room subscribe commands should pass `subscribingMessage` and `listeningMessage` to `setupRoomStatusHandler`.
9. **Grep** for `logJsonStatus("` — all calls must use `JsonStatusType` enum, not raw strings

**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
- Hold commands missing `logJsonStatus` after `logJsonResult` are deviations — JSON consumers need the hold signal
- Hold commands missing `logJsonStatus` after `logJsonResult` are deviations
- Subscribe commands missing `logJsonStatus(Subscribing/Listening)` are deviations
- Raw string arguments to `logJsonStatus` are deviations — use `JsonStatusType` enum

### Agent 6: Test Pattern Sweep

Expand Down
46 changes: 35 additions & 11 deletions .claude/skills/ably-new-command/references/patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ static override flags = {
```

```typescript
import { JsonStatusType } from "../../utils/json-status.js";

async run(): Promise<void> {
const { args, flags } = await this.parse(MySubscribeCommand);

Expand All @@ -47,13 +49,8 @@ async run(): Promise<void> {
if (!this.shouldOutputJson(flags)) {
this.log(formatProgress("Attaching to channel: " + formatResource(args.channel)));
}

channel.once("attached", () => {
if (!this.shouldOutputJson(flags)) {
this.log(formatSuccess("Attached to channel: " + formatResource(args.channel) + "."));
this.log(formatListening("Listening for events."));
}
});
// JSON subscribe status — logJsonStatus has built-in shouldOutputJson guard
this.logJsonStatus(JsonStatusType.Subscribing, `Subscribing to channel: ${args.channel}.`, flags);

let sequenceCounter = 0;
await channel.subscribe((message) => {
Expand Down Expand Up @@ -89,10 +86,35 @@ async run(): Promise<void> {
}
});

if (!this.shouldOutputJson(flags)) {
this.log(formatSuccess("Subscribed to channel: " + formatResource(args.channel) + "."));
this.log(formatListening("Listening for events."));
}
// JSON listening status — emitted after successful attach
this.logJsonStatus(JsonStatusType.Listening, "Listening for events.", flags);

await this.waitAndTrackCleanup(flags, "subscribe", flags.duration);
}
```

**JSON status lifecycle for subscribe commands:**
All subscribe commands must emit two JSON status messages so consumers can distinguish "connecting" from "ready":
```jsonl
{"type":"status","command":"channels:subscribe","status":"subscribing","message":"Subscribing to channel: my-channel."}
{"type":"status","command":"channels:subscribe","status":"listening","message":"Listening for events."}
{"type":"event","command":"channels:subscribe","message":{...}}
```

For **room commands** (extending `ChatBaseCommand`), pass `subscribingMessage` and `listeningMessage` to `setupRoomStatusHandler` — the handler emits both statuses automatically:
```typescript
this.setupRoomStatusHandler(room, flags, {
roomName,
successMessage: `Subscribed to room: ${formatResource(roomName)}.`,
listeningMessage: "Listening for messages.",
subscribingMessage: `Subscribing to room: ${roomName}.`,
});
```

---

## Publish/Send Pattern
Expand Down Expand Up @@ -669,7 +691,7 @@ Commands must behave strictly according to their documented purpose — no unint

**Set / enter / acquire commands** — hold state until Ctrl+C / `--duration`:
- Enter space (manual: `enterSpace: false` + `space.enter()` + `markAsEntered()`), perform operation, output confirmation, then hold with `waitAndTrackCleanup`
- Emit `formatListening("Holding <thing>.")` (human) and `logJsonStatus("holding", ...)` (JSON)
- Emit `formatListening("Holding <thing>.")` (human) and `logJsonStatus(JsonStatusType.Holding, ...)` (JSON)
- **NOT subscribe** to other events — that is what subscribe commands are for

**Side-effect rules:**
Expand Down Expand Up @@ -700,7 +722,7 @@ this.markAsEntered();
await this.space!.locations.set(location);
// output result...
this.log(formatListening("Holding location."));
this.logJsonStatus("holding", "Holding location. Press Ctrl+C to exit.", flags);
this.logJsonStatus(JsonStatusType.Holding, "Holding location. Press Ctrl+C to exit.", flags);
await this.waitAndTrackCleanup(flags, "location", flags.duration);
```

Expand Down Expand Up @@ -747,9 +769,11 @@ Metadata fields (`total`, `timestamp`, `hasMore`, `appId`) may sit alongside the

### Hold status for long-running commands (logJsonStatus)

Long-running commands that hold state (e.g. `spaces members enter`, `spaces locations set`, `spaces locks acquire`, `spaces cursors set`) must emit a status line after the result so JSON consumers know the command is alive and waiting:
Long-running commands that hold state (e.g. `spaces members enter`, `spaces locations set`, `spaces locks acquire`, `spaces cursors set`) must emit a status line after the result so JSON consumers know the command is alive and waiting. Always use the `JsonStatusType` enum from `src/utils/json-status.ts` — never pass raw strings:

```typescript
import { JsonStatusType } from "../../../utils/json-status.js";

// After the result output:
if (this.shouldOutputJson(flags)) {
this.logJsonResult({ member: formatMemberOutput(self!) }, flags);
Expand All @@ -760,7 +784,7 @@ if (this.shouldOutputJson(flags)) {
}

// logJsonStatus has built-in shouldOutputJson guard — no outer if needed
this.logJsonStatus("holding", "Holding presence. Press Ctrl+C to exit.", flags);
this.logJsonStatus(JsonStatusType.Holding, "Holding presence. Press Ctrl+C to exit.", flags);

await this.waitAndTrackCleanup(flags, "member", flags.duration);
```
Expand Down
4 changes: 3 additions & 1 deletion .claude/skills/ably-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ For each changed command file, run the relevant checks. Spawn agents for paralle
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.
6. **Check** hold commands (set, enter, acquire) emit `logJsonStatus("holding", ...)` after `logJsonResult` — this signals to JSON consumers that the command is alive and waiting for Ctrl+C / `--duration`
6. **Check** hold commands (set, enter, acquire) emit `logJsonStatus(JsonStatusType.Holding, ...)` after `logJsonResult`
7. **Check** subscribe commands emit `logJsonStatus(JsonStatusType.Subscribing, ...)` when connecting and `logJsonStatus(JsonStatusType.Listening, ...)` when attached. Room subscribe commands should pass `subscribingMessage` and `listeningMessage` to `setupRoomStatusHandler`.
8. **Grep** for `logJsonStatus("` — all calls must use `JsonStatusType` enum, not raw strings

**Control API helper check (grep — for Control API commands only):**
1. **Grep** for `resolveAppId` — should use `requireAppId` instead (encapsulates null check and `fail()`)
Expand Down
4 changes: 3 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ All output helpers use the `format` prefix and are exported from `src/utils/outp
- **Pagination next hint**: `buildPaginationNext(hasMore, lastTimestamp?)` — returns `{ hint, start? }` for JSON output when `hasMore` is true. Pass `lastTimestamp` only for history commands (which have `--start`).
- **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, `this.logJsonEvent(data, flags)` for streaming events, and `this.logJsonStatus(status, message, flags)` for hold/status signals in long-running commands. The envelope adds 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 hold status**: Long-running hold commands (e.g. `spaces members enter`, `spaces locations set`, `spaces locks acquire`, `spaces cursors set`) must emit a `logJsonStatus("holding", "Holding <thing>. Press Ctrl+C to exit.", flags)` line after the result. This tells LLM agents and scripts that the command is alive and waiting. `logJsonStatus` has a built-in `shouldOutputJson` guard — no outer `if` needed.
- **JSON status types**: Use the `JsonStatusType` enum from `src/utils/json-status.ts` for all `logJsonStatus` calls — never pass raw strings. Available values: `Subscribing`, `Listening`, `Holding`, `Complete`, `Warning`, `Disconnected`. `logJsonStatus` has a built-in `shouldOutputJson` guard — no outer `if` needed. Two command-type-specific patterns:
- **Hold commands** (set, enter, acquire): emit `JsonStatusType.Holding` after the result.
- **Subscribe commands**: emit `JsonStatusType.Subscribing` when connecting, `JsonStatusType.Listening` when attached. Room commands pass `subscribingMessage`/`listeningMessage` to `setupRoomStatusHandler`; channel/space/log commands call `logJsonStatus` directly. See `references/patterns.md` for templates.
- **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]` 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.

Expand Down
3 changes: 2 additions & 1 deletion src/base-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
buildJsonRecord,
formatWarning,
} from "./utils/output.js";
import { JsonStatusType } from "./utils/json-status.js";
import { getCliVersion } from "./utils/version.js";
import Spaces from "@ably/spaces";
import { ChatClient } from "@ably/chat";
Expand Down Expand Up @@ -1621,7 +1622,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand {
if (exitReason === "timeout" && !isTestMode()) {
const message = "Duration elapsed – command finished cleanly.";
if (this.shouldOutputJson(flags)) {
this.logJsonStatus("complete", message, flags);
this.logJsonStatus(JsonStatusType.Complete, message, flags);
} else {
Comment on lines 1624 to 1626
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

logJsonStatus is now documented/used with JsonStatusType, but the method signature still accepts status: string, so passing raw strings will continue to compile and can silently violate the new convention. Consider typing the parameter as status: JsonStatusType (or JsonStatusType | string temporarily) to enforce the rule at compile time.

Copilot uses AI. Check for mistakes.
this.log(message);
}
Expand Down
22 changes: 22 additions & 0 deletions src/chat-base-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { AblyBaseCommand } from "./base-command.js";
import { productApiFlags } from "./flags.js";
import { BaseFlags } from "./types/cli.js";

import { JsonStatusType } from "./utils/json-status.js";
import {
formatSuccess,
formatListening,
Expand Down Expand Up @@ -119,8 +120,17 @@ export abstract class ChatBaseCommand extends AblyBaseCommand {
roomName: string;
successMessage?: string;
listeningMessage?: string;
subscribingMessage?: string;
},
): void {
if (options.subscribingMessage) {
this.logJsonStatus(
JsonStatusType.Subscribing,
options.subscribingMessage,
flags as BaseFlags,
);
}

room.onStatusChange((statusChange) => {
let reason: Error | null | string | undefined;
if (statusChange.current === RoomStatus.Failed) {
Expand All @@ -144,12 +154,24 @@ export abstract class ChatBaseCommand extends AblyBaseCommand {
this.log(formatListening(options.listeningMessage));
}
}
if (options.listeningMessage) {
this.logJsonStatus(
JsonStatusType.Listening,
options.listeningMessage,
flags as BaseFlags,
);
}
break;
}
case RoomStatus.Detached: {
if (!this.shouldOutputJson(flags)) {
this.log(formatWarning("Disconnected from Ably"));
}
this.logJsonStatus(
JsonStatusType.Disconnected,
"Disconnected from Ably.",
flags as BaseFlags,
);
break;
}
case RoomStatus.Failed: {
Expand Down
11 changes: 11 additions & 0 deletions src/commands/channels/annotations/subscribe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
productApiFlags,
rewindFlag,
} from "../../../flags.js";
import { JsonStatusType } from "../../../utils/json-status.js";
import {
formatAnnotationsOutput,
formatListening,
Expand Down Expand Up @@ -91,6 +92,11 @@ export default class ChannelsAnnotationsSubscribe extends AblyBaseCommand {
),
);
}
this.logJsonStatus(
JsonStatusType.Subscribing,
`Subscribing to annotations on channel: ${channelName}.`,
flags,
);

this.setupChannelStateLogging(channel, flags, {
includeUserFriendlyMessages: true,
Expand Down Expand Up @@ -172,6 +178,11 @@ export default class ChannelsAnnotationsSubscribe extends AblyBaseCommand {
this.log(formatListening("Listening for annotations."));
this.log("");
}
this.logJsonStatus(
JsonStatusType.Listening,
"Listening for annotations.",
flags,
);

this.logCliEvent(
flags,
Expand Down
11 changes: 11 additions & 0 deletions src/commands/channels/occupancy/subscribe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Args } from "@oclif/core";
import * as Ably from "ably";
import { AblyBaseCommand } from "../../../base-command.js";
import { clientIdFlag, durationFlag, productApiFlags } from "../../../flags.js";
import { JsonStatusType } from "../../../utils/json-status.js";
import {
formatListening,
formatProgress,
Expand Down Expand Up @@ -85,6 +86,11 @@ export default class ChannelsOccupancySubscribe extends AblyBaseCommand {
),
);
}
this.logJsonStatus(
JsonStatusType.Subscribing,
`Subscribing to occupancy events on channel: ${channelName}.`,
flags,
);

await channel.subscribe(occupancyEventName, (message: Ably.Message) => {
const timestamp = formatMessageTimestamp(message.timestamp);
Expand Down Expand Up @@ -127,6 +133,11 @@ export default class ChannelsOccupancySubscribe extends AblyBaseCommand {
);
this.log(formatListening("Listening for occupancy events."));
}
this.logJsonStatus(
Copy link
Contributor Author

@sacOO7 sacOO7 Mar 26, 2026

Choose a reason for hiding this comment

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

Internally checks if json flag is enabled, if not, then it won't print anything.

JsonStatusType.Listening,
"Listening for occupancy events.",
flags,
);

this.logCliEvent(
flags,
Expand Down
3 changes: 2 additions & 1 deletion src/commands/channels/presence/enter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as Ably from "ably";
import { AblyBaseCommand } from "../../../base-command.js";
import { clientIdFlag, durationFlag, productApiFlags } from "../../../flags.js";
import { isJsonData } from "../../../utils/json-formatter.js";
import { JsonStatusType } from "../../../utils/json-status.js";
import {
formatClientId,
formatEventType,
Expand Down Expand Up @@ -207,7 +208,7 @@ export default class ChannelsPresenceEnter extends AblyBaseCommand {
}

this.logJsonStatus(
"holding",
JsonStatusType.Holding,
"Holding presence. Press Ctrl+C to exit.",
flags,
);
Expand Down
11 changes: 11 additions & 0 deletions src/commands/channels/presence/subscribe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Args } from "@oclif/core";
import * as Ably from "ably";
import { AblyBaseCommand } from "../../../base-command.js";
import { clientIdFlag, durationFlag, productApiFlags } from "../../../flags.js";
import { JsonStatusType } from "../../../utils/json-status.js";
import {
formatListening,
formatProgress,
Expand Down Expand Up @@ -78,6 +79,11 @@ export default class ChannelsPresenceSubscribe extends AblyBaseCommand {
),
);
}
this.logJsonStatus(
JsonStatusType.Subscribing,
`Subscribing to presence events on channel: ${channelName}.`,
flags,
);

await channel.presence.subscribe(
(presenceMessage: Ably.PresenceMessage) => {
Expand Down Expand Up @@ -126,6 +132,11 @@ export default class ChannelsPresenceSubscribe extends AblyBaseCommand {
this.log(formatListening("Listening for presence events."));
this.log("");
}
this.logJsonStatus(
JsonStatusType.Listening,
"Listening for presence events.",
flags,
);

this.logCliEvent(
flags,
Expand Down
3 changes: 2 additions & 1 deletion src/commands/channels/presence/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Args, Flags } from "@oclif/core";
import * as Ably from "ably";
import { AblyBaseCommand } from "../../../base-command.js";
import { clientIdFlag, durationFlag, productApiFlags } from "../../../flags.js";
import { JsonStatusType } from "../../../utils/json-status.js";
import {
formatClientId,
formatLabel,
Expand Down Expand Up @@ -133,7 +134,7 @@ export default class ChannelsPresenceUpdate extends AblyBaseCommand {
}

this.logJsonStatus(
"holding",
JsonStatusType.Holding,
"Holding presence. Press Ctrl+C to exit.",
flags,
);
Expand Down
11 changes: 11 additions & 0 deletions src/commands/channels/subscribe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
productApiFlags,
rewindFlag,
} from "../../flags.js";
import { JsonStatusType } from "../../utils/json-status.js";
import {
formatListening,
formatProgress,
Expand Down Expand Up @@ -171,6 +172,11 @@ export default class ChannelsSubscribe extends AblyBaseCommand {
),
);
}
this.logJsonStatus(
JsonStatusType.Subscribing,
`Subscribing to channel: ${channel.name}.`,
flags,
);

// Set up channel state logging
this.setupChannelStateLogging(channel, flags, {
Expand Down Expand Up @@ -268,6 +274,11 @@ export default class ChannelsSubscribe extends AblyBaseCommand {
this.log(formatListening("Listening for messages."));
this.log("");
}
this.logJsonStatus(
JsonStatusType.Listening,
"Listening for messages.",
flags,
);

this.logCliEvent(
flags,
Expand Down
Loading
Loading