diff --git a/.claude/skills/ably-new-command/SKILL.md b/.claude/skills/ably-new-command/SKILL.md index aa528e5d8..2190b7bfa 100644 --- a/.claude/skills/ably-new-command/SKILL.md +++ b/.claude/skills/ably-new-command/SKILL.md @@ -137,7 +137,9 @@ static flags = { - `await this.requireAppId(flags)` — resolves and validates the app ID, returns `Promise` (non-nullable). Calls `this.fail()` internally if no app found — no manual null check needed. - `await this.runControlCommand(flags, api => api.method(appId))` — creates the Control API client, executes the call, and handles errors in one step. Returns `Promise` (non-nullable). Useful for single API calls; for multi-step flows, use `this.createControlApi(flags)` directly. -**When to include `clientIdFlag`:** Add `...clientIdFlag` to commands where client identity affects the operation: subscribe, publish, enter, set, acquire, update, delete, append, annotate. The reason is that users may want to test auth scenarios — e.g., "can client B update client A's message?" — so they need the ability to set their client ID. Do NOT add to read-only queries (get, get-all, history, occupancy get) — Ably capabilities are operation-based, not clientId-based, so client identity is irrelevant for pure reads. +**When to include `clientIdFlag`:** Add `...clientIdFlag` to commands where client identity affects the operation: subscribe, publish, enter, set, acquire, update, delete, append, annotate. The reason is that users may want to test auth scenarios — e.g., "can client B update client A's message?" — so they need the ability to set their client ID. Do NOT add to read-only queries (get, get-all, history, occupancy get). Reads still carry identity — every command acts as the session's client ID (`ABLY_CLIENT_ID` > config > generated default) and Ably counts it under MAU billing — but a read has no reason to act as someone else, so the session identity is enough. + +**Target client IDs are not identity:** the base command treats `--client-id` as the client ID the CLI acts as only when the command declares it via the shared `clientIdFlag` object. When a command's `--client-id` names a *target* or *filter* (a push recipient, a token's subject), declare a command-local `Flags.string()` instead — never spread `clientIdFlag` for that — so the value can't become the CLI's own identity. `test/unit/base/client-id-target-guard.test.ts` enforces this for every command. For history commands, also use `timeRangeFlags`: ```typescript @@ -472,7 +474,7 @@ See the "Keeping Skills Up to Date" section in `CLAUDE.md` for the full list of - [ ] JSON output uses `logJsonResult()` (one-shot) or `logJsonEvent()` (streaming), not direct `formatJsonRecord()` - [ ] JSON data nested under a domain key (singular for events/single items, plural for collections) — not spread at top level (see `patterns.md` "JSON Data Nesting Convention") - [ ] `room.attach()` / `space.enter()` only called when the SDK method requires a realtime connection (subscribe, send, presence) — NOT for REST mutations (update, delete, annotate, history) -- [ ] Subscribe/enter commands use `this.waitAndTrackCleanup(flags, component, flags.duration)` (not `waitUntilInterruptedOrTimeout`) +- [ ] Subscribe/enter commands use `this.waitAndTrackCleanup(flags, component, flags.duration)` (not `waitUntilInterruptedOrTimeout`) and call `this.setupConnectionStateLogging(client, flags)` — together they fail the command (non-zero exit) when the connection fails or keeps dropping, instead of exiting 0 - [ ] 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"`) - [ ] At least one `--json` example in `static examples` diff --git a/.claude/skills/ably-new-command/references/patterns.md b/.claude/skills/ably-new-command/references/patterns.md index 5f64d5492..e359f6040 100644 --- a/.claude/skills/ably-new-command/references/patterns.md +++ b/.claude/skills/ably-new-command/references/patterns.md @@ -248,7 +248,7 @@ async run(): Promise { ## Get Pattern -Get commands perform one-shot read-only queries for current state. They don't need `clientIdFlag` (Ably capabilities are operation-based, not clientId-based — client identity is irrelevant for reads), `durationFlag`, or `rewindFlag`. +Get commands perform one-shot read-only queries for current state. They don't need `clientIdFlag` (they act as the session's client ID, which is enough for a read), `durationFlag`, or `rewindFlag`. ```typescript static override flags = { diff --git a/AGENTS.md b/AGENTS.md index f14de981f..b1bd33c80 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ This is the Ably CLI npm package (`@ably/cli`), built with the [oclif framework] │ ├── integration/ # Multi-component, mocked external services │ ├── e2e/ # Full scenarios against real Ably │ └── helpers/ # runCommand(), MockConfigManager, etc. -├── docs/ # Project docs (Testing.md, Project-Structure.md, etc.) +├── docs/ # Project docs (Testing.md, Project-Structure.md, Client-Identity.md, etc.) └── package.json # Scripts defined here ``` @@ -100,7 +100,7 @@ Flags are NOT global. Each command explicitly declares only the flags it needs v - **`coreGlobalFlags`** — `--verbose`, `--json`, `--pretty-json`, `--web-cli-help` (hidden) (on every command via `AblyBaseCommand.globalFlags`) - **`productApiFlags`** — core + hidden product API flags (`port`, `tlsPort`, `tls`). Use for commands that talk to the Ably product API. - **`controlApiFlags`** — core + hidden control API flags (`control-host`, `dashboard-host`). Use for commands that talk to the Control API. -- **`clientIdFlag`** — `--client-id`. Add to commands where client identity affects the operation: subscribe, publish, enter, set, acquire, update, delete, append. Do NOT add to read-only queries (get, get-all, occupancy get) — Ably capabilities are operation-based, not clientId-based, so client identity is irrelevant for pure reads. Do NOT add globally. +- **`clientIdFlag`** — `--client-id`. Add to commands where acting as a particular client is part of the operation: subscribe, publish, enter, set, acquire, update, delete, append. Do NOT add to read-only queries (get, get-all, occupancy get): every command, reads included, already acts as the session's client ID (`ABLY_CLIENT_ID` > config > generated default, see `docs/Client-Identity.md`) and Ably counts it under MAU billing, but a read has no reason to act as someone else. Do NOT add globally. A `--client-id` that names a *target* (push recipient, token subject) must be a command-local flag, never `clientIdFlag` — the base command treats only `clientIdFlag` as identity. - **`durationFlag`** — `--duration` / `-D`. Use for long-running subscribe/stream commands that auto-exit after N seconds. - **`rewindFlag`** — `--rewind`. Use for subscribe commands that support message replay (default: 0). - **`timeRangeFlags`** — `--start`, `--end`. Use for history and stats commands. Parse with `parseTimestamp()` from `src/utils/time.ts`. Accepts ISO 8601, Unix ms, or relative (e.g., `"1h"`, `"30m"`, `"2d"`). @@ -282,7 +282,7 @@ this.error() ← oclif exit (ONLY inside fail, nowhere else) - **Never use `this.error()` directly** — it is an internal implementation detail of `this.fail()`. - **`requireAppId`** returns `Promise` (not nullable) — calls `this.fail()` internally if no app found. - **`runControlCommand`** returns `Promise` (not nullable) — calls `this.fail()` internally on error. -- **Error hints**: `fail()` appends a CLI-specific hint from `src/utils/errors.ts` if one exists for the Ably error code. Hints must only contain actionable CLI advice (e.g., "run `ably login`"), not restate the upstream error message (which is already shown). When adding new error codes, **fetch** https://ably.com/docs/platform/errors/codes using WebFetch to get the official description — do NOT rely on memory or assumptions about what an error code means. +- **Error hints**: `fail()` appends a CLI-specific hint from `src/utils/errors.ts` if one exists for the Ably error code. A hint may be a function of `HintContext` (token auth or not, whether the command declares `--client-id`) so it never suggests a flag the command lacks. Hints must only contain actionable CLI advice (e.g., "run `ably login`"), not restate the upstream error message (which is already shown). When adding new error codes, **fetch** https://ably.com/docs/platform/errors/codes using WebFetch to get the official description — do NOT rely on memory or assumptions about what an error code means. - **`extractErrorInfo(error)`** (`src/utils/errors.ts`) — extracts `{ message, code?, statusCode?, href? }` from an unknown error value. Use in commands that report per-item errors inline (batch publish results, connection test summaries) rather than exiting via `this.fail()`. ### Additional output patterns (direct chalk, not helpers) diff --git a/docs/Client-Identity.md b/docs/Client-Identity.md new file mode 100644 index 000000000..6dec19c46 --- /dev/null +++ b/docs/Client-Identity.md @@ -0,0 +1,68 @@ +# Client Identity and MAU Classification + +Ably bills monthly active users (MAU) per distinct client ID on **device** traffic; **server** traffic is exempt from MAU counting, from the per-client-ID connection cap, and from the requirement to carry a client ID. This page describes how the CLI declares its own traffic and which client ID it acts as. Design rationale: DXRFC-029 (CLI Classification and Identity for MAU Pricing). + +## Always a server + +The CLI always connects as a server. Every Pub/Sub client is built with `@ably/pubsub-server`, whatever the CLI authenticates with, and Chat and Spaces wrap that same client, so rooms and spaces traffic is server traffic too. + +With an API key (`ably login`, `ABLY_API_KEY`) that is all Ably needs. Under token auth (`ABLY_TOKEN`) Ably grants the server side only through a signed `x-ably-clientType=server` claim on the token, and rejects a client that declares itself a server without it. Native Ably tokens cannot carry the claim, so use a JWT issued with: + +```bash +ably auth issue-jwt-token --client-type server --client-id backend-worker +``` + +## Which client ID the CLI acts as + +Under API key auth, the client ID is resolved once per process, from the first of: + +1. `--client-id `, on commands that offer it +2. `ABLY_CLIENT_ID` +3. `client.id` in the config file (`~/.ably/config`) +4. A default generated once per install (`ably-cli-<8 hex chars>`) and saved as `client.defaultId` in the config + +Every client the command builds, every command in an interactive session, and every token minted by `ably auth issue-ably-token` / `issue-jwt-token` without `--client-id` uses that one ID. Commands without a `--client-id` flag still act as it. `ably bench` commands append a per-process suffix to the default, so that many concurrent bench processes stay under the per-client-ID connection cap. + +To give a machine or CI job a fixed identity, set it once: + +```bash +export ABLY_CLIENT_ID="deploy-bot" +``` + +or in the config file: + +```toml +[client] +id = "deploy-bot" +``` + +Values the CLI refuses: + +- `""` — an empty client ID. It used to fall back silently to a random ID. +- `"*"` — the wildcard. The CLI acts as, and issues tokens to, one concrete client ID. +- `"none"` — deprecated. It still acts with no client ID, with a warning, but apps that require identified clients reject that traffic. + +Under token auth the client ID is the token's and is never overridden; `--client-id` is ignored with a warning. A JWT without an `x-ably-clientId` claim is rejected before connecting. + +### Target client IDs are not identity + +On push and token commands, `--client-id` names a *target* rather than the CLI's identity: `ably push devices list --client-id user123` lists that user's devices, and `ably auth issue-jwt-token --client-id alice` issues a token to alice. These never change who the CLI acts as. + +## Simulating multiple users + +An explicit `--client-id` always wins, so two terminals can act as two different clients: + +```bash +# Terminal 1 +ably channels presence enter my-channel --client-id alice + +# Terminal 2 +ably channels presence enter my-channel --client-id bob +``` + +Both connect as servers, so they are not counted or capped as devices. Ably counts client IDs wherever they appear, including in message payloads, so each simulated name may still register as an MAU. The CLI cannot reproduce device behaviour (MAU counting, the per-client-ID connection cap, identified-client enforcement); use an Ably SDK on the device side for that. + +## When something goes wrong + +- Rejections about the client ID (`40012`, `40161`, `91000`) carry a hint for how you authenticated: set `--client-id` / `ABLY_CLIENT_ID` under API key auth, or re-issue the token with a client ID under token auth. +- A long-running command whose connection fails, or keeps dropping without staying connected, exits non-zero instead of reporting success. diff --git a/docs/Environment-Variables/General-Usage.md b/docs/Environment-Variables/General-Usage.md index 58ffeca51..badfd9df1 100644 --- a/docs/Environment-Variables/General-Usage.md +++ b/docs/Environment-Variables/General-Usage.md @@ -23,3 +23,5 @@ These environment variables are most commonly used during development as well as | `ABLY_ENDPOINT` | Host Override | Override Realtime/REST API endpoint (host only) | SDK default | > For development, testing, debugging, and internal variables, see [Development Stage Usage](Development-Usage.md). +> +> For how `ABLY_CLIENT_ID`, `--client-id` and tokens decide which client ID the CLI acts as, and why the CLI always connects as a server for MAU billing, see [Client Identity](../Client-Identity.md). diff --git a/docs/Project-Structure.md b/docs/Project-Structure.md index 6b7dca448..8a83dd029 100644 --- a/docs/Project-Structure.md +++ b/docs/Project-Structure.md @@ -74,8 +74,9 @@ This document outlines the directory structure of the Ably CLI project. │ └── utils/ │ ├── channel-rule-display.ts # Channel rule human-readable display │ ├── chat-constants.ts # Shared Chat SDK constants (REACTION_TYPE_MAP) +│ ├── connection-health.ts # Detects dead connections (failed, reconnect loops) │ ├── server-url.ts # Local server URL parsing (parseServerUrl, formatServerUrl) -│ ├── errors.ts # Error utilities (errorMessage) +│ ├── errors.ts # Error utilities (errorMessage, errorWithReason, hints) │ ├── interrupt-feedback.ts # Ctrl+C feedback messages │ ├── json-formatter.ts # JSON output formatting (formatJson, formatMessageData) │ ├── key-display.ts # Key capability formatting diff --git a/src/base-command.ts b/src/base-command.ts index e3458646f..21962d2cd 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -29,7 +29,7 @@ import { type ServerUrl, } from "./utils/server-url.js"; import { errorMessage, getFriendlyAblyErrorHint } from "./utils/errors.js"; -import { coreGlobalFlags } from "./flags.js"; +import { clientIdFlag, coreGlobalFlags } from "./flags.js"; import { InteractiveHelper } from "./services/interactive-helper.js"; import { promptForConfirmation } from "./utils/prompt-confirmation.js"; import { BaseFlags, CommandConfig } from "./types/cli.js"; @@ -50,6 +50,7 @@ import { waitUntilInterruptedOrTimeout, type ExitReason, } from "./utils/long-running.js"; +import { ConnectionHealthMonitor } from "./utils/connection-health.js"; import isTestMode from "./utils/test-mode.js"; import isWebCliMode from "./utils/web-mode.js"; import * as fs from "node:fs"; @@ -160,6 +161,9 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { */ protected suffixClientIdPerProcess = false; + /** Aborted with the cause when a watched connection dies mid-command. */ + private readonly connectionLost = new AbortController(); + // Core global flags available to all commands (verbose, json, pretty-json, web-cli-help) static globalFlags = { ...coreGlobalFlags }; @@ -772,7 +776,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { } try { - return resolveClientIdentity(flags["client-id"], this.configManager) + return resolveClientIdentity(this.identityFlag(flags), this.configManager) .clientId; } catch { return undefined; @@ -1004,6 +1008,28 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { * This hook runs before command execution * It's the oclif standard hook that runs before the run() method */ + /** + * Emit a terminal "completed" line so JSON consumers know the command is + * done. Suppressed when the command is being delegated to from another + * command (the outer command emits its own terminator). + */ + private logCompletedStatus(exitCode: 0 | 1): void { + const isJsonMode = + this.argv.includes("--json") || this.argv.includes("--pretty-json"); + if (!isJsonMode || this.argv.includes("--skip-completed-status")) return; + + const flags: BaseFlags = this.argv.includes("--pretty-json") + ? { "pretty-json": true } + : { json: true }; + this.log( + this.formatJsonRecord( + JsonRecordType.Status, + { status: "completed", exitCode }, + flags, + ), + ); + } + async finally(err: Error | undefined): Promise { // Clean up cached clients try { @@ -1036,25 +1062,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { this.debug(`Realtime client cleanup error: ${String(error)}`); } - // Emit a terminal "completed" line so JSON consumers know the command is done. - // Suppressed when the command is being delegated to from another command - // (the outer command emits its own terminator). - const isJsonMode = - this.argv.includes("--json") || this.argv.includes("--pretty-json"); - const suppressCompleted = this.argv.includes("--skip-completed-status"); - if (isJsonMode && !suppressCompleted) { - const flags: BaseFlags = this.argv.includes("--pretty-json") - ? { "pretty-json": true } - : { json: true }; - const exitCode = err ? 1 : 0; - this.log( - this.formatJsonRecord( - JsonRecordType.Status, - { status: "completed", exitCode }, - flags, - ), - ); - } + this.logCompletedStatus(err ? 1 : 0); // Call super to maintain the parent class functionality await super.finally(err); @@ -1578,6 +1586,21 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { this.setClientId(options, flags); } + /** + * The --client-id value, but only when the command declares the shared + * identity flag. Push and token commands declare their own --client-id to + * name a target or filter, which must never become the CLI's own identity. + */ + private identityFlag(flags: BaseFlags): string | undefined { + return this.declaresIdentityFlag() ? flags["client-id"] : undefined; + } + + private declaresIdentityFlag(): boolean { + const declared = (this.constructor as { flags?: Record }) + .flags?.["client-id"]; + return declared === clientIdFlag["client-id"]; + } + /** * The client ID this command acts as under API-key auth. Fails the command * on a value the CLI cannot act as, such as "" or "*". @@ -1585,7 +1608,10 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { protected resolveClientId(flags: BaseFlags): string | undefined { let identity; try { - identity = resolveClientIdentity(flags["client-id"], this.configManager); + identity = resolveClientIdentity( + this.identityFlag(flags), + this.configManager, + ); } catch (error) { if (error instanceof InvalidClientIdError) { this.fail(error.message, flags, "clientId"); @@ -1621,6 +1647,37 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { return clientId; } + /** + * The client ID a minted token is issued to: the command's --client-id, or + * the client ID the CLI acts as. "none" issues an anonymous token, with a + * warning, and "*" is refused rather than minting a token that can act as + * any client. + */ + protected resolveTokenClientId(flags: BaseFlags): string | undefined { + const requested = flags["client-id"]; + if (requested === undefined) return this.resolveClientId(flags); + + let identity; + try { + identity = resolveClientIdentity(requested, this.configManager); + } catch (error) { + if (error instanceof InvalidClientIdError) { + this.fail(error.message, flags, "clientId"); + } + + throw error; + } + + if (identity.optedOut) { + this.logWarning( + "Issuing a token with no client ID. Apps that require identified clients reject it.", + flags, + ); + } + + return identity.clientId; + } + private setClientId(options: Ably.ClientOptions, flags: BaseFlags): void { const clientId = this.resolveClientId(flags); if (clientId !== undefined) { @@ -1634,7 +1691,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { * only known once Ably accepts it. */ private applyTokenIdentity(token: string, flags: BaseFlags): void { - if (flags["client-id"] && !this.shouldSuppressOutput(flags)) { + if (this.identityFlag(flags) && !this.shouldSuppressOutput(flags)) { this.logWarning( "--client-id is ignored when using token authentication; the client ID comes from the token.", flags, @@ -1742,10 +1799,15 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { ): () => void { const component = options?.component || "connection"; const showUserMessages = options?.includeUserFriendlyMessages || false; + const health = new ConnectionHealthMonitor((error) => + this.connectionLost.abort(error), + ); const connectionStateHandler = ( stateChange: Ably.ConnectionStateChange, ) => { + health.handle(stateChange); + // Always emit in JSON mode so agents can observe connection lifecycle if (this.shouldOutputJson(flags)) { const eventData: Record = { @@ -2073,6 +2135,10 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { (typeof cmdError.context.errorCode === "number" ? cmdError.context.errorCode : undefined), + { + tokenAuth: Boolean(process.env.ABLY_TOKEN), + hasClientIdFlag: this.declaresIdentityFlag(), + }, ); if (this.shouldOutputJson(flags)) { @@ -2108,27 +2174,29 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { component: string, duration?: number, ): Promise { - const exitReason = await waitUntilInterruptedOrTimeout(duration); + const exitReason = await waitUntilInterruptedOrTimeout( + duration, + this.connectionLost.signal, + ); this.logCliEvent(flags, component, "runComplete", "Exiting wait loop", { exitReason, }); + + // A subscription whose connection died must not report success. + if (exitReason === "aborted") { + this.fail(this.connectionLost.signal.reason, flags, component); + } + this.cleanupInProgress = exitReason === "signal"; // For timeout cases in CLI commands, exit immediately to prevent hanging. // This is especially important for E2E tests and automated scenarios. if (exitReason === "timeout" && !isTestMode()) { - const message = "Duration elapsed – command finished cleanly."; - if (this.shouldOutputJson(flags)) { - this.log( - this.formatJsonRecord( - JsonRecordType.Status, - { status: "complete", message }, - flags, - ), - ); - } else { - this.log(message); + if (!this.shouldOutputJson(flags)) { + this.logToStderr("Duration elapsed – command finished cleanly."); } + // Exiting here skips finally(), so emit its terminal line ourselves. + this.logCompletedStatus(0); // Small delay to ensure output is flushed, then force-exit to prevent // hanging and avoid spurious cleanup messages (e.g. "Detached from channel"). await new Promise((resolve) => setTimeout(resolve, 200)); diff --git a/src/commands/auth/issue-ably-token.ts b/src/commands/auth/issue-ably-token.ts index 867e1601d..aa9f3744d 100644 --- a/src/commands/auth/issue-ably-token.ts +++ b/src/commands/auth/issue-ably-token.ts @@ -17,7 +17,6 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { '$ ably auth issue-ably-token --capability \'{"*":["*"]}\'', '$ ably auth issue-ably-token --capability \'{"chat:*":["publish","subscribe"], "status:*":["subscribe"]}\' --ttl 3600', "$ ably auth issue-ably-token --client-id client123 --ttl 86400", - '$ ably auth issue-ably-token --client-id "none" --ttl 3600', "$ ably auth issue-ably-token --json", "$ ably auth issue-ably-token --pretty-json", "$ ably auth issue-ably-token --token-only", @@ -37,7 +36,7 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { }), "client-id": Flags.string({ description: - 'Client ID to associate with the token. Use "none" to explicitly issue a token with no client ID, otherwise a default will be generated.', + 'Client ID to issue the token to (defaults to the client ID the CLI acts as). "none" issues an anonymous token, which apps requiring identified clients reject.', }), "token-only": Flags.boolean({ default: false, @@ -81,18 +80,8 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { ttl: flags.ttl * 1000, // Convert to milliseconds for Ably SDK }; - // Handle client ID - use special "none" value to explicitly indicate no clientId - if (flags["client-id"]) { - if (flags["client-id"].toLowerCase() === "none") { - // No client ID - leave clientId undefined in the token params - } else { - // Use the provided client ID - tokenParams.clientId = flags["client-id"]; - } - } else { - // Default to the identity the CLI itself acts as - tokenParams.clientId = this.resolveClientId(flags); - } + const clientId = this.resolveTokenClientId(flags); + if (clientId !== undefined) tokenParams.clientId = clientId; // Create Ably REST client and request token const rest = await this.createAblyRestClient( @@ -126,9 +115,7 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { value: tokenDetails.token, issuedAt: new Date(tokenDetails.issued).toISOString(), expiresAt: new Date(tokenDetails.expires).toISOString(), - ...(tokenDetails.clientId - ? { clientId: tokenDetails.clientId } - : {}), + clientId: tokenDetails.clientId ?? null, capability: tokenDetails.capability, }, }, @@ -145,11 +132,9 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { `${formatLabel("Expires")} ${new Date(tokenDetails.expires).toISOString()}`, ); this.log(`${formatLabel("TTL")} ${flags.ttl} seconds`); - if (tokenDetails.clientId) { - this.log( - `${formatLabel("Client ID")} ${formatClientId(tokenDetails.clientId)}`, - ); - } + this.log( + `${formatLabel("Client ID")} ${tokenDetails.clientId ? formatClientId(tokenDetails.clientId) : "anonymous"}`, + ); this.log( `${formatLabel("Capability")} ${this.formatJsonOutput({ capability: tokenDetails.capability }, flags)}`, ); diff --git a/src/commands/auth/issue-jwt-token.ts b/src/commands/auth/issue-jwt-token.ts index 5414f8b0c..a59698a14 100644 --- a/src/commands/auth/issue-jwt-token.ts +++ b/src/commands/auth/issue-jwt-token.ts @@ -17,6 +17,7 @@ interface JwtPayload { "x-ably-appId": string; "x-ably-capability": Record; "x-ably-clientId"?: string; + "x-ably-clientType"?: "server"; } export default class IssueJwtTokenCommand extends AblyBaseCommand { @@ -27,6 +28,7 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { '$ ably auth issue-jwt-token --capability \'{"*":["*"]}\'', '$ ably auth issue-jwt-token --capability \'{"chat:*":["publish","subscribe"], "status:*":["subscribe"]}\' --ttl 3600', "$ ably auth issue-jwt-token --client-id client123 --ttl 86400", + "$ ably auth issue-jwt-token --client-type server --client-id backend-worker", "$ ably auth issue-jwt-token --json", "$ ably auth issue-jwt-token --pretty-json", "$ ably auth issue-jwt-token --token-only", @@ -46,7 +48,12 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { }), "client-id": Flags.string({ description: - 'Client ID to associate with the token. Use "none" to explicitly issue a token with no client ID, otherwise a default will be generated.', + 'Client ID to issue the token to (defaults to the client ID the CLI acts as). "none" issues an anonymous token, which apps requiring identified clients reject.', + }), + "client-type": Flags.string({ + description: + "Classify clients using the token as servers, exempt from MAU counting, by adding the signed x-ably-clientType claim", + options: ["server"], }), "token-only": Flags.boolean({ default: false, @@ -102,22 +109,11 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { "x-ably-capability": capabilities, }; - // Handle client ID - use special "none" value to explicitly indicate no clientId - let clientId: null | string = null; - if (flags["client-id"]) { - if (flags["client-id"].toLowerCase() === "none") { - // No client ID - don't add it to the token - clientId = null; - } else { - // Use the provided client ID - jwtPayload["x-ably-clientId"] = flags["client-id"]; - clientId = flags["client-id"]; - } - } else { - // Default to the identity the CLI itself acts as - clientId = this.resolveClientId(flags) ?? null; - if (clientId) jwtPayload["x-ably-clientId"] = clientId; - } + const clientId = this.resolveTokenClientId(flags); + if (clientId !== undefined) jwtPayload["x-ably-clientId"] = clientId; + + const clientType = flags["client-type"] as "server" | undefined; + if (clientType) jwtPayload["x-ably-clientType"] = clientType; // Sign the JWT const token = jwt.sign(jwtPayload, keySecret, { @@ -141,7 +137,8 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { token: { appId, capability: capabilities, - ...(clientId ? { clientId } : {}), + clientId: clientId ?? null, + ...(clientType ? { clientType } : {}), expires: new Date(jwtPayload.exp * 1000).toISOString(), issued: new Date(jwtPayload.iat * 1000).toISOString(), keyId, @@ -165,8 +162,11 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { this.log(`${formatLabel("TTL")} ${flags.ttl} seconds`); this.log(`${formatLabel("App ID")} ${appId}`); this.log(`${formatLabel("Key ID")} ${keyId}`); - if (clientId) { - this.log(`${formatLabel("Client ID")} ${formatClientId(clientId)}`); + this.log( + `${formatLabel("Client ID")} ${clientId ? formatClientId(clientId) : "anonymous"}`, + ); + if (clientType) { + this.log(`${formatLabel("Client Type")} ${clientType}`); } this.log( `${formatLabel("Capability")} ${this.formatJsonOutput(capabilities, flags)}`, diff --git a/src/commands/auth/revoke-token.ts b/src/commands/auth/revoke-token.ts index 39ff5148e..8beaa90ef 100644 --- a/src/commands/auth/revoke-token.ts +++ b/src/commands/auth/revoke-token.ts @@ -1,5 +1,4 @@ import { Flags } from "@oclif/core"; -import * as https from "node:https"; import stripAnsi from "strip-ansi"; import { AblyBaseCommand } from "../../base-command.js"; @@ -97,128 +96,58 @@ export default class RevokeTokenCommand extends AblyBaseCommand { } } - try { - // Extract the keyName (appId.keyId) from the API key - const keyParts = apiKey.split(":"); - if (keyParts.length !== 2) { - this.fail( - "Invalid API key format. Expected format: appId.keyId:secret", - flags, - "revokeToken", - ); - } + let reauthNote = ""; + if (flags["allow-reauth-margin"]) { + reauthNote = + " Connected clients have a 30s grace period to obtain new tokens before disconnection."; + } - const keyName = keyParts[0]!; - const secret = keyParts[1]!; + try { + const rest = await this.createAblyRestClient({ + ...flags, + "api-key": apiKey, + }); + if (!rest) return; - const requestBody: Record = { - targets: [targetSpecifier], - }; + const response = await rest.auth.revokeTokens( + [{ type: clientId ? "clientId" : "revocationKey", value: targetValue }], + flags["allow-reauth-margin"] ? { allowReauthMargin: true } : undefined, + ); - let reauthNote = ""; - if (flags["allow-reauth-margin"]) { - requestBody.allowReauthMargin = true; - reauthNote = - " Connected clients have a 30s grace period to obtain new tokens before disconnection."; + const failure = response.results.find((result) => "error" in result); + if (failure && "error" in failure) { + this.fail(failure.error, flags, "revokeToken", { + target: targetSpecifier, + }); } - try { - // Make direct HTTPS request to Ably REST API - const response = await this.makeHttpRequest( - keyName, - secret, - requestBody, - ); - const successMessage = `Tokens matching ${targetLabel.toLowerCase()} ${formatResource(targetValue)} have been revoked.${reauthNote}`; - - if (this.shouldOutputJson(flags)) { - this.logJsonResult( - { - revocation: { - allowReauthMargin: flags["allow-reauth-margin"], - message: stripAnsi(successMessage), - target: targetSpecifier, - response, - }, + const successMessage = `Tokens matching ${targetLabel.toLowerCase()} ${formatResource(targetValue)} have been revoked.${reauthNote}`; + + if (this.shouldOutputJson(flags)) { + this.logJsonResult( + { + revocation: { + allowReauthMargin: flags["allow-reauth-margin"], + message: stripAnsi(successMessage), + target: targetSpecifier, + response, }, - flags, - ); - } else { - this.logSuccessMessage(successMessage, flags); - } - } catch (requestError: unknown) { - const error = requestError as Error & { statusCode?: number }; - if (error.statusCode === 404) { - this.fail( - "No matching tokens found or already revoked", - flags, - "revokeToken", - ); - } - throw requestError; + }, + flags, + ); + } else { + this.logSuccessMessage(successMessage, flags); } } catch (error) { + if ((error as { statusCode?: number }).statusCode === 404) { + this.fail( + "No matching tokens found or already revoked", + flags, + "revokeToken", + ); + } + this.fail(error, flags, "revokeToken"); } } - - // Helper method to make a direct HTTP request to the Ably REST API - private makeHttpRequest( - keyName: string, - secret: string, - requestBody: Record, - ): Promise | string | null> { - return new Promise((resolve, reject) => { - const encodedAuth = Buffer.from(`${keyName}:${secret}`).toString( - "base64", - ); - - const options = { - headers: { - Accept: "application/json", - Authorization: `Basic ${encodedAuth}`, - "Content-Type": "application/json", - }, - hostname: "rest.ably.io", - method: "POST", - path: `/keys/${keyName}/revokeTokens`, - port: 443, - }; - - const req = https.request(options, (res) => { - let data = ""; - - res.on("data", (chunk) => { - data += chunk; - }); - - res.on("end", () => { - if (res.statusCode && res.statusCode >= 200 && res.statusCode < 300) { - try { - const jsonResponse: Record | null = - data.length > 0 - ? (JSON.parse(data) as Record) - : null; - resolve(jsonResponse); - } catch { - resolve(data); - } - } else { - const err = new Error( - `Request failed with status code ${res.statusCode}: ${data}`, - ) as Error & { statusCode?: number }; - err.statusCode = res.statusCode; - reject(err); - } - }); - }); - - req.on("error", (error) => { - reject(error); - }); - - req.write(JSON.stringify(requestBody)); - req.end(); - }); - } } diff --git a/src/commands/push/channels/list.ts b/src/commands/push/channels/list.ts index 9cbdafb3b..8fe766752 100644 --- a/src/commands/push/channels/list.ts +++ b/src/commands/push/channels/list.ts @@ -38,7 +38,7 @@ export default class PushChannelsList extends AblyBaseCommand { description: "Filter by device ID", }), "client-id": Flags.string({ - description: "Filter by client ID", + description: "Only list subscriptions for this client ID", }), limit: Flags.integer({ description: "Maximum number of results to return", diff --git a/src/commands/push/channels/remove-where.ts b/src/commands/push/channels/remove-where.ts index 170110cc6..37dd3d920 100644 --- a/src/commands/push/channels/remove-where.ts +++ b/src/commands/push/channels/remove-where.ts @@ -28,7 +28,7 @@ export default class PushChannelsRemoveWhere extends AblyBaseCommand { description: "Filter by device ID", }), "client-id": Flags.string({ - description: "Filter by client ID", + description: "Only remove subscriptions for this client ID", }), ...forceFlag, }; diff --git a/src/commands/push/channels/remove.ts b/src/commands/push/channels/remove.ts index fcdf34931..58b1eab9c 100644 --- a/src/commands/push/channels/remove.ts +++ b/src/commands/push/channels/remove.ts @@ -28,7 +28,7 @@ export default class PushChannelsRemove extends AblyBaseCommand { exclusive: ["client-id"], }), "client-id": Flags.string({ - description: "Client ID to unsubscribe", + description: "Client ID whose subscription to remove", exclusive: ["device-id"], }), ...forceFlag, diff --git a/src/commands/push/channels/save.ts b/src/commands/push/channels/save.ts index 94c105404..e49479a50 100644 --- a/src/commands/push/channels/save.ts +++ b/src/commands/push/channels/save.ts @@ -28,7 +28,7 @@ export default class PushChannelsSave extends AblyBaseCommand { exclusive: ["client-id"], }), "client-id": Flags.string({ - description: "Client ID to subscribe", + description: "Client ID to subscribe to the channel", exclusive: ["device-id"], }), }; diff --git a/src/commands/push/devices/list.ts b/src/commands/push/devices/list.ts index fba46e165..fcf79484b 100644 --- a/src/commands/push/devices/list.ts +++ b/src/commands/push/devices/list.ts @@ -31,7 +31,7 @@ export default class PushDevicesList extends AblyBaseCommand { description: "Filter by device ID", }), "client-id": Flags.string({ - description: "Filter by client ID", + description: "Only list devices registered to this client ID", }), state: Flags.string({ description: "Filter by device state", diff --git a/src/commands/push/devices/remove-where.ts b/src/commands/push/devices/remove-where.ts index 1947a08a1..0515f8871 100644 --- a/src/commands/push/devices/remove-where.ts +++ b/src/commands/push/devices/remove-where.ts @@ -20,7 +20,7 @@ export default class PushDevicesRemoveWhere extends AblyBaseCommand { description: "Filter by device ID", }), "client-id": Flags.string({ - description: "Filter by client ID", + description: "Only remove devices registered to this client ID", }), ...forceFlag, }; diff --git a/src/commands/push/devices/save.ts b/src/commands/push/devices/save.ts index ab3200acd..858f24de4 100644 --- a/src/commands/push/devices/save.ts +++ b/src/commands/push/devices/save.ts @@ -54,7 +54,7 @@ export default class PushDevicesSave extends AblyBaseCommand { description: "Web push auth secret (required for web transport)", }), "client-id": Flags.string({ - description: "Client ID to associate with the device", + description: "Client ID to register the device to", }), metadata: Flags.string({ description: "Device metadata as JSON", diff --git a/src/commands/push/publish.ts b/src/commands/push/publish.ts index dda3dd9d0..ddabbd0e2 100644 --- a/src/commands/push/publish.ts +++ b/src/commands/push/publish.ts @@ -35,7 +35,7 @@ export default class PushPublish extends AblyBaseCommand { exclusive: ["client-id", "recipient"], }), "client-id": Flags.string({ - description: "Target client ID", + description: "Client ID whose devices receive the notification", exclusive: ["device-id", "recipient"], }), recipient: Flags.string({ diff --git a/src/data/env-vars.ts b/src/data/env-vars.ts index 1404e59e6..a8102e939 100644 --- a/src/data/env-vars.ts +++ b/src/data/env-vars.ts @@ -129,7 +129,7 @@ const ABLY_API_KEY = new EnvVarEntry( new DetailSection("Client ID", [ { kind: "paragraph", - text: "Auto-generates a default client ID in the format `ably-cli-{uuid}`. Override with `--client-id `, or pass `--client-id none` to send no client ID.", + text: "Acts as one client ID per install, generated once and saved in the config. Set a fixed one with `ABLY_CLIENT_ID`, or override it per command with `--client-id `.", }, ]), ], @@ -158,7 +158,7 @@ const ABLY_TOKEN = new EnvVarEntry( new DetailSection("Client ID", [ { kind: "paragraph", - text: "`--client-id` is ignored when `ABLY_TOKEN` is set — the client ID is embedded in the token. A warning is logged if `--client-id` is passed.", + text: "Comes from the token, which must carry one (`--client-id` and `ABLY_CLIENT_ID` are ignored). The CLI always connects as a server, which Ably accepts under token auth only from a JWT with the `x-ably-clientType: server` claim (`ably auth issue-jwt-token --client-type server`).", }, ]), new DetailSection("Token expiry", [ diff --git a/src/flags.ts b/src/flags.ts index aec47cbb4..2a678fdc9 100644 --- a/src/flags.ts +++ b/src/flags.ts @@ -69,13 +69,20 @@ export const hiddenControlApiFlags = { }; /** - * client-id flag for commands where client identity matters (e.g., subscribe, publish, enter, update, delete). - * Not needed for read-only queries (get, get-all, occupancy get) — Ably capabilities are operation-based, not clientId-based. + * client-id flag for commands where acting as a particular client is part of the operation + * (e.g., subscribe, publish, enter, update, delete). Read-only queries (get, get-all, occupancy get) + * don't take it: they still carry the session's client ID (ABLY_CLIENT_ID, config, or the generated + * default), and Ably counts that ID, but there's no per-command reason to act as someone else. + * + * This exact definition is the identity flag: the base command honours + * `client-id` as the CLI's own identity only when a command declares it via + * this object. A command-local `client-id` flag (a push target or filter, a + * token's subject) never changes who the CLI acts as. */ export const clientIdFlag = { "client-id": Flags.string({ description: - 'Overrides any default client ID when using API authentication. Use "none" to explicitly set no client ID. Not applicable when using token authentication.', + "Client ID to act as, overriding ABLY_CLIENT_ID and the configured default. Ignored under token authentication, where the token sets it.", }), }; diff --git a/src/services/client-identity.ts b/src/services/client-identity.ts index 2e306ffe2..b56dc6ce4 100644 --- a/src/services/client-identity.ts +++ b/src/services/client-identity.ts @@ -46,7 +46,7 @@ function validateClientId(value: string, source: ClientIdSource): void { if (value === "*") { throw new InvalidClientIdError( - `${origin} cannot be "*". The wildcard is only valid inside a token's capability; the CLI must act as one concrete client ID.`, + `${origin} cannot be "*". The CLI acts as, and issues tokens to, one concrete client ID.`, ); } } diff --git a/src/spaces-base-command.ts b/src/spaces-base-command.ts index f5b8d3a32..05668536a 100644 --- a/src/spaces-base-command.ts +++ b/src/spaces-base-command.ts @@ -8,7 +8,7 @@ import { import { AblyBaseCommand } from "./base-command.js"; import { productApiFlags } from "./flags.js"; import { BaseFlags } from "./types/cli.js"; -import { errorMessage } from "./utils/errors.js"; +import { errorMessage, errorWithReason } from "./utils/errors.js"; import isTestMode from "./utils/test-mode.js"; export abstract class SpacesBaseCommand extends AblyBaseCommand { @@ -226,7 +226,10 @@ export abstract class SpacesBaseCommand extends AblyBaseCommand { state: connection.state, }); this.fail( - `${errorMsg}. Use --verbose to see detailed connection logs.`, + errorWithReason( + `${errorMsg}. Use --verbose to see detailed connection logs.`, + connection.errorReason, + ), flags, "connection", ); @@ -263,7 +266,7 @@ export abstract class SpacesBaseCommand extends AblyBaseCommand { this.logCliEvent(flags, "connection", "failed", errorMsg, { state: "failed", }); - reject(new Error(errorMsg)); + reject(errorWithReason(errorMsg, stateChange.reason)); }; const onClosed = () => { @@ -271,9 +274,9 @@ export abstract class SpacesBaseCommand extends AblyBaseCommand { reject(new Error("Connection closed unexpectedly")); }; - const onSuspended = () => { + const onSuspended = (stateChange: Ably.ConnectionStateChange) => { cleanup(); - reject(new Error("Connection suspended")); + reject(errorWithReason("Connection suspended", stateChange.reason)); }; connection.on("connected", onConnected); @@ -381,8 +384,9 @@ export abstract class SpacesBaseCommand extends AblyBaseCommand { channel.off("attached", onAttached); channel.off("failed", onFailed); reject( - new Error( + errorWithReason( `Cursors channel failed to attach: ${stateChange.reason?.message || "Unknown error"}`, + stateChange.reason, ), ); }; diff --git a/src/utils/connection-health.ts b/src/utils/connection-health.ts new file mode 100644 index 000000000..b47f762af --- /dev/null +++ b/src/utils/connection-health.ts @@ -0,0 +1,67 @@ +import type * as Ably from "@ably/pubsub-server"; + +import { errorWithReason } from "./errors.js"; + +/** Consecutive unstable drops after which a connection counts as dead. */ +export const MAX_UNSTABLE_DROPS = 5; + +/** How long a connection must stay up for its next drop to start a new count. */ +export const STABLE_CONNECTION_MS = 10_000; + +/** + * Watches a connection's state changes and reports, once, when it has died: + * either it failed (a terminal state the SDK never leaves), or it keeps + * dropping without staying connected. The second is how a server-side + * rejection such as an eviction can present, as a loop the SDK retries + * forever rather than an error. + */ +export class ConnectionHealthMonitor { + private connectedAt: number | undefined; + private unstableDrops = 0; + private reported = false; + + constructor( + private readonly onDead: (error: Error) => void, + private readonly now: () => number = Date.now, + ) {} + + handle(change: Ably.ConnectionStateChange): void { + switch (change.current) { + case "connected": { + this.connectedAt = this.now(); + break; + } + + case "failed": { + this.report( + `Connection failed: ${change.reason?.message ?? "Unknown error"}`, + change.reason, + ); + break; + } + + case "disconnected": + case "suspended": { + const wasStable = + this.connectedAt !== undefined && + this.now() - this.connectedAt >= STABLE_CONNECTION_MS; + this.unstableDrops = wasStable ? 1 : this.unstableDrops + 1; + this.connectedAt = undefined; + + if (this.unstableDrops >= MAX_UNSTABLE_DROPS) { + this.report( + `Connection lost ${this.unstableDrops} times in a row without staying connected${change.reason ? `: ${change.reason.message}` : ""}`, + change.reason, + ); + } + break; + } + } + } + + private report(message: string, reason?: Ably.ErrorInfo | null): void { + if (this.reported) return; + this.reported = true; + this.onDead(errorWithReason(message, reason)); + } +} diff --git a/src/utils/errors.ts b/src/utils/errors.ts index d5035c5aa..b90771865 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -45,14 +45,59 @@ export function extractErrorInfo(error: unknown): { } /** - * Return a friendly, actionable hint for known Ably error codes. - * Returns undefined for unknown codes. + * Build an Error with a CLI-facing message that keeps the Ably code, status + * code and help URL of the underlying reason, so `fail()` can still attach + * the code and its hint. */ -const clientIdHint = "Use the --client-id flag to set a client identity."; +export function errorWithReason( + message: string, + reason?: { code?: number; statusCode?: number; href?: string } | null, +): Error { + return Object.assign(new Error(message), { + code: reason?.code, + statusCode: reason?.statusCode, + href: reason?.href, + }); +} + +/** What a hint may depend on: how the CLI authenticated and its flags. */ +export interface HintContext { + /** Authenticating with ABLY_TOKEN, so the client ID comes from the token. */ + tokenAuth?: boolean; + /** The command declares the identity --client-id flag. */ + hasClientIdFlag?: boolean; +} + +type Hint = string | ((context: HintContext) => string); + const tokenExpiredHint = "Generate a new token or use an API key instead. See https://ably.com/docs/auth for details."; -const hints: Record = { +const reissueTokenHint = + 'The client ID comes from your token: re-issue it with one, e.g. "ably auth issue-jwt-token --client-id ".'; + +function clientIdSources({ hasClientIdFlag }: HintContext): string { + return hasClientIdFlag + ? "--client-id or the ABLY_CLIENT_ID environment variable" + : "the ABLY_CLIENT_ID environment variable"; +} + +const clientIdHint: Hint = (context) => + context.tokenAuth + ? reissueTokenHint + : `Set a client ID with ${clientIdSources(context)}.`; + +const invalidClientIdHint: Hint = (context) => + context.tokenAuth + ? reissueTokenHint + : `Set a valid client ID (not empty and not "*") with ${clientIdSources(context)}.`; + +/** + * Return a friendly, actionable hint for known Ably error codes. + * Returns undefined for unknown codes. + */ +const hints: Record = { + 40012: invalidClientIdHint, 40101: 'Check your API key or token, or re-authenticate with "ably login".', 40103: "This is unexpected - TLS is enabled by default. Please report this issue at https://ably.com/support", @@ -71,7 +116,11 @@ const hints: Record = { 91000: clientIdHint, }; -export function getFriendlyAblyErrorHint(code?: number): string | undefined { +export function getFriendlyAblyErrorHint( + code?: number, + context: HintContext = {}, +): string | undefined { if (code === undefined) return undefined; - return hints[code]; + const hint = hints[code]; + return typeof hint === "function" ? hint(context) : hint; } diff --git a/src/utils/long-running.ts b/src/utils/long-running.ts index e488f9b60..28e4a8005 100644 --- a/src/utils/long-running.ts +++ b/src/utils/long-running.ts @@ -8,10 +8,15 @@ import isTestMode from "./test-mode.js"; -export type ExitReason = "signal" | "timeout"; +export type ExitReason = "signal" | "timeout" | "aborted"; +/** + * Wait until Ctrl+C, the duration elapses, or `signal` aborts (which a + * command uses to stop waiting on a connection that has died). + */ export async function waitUntilInterruptedOrTimeout( durationSeconds?: number, + signal?: AbortSignal, ): Promise { // In test mode, we may have many instances running concurrently // Increase the max listeners to avoid warnings @@ -26,6 +31,7 @@ export async function waitUntilInterruptedOrTimeout( let sigintHandler: (() => void) | undefined; let sigtermHandler: (() => void) | undefined; let resolved = false; + const abortHandler = (): void => handleExit("aborted"); const handleExit = (reason: ExitReason): void => { if (resolved) { @@ -38,6 +44,7 @@ export async function waitUntilInterruptedOrTimeout( // Remove signal handlers if they were installed if (sigintHandler) process.removeListener("SIGINT", sigintHandler); if (sigtermHandler) process.removeListener("SIGTERM", sigtermHandler); + signal?.removeEventListener("abort", abortHandler); resolve(reason); }; @@ -66,5 +73,11 @@ export async function waitUntilInterruptedOrTimeout( process.once("SIGINT", sigintHandler); process.once("SIGTERM", sigtermHandler); + + if (signal?.aborted) { + handleExit("aborted"); + } else { + signal?.addEventListener("abort", abortHandler); + } }); } diff --git a/test/helpers/mock-ably-rest.ts b/test/helpers/mock-ably-rest.ts index f3e7a0275..19baf91e2 100644 --- a/test/helpers/mock-ably-rest.ts +++ b/test/helpers/mock-ably-rest.ts @@ -68,6 +68,7 @@ export interface MockRestAuth { clientId: string; createTokenRequest: Mock; requestToken: Mock; + revokeTokens: Mock; } /** @@ -232,6 +233,11 @@ function createMockRestAuth(): MockRestAuth { token: "mock-token", expires: Date.now() + 3600000, }), + revokeTokens: vi.fn().mockResolvedValue({ + successCount: 1, + failureCount: 0, + results: [], + }), }; } diff --git a/test/unit/base/client-id-target-guard.test.ts b/test/unit/base/client-id-target-guard.test.ts new file mode 100644 index 000000000..acaa75467 --- /dev/null +++ b/test/unit/base/client-id-target-guard.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { Config } from "@oclif/core"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const projectRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../..", +); + +const TARGET = "target-client-id"; + +type ClientOptionsReader = { + getClientOptions(flags: Record): { clientId?: string }; +}; + +/** + * Commands such as `push devices remove-where --client-id user123` declare + * their own --client-id to name a target or filter. Before the identity flag + * was singled out, that value also became the CLI's own wire identity, which + * billed an end user who never connected. This guard covers every command, + * including ones added later. + */ +describe("command-local --client-id flags", () => { + let config: Config; + + beforeAll(async () => { + process.env.ABLY_API_KEY = "appId.keyId:keySecret"; + config = await Config.load({ root: projectRoot }); + }); + + afterAll(() => { + delete process.env.ABLY_API_KEY; + }); + + it("never become the client ID the CLI acts as", async () => { + // oclif loads commands outside the test's module graph, so take the shared + // identity flag from a command known to declare it rather than importing it. + const publish = await config.findCommand("channels:publish")!.load(); + const identityFlag = publish.flags["client-id"]; + + const targetCommands: string[] = []; + const leaked: string[] = []; + for (const command of config.commands) { + const CommandClass = await command.load(); + // Topic index commands declare no flags at all, despite the static type. + const flags = CommandClass.flags as Record | undefined; + const flag = flags?.["client-id"]; + if (!flag || flag === identityFlag) continue; + + targetCommands.push(command.id); + const instance = new (CommandClass as unknown as new ( + argv: string[], + config: Config, + ) => ClientOptionsReader)([], config); + + const { clientId } = instance.getClientOptions({ "client-id": TARGET }); + if (clientId === TARGET) leaked.push(command.id); + } + + expect(leaked).toEqual([]); + + // Guards the guard: if this drops to zero the loop above checked nothing. + expect(targetCommands).toContain("push:devices:remove-where"); + }); +}); diff --git a/test/unit/base/client-identity.test.ts b/test/unit/base/client-identity.test.ts index 42738f362..eeda828f3 100644 --- a/test/unit/base/client-identity.test.ts +++ b/test/unit/base/client-identity.test.ts @@ -3,6 +3,7 @@ import { Config } from "@oclif/core"; import jwt from "jsonwebtoken"; import { AblyBaseCommand } from "../../../src/base-command.js"; +import { clientIdFlag } from "../../../src/flags.js"; import { CLIENT_ID_ENV_VAR, readTokenIdentity, @@ -15,6 +16,8 @@ import { } from "../../helpers/mock-config-manager.js"; class TestCommand extends AblyBaseCommand { + static override flags = { ...clientIdFlag }; + async run(): Promise {} public testGetClientOptions(flags: BaseFlags) { diff --git a/test/unit/commands/auth/issue-ably-token.test.ts b/test/unit/commands/auth/issue-ably-token.test.ts index 4390dac61..29505c7c6 100644 --- a/test/unit/commands/auth/issue-ably-token.test.ts +++ b/test/unit/commands/auth/issue-ably-token.test.ts @@ -1,6 +1,9 @@ import { describe, it, expect, beforeEach } from "vitest"; import { runCommand } from "@oclif/test"; -import { getMockConfigManager } from "../../../helpers/mock-config-manager.js"; +import { + MOCK_DEFAULT_CLIENT_ID, + getMockConfigManager, +} from "../../../helpers/mock-config-manager.js"; import { getMockAblyRest } from "../../../helpers/mock-ably-rest.js"; import { standardHelpTests, @@ -136,18 +139,41 @@ describe("auth:issue-ably-token command", () => { restMock.auth.createTokenRequest.mockResolvedValue({}); restMock.auth.requestToken.mockResolvedValue(mockTokenDetails); - const { stdout } = await runCommand( + const { stdout, stderr } = await runCommand( ["auth:issue-ably-token", "--client-id", "none"], import.meta.url, ); expect(stdout).toContain("Ably token generated."); - expect(stdout).not.toContain("Client ID:"); + expect(stdout).toContain("Client ID: anonymous"); + expect(stderr).toContain("Issuing a token with no client ID"); expect(restMock.auth.createTokenRequest).toHaveBeenCalled(); const tokenParams = restMock.auth.createTokenRequest.mock.calls[0][0]; expect(tokenParams.clientId).toBeUndefined(); }); + it("should default to the client ID the CLI acts as", async () => { + const restMock = getMockAblyRest(); + restMock.auth.createTokenRequest.mockResolvedValue({}); + + await runCommand(["auth:issue-ably-token"], import.meta.url); + + const tokenParams = restMock.auth.createTokenRequest.mock.calls[0][0]; + expect(tokenParams.clientId).toBe(MOCK_DEFAULT_CLIENT_ID); + }); + + it("should refuse a wildcard client ID", async () => { + const restMock = getMockAblyRest(); + + const { error } = await runCommand( + ["auth:issue-ably-token", "--client-id", "*"], + import.meta.url, + ); + + expect(error?.message).toContain('cannot be "*"'); + expect(restMock.auth.createTokenRequest).not.toHaveBeenCalled(); + }); + it("should output only token string with --token-only flag", async () => { const restMock = getMockAblyRest(); const mockTokenString = "mock-ably-token-only"; diff --git a/test/unit/commands/auth/issue-jwt-token.test.ts b/test/unit/commands/auth/issue-jwt-token.test.ts index 5790ca336..c12695d33 100644 --- a/test/unit/commands/auth/issue-jwt-token.test.ts +++ b/test/unit/commands/auth/issue-jwt-token.test.ts @@ -1,7 +1,10 @@ import { describe, it, expect } from "vitest"; import { runCommand } from "@oclif/test"; import jwt from "jsonwebtoken"; -import { getMockConfigManager } from "../../../helpers/mock-config-manager.js"; +import { + MOCK_DEFAULT_CLIENT_ID, + getMockConfigManager, +} from "../../../helpers/mock-config-manager.js"; import { standardHelpTests, standardArgValidationTests, @@ -128,6 +131,54 @@ describe("auth:issue-jwt-token command", () => { expect(decoded["x-ably-clientId"]).toBeUndefined(); }); + it("should default to the client ID the CLI acts as", async () => { + const { stdout } = await runCommand( + ["auth:issue-jwt-token", "--token-only"], + import.meta.url, + ); + + expect(jwt.decode(stdout.trim(), { json: true })).toHaveProperty( + "x-ably-clientId", + MOCK_DEFAULT_CLIENT_ID, + ); + }); + + it("should refuse a wildcard client ID", async () => { + const { error } = await runCommand( + ["auth:issue-jwt-token", "--client-id", "*"], + import.meta.url, + ); + + expect(error?.message).toContain('cannot be "*"'); + }); + + it("should add the server claim with --client-type server", async () => { + const { stdout } = await runCommand( + ["auth:issue-jwt-token", "--client-type", "server", "--json"], + import.meta.url, + ); + + const result = parseJsonOutput(stdout); + const token = (result.token as { value: string; clientType: string }) + .value; + expect(result.token).toHaveProperty("clientType", "server"); + expect(jwt.decode(token, { json: true })).toHaveProperty( + "x-ably-clientType", + "server", + ); + }); + + it("should omit the server claim by default", async () => { + const { stdout } = await runCommand( + ["auth:issue-jwt-token", "--token-only"], + import.meta.url, + ); + + expect(jwt.decode(stdout.trim(), { json: true })).not.toHaveProperty( + "x-ably-clientType", + ); + }); + it("should output only token string with --token-only flag", async () => { const { stdout } = await runCommand( ["auth:issue-jwt-token", "--token-only"], @@ -247,13 +298,13 @@ describe("auth:issue-jwt-token command", () => { expect(stdout).toContain("Client ID: my-client"); }); - it("should omit client ID line when not specified with none", async () => { + it("should show the token as anonymous with none", async () => { const { stdout } = await runCommand( ["auth:issue-jwt-token", "--client-id", "none"], import.meta.url, ); - expect(stdout).not.toContain("Client ID:"); + expect(stdout).toContain("Client ID: anonymous"); }); }); }); diff --git a/test/unit/commands/auth/revoke-token.test.ts b/test/unit/commands/auth/revoke-token.test.ts index 9708860c7..41598cae8 100644 --- a/test/unit/commands/auth/revoke-token.test.ts +++ b/test/unit/commands/auth/revoke-token.test.ts @@ -1,9 +1,8 @@ import { Readable } from "node:stream"; -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, afterEach } from "vitest"; import { runCommand } from "@oclif/test"; -import nock from "nock"; -import { getMockConfigManager } from "../../../helpers/mock-config-manager.js"; +import { getMockAblyRest } from "../../../helpers/mock-ably-rest.js"; import { standardHelpTests, standardFlagTests, @@ -14,14 +13,6 @@ describe("auth:revoke-token command", () => { const mockClientId = "test-client-id"; const mockRevocationKey = "group1"; - beforeEach(() => { - nock.cleanAll(); - }); - - afterEach(() => { - nock.cleanAll(); - }); - standardHelpTests("auth:revoke-token", import.meta.url); describe("argument validation", () => { @@ -60,31 +51,22 @@ describe("auth:revoke-token command", () => { describe("functionality", () => { describe("token revocation by client ID", () => { it("should successfully revoke tokens for a client ID", async () => { - const mockConfig = getMockConfigManager(); - const keyId = mockConfig.getKeyId()!; - nock("https://rest.ably.io") - .post(`/keys/${keyId}/revokeTokens`, { - targets: [`clientId:${mockClientId}`], - }) - .reply(200, {}); + const { auth } = getMockAblyRest(); const { stderr } = await runCommand( ["auth:revoke-token", "--client-id", mockClientId, "--force"], import.meta.url, ); + expect(auth.revokeTokens).toHaveBeenCalledWith( + [{ type: "clientId", value: mockClientId }], + undefined, + ); expect(stderr).toContain("have been revoked"); }); it("should include allowReauthMargin when --allow-reauth-margin is provided", async () => { - const mockConfig = getMockConfigManager(); - const keyId = mockConfig.getKeyId()!; - nock("https://rest.ably.io") - .post(`/keys/${keyId}/revokeTokens`, { - targets: [`clientId:${mockClientId}`], - allowReauthMargin: true, - }) - .reply(200, {}); + const { auth } = getMockAblyRest(); const { stderr } = await runCommand( [ @@ -97,18 +79,14 @@ describe("auth:revoke-token command", () => { import.meta.url, ); + expect(auth.revokeTokens).toHaveBeenCalledWith( + [{ type: "clientId", value: mockClientId }], + { allowReauthMargin: true }, + ); expect(stderr).toContain("have been revoked"); }); it("should output JSON format when --json flag is used", async () => { - const mockConfig = getMockConfigManager(); - const keyId = mockConfig.getKeyId()!; - nock("https://rest.ably.io") - .post(`/keys/${keyId}/revokeTokens`, { - targets: [`clientId:${mockClientId}`], - }) - .reply(200, { issuedBefore: 1234567890 }); - const { stdout } = await runCommand( [ "auth:revoke-token", @@ -139,13 +117,7 @@ describe("auth:revoke-token command", () => { describe("token revocation by revocation key", () => { it("should successfully revoke tokens for a revocation key", async () => { - const mockConfig = getMockConfigManager(); - const keyId = mockConfig.getKeyId()!; - nock("https://rest.ably.io") - .post(`/keys/${keyId}/revokeTokens`, { - targets: [`revocationKey:${mockRevocationKey}`], - }) - .reply(200, {}); + const { auth } = getMockAblyRest(); const { stderr } = await runCommand( [ @@ -157,6 +129,10 @@ describe("auth:revoke-token command", () => { import.meta.url, ); + expect(auth.revokeTokens).toHaveBeenCalledWith( + [{ type: "revocationKey", value: mockRevocationKey }], + undefined, + ); expect(stderr).toContain("have been revoked"); }); }); @@ -201,10 +177,7 @@ describe("auth:revoke-token command", () => { it("should cancel when user declines confirmation", async () => { mockStdinAnswer("n"); - - const revokeNock = nock("https://rest.ably.io") - .post(/\/keys\/.*\/revokeTokens/) - .reply(200, {}); + const { auth } = getMockAblyRest(); const { stderr } = await runCommand( ["auth:revoke-token", "--client-id", mockClientId], @@ -212,18 +185,16 @@ describe("auth:revoke-token command", () => { ); expect(stderr).toContain("Revocation cancelled"); - expect(revokeNock.isDone()).toBe(false); + expect(auth.revokeTokens).not.toHaveBeenCalled(); }); }); }); describe("error handling", () => { it("should handle token not found error", async () => { - const mockConfig = getMockConfigManager(); - const keyId = mockConfig.getKeyId()!; - nock("https://rest.ably.io") - .post(`/keys/${keyId}/revokeTokens`) - .reply(404, "token_not_found"); + getMockAblyRest().auth.revokeTokens.mockRejectedValueOnce( + Object.assign(new Error("token_not_found"), { statusCode: 404 }), + ); const { error } = await runCommand( ["auth:revoke-token", "--client-id", mockClientId, "--force"], @@ -236,35 +207,42 @@ describe("auth:revoke-token command", () => { }); it("should handle authentication error (invalid API key)", async () => { - const mockConfig = getMockConfigManager(); - const keyId = mockConfig.getKeyId()!; - nock("https://rest.ably.io") - .post(`/keys/${keyId}/revokeTokens`) - .reply(401, { error: { message: "Unauthorized" } }); + getMockAblyRest().auth.revokeTokens.mockRejectedValueOnce( + Object.assign(new Error("Unauthorized"), { + code: 40100, + statusCode: 401, + }), + ); const { error } = await runCommand( ["auth:revoke-token", "--client-id", mockClientId, "--force"], import.meta.url, ); - expect(error).toBeDefined(); - expect(error?.message).toMatch(/401|error|revoking/i); + expect(error?.message).toContain("Unauthorized"); }); - it("should handle server error", async () => { - const mockConfig = getMockConfigManager(); - const keyId = mockConfig.getKeyId()!; - nock("https://rest.ably.io") - .post(`/keys/${keyId}/revokeTokens`) - .reply(500, { error: "Internal Server Error" }); + it("should report a per-target failure", async () => { + getMockAblyRest().auth.revokeTokens.mockResolvedValueOnce({ + successCount: 0, + failureCount: 1, + results: [ + { + target: `clientId:${mockClientId}`, + error: Object.assign(new Error("Target rejected"), { + code: 40000, + statusCode: 400, + }), + }, + ], + }); const { error } = await runCommand( ["auth:revoke-token", "--client-id", mockClientId, "--force"], import.meta.url, ); - expect(error).toBeDefined(); - expect(error?.message).toMatch(/500|error|revoking/i); + expect(error?.message).toContain("Target rejected"); }); }); diff --git a/test/unit/commands/channels/subscribe.test.ts b/test/unit/commands/channels/subscribe.test.ts index 83c14c67b..463811e00 100644 --- a/test/unit/commands/channels/subscribe.test.ts +++ b/test/unit/commands/channels/subscribe.test.ts @@ -72,6 +72,28 @@ describe("channels:subscribe command", () => { ); }); + it("should exit non-zero when the connection fails mid-subscription", async () => { + const mock = getMockAblyRealtime(); + const commandPromise = runCommand( + ["channels:subscribe", "test-channel"], + import.meta.url, + ); + + await vi.waitFor(() => { + expect(mockSubscribeCallback).not.toBeNull(); + }); + mock.connection._emit({ + current: "failed", + previous: "connected", + reason: { message: "Evicted", code: 40012, statusCode: 400 }, + } as Parameters[0]); + + const { error } = await commandPromise; + expect(error?.message).toContain("Connection failed: Evicted"); + expect(error?.message).toContain("Ably error code: 40012"); + expect(error?.oclif?.exit).toBeGreaterThan(0); + }); + it("should receive and display messages with event name and data", async () => { // Run command in background-like manner const commandPromise = runCommand( diff --git a/test/unit/utils/connection-health.test.ts b/test/unit/utils/connection-health.test.ts new file mode 100644 index 000000000..8b160fd67 --- /dev/null +++ b/test/unit/utils/connection-health.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect, vi } from "vitest"; + +import { + ConnectionHealthMonitor, + MAX_UNSTABLE_DROPS, + STABLE_CONNECTION_MS, +} from "../../../src/utils/connection-health.js"; + +type Change = Parameters[0]; + +function change( + current: Change["current"], + reason?: { message: string; code: number; statusCode: number }, +): Change { + return { current, previous: "connected", reason } as Change; +} + +describe("ConnectionHealthMonitor", () => { + it("reports a failed connection with its reason's code", () => { + const onDead = vi.fn(); + const monitor = new ConnectionHealthMonitor(onDead); + + monitor.handle( + change("failed", { message: "Evicted", code: 40012, statusCode: 400 }), + ); + + expect(onDead).toHaveBeenCalledWith( + expect.objectContaining({ + message: "Connection failed: Evicted", + code: 40012, + statusCode: 400, + }), + ); + }); + + it("reports a reconnect loop that never stays connected", () => { + const onDead = vi.fn(); + const monitor = new ConnectionHealthMonitor(onDead, () => 0); + + for (let i = 0; i < MAX_UNSTABLE_DROPS - 1; i++) { + monitor.handle(change("connected")); + monitor.handle(change("disconnected")); + } + expect(onDead).not.toHaveBeenCalled(); + + monitor.handle(change("connected")); + monitor.handle(change("disconnected")); + expect(onDead).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining(`${MAX_UNSTABLE_DROPS} times`), + }), + ); + }); + + it("counts drops while never reaching connected", () => { + const onDead = vi.fn(); + const monitor = new ConnectionHealthMonitor(onDead); + + for (let i = 0; i < MAX_UNSTABLE_DROPS; i++) { + monitor.handle(change("connecting")); + monitor.handle(change("disconnected")); + } + + expect(onDead).toHaveBeenCalledOnce(); + }); + + it("restarts the count after a stable connection", () => { + const onDead = vi.fn(); + let now = 0; + const monitor = new ConnectionHealthMonitor(onDead, () => now); + + for (let i = 0; i < MAX_UNSTABLE_DROPS - 1; i++) { + monitor.handle(change("connected")); + monitor.handle(change("disconnected")); + } + + monitor.handle(change("connected")); + now += STABLE_CONNECTION_MS; + monitor.handle(change("disconnected")); + + expect(onDead).not.toHaveBeenCalled(); + }); + + it("reports only once", () => { + const onDead = vi.fn(); + const monitor = new ConnectionHealthMonitor(onDead); + + monitor.handle(change("failed")); + monitor.handle(change("failed")); + + expect(onDead).toHaveBeenCalledOnce(); + }); +}); diff --git a/test/unit/utils/errors.test.ts b/test/unit/utils/errors.test.ts index 48ae9201b..af32d0b05 100644 --- a/test/unit/utils/errors.test.ts +++ b/test/unit/utils/errors.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect } from "vitest"; -import { errorMessage } from "../../../src/utils/errors.js"; +import { + errorMessage, + errorWithReason, + getFriendlyAblyErrorHint, +} from "../../../src/utils/errors.js"; describe("errorMessage", () => { it("should extract message from Error instances", () => { @@ -11,3 +15,49 @@ describe("errorMessage", () => { expect(errorMessage(42)).toBe("42"); }); }); + +describe("errorWithReason", () => { + it("keeps the reason's code, status code and help URL", () => { + const error = errorWithReason("Connection failed: nope", { + code: 40012, + statusCode: 400, + href: "https://help.ably.io/error/40012", + }); + + expect(error).toMatchObject({ + message: "Connection failed: nope", + code: 40012, + statusCode: 400, + href: "https://help.ably.io/error/40012", + }); + }); +}); + +describe("getFriendlyAblyErrorHint", () => { + it("returns undefined for unknown codes", () => { + expect(getFriendlyAblyErrorHint(12345)).toBeUndefined(); + }); + + it.each([40012, 40161, 91000])( + "points %i at --client-id only when the command has it", + (code) => { + expect( + getFriendlyAblyErrorHint(code, { hasClientIdFlag: true }), + ).toContain("--client-id or the ABLY_CLIENT_ID"); + expect(getFriendlyAblyErrorHint(code, {})).not.toContain("--client-id"); + expect(getFriendlyAblyErrorHint(code, {})).toContain("ABLY_CLIENT_ID"); + }, + ); + + it.each([40012, 40161, 91000])( + "points %i at re-issuing the token under token auth", + (code) => { + expect( + getFriendlyAblyErrorHint(code, { + tokenAuth: true, + hasClientIdFlag: true, + }), + ).toContain("re-issue it"); + }, + ); +}); diff --git a/test/unit/utils/long-running.test.ts b/test/unit/utils/long-running.test.ts index ed79cff3d..e72c42afc 100644 --- a/test/unit/utils/long-running.test.ts +++ b/test/unit/utils/long-running.test.ts @@ -377,4 +377,37 @@ describe("waitUntilInterruptedOrTimeout", function () { await promise; }); }); + + describe("abort behavior", function () { + it("should resolve with 'aborted' when the signal aborts", async function () { + const controller = new AbortController(); + const promise = waitUntilInterruptedOrTimeout(10, controller.signal); + + controller.abort(new Error("connection died")); + + expect(await promise).toBe("aborted"); + }); + + it("should resolve immediately for an already-aborted signal", async function () { + const controller = new AbortController(); + controller.abort(); + + expect(await waitUntilInterruptedOrTimeout(10, controller.signal)).toBe( + "aborted", + ); + }); + + it("should remove signal handlers when aborted", async function () { + const controller = new AbortController(); + const promise = waitUntilInterruptedOrTimeout(10, controller.signal); + + controller.abort(); + await promise; + + expect(processRemoveListenerStub).toHaveBeenCalledWith( + "SIGINT", + expect.any(Function), + ); + }); + }); });