Skip to content

[DX-1136] Fix inconsistency in accepting both Name or ID in commands - #369

Merged
sacOO7 merged 17 commits into
mainfrom
fix/consistent-name-or-id-arguments
Apr 28, 2026
Merged

[DX-1136] Fix inconsistency in accepting both Name or ID in commands#369
sacOO7 merged 17 commits into
mainfrom
fix/consistent-name-or-id-arguments

Conversation

@sacOO7

@sacOO7sacOO7 commented Apr 23, 2026

Copy link
Copy Markdown
Contributor
  • Fixes https://ably.atlassian.net/browse/DX-1136
  • Commands that previously accepted only an app ID, queue ID, or rule ID as their positional argument now accept
    either a name or ID, resolving via resolveAppIdFromNameOrId (for apps) or list-then-match (for queues/rules)
  • Adds helpful error hints in error message when a resource is not found (e.g., Run "ably queues list" to see available queues.)

Changed commands

CommandPrevious ArgumentNew Argument
accounts logoutACCOUNT_ALIAS (alias only)ACCOUNT_ALIAS_OR_ID (alias or account ID) — account ID is shown by accounts list but wasn't accepted as input
accounts switchACCOUNT_ALIAS (alias only)ACCOUNT_ALIAS_OR_ID (alias or account ID) — account ID is shown by accounts list but wasn't accepted as input
apps updateAPP_ID (app ID)APP_NAME_OR_ID (app name or ID) — consistent with apps switch; resolved via listApps()
apps switchAPP_ID (app ID)APP_NAME_OR_ID (app name or ID) — consistent with apps update; resolved via listApps()
queues deleteQUEUE_ID (queue ID)QUEUE_NAME_OR_ID (queue name or ID) — resolved via listQueues() match by name then ID
stats appAPP_ID (app ID)APP_NAME_OR_ID (app name or ID) — consistent with apps update/switch; resolved via listApps()
apps rules deleteNAME_OR_ID (rule ID only)RULE_NAME_OR_ID (rule name or ID) — for rules, ID and name are the same; renamed for clarity
apps rules updateNAME_OR_ID (rule ID only)RULE_NAME_OR_ID (rule name or ID) — for rules, ID and name are the same; renamed for clarity
auth keys get/revokeKEY_NAME, KEY_LABEL, KEY_ID or KEY_VALUEKEY_NAME_OR_VALUE (Key name "appId.keyId" or value "appId.keyId:keySecret") — Check comment #369 (comment)
auth keys switch/updateKEY_NAME or KEY_IDKEY_NAME_OR_VALUE (Key name "appId.keyId" or value "appId.keyId:keySecret") — Check comment #369 (comment)

Unchanged commands

CommandArgumentReason
apps deleteAPP_IDUses a two-step destructive confirmation flow that prompts the user to type the app name explicitly — accepting name-or-ID at the argument level would conflict with this verification step
integrations delete/get/updateINTEGRATION_IDIntegrations have no user-facing name — only system-generated IDs
push devices get/removeDEVICE_IDDevice IDs are client-generated, no "name" concept exists
spaces locks acquire/getLOCK_IDLock IDs are application-defined, no alternative name exists

TODO

  • - Remove hint specific code, can be merged into the error message itself.
  • - Checks keys specific updated nameOrId commands.
  • - Resolve all copilot comments.
  • - Check if command description/examples are well updated.
  • - Test updated commands

@vercel

vercelBot commented Apr 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
cli-web-cliReadyReadyPreview, CommentApr 28, 2026 5:00pm

Request Review

auth keys revoke/update: Accept key name (APP_ID.KEY_ID), key ID, key
label, or full key value — matching keys get/switch. Fixes a bug where
the raw identifier was passed to the Control API instead of the resolved
key ID, causing failures when using labels.
accounts logout/switch: Accept account alias or account ID, resolving
via a new resolveAccountAlias() helper on ControlBaseCommand.
@sacOO7
sacOO7 requested a review from CopilotApril 23, 2026 10:42
@sacOO7sacOO7 changed the title Accept name or ID in commands that previously required only IDs[DX-1136] Accept name or ID in commands that previously required only IDsApr 23, 2026
@sacOO7sacOO7 changed the title [DX-1136] Accept name or ID in commands that previously required only IDs[DX-1136] Fix inconsistency in accepting both Name or ID in commandsApr 23, 2026

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR broadens several CLI commands that previously required resource IDs so they can accept either a name or an ID (or other identifiers for keys/accounts), and improves “not found” UX with actionable hints. It also updates unit tests to cover the new resolution flows and adds support for surfacing custom hint strings in this.fail() output.

