From ddc0964ff99eee51d557255ad8991dc614d8e085 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 21 Apr 2026 17:04:35 +0530 Subject: [PATCH 1/3] Replace --name, --channel, and --location flags with positional arguments on commands where the flag value represents the primary entity being acted upon, aligning with CLI conventions (docopt, POSIX, sibling commands). --- src/commands/apps/create.ts | 27 +++++---- src/commands/apps/rules/create.ts | 27 +++++---- src/commands/apps/rules/index.ts | 6 +- src/commands/auth/keys/create.ts | 37 ++++++------ src/commands/auth/keys/index.ts | 2 +- src/commands/push/channels/index.ts | 4 +- src/commands/push/channels/list.ts | 25 ++++---- src/commands/push/channels/remove-where.ts | 25 ++++---- src/commands/push/channels/remove.ts | 31 +++++----- src/commands/push/channels/save.ts | 27 +++++---- src/commands/push/index.ts | 2 +- src/commands/queues/create.ts | 27 +++++---- src/commands/queues/index.ts | 4 +- src/commands/spaces/locations/set.ts | 18 +++--- .../e2e/control/control-api-workflows.test.ts | 58 +++++-------------- test/e2e/push/channels-e2e.test.ts | 9 ++- test/e2e/spaces/spaces-e2e.test.ts | 4 +- test/helpers/standard-tests.ts | 2 +- test/unit/commands/apps/create.test.ts | 24 ++++---- test/unit/commands/apps/rules/create.test.ts | 23 +++----- test/unit/commands/auth/keys/create.test.ts | 39 +++++-------- test/unit/commands/push/channels/list.test.ts | 11 ++-- .../push/channels/remove-where.test.ts | 15 ++--- .../commands/push/channels/remove.test.ts | 6 +- test/unit/commands/push/channels/save.test.ts | 36 ++---------- test/unit/commands/queues/create.test.ts | 43 +++++--------- .../commands/spaces/locations/set.test.ts | 31 +++------- test/unit/commands/spaces/spaces.test.ts | 9 +-- 28 files changed, 237 insertions(+), 335 deletions(-) diff --git a/src/commands/apps/create.ts b/src/commands/apps/create.ts index 17c9d8518..fcdb552ca 100644 --- a/src/commands/apps/create.ts +++ b/src/commands/apps/create.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { ControlBaseCommand } from "../../control-base-command.js"; import { formatLabel, formatResource } from "../../utils/output.js"; @@ -6,19 +6,22 @@ import { formatLabel, formatResource } from "../../utils/output.js"; export default class AppsCreateCommand extends ControlBaseCommand { static description = "Create a new app"; + static args = { + appName: Args.string({ + description: "Name of the app", + required: true, + }), + }; + static examples = [ - '$ ably apps create --name "My New App"', - '$ ably apps create --name "My New App" --tls-only', - '$ ably apps create --name "My New App" --json', - '$ ABLY_ACCESS_TOKEN="YOUR_ACCESS_TOKEN" ably apps create --name "My New App"', + '$ ably apps create "My New App"', + '$ ably apps create "My New App" --tls-only', + '$ ably apps create "My New App" --json', + '$ ABLY_ACCESS_TOKEN="YOUR_ACCESS_TOKEN" ably apps create "My New App"', ]; static flags = { ...ControlBaseCommand.globalFlags, - name: Flags.string({ - description: "Name of the app", - required: true, - }), "tls-only": Flags.boolean({ default: false, description: "Whether the app should accept TLS connections only", @@ -26,14 +29,14 @@ export default class AppsCreateCommand extends ControlBaseCommand { }; async run(): Promise { - const { flags } = await this.parse(AppsCreateCommand); + const { args, flags } = await this.parse(AppsCreateCommand); try { const controlApi = this.createControlApi(flags); - this.logProgress(`Creating app ${formatResource(flags.name)}`, flags); + this.logProgress(`Creating app ${formatResource(args.appName)}`, flags); const app = await controlApi.createApp({ - name: flags.name, + name: args.appName, tlsOnly: flags["tls-only"], }); diff --git a/src/commands/apps/rules/create.ts b/src/commands/apps/rules/create.ts index b8a0cb67e..d8e9bc8d0 100644 --- a/src/commands/apps/rules/create.ts +++ b/src/commands/apps/rules/create.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { ControlBaseCommand } from "../../../control-base-command.js"; import { formatChannelRuleDetails } from "../../../utils/channel-rule-display.js"; @@ -7,12 +7,19 @@ import { formatLabel, formatResource } from "../../../utils/output.js"; export default class RulesCreateCommand extends ControlBaseCommand { static description = "Create a rule"; + static args = { + ruleName: Args.string({ + description: "Name of the rule", + required: true, + }), + }; + static examples = [ - '$ ably apps rules create --name "chat" --persisted', - '$ ably apps rules create --name "chat" --mutable-messages', - '$ ably apps rules create --name "events" --push-enabled', - '$ ably apps rules create --name "notifications" --persisted --push-enabled --app "My App"', - '$ ably apps rules create --name "chat" --persisted --json', + '$ ably apps rules create "chat" --persisted', + '$ ably apps rules create "chat" --mutable-messages', + '$ ably apps rules create "events" --push-enabled', + '$ ably apps rules create "notifications" --persisted --push-enabled --app "My App"', + '$ ably apps rules create "chat" --persisted --json', ]; static flags = { @@ -56,10 +63,6 @@ export default class RulesCreateCommand extends ControlBaseCommand { "Whether messages on channels matching this rule can be updated or deleted after publishing. Automatically enables message persistence.", required: false, }), - name: Flags.string({ - description: "Name of the rule", - required: true, - }), "persist-last": Flags.boolean({ description: "Whether to persist only the last message on channels matching this rule", @@ -89,7 +92,7 @@ export default class RulesCreateCommand extends ControlBaseCommand { }; async run(): Promise { - const { flags } = await this.parse(RulesCreateCommand); + const { args, flags } = await this.parse(RulesCreateCommand); const appId = await this.requireAppId(flags); @@ -112,7 +115,7 @@ export default class RulesCreateCommand extends ControlBaseCommand { authenticated: flags.authenticated, batchingEnabled: flags["batching-enabled"], batchingInterval: flags["batching-interval"], - id: flags.name, + id: args.ruleName, conflationEnabled: flags["conflation-enabled"], conflationInterval: flags["conflation-interval"], conflationKey: flags["conflation-key"], diff --git a/src/commands/apps/rules/index.ts b/src/commands/apps/rules/index.ts index 927a2a7a3..8c8b09a7f 100644 --- a/src/commands/apps/rules/index.ts +++ b/src/commands/apps/rules/index.ts @@ -8,8 +8,8 @@ export default class RulesIndexCommand extends BaseTopicCommand { static examples = [ "$ ably apps rules list", - '$ ably apps rules create --name "chat" --persisted', - "$ ably apps rules update chat --push-enabled", - "$ ably apps rules delete chat", + '$ ably apps rules create "chat" --persisted', + '$ ably apps rules update "chat" --push-enabled', + '$ ably apps rules delete "chat"', ]; } diff --git a/src/commands/auth/keys/create.ts b/src/commands/auth/keys/create.ts index ceab87c23..09628dcd7 100644 --- a/src/commands/auth/keys/create.ts +++ b/src/commands/auth/keys/create.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { ControlBaseCommand } from "../../../control-base-command.js"; import { formatCapabilities } from "../../../utils/key-display.js"; @@ -8,16 +8,23 @@ import { formatLabel, formatResource } from "../../../utils/output.js"; export default class KeysCreateCommand extends ControlBaseCommand { static description = "Create a new API key for an app"; + static args = { + keyName: Args.string({ + description: "Name of the key", + required: true, + }), + }; + static examples = [ - `$ ably auth keys create --name "My New Key"`, - `$ ably auth keys create --name "My New Key" --app APP_ID`, - `$ ably auth keys create --name "My New Key" --capabilities '{"*":["*"]}'`, - `$ ably auth keys create --name "My New Key" --capabilities '{"channel1":["publish","subscribe"],"channel2":["history"]}'`, - `$ ably auth keys create --name "My New Key" --capabilities "publish,subscribe"`, - `$ ably auth keys create --name "My New Key" --json`, - `$ ably auth keys create --name "My New Key" --pretty-json`, - `$ ably auth keys create --app APP_ID --name "MyKey" --capabilities '{"channel:*":["publish"]}'`, - `$ ably auth keys create --app APP_ID --name "MyOtherKey" --capabilities '{"channel:chat-*":["subscribe"],"channel:updates":["publish"]}'`, + `$ ably auth keys create "My New Key"`, + `$ ably auth keys create "My New Key" --app APP_ID`, + `$ ably auth keys create "My New Key" --capabilities '{"*":["*"]}'`, + `$ ably auth keys create "My New Key" --capabilities '{"channel1":["publish","subscribe"],"channel2":["history"]}'`, + `$ ably auth keys create "My New Key" --capabilities "publish,subscribe"`, + `$ ably auth keys create "My New Key" --json`, + `$ ably auth keys create "My New Key" --pretty-json`, + `$ ably auth keys create "MyKey" --app APP_ID --capabilities '{"channel:*":["publish"]}'`, + `$ ably auth keys create "MyOtherKey" --app APP_ID --capabilities '{"channel:chat-*":["subscribe"],"channel:updates":["publish"]}'`, ]; static flags = { @@ -31,14 +38,10 @@ export default class KeysCreateCommand extends ControlBaseCommand { description: "Capabilities as JSON object (per-channel) or comma-separated list (all channels)", }), - name: Flags.string({ - description: "Name of the key", - required: true, - }), }; async run(): Promise { - const { flags } = await this.parse(KeysCreateCommand); + const { args, flags } = await this.parse(KeysCreateCommand); const appId = await this.requireAppId(flags); @@ -52,13 +55,13 @@ export default class KeysCreateCommand extends ControlBaseCommand { try { const controlApi = this.createControlApi(flags); this.logProgress( - `Creating key ${formatResource(flags.name)} for app ${formatResource(appId)}`, + `Creating key ${formatResource(args.keyName)} for app ${formatResource(appId)}`, flags, ); const key = await controlApi.createKey(appId, { capability: capabilities, - name: flags.name, + name: args.keyName, }); if (this.shouldOutputJson(flags)) { diff --git a/src/commands/auth/keys/index.ts b/src/commands/auth/keys/index.ts index 63e2759b5..b1243ab30 100644 --- a/src/commands/auth/keys/index.ts +++ b/src/commands/auth/keys/index.ts @@ -8,7 +8,7 @@ export default class AuthKeys extends BaseTopicCommand { static examples = [ "$ ably auth keys list", - '$ ably auth keys create --name "My New Key"', + '$ ably auth keys create "My New Key"', "$ ably auth keys get KEY_ID", "$ ably auth keys revoke KEY_ID", '$ ably auth keys update KEY_ID --name "New Name"', diff --git a/src/commands/push/channels/index.ts b/src/commands/push/channels/index.ts index ef63ed262..2471d387a 100644 --- a/src/commands/push/channels/index.ts +++ b/src/commands/push/channels/index.ts @@ -8,8 +8,8 @@ export default class PushChannels extends BaseTopicCommand { "Manage push notification channel subscriptions"; static override examples = [ - "<%= config.bin %> <%= command.id %> list --channel my-channel", - "<%= config.bin %> <%= command.id %> save --channel my-channel --device-id device-123", + '<%= config.bin %> <%= command.id %> list "my-channel"', + '<%= config.bin %> <%= command.id %> save "my-channel" --device-id device-123', "<%= config.bin %> <%= command.id %> list-channels", ]; } diff --git a/src/commands/push/channels/list.ts b/src/commands/push/channels/list.ts index a1c9b259c..a3e875daf 100644 --- a/src/commands/push/channels/list.ts +++ b/src/commands/push/channels/list.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { AblyBaseCommand } from "../../../base-command.js"; import { productApiFlags } from "../../../flags.js"; @@ -20,18 +20,21 @@ import { export default class PushChannelsList extends AblyBaseCommand { static override description = "List push channel subscriptions"; + static override args = { + channelName: Args.string({ + description: "Channel name to list subscriptions for", + required: true, + }), + }; + static override examples = [ - "<%= config.bin %> <%= command.id %> --channel my-channel", - "<%= config.bin %> <%= command.id %> --channel my-channel --device-id device-123", - "<%= config.bin %> <%= command.id %> --channel my-channel --json", + '<%= config.bin %> <%= command.id %> "my-channel"', + '<%= config.bin %> <%= command.id %> "my-channel" --device-id device-123', + '<%= config.bin %> <%= command.id %> "my-channel" --json', ]; static override flags = { ...productApiFlags, - channel: Flags.string({ - description: "Channel name to list subscriptions for", - required: true, - }), "device-id": Flags.string({ description: "Filter by device ID", }), @@ -46,19 +49,19 @@ export default class PushChannelsList extends AblyBaseCommand { }; async run(): Promise { - const { flags } = await this.parse(PushChannelsList); + const { args, flags } = await this.parse(PushChannelsList); try { const rest = await this.createAblyRestClient(flags as BaseFlags); if (!rest) return; this.logProgress( - `Fetching subscriptions for channel ${formatResource(flags.channel)}`, + `Fetching subscriptions for channel ${formatResource(args.channelName)}`, flags, ); const params: Record = { - channel: flags.channel, + channel: args.channelName, limit: flags.limit, }; if (flags["device-id"]) params.deviceId = flags["device-id"]; diff --git a/src/commands/push/channels/remove-where.ts b/src/commands/push/channels/remove-where.ts index 78e87efbf..842b177ac 100644 --- a/src/commands/push/channels/remove-where.ts +++ b/src/commands/push/channels/remove-where.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { AblyBaseCommand } from "../../../base-command.js"; import { forceFlag, productApiFlags } from "../../../flags.js"; @@ -10,18 +10,21 @@ export default class PushChannelsRemoveWhere extends AblyBaseCommand { static override description = "Remove push channel subscriptions matching filter criteria"; + static override args = { + channelName: Args.string({ + description: "Channel name to filter by", + required: true, + }), + }; + static override examples = [ - "<%= config.bin %> <%= command.id %> --channel my-channel", - "<%= config.bin %> <%= command.id %> --channel my-channel --device-id device-123 --force", - "<%= config.bin %> <%= command.id %> --channel my-channel --json", + '<%= config.bin %> <%= command.id %> "my-channel"', + '<%= config.bin %> <%= command.id %> "my-channel" --device-id device-123 --force', + '<%= config.bin %> <%= command.id %> "my-channel" --json', ]; static override flags = { ...productApiFlags, - channel: Flags.string({ - description: "Channel name to filter by", - required: true, - }), "device-id": Flags.string({ description: "Filter by device ID", }), @@ -32,14 +35,14 @@ export default class PushChannelsRemoveWhere extends AblyBaseCommand { }; async run(): Promise { - const { flags } = await this.parse(PushChannelsRemoveWhere); + const { args, flags } = await this.parse(PushChannelsRemoveWhere); try { const rest = await this.createAblyRestClient(flags as BaseFlags); if (!rest) return; const params: Record = { - channel: flags.channel, + channel: args.channelName, }; if (flags["device-id"]) params.deviceId = flags["device-id"]; if (flags["client-id"]) params.clientId = flags["client-id"]; @@ -69,7 +72,7 @@ export default class PushChannelsRemoveWhere extends AblyBaseCommand { } this.logProgress( - `Removing matching subscriptions from channel ${formatResource(flags.channel)}`, + `Removing matching subscriptions from channel ${formatResource(args.channelName)}`, flags, ); diff --git a/src/commands/push/channels/remove.ts b/src/commands/push/channels/remove.ts index 0cb50d4a9..1c87133b7 100644 --- a/src/commands/push/channels/remove.ts +++ b/src/commands/push/channels/remove.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { AblyBaseCommand } from "../../../base-command.js"; import { forceFlag, productApiFlags } from "../../../flags.js"; @@ -9,18 +9,21 @@ import { promptForConfirmation } from "../../../utils/prompt-confirmation.js"; export default class PushChannelsRemove extends AblyBaseCommand { static override description = "Remove a push channel subscription"; + static override args = { + channelName: Args.string({ + description: "Channel name to unsubscribe from", + required: true, + }), + }; + static override examples = [ - "<%= config.bin %> <%= command.id %> --channel my-channel --device-id device-123", - "<%= config.bin %> <%= command.id %> --channel my-channel --client-id client-1 --force", - "<%= config.bin %> <%= command.id %> --channel my-channel --device-id device-123 --json", + '<%= config.bin %> <%= command.id %> "my-channel" --device-id device-123', + '<%= config.bin %> <%= command.id %> "my-channel" --client-id client-1 --force', + '<%= config.bin %> <%= command.id %> "my-channel" --device-id device-123 --json', ]; static override flags = { ...productApiFlags, - channel: Flags.string({ - description: "Channel name to unsubscribe from", - required: true, - }), "device-id": Flags.string({ description: "Device ID to unsubscribe", exclusive: ["client-id"], @@ -33,7 +36,7 @@ export default class PushChannelsRemove extends AblyBaseCommand { }; async run(): Promise { - const { flags } = await this.parse(PushChannelsRemove); + const { args, flags } = await this.parse(PushChannelsRemove); if (!flags["device-id"] && !flags["client-id"]) { this.fail( @@ -62,7 +65,7 @@ export default class PushChannelsRemove extends AblyBaseCommand { if (!flags.force && !this.shouldOutputJson(flags)) { const confirmed = await promptForConfirmation( - `Are you sure you want to unsubscribe ${target} from channel ${flags.channel}?`, + `Are you sure you want to unsubscribe ${target} from channel ${args.channelName}?`, ); if (!confirmed) { @@ -72,12 +75,12 @@ export default class PushChannelsRemove extends AblyBaseCommand { } this.logProgress( - `Removing subscription from channel ${formatResource(flags.channel)}`, + `Removing subscription from channel ${formatResource(args.channelName)}`, flags, ); const subscription: Record = { - channel: flags.channel, + channel: args.channelName, }; if (flags["device-id"]) subscription.deviceId = flags["device-id"]; if (flags["client-id"]) subscription.clientId = flags["client-id"]; @@ -89,7 +92,7 @@ export default class PushChannelsRemove extends AblyBaseCommand { { subscription: { removed: true, - channel: flags.channel, + channel: args.channelName, ...subscription, }, }, @@ -97,7 +100,7 @@ export default class PushChannelsRemove extends AblyBaseCommand { ); } else { this.logSuccessMessage( - `Subscription removed from channel ${formatResource(flags.channel)}.`, + `Subscription removed from channel ${formatResource(args.channelName)}.`, flags, ); } diff --git a/src/commands/push/channels/save.ts b/src/commands/push/channels/save.ts index fb17f92e8..039086a5d 100644 --- a/src/commands/push/channels/save.ts +++ b/src/commands/push/channels/save.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { AblyBaseCommand } from "../../../base-command.js"; import { productApiFlags } from "../../../flags.js"; @@ -9,18 +9,21 @@ export default class PushChannelsSave extends AblyBaseCommand { static override description = "Subscribe a device or client to push notifications on a channel"; + static override args = { + channelName: Args.string({ + description: "Channel name to subscribe to", + required: true, + }), + }; + static override examples = [ - "<%= config.bin %> <%= command.id %> --channel my-channel --device-id device-123", - "<%= config.bin %> <%= command.id %> --channel my-channel --client-id client-1", - "<%= config.bin %> <%= command.id %> --channel my-channel --device-id device-123 --json", + '<%= config.bin %> <%= command.id %> "my-channel" --device-id device-123', + '<%= config.bin %> <%= command.id %> "my-channel" --client-id client-1', + '<%= config.bin %> <%= command.id %> "my-channel" --device-id device-123 --json', ]; static override flags = { ...productApiFlags, - channel: Flags.string({ - description: "Channel name to subscribe to", - required: true, - }), "device-id": Flags.string({ description: "Device ID to subscribe", exclusive: ["client-id"], @@ -32,7 +35,7 @@ export default class PushChannelsSave extends AblyBaseCommand { }; async run(): Promise { - const { flags } = await this.parse(PushChannelsSave); + const { args, flags } = await this.parse(PushChannelsSave); if (!flags["device-id"] && !flags["client-id"]) { this.fail( @@ -47,12 +50,12 @@ export default class PushChannelsSave extends AblyBaseCommand { if (!rest) return; this.logProgress( - `Subscribing to channel ${formatResource(flags.channel)}`, + `Subscribing to channel ${formatResource(args.channelName)}`, flags, ); const subscription: Record = { - channel: flags.channel, + channel: args.channelName, }; if (flags["device-id"]) subscription.deviceId = flags["device-id"]; if (flags["client-id"]) subscription.clientId = flags["client-id"]; @@ -67,7 +70,7 @@ export default class PushChannelsSave extends AblyBaseCommand { this.logJsonResult({ subscription }, flags); } else { this.logSuccessMessage( - `Subscribed ${target} to channel ${formatResource(flags.channel)}.`, + `Subscribed ${target} to channel ${formatResource(args.channelName)}.`, flags, ); } diff --git a/src/commands/push/index.ts b/src/commands/push/index.ts index 8b722294d..ce8bdbc79 100644 --- a/src/commands/push/index.ts +++ b/src/commands/push/index.ts @@ -9,7 +9,7 @@ export default class Push extends BaseTopicCommand { static override examples = [ "<%= config.bin %> <%= command.id %> publish --device-id device1 --title Hello --body World", "<%= config.bin %> <%= command.id %> devices list", - "<%= config.bin %> <%= command.id %> channels list --channel my-channel", + '<%= config.bin %> <%= command.id %> channels list "my-channel"', "<%= config.bin %> <%= command.id %> config show", ]; } diff --git a/src/commands/queues/create.ts b/src/commands/queues/create.ts index a595e1679..539fe8301 100644 --- a/src/commands/queues/create.ts +++ b/src/commands/queues/create.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Args, Flags } from "@oclif/core"; import { ControlBaseCommand } from "../../control-base-command.js"; import { formatLabel, formatResource } from "../../utils/output.js"; @@ -6,11 +6,18 @@ import { formatLabel, formatResource } from "../../utils/output.js"; export default class QueuesCreateCommand extends ControlBaseCommand { static description = "Create a queue"; + static args = { + queueName: Args.string({ + description: "Name of the queue", + required: true, + }), + }; + static examples = [ - '$ ably queues create --name "my-queue"', - '$ ably queues create --name "my-queue" --ttl 300 --max-length 5000', - '$ ably queues create --name "my-queue" --region "eu-west-1-a" --app "My App"', - '$ ably queues create --name "my-queue" --json', + '$ ably queues create "my-queue"', + '$ ably queues create "my-queue" --ttl 300 --max-length 5000', + '$ ably queues create "my-queue" --region "eu-west-1-a" --app "My App"', + '$ ably queues create "my-queue" --json', ]; static flags = { @@ -24,10 +31,6 @@ export default class QueuesCreateCommand extends ControlBaseCommand { description: "Maximum number of messages in the queue (max: 10000)", required: false, }), - name: Flags.string({ - description: "Name of the queue", - required: true, - }), region: Flags.string({ default: "us-east-1-a", description: "Region for the queue (e.g., us-east-1-a, eu-west-1-a)", @@ -41,8 +44,8 @@ export default class QueuesCreateCommand extends ControlBaseCommand { }; async run(): Promise { - const { flags } = await this.parse(QueuesCreateCommand); - if (!flags.name.trim()) { + const { args, flags } = await this.parse(QueuesCreateCommand); + if (!args.queueName.trim()) { this.fail("Queue name cannot be empty", flags, "parse"); } @@ -60,7 +63,7 @@ export default class QueuesCreateCommand extends ControlBaseCommand { const controlApi = this.createControlApi(flags); const queueData = { maxLength: flags["max-length"], - name: flags.name, + name: args.queueName, region: flags.region, ttl: flags.ttl, }; diff --git a/src/commands/queues/index.ts b/src/commands/queues/index.ts index 1f5306490..942091853 100644 --- a/src/commands/queues/index.ts +++ b/src/commands/queues/index.ts @@ -8,7 +8,7 @@ export default class QueuesIndexCommand extends BaseTopicCommand { static examples = [ "<%= config.bin %> <%= command.id %> list", - '<%= config.bin %> <%= command.id %> create --name "my-queue"', - "<%= config.bin %> <%= command.id %> delete my-queue", + '<%= config.bin %> <%= command.id %> create "my-queue"', + '<%= config.bin %> <%= command.id %> delete "my-queue"', ]; } diff --git a/src/commands/spaces/locations/set.ts b/src/commands/spaces/locations/set.ts index ef8721949..7968735bd 100644 --- a/src/commands/spaces/locations/set.ts +++ b/src/commands/spaces/locations/set.ts @@ -1,4 +1,4 @@ -import { Args, Flags } from "@oclif/core"; +import { Args } from "@oclif/core"; import { productApiFlags, clientIdFlag, durationFlag } from "../../../flags.js"; import { SpacesBaseCommand } from "../../../spaces-base-command.js"; @@ -14,23 +14,23 @@ export default class SpacesLocationsSet extends SpacesBaseCommand { description: "Name of the space to set location in", required: true, }), + location: Args.string({ + description: "Location data to set (JSON format)", + required: true, + }), }; static override description = "Set location in a space"; static override examples = [ - '$ ably spaces locations set my-space --location \'{"x":10,"y":20}\'', - '$ ably spaces locations set my-space --location \'{"sectionId":"section1"}\'', - '$ ably spaces locations set my-space --location \'{"x":10,"y":20}\' --json', + '$ ably spaces locations set "my-space" \'{"x":10,"y":20}\'', + '$ ably spaces locations set "my-space" \'{"sectionId":"section1"}\'', + '$ ably spaces locations set "my-space" \'{"x":10,"y":20}\' --json', ]; static override flags = { ...productApiFlags, ...clientIdFlag, - location: Flags.string({ - description: "Location data to set (JSON format)", - required: true, - }), ...durationFlag, }; @@ -38,7 +38,7 @@ export default class SpacesLocationsSet extends SpacesBaseCommand { const { args, flags } = await this.parse(SpacesLocationsSet); const { spaceName } = args; - const location = this.parseJsonFlag(flags.location, "location", flags); + const location = this.parseJsonFlag(args.location, "location", flags); try { this.logProgress("Entering space", flags); diff --git a/test/e2e/control/control-api-workflows.test.ts b/test/e2e/control/control-api-workflows.test.ts index f6b50e580..618888f29 100644 --- a/test/e2e/control/control-api-workflows.test.ts +++ b/test/e2e/control/control-api-workflows.test.ts @@ -157,7 +157,7 @@ describe("Control API E2E Workflow Tests", () => { // 1. Create app const createResult = await runBackgroundProcessAndGetOutput( - `ABLY_ACCESS_TOKEN=${process.env.E2E_ABLY_ACCESS_TOKEN} ${cliPath} apps create --name "${appName}" --json`, + `ABLY_ACCESS_TOKEN=${process.env.E2E_ABLY_ACCESS_TOKEN} ${cliPath} apps create "${appName}" --json`, 30000, ); @@ -235,7 +235,7 @@ describe("Control API E2E Workflow Tests", () => { // Create a test app first const appName = `E2E Key Test App ${Date.now()}`; const createResult = await runCommand( - ["apps", "create", "--name", appName, "--json"], + ["apps", "create", appName, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -267,16 +267,7 @@ describe("Control API E2E Workflow Tests", () => { const keyName = `Test Key ${Date.now()}`; const createResult = await runCommand( - [ - "auth", - "keys", - "create", - "--app", - testAppId, - "--name", - keyName, - "--json", - ], + ["auth", "keys", "create", keyName, "--app", testAppId, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -322,7 +313,7 @@ describe("Control API E2E Workflow Tests", () => { // Create a test app first const appName = `E2E Queue Test App ${Date.now()}`; const createResult = await runCommand( - ["apps", "create", "--name", appName, "--json"], + ["apps", "create", appName, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -357,10 +348,9 @@ describe("Control API E2E Workflow Tests", () => { [ "queues", "create", + queueName, "--app", testAppId, - "--name", - queueName, "--max-length", "5000", "--ttl", @@ -406,7 +396,7 @@ describe("Control API E2E Workflow Tests", () => { const queueName = `test-delete-queue-${Date.now()}`; // First create a queue await runCommand( - ["queues", "create", "--app", testAppId, "--name", queueName, "--json"], + ["queues", "create", queueName, "--app", testAppId, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -438,7 +428,7 @@ describe("Control API E2E Workflow Tests", () => { // Create a test app first const appName = `E2E Integration Test App ${Date.now()}`; const createResult = await runCommand( - ["apps", "create", "--name", appName, "--json"], + ["apps", "create", appName, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -524,7 +514,7 @@ describe("Control API E2E Workflow Tests", () => { // Create a test app first const appName = `E2E Rules Test App ${Date.now()}`; const createResult = await runCommand( - ["apps", "create", "--name", appName, "--json"], + ["apps", "create", appName, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -565,10 +555,9 @@ describe("Control API E2E Workflow Tests", () => { "apps", "rules", "create", + ruleName, "--app", testAppId, - "--name", - ruleName, "--persisted", "--push-enabled", "--authenticated", @@ -626,10 +615,9 @@ describe("Control API E2E Workflow Tests", () => { "apps", "rules", "create", + ruleName, "--app", testAppId, - "--name", - ruleName, "--persisted", "--json", ], @@ -687,16 +675,7 @@ describe("Control API E2E Workflow Tests", () => { // 1. Create rule const createResult = await runCommand( - [ - "apps", - "rules", - "create", - "--app", - testAppId, - "--name", - ruleName, - "--json", - ], + ["apps", "rules", "create", ruleName, "--app", testAppId, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -801,7 +780,7 @@ describe("Control API E2E Workflow Tests", () => { // 1. Create app const createAppResult = await runCommand( - ["apps", "create", "--name", appName, "--json"], + ["apps", "create", appName, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -815,16 +794,7 @@ describe("Control API E2E Workflow Tests", () => { // 2. Create API key const createKeyResult = await runCommand( - [ - "auth", - "keys", - "create", - "--app", - appId, - "--name", - keyName, - "--json", - ], + ["auth", "keys", "create", keyName, "--app", appId, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, @@ -838,7 +808,7 @@ describe("Control API E2E Workflow Tests", () => { // 3. Create queue const createQueueResult = await runCommand( - ["queues", "create", "--app", appId, "--name", queueName, "--json"], + ["queues", "create", queueName, "--app", appId, "--json"], { env: { ABLY_ACCESS_TOKEN: process.env.E2E_ABLY_ACCESS_TOKEN }, }, diff --git a/test/e2e/push/channels-e2e.test.ts b/test/e2e/push/channels-e2e.test.ts index 92ed85f91..8bc5753e8 100644 --- a/test/e2e/push/channels-e2e.test.ts +++ b/test/e2e/push/channels-e2e.test.ts @@ -73,7 +73,7 @@ describe.skipIf(SHOULD_SKIP_E2E)("Push Channel Subscriptions E2E Tests", () => { describe("push channels save - validation", () => { it("should error when neither device-id nor client-id provided", async () => { const result = await runCommand( - ["push", "channels", "save", "--channel", "test-channel"], + ["push", "channels", "save", "test-channel"], { env: { ABLY_API_KEY: E2E_API_KEY || "" }, timeoutMs: 30000, @@ -92,7 +92,6 @@ describe.skipIf(SHOULD_SKIP_E2E)("Push Channel Subscriptions E2E Tests", () => { "push", "channels", "save", - "--channel", "test-channel", "--device-id", "device1", @@ -111,14 +110,14 @@ describe.skipIf(SHOULD_SKIP_E2E)("Push Channel Subscriptions E2E Tests", () => { }); describe("push channels list - validation", () => { - it("should require --channel flag", async () => { + it("should require channel argument", async () => { const result = await runCommand(["push", "channels", "list"], { env: { ABLY_API_KEY: E2E_API_KEY || "" }, timeoutMs: 30000, }); expect(result.exitCode).not.toBe(0); - expect(result.stderr).toContain("Missing required flag channel"); + expect(result.stderr).toContain("Missing 1 required arg"); }); }); @@ -159,7 +158,7 @@ describe.skipIf(SHOULD_SKIP_E2E)("Push Channel Subscriptions E2E Tests", () => { describe("push channels remove - validation", () => { it("should error when neither device-id nor client-id provided", async () => { const result = await runCommand( - ["push", "channels", "remove", "--channel", "test-channel", "--force"], + ["push", "channels", "remove", "test-channel", "--force"], { env: { ABLY_API_KEY: E2E_API_KEY || "" }, timeoutMs: 30000, diff --git a/test/e2e/spaces/spaces-e2e.test.ts b/test/e2e/spaces/spaces-e2e.test.ts index d7222526f..38cc2a88f 100644 --- a/test/e2e/spaces/spaces-e2e.test.ts +++ b/test/e2e/spaces/spaces-e2e.test.ts @@ -212,7 +212,7 @@ describe("Spaces E2E Tests", () => { }; const setLocationResult = await runBackgroundProcessAndGetOutput( - `bin/run.js spaces locations set ${testSpaceId} --location '${JSON.stringify(locationData)}' --client-id ${client2Id} --duration 5`, + `bin/run.js spaces locations set ${testSpaceId} '${JSON.stringify(locationData)}' --client-id ${client2Id} --duration 5`, process.env.CI ? 15000 : 15000, // Timeout for the command ); @@ -254,7 +254,7 @@ describe("Spaces E2E Tests", () => { }; const updateLocationResult = await runBackgroundProcessAndGetOutput( - `bin/run.js spaces locations set ${testSpaceId} --location '${JSON.stringify(newLocationData)}' --client-id ${client2Id} --duration 5`, + `bin/run.js spaces locations set ${testSpaceId} '${JSON.stringify(newLocationData)}' --client-id ${client2Id} --duration 5`, process.env.CI ? 15000 : 15000, // Increased local timeout ); diff --git a/test/helpers/standard-tests.ts b/test/helpers/standard-tests.ts index c28809ab1..329d605e5 100644 --- a/test/helpers/standard-tests.ts +++ b/test/helpers/standard-tests.ts @@ -90,7 +90,7 @@ export function standardFlagTests( * Options for standardControlApiErrorTests. */ interface ControlApiErrorTestOptions { - /** Command args for runCommand, e.g., ["apps:create", "--name", "test"] */ + /** Command args for runCommand, e.g., ["apps:create", "test"] */ commandArgs: string[]; /** import.meta.url of the test file */ importMetaUrl: string; diff --git a/test/unit/commands/apps/create.test.ts b/test/unit/commands/apps/create.test.ts index aca955a07..50a8dbe2c 100644 --- a/test/unit/commands/apps/create.test.ts +++ b/test/unit/commands/apps/create.test.ts @@ -59,7 +59,6 @@ describe("apps:create command", () => { const { stdout, stderr } = await runCommand([ "apps:create", - "--name", `"${mockAppName}"`, ]); @@ -100,7 +99,7 @@ describe("apps:create command", () => { }); const { stdout, stderr } = await runCommand( - ["apps:create", "--name", `"${mockAppName}"`, "--tls-only"], + ["apps:create", `"${mockAppName}"`, "--tls-only"], import.meta.url, ); @@ -137,7 +136,7 @@ describe("apps:create command", () => { nockControl().post(`/v1/accounts/${accountId}/apps`).reply(201, mockApp); const { stdout } = await runCommand( - ["apps:create", "--name", `"${mockAppName}"`, "--json"], + ["apps:create", `"${mockAppName}"`, "--json"], import.meta.url, ); @@ -195,7 +194,7 @@ describe("apps:create command", () => { }); const { stderr } = await runCommand( - ["apps:create", "--name", mockAppName], + ["apps:create", mockAppName], import.meta.url, ); @@ -229,7 +228,7 @@ describe("apps:create command", () => { }); const { stderr } = await runCommand( - ["apps:create", "--name", `"${mockAppName}"`], + ["apps:create", `"${mockAppName}"`], import.meta.url, ); @@ -247,9 +246,9 @@ describe("apps:create command", () => { standardHelpTests("apps:create", import.meta.url); describe("argument validation", () => { - it("should require --name flag", async () => { + it("should require app name argument", async () => { const { error } = await runCommand(["apps:create"], import.meta.url); - expect(error?.message).toMatch(/Missing required flag.*name/); + expect(error?.message).toMatch(/Missing 1 required arg/); }); }); @@ -257,7 +256,7 @@ describe("apps:create command", () => { describe("error handling", () => { standardControlApiErrorTests({ - commandArgs: ["apps:create", "--name", `"${mockAppName}"`], + commandArgs: ["apps:create", `"${mockAppName}"`], importMetaUrl: import.meta.url, setupNock: (scenario) => { if (scenario === "401") { @@ -305,7 +304,7 @@ describe("apps:create command", () => { .reply(403, { error: "Forbidden" }); const { error } = await runCommand( - ["apps:create", "--name", `"${mockAppName}"`], + ["apps:create", `"${mockAppName}"`], import.meta.url, ); expect(error).toBeDefined(); @@ -333,7 +332,7 @@ describe("apps:create command", () => { .reply(404, { error: "Not Found" }); const { error } = await runCommand( - ["apps:create", "--name", `"${mockAppName}"`], + ["apps:create", `"${mockAppName}"`], import.meta.url, ); expect(error).toBeDefined(); @@ -344,8 +343,7 @@ describe("apps:create command", () => { it("should require name parameter", async () => { const { error } = await runCommand(["apps:create"], import.meta.url); expect(error).toBeDefined(); - expect(error?.message).toMatch(/Missing required flag.*name/); - expect(error?.oclif?.exit).toBeGreaterThan(0); + expect(error?.message).toMatch(/Missing 1 required arg/); }); it("should handle validation errors from API", async () => { @@ -369,7 +367,7 @@ describe("apps:create command", () => { }); const { error } = await runCommand( - ["apps:create", "--name", `"${mockAppName}"`], + ["apps:create", `"${mockAppName}"`], import.meta.url, ); expect(error).toBeDefined(); diff --git a/test/unit/commands/apps/rules/create.test.ts b/test/unit/commands/apps/rules/create.test.ts index 6cc964dad..5fa78033b 100644 --- a/test/unit/commands/apps/rules/create.test.ts +++ b/test/unit/commands/apps/rules/create.test.ts @@ -27,7 +27,6 @@ describe("apps:rules:create command", () => { standardFlagTests("apps:rules:create", import.meta.url, [ "--json", "--app", - "--name", "--persisted", "--push-enabled", "--mutable-messages", @@ -41,7 +40,7 @@ describe("apps:rules:create command", () => { .reply(201, mockNamespace({ id: mockRuleId })); const { stderr } = await runCommand( - ["apps:rules:create", "--name", mockRuleName], + ["apps:rules:create", mockRuleName], import.meta.url, ); @@ -57,7 +56,7 @@ describe("apps:rules:create command", () => { .reply(201, mockNamespace({ id: mockRuleId, persisted: true })); const { stdout, stderr } = await runCommand( - ["apps:rules:create", "--name", mockRuleName, "--persisted"], + ["apps:rules:create", mockRuleName, "--persisted"], import.meta.url, ); @@ -82,7 +81,7 @@ describe("apps:rules:create command", () => { ); const { stdout, stderr } = await runCommand( - ["apps:rules:create", "--name", mockRuleName, "--mutable-messages"], + ["apps:rules:create", mockRuleName, "--mutable-messages"], import.meta.url, ); @@ -101,7 +100,7 @@ describe("apps:rules:create command", () => { .reply(201, mockNamespace({ id: mockRuleId, pushEnabled: true })); const { stdout, stderr } = await runCommand( - ["apps:rules:create", "--name", mockRuleName, "--push-enabled"], + ["apps:rules:create", mockRuleName, "--push-enabled"], import.meta.url, ); @@ -116,7 +115,7 @@ describe("apps:rules:create command", () => { .reply(201, mockNamespace({ id: mockRuleId })); const { stdout } = await runCommand( - ["apps:rules:create", "--name", mockRuleName, "--json"], + ["apps:rules:create", mockRuleName, "--json"], import.meta.url, ); @@ -143,13 +142,7 @@ describe("apps:rules:create command", () => { ); const { stdout } = await runCommand( - [ - "apps:rules:create", - "--name", - mockRuleName, - "--mutable-messages", - "--json", - ], + ["apps:rules:create", mockRuleName, "--mutable-messages", "--json"], import.meta.url, ); @@ -168,7 +161,7 @@ describe("apps:rules:create command", () => { describe("error handling", () => { standardControlApiErrorTests({ - commandArgs: ["apps:rules:create", "--name", mockRuleName], + commandArgs: ["apps:rules:create", mockRuleName], importMetaUrl: import.meta.url, setupNock: (scenario) => { const appId = getMockConfigManager().getCurrentAppId()!; @@ -187,7 +180,7 @@ describe("apps:rules:create command", () => { .reply(400, { error: "Validation failed" }); const { error } = await runCommand( - ["apps:rules:create", "--name", mockRuleName], + ["apps:rules:create", mockRuleName], import.meta.url, ); diff --git a/test/unit/commands/auth/keys/create.test.ts b/test/unit/commands/auth/keys/create.test.ts index fedbda046..d2ab10e83 100644 --- a/test/unit/commands/auth/keys/create.test.ts +++ b/test/unit/commands/auth/keys/create.test.ts @@ -54,7 +54,7 @@ describe("auth:keys:create command", () => { }); const { stdout, stderr } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); @@ -86,7 +86,6 @@ describe("auth:keys:create command", () => { const { stdout, stderr } = await runCommand( [ "auth:keys:create", - "--name", `"${mockKeyName}"`, "--app", appId, @@ -131,7 +130,6 @@ describe("auth:keys:create command", () => { const { stdout, stderr } = await runCommand( [ "auth:keys:create", - "--name", `"${mockKeyName}"`, "--app", appId, @@ -166,14 +164,7 @@ describe("auth:keys:create command", () => { nockControl().post(`/v1/apps/${appId}/keys`).reply(201, mockKey); const { stdout } = await runCommand( - [ - "auth:keys:create", - "--name", - `"${mockKeyName}"`, - "--app", - appId, - "--json", - ], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId, "--json"], import.meta.url, ); @@ -226,7 +217,7 @@ describe("auth:keys:create command", () => { }); const { stderr } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); @@ -239,7 +230,7 @@ describe("auth:keys:create command", () => { standardFlagTests("auth:keys:create", import.meta.url, ["--json"]); describe("argument validation", () => { - it("should require name parameter", async () => { + it("should require key name argument", async () => { const appId = getMockConfigManager().getRegisteredAppId(); mockAppResolution(appId); const { error } = await runCommand( @@ -247,7 +238,7 @@ describe("auth:keys:create command", () => { import.meta.url, ); expect(error).toBeDefined(); - expect(error?.message).toMatch(/Missing required flag.*name/); + expect(error?.message).toMatch(/Missing 1 required arg/); expect(error?.oclif?.exit).toBeGreaterThan(0); }); @@ -263,7 +254,7 @@ describe("auth:keys:create command", () => { nockControl().get(`/v1/accounts/${accountId}/apps`).reply(200, []); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`], + ["auth:keys:create", `"${mockKeyName}"`], import.meta.url, ); expect(error).toBeDefined(); @@ -278,7 +269,6 @@ describe("auth:keys:create command", () => { const { error } = await runCommand( [ "auth:keys:create", - "--name", `"${mockKeyName}"`, "--app", appId, @@ -301,7 +291,6 @@ describe("auth:keys:create command", () => { const { error } = await runCommand( [ "auth:keys:create", - "--name", `"${mockKeyName}"`, "--app", appId, @@ -328,7 +317,7 @@ describe("auth:keys:create command", () => { .reply(401, { error: "Unauthorized" }); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); expect(error).toBeDefined(); @@ -345,7 +334,7 @@ describe("auth:keys:create command", () => { .reply(403, { error: "Forbidden" }); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); expect(error).toBeDefined(); @@ -362,7 +351,7 @@ describe("auth:keys:create command", () => { .reply(404, { error: "App not found" }); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); expect(error).toBeDefined(); @@ -379,7 +368,7 @@ describe("auth:keys:create command", () => { .reply(500, { error: "Internal Server Error" }); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); expect(error).toBeDefined(); @@ -396,7 +385,7 @@ describe("auth:keys:create command", () => { .replyWithError("Network error"); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); expect(error).toBeDefined(); @@ -414,7 +403,7 @@ describe("auth:keys:create command", () => { }); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); expect(error).toBeDefined(); @@ -431,7 +420,7 @@ describe("auth:keys:create command", () => { .reply(429, { error: "Rate limit exceeded" }); const { error } = await runCommand( - ["auth:keys:create", "--name", `"${mockKeyName}"`, "--app", appId], + ["auth:keys:create", `"${mockKeyName}"`, "--app", appId], import.meta.url, ); expect(error).toBeDefined(); @@ -465,7 +454,6 @@ describe("auth:keys:create command", () => { const { stdout, stderr } = await runCommand( [ "auth:keys:create", - "--name", `"${mockKeyName}"`, "--app", appId, @@ -509,7 +497,6 @@ describe("auth:keys:create command", () => { const { stdout, stderr } = await runCommand( [ "auth:keys:create", - "--name", `"${mockKeyName}"`, "--app", appId, diff --git a/test/unit/commands/push/channels/list.test.ts b/test/unit/commands/push/channels/list.test.ts index aa7f8ed9d..fdadcb2f0 100644 --- a/test/unit/commands/push/channels/list.test.ts +++ b/test/unit/commands/push/channels/list.test.ts @@ -19,7 +19,6 @@ describe("push:channels:list command", () => { standardArgValidationTests("push:channels:list", import.meta.url); standardFlagTests("push:channels:list", import.meta.url, [ "--json", - "--channel", "--device-id", "--client-id", "--limit", @@ -36,7 +35,7 @@ describe("push:channels:list command", () => { ); const { stdout } = await runCommand( - ["push:channels:list", "--channel", "my-channel"], + ["push:channels:list", "my-channel"], import.meta.url, ); @@ -51,7 +50,7 @@ describe("push:channels:list command", () => { ); const { stderr } = await runCommand( - ["push:channels:list", "--channel", "my-channel"], + ["push:channels:list", "my-channel"], import.meta.url, ); @@ -67,7 +66,7 @@ describe("push:channels:list command", () => { ); const { stdout } = await runCommand( - ["push:channels:list", "--channel", "my-channel", "--json"], + ["push:channels:list", "my-channel", "--json"], import.meta.url, ); @@ -85,7 +84,7 @@ describe("push:channels:list command", () => { }); describe("argument validation", () => { - it("should require --channel flag", async () => { + it("should require channel argument", async () => { const { error } = await runCommand( ["push:channels:list"], import.meta.url, @@ -103,7 +102,7 @@ describe("push:channels:list command", () => { ); const { error } = await runCommand( - ["push:channels:list", "--channel", "my-channel"], + ["push:channels:list", "my-channel"], import.meta.url, ); diff --git a/test/unit/commands/push/channels/remove-where.test.ts b/test/unit/commands/push/channels/remove-where.test.ts index 8e7d56b29..1fa54f0af 100644 --- a/test/unit/commands/push/channels/remove-where.test.ts +++ b/test/unit/commands/push/channels/remove-where.test.ts @@ -16,7 +16,6 @@ describe("push:channels:remove-where command", () => { standardArgValidationTests("push:channels:remove-where", import.meta.url); standardFlagTests("push:channels:remove-where", import.meta.url, [ "--json", - "--channel", "--device-id", "--client-id", "--force", @@ -27,7 +26,7 @@ describe("push:channels:remove-where command", () => { const mock = getMockAblyRest(); const { stderr } = await runCommand( - ["push:channels:remove-where", "--channel", "my-channel", "--force"], + ["push:channels:remove-where", "my-channel", "--force"], import.meta.url, ); @@ -39,7 +38,7 @@ describe("push:channels:remove-where command", () => { ); }); - it("should require --channel flag", async () => { + it("should require channel argument", async () => { const { error } = await runCommand( ["push:channels:remove-where", "--force"], import.meta.url, @@ -50,13 +49,7 @@ describe("push:channels:remove-where command", () => { it("should output JSON when requested", async () => { const { stdout } = await runCommand( - [ - "push:channels:remove-where", - "--channel", - "my-channel", - "--force", - "--json", - ], + ["push:channels:remove-where", "my-channel", "--force", "--json"], import.meta.url, ); @@ -82,7 +75,7 @@ describe("push:channels:remove-where command", () => { ); const { error } = await runCommand( - ["push:channels:remove-where", "--channel", "my-channel", "--force"], + ["push:channels:remove-where", "my-channel", "--force"], import.meta.url, ); diff --git a/test/unit/commands/push/channels/remove.test.ts b/test/unit/commands/push/channels/remove.test.ts index 0021e3b2d..543237087 100644 --- a/test/unit/commands/push/channels/remove.test.ts +++ b/test/unit/commands/push/channels/remove.test.ts @@ -16,7 +16,6 @@ describe("push:channels:remove command", () => { standardArgValidationTests("push:channels:remove", import.meta.url); standardFlagTests("push:channels:remove", import.meta.url, [ "--json", - "--channel", "--device-id", "--client-id", "--force", @@ -29,7 +28,6 @@ describe("push:channels:remove command", () => { const { stderr } = await runCommand( [ "push:channels:remove", - "--channel", "my-channel", "--device-id", "dev-1", @@ -49,7 +47,7 @@ describe("push:channels:remove command", () => { it("should require either device-id or client-id", async () => { const { error } = await runCommand( - ["push:channels:remove", "--channel", "my-channel", "--force"], + ["push:channels:remove", "my-channel", "--force"], import.meta.url, ); @@ -60,7 +58,6 @@ describe("push:channels:remove command", () => { const { stdout } = await runCommand( [ "push:channels:remove", - "--channel", "my-channel", "--device-id", "dev-1", @@ -94,7 +91,6 @@ describe("push:channels:remove command", () => { const { error } = await runCommand( [ "push:channels:remove", - "--channel", "my-channel", "--device-id", "dev-1", diff --git a/test/unit/commands/push/channels/save.test.ts b/test/unit/commands/push/channels/save.test.ts index 028b5885b..ea6be8c76 100644 --- a/test/unit/commands/push/channels/save.test.ts +++ b/test/unit/commands/push/channels/save.test.ts @@ -16,7 +16,6 @@ describe("push:channels:save command", () => { standardArgValidationTests("push:channels:save", import.meta.url); standardFlagTests("push:channels:save", import.meta.url, [ "--json", - "--channel", "--device-id", "--client-id", ]); @@ -26,13 +25,7 @@ describe("push:channels:save command", () => { const mock = getMockAblyRest(); const { stderr } = await runCommand( - [ - "push:channels:save", - "--channel", - "my-channel", - "--device-id", - "device-1", - ], + ["push:channels:save", "my-channel", "--device-id", "device-1"], import.meta.url, ); @@ -50,13 +43,7 @@ describe("push:channels:save command", () => { const mock = getMockAblyRest(); const { stderr } = await runCommand( - [ - "push:channels:save", - "--channel", - "my-channel", - "--client-id", - "client-1", - ], + ["push:channels:save", "my-channel", "--client-id", "client-1"], import.meta.url, ); @@ -71,7 +58,7 @@ describe("push:channels:save command", () => { it("should require either device-id or client-id", async () => { const { error } = await runCommand( - ["push:channels:save", "--channel", "my-channel"], + ["push:channels:save", "my-channel"], import.meta.url, ); @@ -80,14 +67,7 @@ describe("push:channels:save command", () => { it("should output JSON when requested", async () => { const { stdout } = await runCommand( - [ - "push:channels:save", - "--channel", - "my-channel", - "--device-id", - "dev-1", - "--json", - ], + ["push:channels:save", "my-channel", "--device-id", "dev-1", "--json"], import.meta.url, ); @@ -112,13 +92,7 @@ describe("push:channels:save command", () => { ); const { error } = await runCommand( - [ - "push:channels:save", - "--channel", - "my-channel", - "--device-id", - "dev-1", - ], + ["push:channels:save", "my-channel", "--device-id", "dev-1"], import.meta.url, ); diff --git a/test/unit/commands/queues/create.test.ts b/test/unit/commands/queues/create.test.ts index a05ff1923..3e7b54f46 100644 --- a/test/unit/commands/queues/create.test.ts +++ b/test/unit/commands/queues/create.test.ts @@ -56,7 +56,7 @@ describe("queues:create command", () => { .reply(201, createMockQueueResponse(appId)); const { stdout, stderr } = await runCommand( - ["queues:create", "--name", mockQueueName], + ["queues:create", mockQueueName], import.meta.url, ); @@ -99,7 +99,6 @@ describe("queues:create command", () => { const { stdout, stderr } = await runCommand( [ "queues:create", - "--name", mockQueueName, "--max-length", "5000", @@ -134,7 +133,7 @@ describe("queues:create command", () => { .reply(201, createMockQueueResponse(appId)); const { stdout } = await runCommand( - ["queues:create", "--name", mockQueueName, "--json"], + ["queues:create", mockQueueName, "--json"], import.meta.url, ); @@ -171,7 +170,7 @@ describe("queues:create command", () => { }); const { stderr } = await runCommand( - ["queues:create", "--name", mockQueueName, "--app", "custom-app-id"], + ["queues:create", mockQueueName, "--app", "custom-app-id"], import.meta.url, ); @@ -210,7 +209,7 @@ describe("queues:create command", () => { .reply(201, createMockQueueResponse(appId)); const { stderr } = await runCommand( - ["queues:create", "--name", mockQueueName], + ["queues:create", mockQueueName], import.meta.url, ); @@ -220,7 +219,7 @@ describe("queues:create command", () => { describe("error handling", () => { standardControlApiErrorTests({ - commandArgs: ["queues:create", "--name", mockQueueName], + commandArgs: ["queues:create", mockQueueName], importMetaUrl: import.meta.url, setupNock: (scenario) => { const mockConfig = getMockConfigManager(); @@ -258,7 +257,7 @@ describe("queues:create command", () => { .reply(403, { error: "Forbidden" }); const { error } = await runCommand( - ["queues:create", "--name", mockQueueName], + ["queues:create", mockQueueName], import.meta.url, ); @@ -284,7 +283,7 @@ describe("queues:create command", () => { .reply(404, { error: "App not found" }); const { error } = await runCommand( - ["queues:create", "--name", mockQueueName], + ["queues:create", mockQueueName], import.meta.url, ); @@ -293,12 +292,11 @@ describe("queues:create command", () => { expect(error?.oclif?.exit).toBeGreaterThan(0); }); - it("should require name parameter", async () => { + it("should require queue name argument", async () => { const { error } = await runCommand(["queues:create"], import.meta.url); expect(error).toBeDefined(); - expect(error?.message).toMatch(/Missing required flag/); - expect(error?.message).toMatch(/name/); + expect(error?.message).toMatch(/Missing 1 required arg/); expect(error?.oclif?.exit).toBeGreaterThan(0); }); @@ -306,7 +304,7 @@ describe("queues:create command", () => { getMockConfigManager().clearAccounts(); const { error } = await runCommand( - ["queues:create", "--name", mockQueueName], + ["queues:create", mockQueueName], import.meta.url, ); @@ -332,7 +330,7 @@ describe("queues:create command", () => { }); const { error } = await runCommand( - ["queues:create", "--name", mockQueueName], + ["queues:create", mockQueueName], import.meta.url, ); @@ -359,7 +357,7 @@ describe("queues:create command", () => { }); const { error } = await runCommand( - ["queues:create", "--name", mockQueueName], + ["queues:create", mockQueueName], import.meta.url, ); @@ -396,15 +394,7 @@ describe("queues:create command", () => { }); const { stdout, stderr } = await runCommand( - [ - "queues:create", - "--name", - mockQueueName, - "--max-length", - "1", - "--ttl", - "1", - ], + ["queues:create", mockQueueName, "--max-length", "1", "--ttl", "1"], import.meta.url, ); @@ -442,7 +432,6 @@ describe("queues:create command", () => { const { stdout, stderr } = await runCommand( [ "queues:create", - "--name", mockQueueName, "--max-length", "10000", @@ -462,7 +451,7 @@ describe("queues:create command", () => { it("should reject max-length exceeding 10000", async () => { const { error } = await runCommand( - ["queues:create", "--name", mockQueueName, "--max-length", "10001"], + ["queues:create", mockQueueName, "--max-length", "10001"], import.meta.url, ); @@ -472,7 +461,7 @@ describe("queues:create command", () => { it("should reject ttl exceeding 3600", async () => { const { error } = await runCommand( - ["queues:create", "--name", mockQueueName, "--ttl", "3601"], + ["queues:create", mockQueueName, "--ttl", "3601"], import.meta.url, ); @@ -483,5 +472,5 @@ describe("queues:create command", () => { standardHelpTests("queues:create", import.meta.url); standardArgValidationTests("queues:create", import.meta.url); - standardFlagTests("queues:create", import.meta.url, ["--name", "--json"]); + standardFlagTests("queues:create", import.meta.url, ["--json"]); }); diff --git a/test/unit/commands/spaces/locations/set.test.ts b/test/unit/commands/spaces/locations/set.test.ts index 48b140edc..f44376a62 100644 --- a/test/unit/commands/spaces/locations/set.test.ts +++ b/test/unit/commands/spaces/locations/set.test.ts @@ -32,7 +32,6 @@ describe("spaces:locations:set command", () => { const args = [ "spaces:locations:set", "test-space", - "--location", '{"x":1}', "--unknown-flag-xyz", ]; @@ -41,23 +40,21 @@ describe("spaces:locations:set command", () => { expect(error?.message).toMatch(/unknown|Nonexistent flag/i); }); - it("should require --location flag", async () => { + it("should require location argument", async () => { const { error } = await runCommand( ["spaces:locations:set", "test-space"], import.meta.url, ); expect(error).toBeDefined(); - expect(error?.message).toMatch( - /--location.*required|Missing required flag/i, - ); + expect(error?.message).toMatch(/Missing 1 required arg/i); }); }); describe("functionality", () => { - it("should error on invalid --location JSON", async () => { + it("should error on invalid location JSON", async () => { const { error } = await runCommand( - ["spaces:locations:set", "test-space", "--location", "not-valid-json"], + ["spaces:locations:set", "test-space", "not-valid-json"], import.meta.url, ); @@ -74,12 +71,7 @@ describe("spaces:locations:set command", () => { const location = { x: 10, y: 20, sectionId: "main" }; const { stderr } = await runCommand( - [ - "spaces:locations:set", - "test-space", - "--location", - JSON.stringify(location), - ], + ["spaces:locations:set", "test-space", JSON.stringify(location)], import.meta.url, ); @@ -94,7 +86,7 @@ describe("spaces:locations:set command", () => { spacesMock._getSpace("test-space"); const { stderr } = await runCommand( - ["spaces:locations:set", "test-space", "--location", '{"x":1}'], + ["spaces:locations:set", "test-space", '{"x":1}'], import.meta.url, ); @@ -114,7 +106,6 @@ describe("spaces:locations:set command", () => { [ "spaces:locations:set", "test-space", - "--location", JSON.stringify(location), "--json", ], @@ -136,13 +127,7 @@ describe("spaces:locations:set command", () => { it("should output JSON error on invalid location", async () => { const { stdout, error } = await runCommand( - [ - "spaces:locations:set", - "test-space", - "--location", - "not-valid-json", - "--json", - ], + ["spaces:locations:set", "test-space", "not-valid-json", "--json"], import.meta.url, ); @@ -168,7 +153,7 @@ describe("spaces:locations:set command", () => { ); const { error } = await runCommand( - ["spaces:locations:set", "test-space", "--location", '{"x":10,"y":20}'], + ["spaces:locations:set", "test-space", '{"x":10,"y":20}'], import.meta.url, ); diff --git a/test/unit/commands/spaces/spaces.test.ts b/test/unit/commands/spaces/spaces.test.ts index d173a0019..167c20fda 100644 --- a/test/unit/commands/spaces/spaces.test.ts +++ b/test/unit/commands/spaces/spaces.test.ts @@ -180,17 +180,12 @@ describe("spaces commands", () => { expect(stdout).toContain("SPACE_NAME"); }); - it("should set location with --location flag", async () => { + it("should set location with positional argument", async () => { const spacesMock = getMockAblySpaces(); const space = spacesMock._getSpace("test-space"); const { stderr } = await runCommand( - [ - "spaces:locations:set", - "test-space", - "--location", - '{"x":100,"y":200}', - ], + ["spaces:locations:set", "test-space", '{"x":100,"y":200}'], import.meta.url, ); From b82ad266b85e53ff4af4add8b58a9bf1a78c097b Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 21 Apr 2026 17:46:33 +0530 Subject: [PATCH 2/3] Updated AGENTS.md for the relevant implementation --- AGENTS.md | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5a1337293..3bae0fd2f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,6 +45,7 @@ This is the Ably CLI npm package (`@ably/cli`), built with the [oclif framework] 6. **NODE_ENV** - To check if the CLI is in test mode, use the `isTestMode()` helper function. 7. **`process.exit`** - When creating a command, use `this.exit()` for consistent test mode handling. 8. **`console.log` / `console.error`** - In commands, always use `this.log()` (stdout) for data/results and the logging helpers (`this.logProgress()`, `this.logSuccessMessage()`, `this.logListening()`, `this.logHolding()`, `this.logWarning()`) for status messages. `console.*` bypasses oclif and can't be captured by tests. +9. **Use `Args.string()` for primary entity identifiers** - If the value is "what is being acted on" (name, ID, channel), it must be a positional `Args.string()`/`Args.boolean()`/`Args.integer()`, not a `Flags.string().`. primary entity identifier should always be represented as a camelCase. ## Correct Practices @@ -105,23 +106,43 @@ Flags are NOT global. Each command explicitly declares only the flags it needs v - **`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"`). - **`endpointFlag`** — `--endpoint`. Hidden, only on `accounts login` and `accounts switch`. +**Flags vs positional arguments (POSIX / docopt convention):** +- If a value answers **"what is being created/deleted/acted on?"** → **positional argument** (`Args.string()`) +- If a value answers **"how should the operation be performed?"** → **flag** (`Flags.string()`) +- The primary entity identifier (name, ID, channel) must always be a positional argument, never a `--flag`. +- Exceptions where required flags are correct: enum-constrained config values (e.g., `--rule-type` on `integrations create`), file path inputs (e.g., `--service-account` on `push config set-fcm`). + **When creating a new command:** ```typescript // Product API command (channels, spaces, rooms, etc.) import { productApiFlags, clientIdFlag, durationFlag, rewindFlag } from "../../flags.js"; +static override args = { + // entityName should always be camelCase for `Args.*`. + entityName: Args.string({ + description: "The primary entity being acted on", + required: true, // or false if interactive fallback exists + }), +}; static override flags = { ...productApiFlags, ...clientIdFlag, // Only if command needs client identity ...durationFlag, // Only if long-running (subscribe/stream commands) ...rewindFlag, // Only if supports message replay - // command-specific flags... + // command-specific flags (modifiers only, NOT primary entity identifiers)... }; // Control API command (apps, keys, queues, etc.) // controlApiFlags come from ControlBaseCommand.globalFlags automatically +static args = { + // entityName should always be camelCase for `Args.*` + entityName: Args.string({ + description: "The primary entity being acted on", + required: true, + }), +}; static flags = { ...ControlBaseCommand.globalFlags, - // command-specific flags... + // command-specific flags (modifiers only, NOT primary entity identifiers)... }; ``` From fb9b7f749e3c99e41534ccf5f43839d379f97dad Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 21 Apr 2026 19:40:55 +0530 Subject: [PATCH 3/3] Addressed review comments by copilot --- AGENTS.md | 2 +- test/e2e/push/push-config-e2e.test.ts | 2 +- test/unit/commands/apps/create.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3bae0fd2f..e6f3ad3c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,7 @@ This is the Ably CLI npm package (`@ably/cli`), built with the [oclif framework] 6. **NODE_ENV** - To check if the CLI is in test mode, use the `isTestMode()` helper function. 7. **`process.exit`** - When creating a command, use `this.exit()` for consistent test mode handling. 8. **`console.log` / `console.error`** - In commands, always use `this.log()` (stdout) for data/results and the logging helpers (`this.logProgress()`, `this.logSuccessMessage()`, `this.logListening()`, `this.logHolding()`, `this.logWarning()`) for status messages. `console.*` bypasses oclif and can't be captured by tests. -9. **Use `Args.string()` for primary entity identifiers** - If the value is "what is being acted on" (name, ID, channel), it must be a positional `Args.string()`/`Args.boolean()`/`Args.integer()`, not a `Flags.string().`. primary entity identifier should always be represented as a camelCase. +9. **Use `Args.string()` for primary entity identifiers** - If the value is "what is being acted on" (name, ID, channel), represent it as a positional `Args.string()`, not a `Flags.string()`. Primary entity identifiers should always use camelCase. ## Correct Practices diff --git a/test/e2e/push/push-config-e2e.test.ts b/test/e2e/push/push-config-e2e.test.ts index dc565988c..4b4125db6 100644 --- a/test/e2e/push/push-config-e2e.test.ts +++ b/test/e2e/push/push-config-e2e.test.ts @@ -46,7 +46,7 @@ describe("Push Config E2E Tests", () => { // Let setup failures propagate — only missing credentials should skip const appName = `E2E Push Config Test ${Date.now()}`; const createResult = await runCommand( - ["apps", "create", "--name", appName, "--json"], + ["apps", "create", appName, "--json"], { env: { ABLY_ACCESS_TOKEN: accessToken }, }, diff --git a/test/unit/commands/apps/create.test.ts b/test/unit/commands/apps/create.test.ts index 50a8dbe2c..e1b65a14a 100644 --- a/test/unit/commands/apps/create.test.ts +++ b/test/unit/commands/apps/create.test.ts @@ -340,7 +340,7 @@ describe("apps:create command", () => { expect(error?.oclif?.exit).toBeGreaterThan(0); }); - it("should require name parameter", async () => { + it("should require app name positional argument", async () => { const { error } = await runCommand(["apps:create"], import.meta.url); expect(error).toBeDefined(); expect(error?.message).toMatch(/Missing 1 required arg/);