Changes:

  • Accept name-or-ID positional args for apps/queues/stats/rules, plus richer key identifiers (ID/label/full value) and account alias-or-ID.
  • Add “try listing resources” hints when a target queue/rule/account can’t be found.
  • Update command error handling to allow custom hint passthrough and refresh unit tests accordingly.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
test/unit/commands/stats/app.test.tsMocks app name→ID resolution before stats calls.
test/unit/commands/queues/delete.test.tsAdds deletion-by-name test and asserts new hint/error text.
test/unit/commands/auth/keys/update.test.tsAdds update-by-label test and adjusts arg wording.
test/unit/commands/auth/keys/revoke.test.tsAdds revoke-by-label and revoke-by-full-key-value tests.
test/unit/commands/apps/update.test.tsAdds app resolution mocking to update scenarios (including custom token).
test/unit/commands/apps/switch.test.tsAdjusts mocks for additional listApps calls and updated /me data.
test/unit/commands/accounts/switch.test.tsAdds switching by account ID coverage.
test/unit/commands/accounts/logout.test.tsAdds logout-by-account-ID coverage (single + multiple accounts).
src/control-base-command.tsIntroduces resolveAccountAlias() helper (alias or accountId → alias).
src/commands/stats/app.tsRenames arg to appNameOrId and resolves name→ID when provided.
src/commands/queues/delete.tsRenames arg to queueNameOrId, matches queue by name then id, adds hint when not found.
src/commands/auth/keys/update.tsRenames arg to keyNameOrValue, resolves appId more flexibly, updates by resolved key id.
src/commands/auth/keys/revoke.tsRenames arg to keyNameOrValue, resolves appId more flexibly, revokes resolved key id.
src/commands/apps/update.tsRenames arg to appNameOrId and resolves to appId before update calls.
src/commands/apps/switch.tsAccepts app name-or-ID and resolves before switching.
src/commands/apps/rules/update.tsRenames arg to ruleNameOrId and adds “list rules” hint on not found.
src/commands/apps/rules/delete.tsRenames arg to ruleNameOrId and adds “list rules” hint on not found.
src/commands/accounts/switch.tsAccepts account alias-or-ID and resolves via resolveAccountAlias().
src/commands/accounts/logout.tsAccepts account alias-or-ID and resolves via resolveAccountAlias().
src/base-command.tsAdds custom hint passthrough support in fail() (Ably-code hint still takes priority).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadsrc/commands/apps/switch.ts Outdated
Comment threadsrc/commands/queues/delete.ts Outdated
Comment threadsrc/commands/apps/update.ts Outdated

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadsrc/commands/auth/keys/revoke.ts Outdated
Comment threadsrc/commands/apps/switch.ts Outdated
Comment threadsrc/commands/auth/keys/update.ts Outdated

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadsrc/commands/accounts/logout.ts Outdated
… comments
- Added resolveAccountAlias check for accounts logout command that checks for stale config
Comment threadsrc/commands/auth/keys/get.ts Outdated
keyNameOrValue: Args.string({
description:
"Key name (APP_ID.KEY_ID), key ID, key label (e.g. Root), or full key value to get details for",
'Key name "<appId>.<keyId>" or value "<appId>.<keyId>:<keySecret>"',

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  • The key label is not a reliable unique identifier. Multiple keys can share the same label, and it also allows both . and : characters. Because of this ambiguity, it shouldn't be used to uniquely identify or retrieve a specific key.

  • The key ID currently requires the app flag to determine which app it belongs to. Instead of extracting keyId and appId separately from a string like <appId>.<keyId> and passing them as different parameters, it’s more intuitive to use the combined identifier directly.

Instead of:

ably auth keys get key_id --app appID

Prefer

ably auth keys get KEY_ID.APP_ID

This approach aligns with the existing key format and simplifies usage by allowing the identifier to be copied and used as-is.

…r-id-arguments
# Conflicts:
#	src/commands/accounts/switch.ts
…ption and examples
Refactored auth keys update command to with enhanced description and examples
@sacOO7
sacOO7force-pushed the fix/consistent-name-or-id-arguments branch from 5639deb to 12391faCompareApril 28, 2026 16:59
@sacOO7
sacOO7 merged commit 79ef79b into mainApr 28, 2026
11 checks passed
@sacOO7
sacOO7 deleted the fix/consistent-name-or-id-arguments branch April 28, 2026 17:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@sacOO7@AndyTWF