Typescript lint fixes #6
Conversation
Official Did you mean plugin (https://github.com/oclif/plugin-not-found) had incompatibilities, such as assuming `help` command is available yet we use --help.
Largely automated fixes with `pnpm exec eslint . --fix` With a few exceptions and manual clean ups
Reduced reported errors from 3k+ to ~1.5k
pnpm exec eslint . --fix
Down by around 100 lint issues: ✖ 220 problems (14 errors, 206 warnings)
- Ignored examples/web-cli/vite.config.ts in ESLint to fix the parsing error. - Adjusted the files array in package.json to potentially resolve the n/no-unpublished-bin error for bin/run.js. - Renamed bin/dev.js to bin/development.js. - Temporarily added @ts-nocheck back to src/mcp/mcp-server.ts to suppress TypeScript errors revealed by its removal. We also noted the persistent n/no-missing-import lint warnings for @modelcontextprotocol/sdk imports in this file, which might need investigation later if they cause issues, but the build currently passes. - Exported BaseFlags from src/base-command.ts.
…slint/no-unused-vars) - Replaced JSON.parse(JSON.stringify(...)) with structuredClone(...) (and appropriate type casting) in integrations/get.ts, queues/create.ts, and spaces/locks/get.ts. - Disabled the unicorn/prefer-top-level-await rule for test/hooks/command_not_found/did-you-mean.test.ts. - Disabled the unicorn/consistent-function-scoping rule for the cleanup function in src/commands/spaces/members/subscribe.ts. ✖ 192 problems (4 errors, 188 warnings)
Lint is now passing! ✖ 181 problems (0 errors, 181 warnings)
✖ 180 problems (0 errors, 180 warnings)
Removed unused variables, imports, methods, and parameters identified by the `@typescript-eslint/no-unused-vars` rule across various files. ✖ 116 problems (0 errors, 116 warnings)
For core services only. ✖ 109 problems (0 errors, 109 warnings)
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces significant refactoring and cleanup across the codebase. Numerous alias command files, which previously forwarded execution to main commands, have been removed, streamlining the command structure. The base command classes and several command implementations are refactored for improved modularity, type safety, argument and flag handling, and error reporting. Many commands now explicitly declare their arguments and flags, with enhanced output formatting and resource cleanup. The ESLint and Prettier configurations are overhauled, introducing new flat config files and updated ignore patterns. Dependency management is modernized, with scripts and dependencies updated in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant AblyBaseCommand
participant AblyAPI
participant ConfigManager
User->>CLI: Runs a command with flags/args
CLI->>AblyBaseCommand: Parses args/flags
AblyBaseCommand->>ConfigManager: Ensures app/key or token
AblyBaseCommand->>AblyAPI: Creates client (token or API key)
AblyAPI-->>AblyBaseCommand: Returns client or error
AblyBaseCommand->>CLI: Executes command logic, handles output/error
sequenceDiagram
participant User
participant TerminalServer
participant Docker
participant WebSocketClient
User->>TerminalServer: Connects via WebSocket
TerminalServer->>User: Requests authentication
User->>TerminalServer: Sends token
TerminalServer->>Docker: Starts container for session
TerminalServer->>WebSocketClient: Attaches streams (stdin/stdout)
WebSocketClient->>TerminalServer: Sends input/data
TerminalServer->>Docker: Forwards input
Docker->>TerminalServer: Sends output
TerminalServer->>WebSocketClient: Forwards output
Note over TerminalServer: Handles cleanup, errors, and timeouts
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (9)
src/commands/auth/revoke-token.ts (1)
80-87: 🛠️ Refactor suggestionValidate API key format with a regex rather than a naive split
apiKey.split(':')assumes exactly one colon and no surrounding whitespace. A malformed key such as" app.key : secret "will silently bypass validation.- const keyParts = apiKey.split(':') - if (keyParts.length !== 2) { + const match = apiKey.trim().match(/^([^.]+\.[^.]+)\:([A-Za-z0-9+/=]+)$/) + if (!match) { this.error('Invalid API key format. Expected format: appId.keyId:secret') return } - const keyName = keyParts[0] - const secret = keyParts[1] + const [, keyName, secret] = matchThis prevents subtle parsing errors and avoids accidental requests with an incorrect header.
src/commands/channels/subscribe.ts (2)
100-110:⚠️ Potential issueCipher
keyshould be aBuffer, not a hex string
Ably JS expectschannelOptions.cipher.keyto be raw bytes (ArrayBuffer/Buffer), but the flag accepts a hex‑encoded string. Provide decoding to avoid runtime “Invalid key length” errors:- key: flags['cipher-key'], + key: Buffer.from(flags['cipher-key'], 'hex'),Document the encoding in the flag description as well.
134-158: 🛠️ Refactor suggestionThrowing
this.errorinside connection event handlers exits the whole CLI
Inside the'failed'case the command callsthis.error(...), which triggersprocess.exit(1)from within an asynchronous callback. This skips cleanup logic and can leave the connection hanging. Prefer emitting an error event or calling the internalcleanup()routine so resources are released gracefully.src/commands/apps/stats/index.ts (1)
80-83: 🛠️ Refactor suggestionAvoid mutating the parsed
flagsobject in‑place
flagsreturned bythis.parse()is generally treated as an immutable view of CLI input. Re‑assigningflags.unitnot only breaks that assumption, it can also lead to surprising side‑effects for any downstream code that still holds a reference to the original flags object (e.g. telemetry, shared helpers). Prefer copying to a local variable instead:-if (flags.live && flags.unit !== 'minute') { - this.warn('Live stats only support minute intervals. Using minute interval.') - flags.unit = 'minute' -} +let unit: 'minute' | 'hour' | 'day' | 'month' = + flags.unit as typeof flags.unit +if (flags.live && unit !== 'minute') { + this.warn('Live stats only support minute intervals. Using minute interval.') + unit = 'minute' +}Then use
uniteverywhere that currently referencesflags.unit.packages/react-web-cli/src/AblyCliTerminal.tsx (1)
436-437:⚠️ Potential issueDependency array uses boolean expression
useEffect([... , sessionState === 'inactive', ...])stores a boolean value, not thesessionStateitself.
This means the effect will not re‑run whensessionStatelater changes between non‑inactive values (e.g.,'connecting'→'active').Replace with the actual state:
-}, [ablyApiKey, ablyAccessToken, websocketUrl, sessionState === 'inactive', handleSessionEnd, onConnectionStatusChange]); +}, [ablyApiKey, ablyAccessToken, websocketUrl, sessionState, handleSessionEnd, onConnectionStatusChange]);This guarantees the connection logic responds to every state transition.
src/commands/channels/publish.ts (1)
165-203:⚠️ Potential issue
prepareMessagethrows when the parsed JSON is a primitiveIf the user supplies
"\"hello\""(valid JSON string) or a number,messageDatabecomes a primitive.
Theinoperator ('data' in messageData) throwsTypeError: right‑hand side of 'in' should be an object.Safely guard for non‑object payloads:
- // If data is explicitly provided in the message, use it - if ('data' in messageData) { - message.data = messageData.data + const isObj = typeof messageData === 'object' && messageData !== null + + if (isObj && 'data' in messageData) { + // messageData is a record and contains an explicit `data` property + // @ts-expect-error indexing safe after guard + message.data = messageData.data } else { - // Otherwise use the entire messageData as the data - message.data = messageData + // Otherwise treat the parsed value verbatim as message payload + message.data = messageData }Doing the same type‑guard before accessing
messageData.nameprevents another potential crash.🧰 Tools
🪛 Biome (1.9.4)
[error] 187-188: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
scripts/terminal-server.ts (1)
272-547: 🛠️ Refactor suggestionDead code:
_handleAuth()is never invokedThe connection flow now performs authentication inside the
ws.once('message' …)handler._handleAuth()(≈275 LOC) is therefore unreachable, yet it still creates containers, attaches streams, and holds extensive error handling logic. Keeping it:
- Bloats the file (~25 % extra lines).
- Risks maintenance divergence – fixes in one path may not land in the other.
- Can confuse future reviewers (is there a second auth path?).
Either call
_handleAuth()from the'connection'callback or delete it entirely.-async function _handleAuth(ws: WebSocket, req: http.IncomingMessage): Promise<boolean> { - …huge block… -}🧰 Tools
🪛 Biome (1.9.4)
[error] 472-472: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 487-487: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/base-command.ts (2)
42-83:⚠️ Potential issue
--token-onlyflag is referenced but never declared
shouldSuppressOutput()(lines 701‑703) relies on atoken-onlyboolean flag, yet it is missing fromstatic globalFlags. CLI help will not list it and Oclif will treat it as an unknown flag, breaking the suppression logic.'pretty-json': Flags.boolean({ description: 'Output in colorized JSON format', exclusive: ['json'], // Cannot use with json }), + 'token-only': Flags.boolean({ + description: 'Suppress all output except the issued token (useful for scripting)', + hidden: true, + }),
483-521:⚠️ Potential issue
options.logHandler&options.logLeveldo not match Ably SDK APIThe Ably JS SDK expects:
options.log = { level: 4, handler: (msg, level) => { … } }Setting
logHandler/logLevelseparately is ignored, so none of the verbose/error filtering will ever fire.- // Always add a log handler … - options.logHandler = (message: string, level: number) => { + // Attach SDK log handler + options.log = { + level: 4, + handler: (message: string, level: number) => { /* existing body unchanged */ - }; - - // Set logLevel to highest ONLY when using custom handler … - options.logLevel = 4; + } + }
🧹 Nitpick comments (30)
src/commands/apps/logs/subscribe.ts (1)
43-43: Ensure no trailing whitespace at end of file.There appears to be trailing whitespace at the end of line 43, which should be removed to comply with linting rules.
-} +}.github/workflows/test.yml (1)
34-34: Fix YAML formatting issues.The file has two minor YAML linting issues that should be addressed:
- Missing newline at the end of file
- Trailing whitespace after
pnpm test- - name: Run tests - run: pnpm test + - name: Run tests + run: pnpm test +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 34-34: no new line character at the end of file
(new-line-at-end-of-file)
[error] 34-34: trailing spaces
(trailing-spaces)
src/commands/auth/revoke-token.ts (2)
68-69: Remove unused_restvariable to prevent future lint violations
_restis instantiated but never referenced. Even though the underscore prefix often signals “intentionally unused”, the current project goal is zero‑lint warnings; most ESLint configs still flag any unused variable regardless of naming.- const _rest = new Ably.Rest(this.getClientOptions(flags)) + // const rest = new Ably.Rest(this.getClientOptions(flags)) // uncomment when REST client is requiredIf the REST client is not required, simply delete the line.
160-178: Add explicittimeoutand handle TLS errors in HTTPS request options
Long‑running connections can hang indefinitely; additionally, TLS failures are currently surfaced only through the genericerrorlistener.port: 443, + timeout: 10000, // 10 seconds + rejectUnauthorized: true,Also attach a
req.setTimeouthandler to abort and reject with a clear “timeout” error.src/commands/channels/occupancy/subscribe.ts (1)
206-208: Detach SIGINT/SIGTERM listeners after cleanup to avoid leaks
When the promise resolves the process usually exits, but if another long‑lived command ever re‑uses this class in‑process, the listener remains and will fire multiple times.process.on('SIGINT', cleanup) process.on('SIGTERM', cleanup) + +const removeHandlers = () => { + process.off('SIGINT', cleanup) + process.off('SIGTERM', cleanup) +} + ... - await new Promise<void>((resolve) => { + await new Promise<void>((resolve) => { const cleanup = () => { ... - resolve() + removeHandlers() + resolve() }This ensures only one active handler per invocation.
src/commands/channels/subscribe.ts (1)
73-78: Avoid double‑parsing CLI input
this.parse()is invoked twice which needlessly repeats validation and flag defaulting.- const { flags } = await this.parse(ChannelsSubscribe); - const _args = await this.parse(ChannelsSubscribe) + const { args, flags, argv } = await this.parse(ChannelsSubscribe)Then use
argv(orargs.channelsifmultiple: true) for channel names.src/commands/apps/stats/index.ts (2)
145-155: Register shutdown handlers withonceor remove after first useBoth
SIGINTandSIGTERMhandlers re‑invokecleanup()every time the signal is delivered. Although the function is idempotent, repeated listeners accumulate across multiple invocations of this command in the same Node process (e.g. test harnesses), causing memory‑leak warnings. Either:-process.on('SIGINT', cleanup) -process.on('SIGTERM', cleanup) +process.once('SIGINT', cleanup) +process.once('SIGTERM', cleanup)or manually call
process.off()insidecleanup.
91-97: ValidateintervalSecondsand guard against nonsensical values
flags.intervalis user‑supplied; negative or zero values will create a non‑functional timer. A quick guard makes the UX friendlier:-intervalSeconds: flags.interval as number, +intervalSeconds: Math.max(1, flags.interval as number),Consider emitting a warning when coercion occurs.
src/commands/connections/test.ts (2)
29-38:finallyduplication results in double‑closeYou close the clients here and again in each individual test’s
finally, and again inrun()’sfinallycomment. Multipleclose()calls are safe but noisy (extra state‑change events). Centralise the cleanup either in this override or in each helper, not both.
129-134: Prefer a typed flags interface overRecord<string, unknown>Using
Record<string, unknown>loses static safety for downstream helpers (logCliEvent,shouldOutputJson). Define:interface TestFlags extends BaseFlags { transport: 'ws' | 'xhr' | 'all' }and use
const {flags} = await this.parse<{}, TestFlags>(ConnectionsTest).src/commands/channels/presence/subscribe.ts (2)
232-315: Signal handlers are attached inside an ever‑running PromiseEach execution of the command adds fresh
SIGINT/SIGTERMlisteners without removing them after cleanup. Re‑invoking the command within the same process (unit tests, programmatic usage) will trigger MaxListenersExceeded warnings.Attach the listeners at class scope with
once, or callprocess.off()at the end ofcleanup():process.once('SIGINT', onSignal) process.once('SIGTERM', onSignal) … const onSignal = () => cleanup().catch(reject) - process.once('SIGINT', …) - process.once('SIGTERM', …) + const remove = () => { + process.off('SIGINT', onSignal) + process.off('SIGTERM', onSignal) + } + await cleanup(); remove()
295-299: Suppressing ESLint with// eslint-disable-next-line unicorn/no-useless-undefinedis unnecessaryReturning
resolve()without arguments is perfectly acceptable; remove the disable and returnvoidexplicitly:-// eslint-disable-next-line unicorn/no-useless-undefined -resolve(undefined) +resolve()Cleaning up stray lint ignores keeps the codebase pristine.
src/commands/accounts/stats/index.ts (2)
55-58: Race condition flag:isPollingcomment misleading
isPollingis declared after the comment// Track when we're already fetching stats.
The comment makes it sound like it referencesstatsDisplay. Consider renaming toisFetchingStatsor moving the comment next to the flag to avoid confusion.
130-141: Leaking process listeners on multiple invocations
process.on('SIGINT', cleanup)andprocess.on('SIGTERM', cleanup)are registered every timerunLiveStatsis executed. If the command is invoked repeatedly in the same Node process (e.g., via tests), multiple identical listeners build up.Add a one‑off guard:
const signalsInstalled = new Set<string>() for (const sig of ['SIGINT', 'SIGTERM']) { if (!signalsInstalled.has(sig)) { process.on(sig, cleanup) signalsInstalled.add(sig) } }or use
process.once.packages/react-web-cli/src/AblyCliTerminal.tsx (2)
64-75: Mixed event‑attachment styles
addEventListeneris used foropen/close, but the legacyonmessage=assignment is left in place.
Mixing styles is harmless but inconsistent and silently overrides other listeners.- socket.onmessage = (event) => { - callbacks.onMessage?.(event); - }; + socket.addEventListener('message', (event) => { + callbacks.onMessage?.(event); + });Consider converting
onmessagetoaddEventListener('message', …)for parity.
148-156: Swallowing send errors loses context
catch (error) { console.error('[AblyCLITerminal] Error sending resize message:', error); }Because
debouncedSendResizesuppresses the exception, callers cannot react (e.g., mark the socket as errored). Consider rethrowing or invoking an error callback so higher‑level code can decide whether to terminate the session.README.md (4)
20-20: Extra blank line after TOC section
There's an added blank line at line 20 after<!-- tocstop -->. Consider removing it to keep the document compact and avoid unintended vertical whitespace.
3873-3873: Specify language for fenced code block
The code block at line 3873 is missing a language identifier, which is required by markdownlint (MD040). Add a language (e.g.shorbash) after the opening backticks.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3873-3873: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
4095-4095: Specify language for fenced code block
The code block at line 4095 lacks a language tag, triggering MD040. Please annotate it (e.g.sh) to improve syntax highlighting and satisfy lint rules.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
4095-4095: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
4157-4157: Specify language for fenced code block
Similarly, the code block starting at line 4157 needs a language annotation for consistency and to resolve MD040.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
4157-4157: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
src/commands/channels/presence/enter.ts (1)
121-132:parsePresenceDatareturnsnullonly afterthis.error()throws – the surrounding checks are unreachable
this.error()inside thecatchbranch throws anoclif-specific error that terminates the command.
Therefore:
- The subsequent
return nullwill never be executed.- The
presenceData === nullguard inrun()(lines 55‑56) is redundant dead code.Removing this defensive layer will simplify control‑flow and satisfy TypeScript by using the
neverreturn type fromthis.error().- } catch (error) { - const errorMsg = 'Invalid JSON data format. Please provide a valid JSON string.'; - this.logCliEvent(flags, 'presence', 'dataParseError', errorMsg, { error: error instanceof Error ? error.message : String(error) }); - this.error(errorMsg); - return null; - } + } catch (error) { + const errorMsg = 'Invalid JSON data format. Please provide a valid JSON string.'; + this.logCliEvent(flags, 'presence', 'dataParseError', errorMsg, { error: error instanceof Error ? error.message : String(error) }); + this.error(errorMsg); // throws – no further code needed + }Follow‑up: delete the
presenceData === nullshort‑circuit inrun().src/commands/connections/stats.ts (2)
278-285: Duplicate rendering of the same stats interval
fetchAndDisplayStatsfirst displaysstats[0](line 279) and then iterates all stats including element 0 (lines 282‑284), so the first interval is shown twice.- this.statsDisplay!.display(stats[0] as StatsDisplayData) - - // Display each stat interval - for (const stat of stats) { + // Display each stat interval + for (const stat of stats) { this.statsDisplay!.display(stat as StatsDisplayData) }Removing the initial call ensures each interval is rendered exactly once.
125-127: Listeners attached withprocess.onaccumulate across tests / multiple invocations
runLiveStatsregistersSIGINT/SIGTERMhandlers withprocess.on, notonce.
When this command is executed repeatedly in the same Node process (e.g., in integration tests) handlers will stack up, triggering cleanup multiple times.Prefer
process.onceor store the bound function so you canprocess.offit infinally().src/commands/channels/publish.ts (1)
318-331: Progress indicator interval is never cleared on early termination
setupProgressIndicatorstores the interval id, but if the user terminates withCtrl+Cbefore publishing completes,finally()clears onlyprogressIntervalId, not the SIGINT listeners created insidepublishWithRealtime.Consider:
- Clearing the interval in the SIGINT handler to avoid one‑off logs after exit.
- Using
process.once('SIGINT', …)to match single‑run CLI semantics.scripts/terminal-server.ts (1)
532-536: Duplicate attachment logic
attachToContainer()is invoked here even though the same streams were already wired earlier in_handleAuth(now unused). When the earlier code path is re‑used, this double attachment will attempt to hijack the same exec twice, leading to “already attached” Docker errors.If you keep
_handleAuth, make sure only one of the two attachment paths executes by:-if (!session.execInstance) { - await attachToContainer(fullSession, ws); -}src/commands/bench/publisher.ts (2)
102-103: Remove unusedpresenceCountproperty
private presenceCount = 0;is never incremented or read, so it adds noise and hints at half‑implemented logic.- private presenceCount = 0;
413-517: Duplicate publish loops – consolidate REST vs Realtime paths
publishMessagesRealtime()andpublishMessagesRest()are identical except for the log header. Maintaining two 100‑line functions invites drift and doubles bug‑fix effort.Consider:
private async publishMessages( channel: Ably.RealtimeChannel, transport: 'rest' | 'realtime', … ) { /* shared implementation */ }Dispatch with
await publishMessages(channel, flags.transport as 'rest'|'realtime', …).Besides readability, this trims ~100 LOC and prevents future inconsistencies (e.g. one path capturing errors you forgot to port).
src/base-command.ts (3)
136-143: Unnecessary mutation of the parsedflagsobject
flags['api-key'] = appAndKey.apiKeymutates the frozen object returned by Oclif.
Although it happens to work because Oclif returns a plain object, mutating shared input increases coupling and can cause surprising side‑effects in downstream code.- flags['api-key'] = appAndKey.apiKey + const updatedFlags = {...flags, 'api-key': appAndKey.apiKey}Pass
updatedFlagstogetClientOptions()and other helpers instead of the original.
136-141:return nullis unreachable afterthis.error()
this.error()in Oclif throws (typenever). The followingreturn nullis therefore dead code.- this.error(`${chalk.yellow('No app or API key configured …')}`) - return null + this.error(`${chalk.yellow('No app or API key configured …')}`)
701-703:shouldSuppressOutput()duplicatestoken-onlycheckOnce
token-onlyis formally declared (see earlier comment), consider reusingshouldSuppressOutput()wherever raw token output mode is needed to avoid scattered quiet‑mode conditionals.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (107)
.eslintignore(0 hunks).eslintrc.json(0 hunks).github/workflows/test.yml(1 hunks).prettierrc.json(1 hunks)README.md(103 hunks)bin/development.js(0 hunks)eslint.config.js(1 hunks)examples/web-cli/eslint.config.js(1 hunks)examples/web-cli/src/App.tsx(3 hunks)examples/web-cli/src/main.tsx(1 hunks)examples/web-cli/vite.config.ts(1 hunks)package.json(5 hunks)packages/react-web-cli/src/AblyCliTerminal.tsx(18 hunks)packages/react-web-cli/src/index.ts(1 hunks)prd.md(2 hunks)scripts/restricted-shell.sh(1 hunks)scripts/terminal-server.ts(17 hunks)scripts/terminal-test-client.ts(2 hunks)scripts/update-json-flag.js(0 hunks)src/base-command.ts(12 hunks)src/chat-base-command.ts(1 hunks)src/commands/account/index.ts(0 hunks)src/commands/account/list.ts(0 hunks)src/commands/accounts/current.ts(5 hunks)src/commands/accounts/index.ts(1 hunks)src/commands/accounts/list.ts(2 hunks)src/commands/accounts/login.ts(7 hunks)src/commands/accounts/logout.ts(6 hunks)src/commands/accounts/stats/index.ts(6 hunks)src/commands/accounts/switch.ts(5 hunks)src/commands/aliases.ts(0 hunks)src/commands/app/index.ts(0 hunks)src/commands/app/list.ts(0 hunks)src/commands/apps/channel-rules/create.ts(6 hunks)src/commands/apps/channel-rules/delete.ts(7 hunks)src/commands/apps/channel-rules/index.ts(1 hunks)src/commands/apps/channel-rules/list.ts(5 hunks)src/commands/apps/channel-rules/update.ts(8 hunks)src/commands/apps/create.ts(4 hunks)src/commands/apps/current.ts(3 hunks)src/commands/apps/delete.ts(7 hunks)src/commands/apps/list.ts(4 hunks)src/commands/apps/logs/history.ts(7 hunks)src/commands/apps/logs/subscribe.ts(2 hunks)src/commands/apps/set-apns-p12.ts(3 hunks)src/commands/apps/stats/index.ts(6 hunks)src/commands/apps/switch.ts(3 hunks)src/commands/apps/update.ts(4 hunks)src/commands/auth/issue-ably-token.ts(6 hunks)src/commands/auth/issue-jwt-token.ts(6 hunks)src/commands/auth/keys/create.ts(5 hunks)src/commands/auth/keys/current.ts(3 hunks)src/commands/auth/keys/get.ts(5 hunks)src/commands/auth/keys/index.ts(1 hunks)src/commands/auth/keys/list.ts(5 hunks)src/commands/auth/keys/revoke.ts(6 hunks)src/commands/auth/keys/switch.ts(6 hunks)src/commands/auth/keys/update.ts(4 hunks)src/commands/auth/revoke-token.ts(8 hunks)src/commands/bench/publisher.ts(7 hunks)src/commands/bench/subscriber.ts(7 hunks)src/commands/channel-rule/create.ts(1 hunks)src/commands/channel-rule/delete.ts(1 hunks)src/commands/channel-rule/index.ts(1 hunks)src/commands/channel-rule/list.ts(1 hunks)src/commands/channel-rule/update.ts(1 hunks)src/commands/channel/history.ts(0 hunks)src/commands/channel/index.ts(0 hunks)src/commands/channel/list.ts(0 hunks)src/commands/channel/occupancy/get.ts(0 hunks)src/commands/channel/occupancy/index.ts(0 hunks)src/commands/channel/occupancy/subscribe.ts(0 hunks)src/commands/channel/presence/enter.ts(0 hunks)src/commands/channel/presence/index.ts(0 hunks)src/commands/channel/presence/subscribe.ts(0 hunks)src/commands/channel/publish.ts(0 hunks)src/commands/channel/subscribe.ts(0 hunks)src/commands/channels/batch-publish.ts(11 hunks)src/commands/channels/history.ts(6 hunks)src/commands/channels/list.ts(9 hunks)src/commands/channels/logs.ts(2 hunks)src/commands/channels/occupancy/get.ts(4 hunks)src/commands/channels/occupancy/subscribe.ts(10 hunks)src/commands/channels/presence/enter.ts(3 hunks)src/commands/channels/presence/subscribe.ts(13 hunks)src/commands/channels/publish.ts(3 hunks)src/commands/channels/subscribe.ts(8 hunks)src/commands/config.ts(3 hunks)src/commands/connection/index.ts(0 hunks)src/commands/connections/logs.ts(2 hunks)src/commands/connections/stats.ts(6 hunks)src/commands/connections/test.ts(3 hunks)src/commands/help/ask.ts(4 hunks)src/commands/help/contact.ts(2 hunks)src/commands/help/index.ts(3 hunks)src/commands/help/status.ts(2 hunks)src/commands/help/support.ts(2 hunks)src/commands/help/web-cli.ts(1 hunks)src/commands/integration/create.ts(0 hunks)src/commands/integration/delete.ts(0 hunks)src/commands/integration/get.ts(0 hunks)src/commands/integration/index.ts(0 hunks)src/commands/integration/list.ts(0 hunks)src/commands/integration/update.ts(0 hunks)src/commands/integrations/create.ts(3 hunks)src/commands/integrations/delete.ts(5 hunks)src/commands/integrations/get.ts(3 hunks)
⛔ Files not processed due to max files limit (45)
- src/commands/integrations/list.ts
- src/commands/integrations/update.ts
- src/commands/log/index.ts
- src/commands/login.ts
- src/commands/logs/app/history.ts
- src/commands/logs/app/subscribe.ts
- src/commands/logs/channel-lifecycle.ts
- src/commands/logs/channel-lifecycle/subscribe.ts
- src/commands/logs/connection-lifecycle/history.ts
- src/commands/logs/connection-lifecycle/subscribe.ts
- src/commands/logs/connection/subscribe.ts
- src/commands/logs/push/history.ts
- src/commands/logs/push/subscribe.ts
- src/commands/mcp/index.ts
- src/commands/mcp/start-server.ts
- src/commands/queue/create.ts
- src/commands/queue/delete.ts
- src/commands/queue/index.ts
- src/commands/queue/list.ts
- src/commands/queues/create.ts
- src/commands/queues/delete.ts
- src/commands/queues/list.ts
- src/commands/room/index.ts
- src/commands/rooms/index.ts
- src/commands/rooms/list.ts
- src/commands/rooms/messages/get.ts
- src/commands/rooms/messages/send.ts
- src/commands/rooms/messages/subscribe.ts
- src/commands/rooms/occupancy/get.ts
- src/commands/rooms/occupancy/subscribe.ts
- src/commands/rooms/presence/enter.ts
- src/commands/rooms/presence/subscribe.ts
- src/commands/rooms/reactions/send.ts
- src/commands/rooms/reactions/subscribe.ts
- src/commands/rooms/typing/start.ts
- src/commands/rooms/typing/subscribe.ts
- src/commands/space/cursors.ts
- src/commands/space/index.ts
- src/commands/space/locations.ts
- src/commands/space/locks.ts
- src/commands/space/members.ts
- src/commands/spaces/cursors.ts
- src/commands/spaces/cursors/get-all.ts
- src/commands/spaces/cursors/set.ts
- src/commands/spaces/cursors/subscribe.ts
💤 Files with no reviewable changes (27)
- .eslintrc.json
- bin/development.js
- src/commands/account/list.ts
- src/commands/channel/presence/enter.ts
- src/commands/channel/occupancy/get.ts
- src/commands/channel/occupancy/index.ts
- .eslintignore
- src/commands/channel/presence/index.ts
- src/commands/connection/index.ts
- src/commands/channel/occupancy/subscribe.ts
- src/commands/channel/subscribe.ts
- src/commands/channel/presence/subscribe.ts
- src/commands/account/index.ts
- src/commands/integration/delete.ts
- src/commands/integration/list.ts
- src/commands/app/index.ts
- src/commands/integration/update.ts
- src/commands/channel/list.ts
- src/commands/app/list.ts
- src/commands/channel/publish.ts
- src/commands/aliases.ts
- src/commands/integration/create.ts
- src/commands/integration/get.ts
- scripts/update-json-flag.js
- src/commands/channel/history.ts
- src/commands/integration/index.ts
- src/commands/channel/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (12)
src/commands/accounts/current.ts (1)
src/services/control-api.ts (1)
ControlApi(162-490)
src/commands/channels/occupancy/get.ts (1)
src/commands/rooms/occupancy/subscribe.ts (1)
OccupancyMetrics(8-11)
src/commands/accounts/switch.ts (1)
src/services/control-api.ts (1)
ControlApi(162-490)
src/commands/auth/keys/switch.ts (1)
src/services/control-api.ts (1)
ControlApi(162-490)
src/commands/help/ask.ts (1)
src/services/control-api.ts (1)
Conversation(149-154)
src/commands/apps/switch.ts (1)
src/services/control-api.ts (1)
ControlApi(162-490)
src/chat-base-command.ts (2)
src/types/cli.ts (1)
BaseFlags(7-24)src/base-command.ts (1)
BaseFlags(773-773)
src/commands/help/web-cli.ts (1)
src/help.ts (1)
CustomHelp(10-281)
src/commands/integrations/create.ts (1)
src/services/control-api.ts (1)
RuleData(83-92)
src/commands/connections/stats.ts (1)
src/services/stats-display.ts (2)
StatsDisplay(24-676)StatsDisplayData(13-22)
src/commands/accounts/stats/index.ts (3)
src/services/stats-display.ts (1)
StatsDisplay(24-676)src/types/cli.ts (1)
BaseFlags(7-24)src/services/control-api.ts (1)
ControlApi(162-490)
src/base-command.ts (4)
src/services/config-manager.ts (1)
ConfigManager(41-461)src/services/interactive-helper.ts (1)
InteractiveHelper(5-126)src/types/cli.ts (3)
CommandConfig(40-40)BaseFlags(7-24)ErrorDetails(30-35)src/services/control-api.ts (1)
ControlApi(162-490)
🪛 markdownlint-cli2 (0.17.2)
prd.md
46-46: Spaces inside code span elements
null
(MD038, no-space-in-code)
README.md
3873-3873: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
4095-4095: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
4157-4157: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 34-34: no new line character at the end of file
(new-line-at-end-of-file)
[error] 34-34: trailing spaces
(trailing-spaces)
🪛 Biome (1.9.4)
src/commands/bench/publisher.ts
[error] 563-564: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
scripts/terminal-server.ts
[error] 487-487: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (244)
src/commands/accounts/index.ts (2)
6-7: Reordering state properties for clarity
MovinglastExecutionTimestampdirectly belowisExecutinggroups related state together, improving readability without affecting functionality.
8-10: Grouping thedone()mutator with state declarations
Placing thedone()method immediately after the state fields logically groups the mutator with the fields it affects. No behavior change introduced.src/commands/config.ts (2)
2-3: Confirm Node.jsnode:specifier compatibilityPlease verify that the project's minimum supported Node.js version supports the newer
node:built‑in module specifiers (introduced in Node.js v16.0). If not already enforced, consider updating theenginesfield inpackage.jsonor your CI matrix to ensure compatibility.
57-62: Approve independent platform checks for default editorRefactoring from an
else ifchain to separateifstatements retains the same behavior while simplifying control flow. Each platform branch returns early, so only the matching block executes.src/commands/apps/set-apns-p12.ts (4)
2-3: Verify Node.jsnode:imports are supportedThe
node:fsandnode:pathspecifiers require Node.js ≥16.0. Ensure the CLI’s supported Node versions align with this change, and update theenginesfield if necessary.
8-13: Args declaration moved above flags per oclif conventionsDefining
static argsbeforestatic flagsimproves readability and follows oclif guidelines. This change has no behavioral impact.
32-35: Reordered flag properties for consistencyMoving the
descriptionafterdefaultin theuse-for-sandboxflag aligns with the style used across other commands. No functional change.
64-66: Swapped option order inuploadApnsP12callReordering
passwordbeforeuseForSandboxin the options object is purely cosmetic and does not affect the API invocation.examples/web-cli/vite.config.ts (1)
2-2: Approve import order change in Vite configSwapping the positions of the React plugin and
defineConfigimports is a non-functional, stylistic update that improves consistency with other example files.src/commands/apps/channel-rules/index.ts (1)
22-24: Approvefor...ofloop replacingforEachSwitching to a
for...ofloop for printing examples simplifies the iteration and avoids issues with asynchronous callbacks (even though none are present). Behavior remains the same.scripts/restricted-shell.sh (1)
78-78: Validate updated help command invocationThe welcome message now uses
ably help web-cli. Please confirm that this subcommand exists and produces the expected output after the recent CLI help refactoring.examples/web-cli/src/main.tsx (2)
3-3: Consistent import spacing appliedThe added empty line after imports follows common code style conventions, improving readability by separating import statements from the rest of the code.
7-7:Details
✅ Verification successful
Changed DOM selection method
Switching from
getElementByIdtoquerySelectoris a reasonable stylistic choice. Both methods accomplish the same task in this context, thoughquerySelectoris slightly more versatile at a minor performance cost.
🏁 Script executed:
#!/bin/bash # Check if this pattern is used consistently across the codebase echo "Checking for DOM element selection patterns in the codebase:" rg -t tsx -t jsx "document\.(getElementById|querySelector)" --statsLength of output: 228
🏁 Script executed:
#!/bin/bash echo "Checking for DOM element selection patterns in .tsx and .jsx files:" rg -n "document\.(getElementById|querySelector)" --glob "*.tsx" --glob "*.jsx" --statsLength of output: 485
DOM selection change verified
Switching to
querySelectoronly affects examples/web-cli/src/main.tsx (line 7). No other occurrences ofgetElementByIdorquerySelectorwere found in .tsx/.jsx files, so this stylistic update is safe to approve.src/commands/help/status.ts (2)
3-3: Import ordering standardizedReordering the imports is a good styling change that improves consistency. This aligns with code quality improvements targeted by this lint-focused PR.
23-23: Flag property ordering standardizedMoving the description property below the default property creates a consistent ordering pattern in flag definitions throughout the codebase.
src/commands/auth/keys/index.ts (1)
32-34:Details
✅ Verification successful
Improved user feedback with authentication information
Great addition! This enhances the user experience by providing relevant authentication context after displaying command help. This change is part of a broader effort to standardize authentication feedback across commands.
🏁 Script executed:
#!/bin/bash # Check if showAuthInfoIfNeeded is properly implemented in the base command echo "Checking implementation of showAuthInfoIfNeeded method:" rg -A 10 "showAuthInfoIfNeeded" src/base-command.tsLength of output: 1116
Confirm
showAuthInfoIfNeededIntegration
TheshowAuthInfoIfNeededmethod is already implemented insrc/base-command.tsand correctly handles:
- Skipping display via
shouldShowAuthInfo()exceptions- Suppressing output for
quiet, JSON,token-only, and other suppression flagsThis standardizes post‑help authentication feedback across commands as intended.
.prettierrc.json (1)
1-4: Enhanced Prettier configurationGood improvement to the Prettier configuration. Changing from a simple preset string to an object that extends the preset while adding custom settings gives more flexibility while maintaining consistency with the oclif ecosystem.
The explicit tabWidth setting ensures consistent indentation across the project.
src/commands/help/contact.ts (3)
3-3: Reordered imports for consistency.The import statement for
openhas been properly moved to appear after thechalkimport. This change aligns with common import ordering practices where third-party packages are grouped together.
17-17: Simplified parse call by removing unused destructuring.Good change - since the flags aren't being used in this method, removing the destructuring assignment makes the code cleaner and more maintainable.
22-22: Added trailing newline at the end of file.Adding a trailing newline is a common best practice and is enforced by many linters.
src/commands/apps/logs/subscribe.ts (2)
2-2: Added blank line after imports.This spacing improves readability by clearly separating imports from the code implementation.
15-22: Reordered flag properties for consistency.The properties for both the
jsonandrewindflags have been properly reordered to follow a consistent pattern, withdefaultfollowed bydescription. This standardization improves code maintainability and readability.packages/react-web-cli/src/index.ts (1)
1-1: Improved export specificity with explicit named export.This change from wildcard export to a named export with explicit file extension is an improvement that:
- Makes imports more explicit and improves code maintainability
- Adheres to modern ES module best practices by including the
.jsextension- Provides better support for tree-shaking in module bundlers
The explicit export approach is preferred over wildcard exports as it clearly documents what's being exposed from the module.
src/commands/help/support.ts (3)
3-3: Reordered imports for consistency.The import statement for
openhas been properly moved to appear after thechalkimport. This change aligns with common import ordering practices where third-party packages are grouped together.
17-17: Simplified parse call by removing unused destructuring.Good change - since the flags aren't being used in this method, removing the destructuring assignment makes the code cleaner and more maintainable.
22-22: Added trailing newline at the end of file.Adding a trailing newline is a common best practice and is enforced by many linters.
src/commands/apps/list.ts (3)
2-3: Import ordering looks good.The improved import ordering follows standard TypeScript conventions, with external libraries first (
chalk) followed by internal imports.
37-37: JSON structure improvement.Good change to wrap the apps array in an object with an
appskey. This provides better context in the JSON output and follows the best practice of using objects rather than arrays as root elements in JSON responses.
58-78: Loop style enhancement.Replacing
Array.forEachwith afor...ofloop improves readability and makes the code more maintainable. This modern syntax is also more flexible if you ever need to useawaitwithin the loop.src/commands/channel-rule/index.ts (2)
2-2: Consistent spacing after imports.The added blank line after imports improves readability and follows consistent code style.
6-9: Standardized property ordering.The reordering of static properties (
argsbeforehidden) aligns with TypeScript best practices and improves consistency across the codebase.src/commands/channel-rule/list.ts (2)
2-2: Consistent spacing after imports.The added blank line after imports improves readability and follows consistent code style.
6-9: Standardized property ordering.The reordering of static properties (
argsbeforehidden) aligns with TypeScript best practices and improves consistency across the codebase.src/commands/channel-rule/update.ts (2)
2-2: Consistent spacing after imports.The added blank line after imports improves readability and follows consistent code style.
6-9: Standardized property ordering.The reordering of static properties (
argsbeforehidden) aligns with TypeScript best practices and improves consistency across the codebase.src/commands/channel-rule/create.ts (1)
2-2: LGTM: Consistent ordering of static properties.The reordering of static properties with
argsbeforehiddenand adding the empty line after imports improves consistency with other command files in the codebase.Also applies to: 6-9
prd.md (1)
245-245: LGTM: Improved user experience for command suggestions.Replacing hidden aliases with command suggestions is a more intuitive approach that aligns with modern CLI design patterns.
src/commands/channel-rule/delete.ts (1)
2-2: LGTM: Consistent ordering of static properties.The reordering of static properties with
argsbeforehiddenand adding the empty line after imports improves consistency with other command files in the codebase.Also applies to: 6-9
src/commands/apps/current.ts (3)
3-4: LGTM: Improved import organization.Reordering the imports enhances readability and maintains consistency with other files in the codebase.
100-100: LGTM: Enhanced type safety with specific type annotation.Replacing
anywithRecord<string, unknown>is a good TypeScript practice that improves type safety and code clarity.
139-139: LGTM: Simplified catch block.Removing the unused error parameter in the catch block improves code cleanliness.
src/commands/apps/create.ts (3)
22-23: Consistently ordered flag properties.The flag description has been correctly positioned after the default property, improving consistency with other flag declarations in the codebase.
43-54: Improved JSON output structure and date formatting.The JSON response structure has been improved with:
- Logical grouping of related fields (timestamps together at the top)
- Explicit ISO string conversion for dates
- Consistent property naming (
tlsOnlyin the output matches the camelCase property name)- Standardized order with
successat the endThis standardization improves consistency across all CLI commands.
77-79: Standardized JSON error response structure.The error response now follows the standard pattern of placing
successfield at the end of the JSON object, consistent with the success response formatting and improving consistency across the codebase.scripts/terminal-test-client.ts (3)
17-21: Improved authentication message property ordering.The authentication message properties have been reordered to place
accessTokenfirst andtypelast, improving readability and consistency with other command outputs in the codebase.
56-56: Enhanced number readability with numeric separator.Using the numeric separator (
15_000instead of15000) improves code readability for larger numbers, which is a good practice in modern JavaScript/TypeScript.
62-62: Improved string replacement with replaceAll.Replacing regex-based
.replace()with.replaceAll()is a good modernization that improves code readability and maintainability for simple string replacements..github/workflows/test.yml (1)
1-34: Well-structured GitHub Actions workflow for automated testing.The workflow is properly configured to:
- Run on all push and PR events across branches
- Set up a modern development environment with pnpm 10 and Node.js 22.x
- Install dependencies with frozen lockfile (good for CI reliability)
- Execute the test suite
This addition significantly improves the project's CI capabilities.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 34-34: no new line character at the end of file
(new-line-at-end-of-file)
[error] 34-34: trailing spaces
(trailing-spaces)
src/commands/accounts/logout.ts (4)
2-3: Improved import using Node.js module specifier.Using
node:readlineinstead ofreadlineis a best practice that explicitly identifies this as a Node.js built-in module rather than a third-party package.
7-12: Enhanced argument declaration with TypeScript override.The
argsdeclaration has been:
- Properly moved to the top of the class
- Correctly marked with
overridekeyword- Well-structured with clear type definitions
This improves type safety and follows TypeScript best practices.
41-45: Standardized JSON response structure.Error responses now consistently place the
errorfield beforesuccess: false, maintaining a consistent response structure across all commands.Also applies to: 58-62, 106-110
86-92: Improved success response structure.The success response maintains the consistent pattern of placing
success: trueat the end and adds a trailing comma afterremainingAccountsfor better code formatting.src/commands/auth/keys/list.ts (5)
4-5: LGTM: Improved import organizationMoving the ControlBaseCommand import below the chalk import follows a consistent import ordering pattern, with external dependencies first, followed by internal modules.
37-39: LGTM: Standardized JSON output formatThe error message is now listed before the success flag, providing a more consistent and readable JSON structure.
68-70: LGTM: Improved JSON property orderingPlacing keys before the success flag in the JSON output improves readability by grouping the data fields together.
79-80: LGTM: Modernized iteration styleReplacing
forEachwithfor...ofloops improves code readability and follows modern JavaScript/TypeScript best practices.Also applies to: 98-100, 107-107
112-115: LGTM: Standardized error response structureReorganizing the error response properties to include appId first, followed by the error message and success flag, creates a more consistent pattern across commands.
examples/web-cli/eslint.config.js (3)
4-4: LGTM: Reordered importMoving the globals import to a consistent position in the import list.
8-17: LGTM: Expanded ignore patternsThe ignore patterns have been expanded to include additional file types and directories, which helps prevent ESLint from processing files that don't need linting.
This expanded configuration properly ignores:
- Distribution directories
- Node modules
- TypeScript declaration files
- Specific environment and configuration files
24-26: LGTM: Disabled TypeScript project referencesSetting
project: nulldisables TypeScript project references, which can speed up linting and prevent issues with project file resolution.src/commands/accounts/current.ts (5)
1-2: LGTM: Improved import organizationMoving the chalk import to the top follows a consistent pattern of listing external dependencies first.
39-39: LGTM: Simplified object destructuringThe more concise destructuring syntax improves code readability.
46-46: LGTM: Standardized property orderingReordering the destructured properties from
controlApi.getMe()to consistently putaccountbeforeuseracross multiple locations improves code consistency.Also applies to: 102-102
72-72: LGTM: Simplified error handlingRemoving the unused error parameter in the catch clause is a good practice to avoid unused variables.
91-91: LGTM: Improved type safetyReplacing
anywithRecord<string, unknown>for the flags parameter provides better type safety while maintaining flexibility.src/commands/auth/keys/revoke.ts (5)
6-11: LGTM: Improved command structureMoving the args definition to the top of the class provides a clearer structure, with arguments defined before flags.
31-31: LGTM: Standardized flag property orderingReordering the properties in the force flag definition for consistency.
56-58: LGTM: Standardized JSON error responsePlacing the error message before the success flag creates a more consistent pattern across error responses.
88-90: LGTM: Modernized iteration styleReplacing
forEachwith afor...ofloop improves code readability and follows modern JavaScript/TypeScript best practices.
111-113: LGTM: Standardized JSON response structureConsistently placing the
successproperty last in all JSON responses improves readability and maintainability.Also applies to: 126-128, 152-155
src/commands/auth/keys/current.ts (3)
4-5: Import reordering looks good.The import for
ControlBaseCommandhas been moved after the standard library imports, which follows a common convention of organizing imports by source type.
81-81: Improved type safety by using specific types.Changing the
flagsparameter fromanytoRecord<string, unknown>is a good practice that increases type safety while still allowing for flexible flag handling.
113-116: Simplified destructuring and error handling.The changes to only destructure the
accountproperty from the API response and simplifying the catch block to omit the unused error parameter are good cleanup steps that improve code clarity.src/commands/auth/keys/get.ts (4)
6-11: Improved command definition structure.Moving the command argument declaration to the top of the class improves readability and follows a consistent pattern for CLI command structure. This makes it easier to understand the command's interface at a glance.
52-55: Field reordering in JSON output improves consistency.Reordering the fields in the JSON output to have a consistent structure across error and success cases is a good practice. The consistent field ordering makes parsing results more predictable for consumers.
92-94: Improved loop pattern using for...of.Replacing
.forEach()with afor...ofloop is a good modernization that provides better readability and potentially better performance for iterating over capability entries.
107-111: Consistent field ordering in error output.Reordering fields in the error JSON output to include
appIdbeforeerrorandkeyIdcreates a more consistent structure that aligns with other response patterns in the codebase.examples/web-cli/src/App.tsx (4)
2-3: Clean import structure.Reordering React hook imports alphabetically improves code consistency and maintainability.
14-14: More explicit boolean conversion.Using
Boolean()instead of the double negation operator (!!) makes the intention clearer for readers and follows better TypeScript practices.
17-17: Consistent TypeScript union type ordering.The connection status types and the callback parameter types are now consistently ordered, which improves readability and maintainability.
Also applies to: 24-24
55-69: Consistent prop ordering in JSX.Reordering the props in the
AblyCliTerminalcomponent creates a consistent pattern that improves code readability and maintainability.src/commands/apps/switch.ts (4)
4-4: Added explicit import for type annotation.Adding the import for
ControlApito support the type annotation is a good practice that improves code clarity and ensures types are properly resolved.
7-12: Improved command arguments declaration position.Moving the static
argsproperty to the top of the class improves readability and follows a consistent pattern across the codebase for CLI command structures.
50-50: Enhanced type safety for method parameters.Changing the
switchToAppmethod's second parameter type fromanyto the specificControlApitype is a good improvement that enhances type safety and IDE support.
60-62: Simplified error handling.Removing the unused error parameter in the catch block simplifies the code while maintaining the same error handling behavior. This is a good cleanup step.
package.json (6)
13-13: Adding lint:fix script is a good practice.The addition of a dedicated lint:fix script makes it easier for developers to automatically fix eslint issues.
15-17: Good switch from npm to pnpm commands.The migration from npm to pnpm is a positive change. pnpm offers better dependency management with disk space efficiency and a more reliable dependency resolution.
42-42: Command not found hook changed from alias-command to did-you-mean.This looks like a feature change from using aliases to providing "did you mean" suggestions when a command is not found. This aligns with the PR objective of cleaning up linting issues and improving the user experience.
52-53: New dependencies for improved command suggestions.Added
@inquirer/promptsandfast-levenshteinpackages likely support the new "did-you-mean" feature by providing interactive prompts and string distance calculation for command suggestions.Also applies to: 63-63
76-99: ESLint configuration updated to modern standards.The extensive updates to ESLint-related dependencies reflect a modernization of the linting setup. The move to ESLint v9.x and addition of specialized plugins (
eslint-plugin-unicorn,eslint-plugin-n, etc.) aligns with current best practices.
113-116: Cleaned up file paths in package.json.Removed leading slashes from paths in the
filesarray for better consistency and to follow npm package conventions.src/commands/connections/logs.ts (2)
6-11: Improved code organization by moving args declaration above flags.Moving the static
argsdefinition aboveflagscreates a more consistent structure across command files. This ordering is more logical as arguments are typically processed before flags.
20-29: Standardized flag ordering and description formatting.Reordered flags to place
jsonbeforerewindand standardized the formatting of description properties. This improves consistency across the codebase.src/commands/accounts/list.ts (4)
2-3: Improved import ordering.Reorganized imports by grouping them logically, separating the chalk import from the command imports with an empty line for better readability.
27-31: Standardized JSON output structure for error case.Reordered properties in the JSON output for a more consistent structure:
accountsarray first, thenerrormessage, followed bysuccessstatus. This improves predictability of the API response.
41-55: Standardized JSON output structure for success case.Reordered account properties and overall response structure for consistency, placing the
accountsarray first, followed bycurrentAccountandsuccessstatus. This makes the API response more predictable and easier to parse.
61-61: Improved destructuring pattern consistency.Changed the destructuring order in the loop to match the order used in the JSON output mapping, enhancing code readability and maintainability.
src/commands/channels/logs.ts (2)
6-11: Improved code organization by moving args declaration above flags.Moving the static
argsdefinition aboveflagscreates a more consistent structure across command files. This ordering is more logical as arguments are typically processed before flags.
20-29: Standardized flag ordering and description formatting.Reordered flags to place
jsonbeforerewindand standardized the formatting of description properties. This improves consistency across the codebase.src/commands/channels/occupancy/get.ts (5)
1-1: Removed unused import for better code hygiene.Removed the unused
Flagsimport from@oclif/core, keeping only theArgsimport that's actually used. This reduces unnecessary imports and improves code clarity.
6-13: Improved interface property ordering.Reordered the properties in the
OccupancyMetricsinterface to group related fields together - presence-related fields first, then publisher/subscriber fields. This makes the interface more logically structured.
16-21: Added explicit args definition for better type safety.Added a static
argsproperty with explicit definition of the requiredchannelargument. This improves type safety, documentation, and command-line help output.
71-71: Improved type safety by specifying message type.Changed the message parameter type from the implicit
anyto the explicitAbly.Messagetype. This enhances type safety and prevents potential runtime errors.
86-90: Standardized JSON output structure.Reordered properties in JSON output for both success and error cases to follow a consistent pattern across all commands. In the success case,
metricsis followed bysuccess: true; in the error case,channelis included before the error message and success status.Also applies to: 114-118
src/commands/auth/issue-ably-token.ts (6)
3-5: Import organization improved.The imports are now better organized by grouping system imports first, followed by local imports with a clear separation between them.
30-30: Flag description formatting standardized.The capability flag description has been reformatted for consistency with other flags in the codebase.
35-43: Improved flag organization.Flags have been reordered and reformatted for better consistency, with output-related flags (
token-only) placed before operational flags (ttl).
55-55: Cleaner object destructuring.The destructuring is simplified to extract only the properties actually used in the code, which is a good practice.
87-87: Replaced substring with slice.Using
slice()instead ofsubstring()is preferred in modern JavaScript/TypeScript and is more consistent with other array operations.
104-104: Standardized JSON output structure.The capability output is now wrapped in a proper object with a named field, which improves the structure and readability of the JSON output.
Also applies to: 113-113
src/commands/apps/logs/history.ts (6)
1-5: Improved import organization.The imports have been reorganized with a renamed import variable (
_Flags) and a clear separation between external and internal imports.
19-31: Standardized flag definitions.The flag definitions have been reorganized with consistent property ordering:
defaultappears beforedescription- Property formatting is consistent across all flags
This improves code readability and maintenance.
46-47: Improved code spacing.Added a blank line after the conditional return for better visual separation of code blocks.
61-64: Consistent parameter ordering.The
historyParamsobject properties have been ordered consistently with other similar objects in the codebase.
72-72: Standardized JSON output structure.The messages array is now wrapped in a proper object with a named field, improving the structure and readability of the JSON output.
82-99: Replaced forEach with for...of loop.Changed from array
forEachmethod to afor...ofloop, which is generally more readable and allows for easier use ofawaitinside the loop if needed in the future.src/commands/help/web-cli.ts (5)
2-3: Updated import path.The import path for
CustomHelphas been updated to reflect the correct relative path, ensuring proper module resolution.
7-10: Improved class property organization.The static properties have been reordered for better readability, and
isAlias = truewas added to properly identify this as an alias command, allowing arbitrary arguments.
13-13: Clearer property documentation.The comment for the
strict = falseproperty has been updated to better explain its purpose in relation to the custom help formatter.
20-21: Improved TypeScript annotation.Changed from
@ts-ignoreto@ts-expect-error, which is more specific and requires an actual error to be thrown, preventing it from hiding unrelated type errors.
24-24: Updated help display method.Changed from
showRootHelp()toshowHelp([]), which is the more appropriate method based on theCustomHelpclass implementation. This ensures the help output is properly formatted for the web CLI mode.src/commands/apps/update.ts (4)
6-11: Improved command structure with explicit args declaration.The
static argsdeclaration has been moved to the top of the class, making the app ID argument more explicit and visible. This improves code organization by placing arguments before flags and examples.
38-43: Standardized JSON error output format.The JSON output for errors has been restructured to include the
appIdfirst andsuccesslast, providing a more consistent API response structure across all commands.
71-84: Improved JSON output object structure.The JSON output for successful updates has been reorganized to:
- Place app details first, followed by operation status
- Order app properties consistently with other app-related commands
- Format dates as ISO strings directly in the object construction
- Follow a consistent pattern with
successandtimestampat the endThis improves API consistency and readability.
100-105: Standardized JSON error response format.The error JSON output now follows the same consistent structure as other commands with
appIdfirst andsuccesslast, maintaining a predictable response format throughout the CLI.src/commands/integrations/get.ts (5)
1-1: Improved module imports with explicit extensionsThe import statements now use explicit
.jsextensions, which aligns with ES modules best practices and helps bundlers and TypeScript correctly resolve the imports.Also applies to: 4-5
7-12: Better command argument structureMoving the
ruleIdargument definition to a static property at the top of the class improves code organization and makes the command interface more discoverable.
40-40: Consistent method naming with resolveAppIdChanged from
getAppId(flags)toresolveAppId(flags), providing a more descriptive name that better reflects the method's purpose of resolving an app ID from various sources.
50-50: Improved JSON output with structuredCloneUsing
structuredClone()ensures immutability of the rule object when formatting it for output, preventing unintended side effects.
52-60: Enhanced output formattingThe non-JSON output has been restructured for better readability and consistency:
- Added a green header line
- Improved field labeling
- Restructured and reordered source information
- Better formatting of the target property with proper indentation
This makes the output more user-friendly and consistent with other commands.
src/commands/auth/issue-jwt-token.ts (6)
3-5: Improved imports with Node.js built-in module specifierUsing the
node:cryptospecifier for a Node.js built-in module follows current best practices and makes the import source explicit.
7-14: Enhanced type safety in JwtPayload interfaceThe JwtPayload interface now has explicit typing for the
capabilityfield asRecord<string, string[]>, improving type safety and code predictability.
38-38: Improved flag descriptions and organizationFlag descriptions are now clearer, and the flags have been reordered in a more logical sequence, improving the command's documentation and developer experience.
Also applies to: 45-51
82-88: Logical reordering of JWT payload constructionThe JWT payload is now constructed in a more logical order, with expiration time first, followed by other fields. This improves code readability and maintenance.
103-103: Preferred string manipulation methodChanged from
.substring(0, 8)to.slice(0, 8)for truncating the UUID. While functionally equivalent in this case,slice()is generally preferred in modern JavaScript.
121-131: Improved JSON output field orderingThe JSON output fields have been reordered to place the most important information (appId, capability, clientId) first, followed by other token details, improving readability.
src/commands/auth/keys/create.ts (6)
8-18: Enhanced examples with template literalsThe command examples now use template literals instead of escaped single quotes, making them more readable and maintainable. Additionally, more usage scenarios have been added to better illustrate the command's capabilities.
26-33: Improved flag descriptions and logical orderingThe flags have been reordered to group related options together, and the capabilities description now includes a clearer example format that better illustrates expected input.
41-42: Proper variable declarationChanged from
let appIdtoconst appIdsince the variable is not being reassigned. This improves code quality by using the most restrictive declaration possible.
59-70: Simplified error handlingThe catch block no longer explicitly captures the error object since it's not used, which simplifies the code without affecting functionality.
107-110: Modern iteration with for...of loopChanged from
forEachto afor...ofloop for iterating over capability entries, which is more readable and generally performs better than the callback-based approach.
124-128: Consistent JSON error response formatError response JSON now includes
appIdbeforeerrorand placessuccess: falseat the end, providing a more consistent structure that aligns with other commands.src/chat-base-command.ts (2)
1-4: Improved imports and type safetyImports have been reorganized for better readability, and the explicit import of
BaseFlagsfrom './types/cli.js' enhances type safety throughout the module.
7-20: Simplified client creation with improved type safetyThe
createChatClientmethod has been streamlined by:
- Adding explicit typing with
flags: BaseFlagsparameter andPromise<ChatClient | null>return type- Removing redundant error handling that's now handled elsewhere
- Adding clearer comments explaining the method's purpose
- Improving code formatting with consistent spacing
This simplification makes the code more maintainable while ensuring proper type safety.
src/commands/auth/keys/switch.ts (5)
7-12: Improvement: Good standardization of argument declaration.Moving the
keyNameOrValueargument to a staticargsproperty follows standard OCLIF patterns and improves command structure and documentation.
4-4: Improvement: Better type safety with explicit import.Adding the explicit
ControlApiimport allows for proper typing in the codebase.
103-103: Improved type safety for private method.Replacing
anywith a properControlApitype improves type safety and code clarity.
78-81: Simplified error handling.Removing unused error parameters in catch blocks makes the code cleaner while preserving the error handling functionality.
Also applies to: 118-121, 136-138
89-92: Consistent property ordering.Reordering properties to place
appNamebeforekeyIdandkeyNameimproves consistency with other commands in the codebase.Also applies to: 129-132
src/commands/channels/history.ts (4)
9-14: Improvement: Standardized argument declaration.Moving the
channelargument definition to a static property at the top of the class follows the standard OCLIF pattern and improves command documentation.
29-47: Improved flag organization.Reordering flags to group related ones together (
cipherbeforedirection, etc.) and standardizing descriptions improves readability and consistency.
104-104: Enhanced JSON output structure.Wrapping the messages array in a descriptive object with a
messageskey is a better API design and makes the output more consistent with other commands.
114-134: Modern iteration pattern.Switching from
forEachto afor...ofloop withentries()provides cleaner access to both index and message, following modern JavaScript best practices.src/commands/apps/channel-rules/list.ts (4)
41-41: Improved method naming clarity.Renaming
getAppIdtoresolveAppIdbetter describes the method's purpose of resolving the app ID from various sources.
62-84: Standardized JSON output structure.Reordering JSON output properties for consistency across commands improves maintainability. Moving
successandtimestampto the end while keeping related properties grouped together makes the output more readable.
100-100: Improved code readability.Adding blank lines between conditional property logging blocks greatly improves readability and makes the code structure clearer.
Also applies to: 104-104, 108-108, 112-112, 116-116, 120-120, 124-124, 128-128, 132-132, 136-136
143-149: Consistent error response format.Reordering properties in the error JSON output to include
appIdbeforesuccess: falsemaintains consistency with successful responses.src/commands/help/ask.ts (3)
9-14: Improvement: Standardized argument declaration.Adding an explicit
argsstatic property with a clear description for the requiredquestionargument follows OCLIF best practices and improves command documentation.
41-55: Logical flow improvement for conversation context.Inverting the continuation logic to first check for existing context before attempting to continue a conversation improves code readability and logical flow.
94-96: Modern iteration pattern.Switching to a
for...ofloop withentries()provides cleaner access to both index and link data, following modern JavaScript best practices.src/commands/apps/channel-rules/create.ts (8)
4-5: Import organization looks good.The import statement for
ControlBaseCommandis now properly separated from other imports with a blank line, improving readability.
15-74: Improved flag organization and defaults.The flag organization has been standardized with:
appflag moved to a more logical position- Boolean flags given explicit defaults where appropriate
- Related flags grouped together
- Consistent description formatting
This makes the command interface more predictable and self-documenting.
84-87: App ID resolution logic improved.The app ID resolution now checks
flags.appfirst before falling back toresolveAppId, making the logic more explicit and consistent with other commands.
90-100: Error handling structure standardized.The error response JSON structure now follows a consistent pattern with
statusandsuccessfields in the same order as other commands.
103-117: Object property ordering improved.The
namespaceDataobject construction has been reordered to group related properties logically, improving readability and maintainability.
122-143: JSON output formatting standardized.The JSON output structure now follows a consistent format with:
appIdincluded for context- Properties arranged logically
- Consistent date formatting
successandtimestampat the endThis standardization improves the CLI's output consistency.
153-189: Clean spacing in conditional output sections.Added consistent blank lines between conditional output sections, improving readability of the console output code.
192-203: Error response enriched with context.Error responses now include
appIdfor better context, helping with debugging and error correlation.src/commands/integrations/delete.ts (6)
1-4: Import updates improve modularity.The changes improve modularity by:
- Using the Node.js built-in module prefix (
node:readline)- Adding
chalkfor better terminal output styling- Organizing imports with a blank line separator
These changes align with modern best practices for Node.js modules.
8-13: Standardized arguments declaration.Moving the positional argument definition to a static
argsproperty follows the oclif standard pattern and improves API clarity.
29-34: Enhanced flag declaration with short alias.The
forceflag now includes:
- A short alias
-ffor better CLI usability- An explicit default of
falsefor clarityThese changes improve the command's usability and documentation.
44-44: Improved app ID resolution method.Renamed method from
getAppIdtoresolveAppIdwhich better reflects its function and is consistent with other commands in the codebase.
73-77: Improved success feedback with colored output.The success message now:
- Uses green color highlighting for better visibility
- Displays in a multi-line format with detailed information
- Includes key context (ID, App ID, Rule Type, Source Type)
This provides more informative feedback to users.
89-93: Simplified confirmation prompt logic.The confirmation prompt now only accepts "y" (case-insensitive) without trailing spaces, making the confirmation check more precise and predictable.
src/commands/apps/channel-rules/update.ts (8)
1-5: Import organization improved.The imports are now properly organized with:
- Appropriate spacing between import groups
- Importing Args alongside Flags from @oclif/core
This improves code readability and organization.
7-12: Standardized arguments declaration.Moving the positional argument definition to a static
argsproperty follows the oclif standard pattern and improves API clarity.
24-84: Improved flag organization with consistent options.All boolean flags now consistently include
allowNo: true, which enables explicit negation (e.g.,--no-persisted). Flags are also reordered for better logical grouping, with app flag positioned early.This allows users more flexibility when specifying flag values.
94-97: App ID resolution logic improved.The app ID resolution now checks
flags.appfirst before falling back toresolveAppId, making the logic more explicit and consistent with other commands.
186-192: Enhanced error response with context.Error responses now include
appIdandruleIdfor better context, helping with debugging and error correlation.
203-224: JSON output formatting standardized.The JSON output structure now follows a consistent format with:
appIdincluded for context- Properties arranged logically
- Consistent date formatting for both created and modified timestamps
successandtimestampat the endThis standardization improves the CLI's output consistency.
234-269: Clean spacing in conditional output sections.Added consistent blank lines between conditional output sections, improving readability of the console output code.
276-281: Error response enriched with context.Error responses now include
appIdfor better context, helping with debugging and error correlation.src/commands/channels/list.ts (10)
5-6: Import organization improved.The import for
AblyBaseCommandis now properly separated from other imports with a blank line, improving readability.
7-13: Logical reordering of interface properties.The
ChannelMetricsinterface properties have been reordered to group related fields together, withpublishersandsubscribersappearing afterpresenceMembers.
26-30: Added type safety with interface definition.A new interface
ChannelListParamshas been created to properly type the channel listing request parameters, improving type safety and code clarity.
45-52: Improved flag definitions with character alias.The
prefixflag now includes a character alias-pfor improved CLI usability, and flag descriptions have been standardized.
67-68: Improved type safety for params object.The params object is now explicitly typed with the new
ChannelListParamsinterface, enhancing type safety and code clarity.
76-76: Updated REST request parameters.The REST request now uses a more explicit parameter structure, which is clearer and more maintainable.
92-96: Standardized JSON output property ordering.The JSON output properties have been reordered to follow a consistent pattern, with:
hasMorebefore success fieldssuccessandtimestampgrouped togethertotalat the endThis improves consistency across all commands.
105-125: Improved iteration with for...of loop.Changed from
forEachto afor...ofloop with explicit typing, which is:
- More idiomatic and readable
- Allows for easier debugging
- Provides better type safety through explicit casting
This is a good modernization of the code.
110-110: Improved object destructuring syntax.Updated to use cleaner destructuring syntax when extracting
metricsfromchannel.status.occupancy, making the code more concise.
135-137: Standardized error response structure.The error response JSON structure now follows a consistent pattern with
statusandsuccessfields in a standard order.src/commands/apps/channel-rules/delete.ts (6)
1-5: Improved imports with Node.js built-in module specifier.Good update to use
node:readlineinstead of justreadline. This follows the Node.js best practice of using thenode:prefix for built-in modules, which makes dependencies more explicit and prevents potential conflicts with similarly named modules in node_modules.
8-13: Good addition of explicit args declaration.Defining static
argsproperty with proper typing and description improves command structure and documentation. This makes the command interface more self-documenting and consistent with other commands.
31-35: Properties reordering for consistency.The reordering of flag properties (moving
defaultbeforedescription) doesn't change functionality but improves consistency with other command definitions across the codebase.
46-49: Improved app ID resolution logic.The modified app ID resolution now explicitly checks
flags.appfirst before callingresolveAppId(). This change aligns with other commands' implementation patterns and provides more predictable behavior.
91-130: Enhanced conditional property display in confirmation prompts.The expanded confirmation prompt now displays detailed properties of the namespace conditionally, improving user feedback. This gives users more context about what they're about to delete.
138-143: Standardized JSON output structure.JSON output objects have been consistently reordered to place
successlast and include contextual properties likeappIdandruleId. This improves consistency across the CLI and makes parsing outputs more predictable.Also applies to: 156-162, 169-173
eslint.config.js (7)
1-11: Well-structured imports for ESLint flat config.Good job importing the necessary ESLint plugins, parsers, and configurations. The commented-out imports show thoughtful refactoring process as unnecessary imports were removed.
14-32: Comprehensive ignore patterns.The ignore patterns are well organized and cover all necessary build artifacts, dependencies, and temporary directories. This matches and enhances the patterns from the previous
.eslintignorefile, centralizing configuration in one place.
33-69: Strong base configuration for all JavaScript/TypeScript files.The base configuration correctly sets up ECMAScript version, module source type, and Node.js globals. It appropriately extends recommended rules from ESLint core, Node.js, and Unicorn plugins while disabling overly strict or stylistic rules that could impede development.
70-93: Well-tailored TypeScript-specific configuration.The TypeScript configuration correctly sets up the parser with project references and applies TypeScript-specific rules. The warning level for explicit
anyand unused variables with appropriate ignore patterns strikes a good balance between strictness and practicality.
94-105: Environment-specific config for React Web CLI package.Good configuration for the React Web CLI package that correctly sets browser globals and disables the
prefer-modulerule which would be inappropriate for browser-targeted code.
106-131: Comprehensive test configuration.The test-specific configuration correctly sets up Mocha globals and rules, disables TypeScript rules that would interfere with testing patterns, and adds important safeguards against exclusive or skipped tests.
132-134: Proper Prettier integration.Placing the Prettier config last ensures it can override any conflicting rules from other configurations, preventing formatting conflicts between ESLint and Prettier.
src/commands/apps/delete.ts (6)
2-3: Updated to use Node.js built-in module specifier.Good update to use
node:readlineinstead of justreadline. This follows Node.js best practices and ensures proper module resolution.
8-13: Explicit argument declaration improves command interface.Adding a static
argsproperty with proper typing and description for the optional app ID argument improves the command structure and documentation. The description clearly indicates that the current app is used if not specified.
29-32: Reordered flag properties for consistency.The reordering of the
forceflag properties (movingdefaultbeforedescription) improves consistency with other commands without changing functionality.
46-56: Standardized JSON error output format.The JSON error output has been reordered to place
successlast, which is consistent with the pattern applied across other commands. This improves API consistency for programmatic consumers of the CLI.
152-164: Improved app name confirmation method.The
promptForAppNamemethod now requires exact case-sensitive matching of the app name, which is a good security practice for destructive operations like deletion. This ensures users are fully aware of which app they're deleting.
166-178: Enhanced confirmation prompt with better user experience.The
promptForConfirmationmethod now accepts both 'y' and 'yes' case-insensitively, making the CLI more user-friendly while maintaining proper safeguards for destructive operations.src/commands/accounts/switch.ts (6)
7-12: Better argument declaration structure.Moving the static
argsproperty above flags and usingoverridemodifier properly follows OOP principles and makes the command structure more consistent with other commands in the codebase.
37-45: Standardized JSON error output format.The JSON error output has been reordered to place
errorbeforesuccess, which is consistent with the pattern applied across other commands. The additional whitespace after the if-block improves readability.
78-82: Improved method signature with explicit typing.The enhanced type signatures for the
switchToAccountmethod parameters improve type safety and code readability. Properly typing the accounts array structure and flags object will help catch potential errors during development.
127-127: Added explicit type casting for controlHost.Adding the explicit type cast
as string | undefinedfor the controlHost parameter ensures type safety when interacting with the ControlApi service.
130-130: Reordered destructuring for better readability.Reordering the destructuring of
controlApi.getMe()response to{ account, user }improves readability by placing the more frequently used object first.
148-159: Improved error handling with better user guidance.Simplifying the catch block by removing the unused error variable and adding specific advice to log in again with the alias provides better user guidance. The warning message is well-crafted to help users resolve authentication issues.
src/commands/help/index.ts (6)
1-5: Good imports restructuring.Added relevant imports for
Config,stripAnsi, andConfigManagerto support the new functionality in the class.
21-23: Good implementation of class properties.Clear declaration of protected properties with appropriate types that will store state needed by the class.
25-29: Well-structured constructor implementation.The constructor properly initializes the class properties, including reading the environment variable and creating a new ConfigManager instance.
31-33: Improved async/await pattern.Simplified the run method to properly await the parse call without destructuring flags.
54-59: Nice addition of environment-aware output formatting.The new
formatHelpOutputmethod improves the CLI's flexibility by stripping ANSI codes when generating README documentation.
61-88: Improved command filtering and display logic.The refactored
displayAllCommandsmethod includes several improvements:
- Better command filtering logic with clear conditionals
- Robust error handling for command loading with try/catch
- More efficient iteration using for...of loop
- Consistent command formatting for better readability
src/commands/auth/keys/update.ts (6)
6-11: Good addition of explicit argument definition.Adding a static
argsproperty with a properly definedkeyNameargument improves command-line argument handling and documentation.
31-34: Flag re-organization looks good.Moved the
nameflag definition aftercapabilitiesfor better organization.
69-69: Improved type safety with explicit interface.Replaced
anytype with a properly typed object structure forupdateData, enhancing type safety.
76-87: Enhanced capability parsing with proper error handling.The capabilities parsing has been improved with:
- Clear comments explaining the expected format
- Proper error handling with try/catch
- Better structured capability object creation
102-103: Type-safe function calls.Added explicit type casting when calling
formatCapabilityto ensure type safety.
111-123: Well-structured helper method.The
formatCapabilitymethod has been:
- Relocated to the end of the class for better organization
- Improved with proper parameter typing
- Enhanced with clearer implementation logic
src/commands/accounts/login.ts (5)
10-33: Good extraction of validation logic.Moving the
validateAndGetAliasfunction outside the class improves modularity and separation of concerns. The validation logic is well-structured with clear error messages.
36-41: Good addition of explicit argument definition.Adding a static
argsproperty with a properly defined optionaltokenargument improves command-line argument handling and documentation.
82-82: Simplified token retrieval URL logic.The token retrieval URL construction has been streamlined into a single, more readable line.
211-225: Improved function binding for validation.Good use of binding
this.logto ensure proper context when passing the log function to the external validator.
237-249: Added missing prompt method implementation.Properly implemented the previously missing or commented
promptForTokenmethod with clean readline interface handling.src/commands/integrations/create.ts (6)
6-16: Good addition of type interface for rule data.Creating a dedicated
RuleDatainterface improves type safety and provides clear documentation of the expected data structure. This interface matches the one from the Control API service.
37-51: Improved flag definitions with sensible defaults.Enhanced flag definitions with:
- Sensible default for
request-modeset to 'single'- Better organization of related flags
- Clearer descriptions and required/optional status
53-57: Good default for status flag.Added a sensible default of 'enabled' for the status flag, improving user experience.
80-89: Improved type safety in rule data construction.The rule data construction now uses the defined
RuleDatainterface and properly types all properties, improving code reliability.
93-127: Enhanced rule type handling with block scoping.The switch statement for different rule types has been improved with:
- Block scoping for case statements
- Better validation for required fields
- Clearer property organization
- Helpful comments explaining the purpose
132-142: Improved output formatting with color.Enhanced output formatting with:
- Added color to success messages using chalk
- Better structured JSON output
- More comprehensive information display
- Clearer formatting of complex data
src/commands/auth/revoke-token.ts (1)
70-77: Using the token itself as aclientIdis risky & ambiguous
When the--client-idflag isn’t provided the code silently falls back to the raw token string.
- A token may contain characters that are invalid in a client‑id context.
- Revoking an entire client‑id can unintentionally invalidate all tokens for that client rather than the single token supplied.
Consider aborting with an error (or at least requiring an explicit
--client-idoverride) instead of defaulting to a potentially dangerous assumption.Would you like a patch that enforces an explicit
--client-idflag when the user attempts per‑client revocation?src/commands/channels/batch-publish.ts (1)
228-241: Potential failure when--specis an arrayWhen
flags.specis an array of batch specs,batchContentObj.channelsandbatchContentObj.messageswill beundefined, leading to"channels": [undefined]in the JSON output.Guard for the array case:
-const batchContentObj = batchContent as Record<string, unknown> +const batchContentObj = Array.isArray(batchContent) + ? ({} as Record<string, unknown>) // will be handled separately + : (batchContent as Record<string, unknown>)and, before logging, either loop over the array or emit a clearer error.
README.md (2)
18-18: TOC Extended: MCP Server
The Table of Contents correctly includes the new MCP Server entry, improving discoverability.
49-49: Link to MCP Server section
The new reference to the MCP Server section ensures users know where to find details on the experimental feature. This aligns nicely with the TOC update.
| *Note: The `topicSeparator` configured for oclif is not a `:` but instead a space (` `). All commands must respect this configuration* | ||
|
|
There was a problem hiding this comment.
Fix Markdown linting issue with code span formatting.
The backtick code span contains spaces which is triggering a Markdown linting error (MD038, no-space-in-code).
-*Note: The `topicSeparator` configured for oclif is not a `:` but instead a space (` `). All commands must respect this configuration*
+*Note: The `topicSeparator` configured for oclif is not a `:` but instead a space (`␣`). All commands must respect this configuration*Alternatively, you could use something like "space character" outside the code span to avoid the linting issue.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| *Note: The `topicSeparator` configured for oclif is not a `:` but instead a space (` `). All commands must respect this configuration* | |
| *Note: The `topicSeparator` configured for oclif is not a `:` but instead a space (`␣`). All commands must respect this configuration* |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
46-46: Spaces inside code span elements
null
(MD038, no-space-in-code)
| const processedWithCodeBlocks = response.answer.replaceAll( | ||
| /```(?:javascript|js|html)?\n([\S\s]*?)```/g, | ||
| (_, codeContent) => |
There was a problem hiding this comment.
Fix: Global replacement of markdown patterns.
Switching from replace to replaceAll for regex replacements ensures all occurrences of patterns are replaced, not just the first match. This is critical for correct markdown processing.
Also applies to: 79-86
| static override args = { | ||
| channels: Args.string({ | ||
| description: 'Channel name(s) to subscribe to', | ||
| multiple: false, | ||
| required: true, | ||
| }), | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
multiple: false contradicts example usage with multiple channels
Examples show ably channels subscribe my-channel another-channel, yet the arg definition explicitly disables multiples.
Change to multiple: true (and rename to channel perhaps) or rely solely on strict = false with argv, but be consistent to avoid confusing users & typings.
| await new Promise<void>((resolve, reject) => { | ||
| const connectionTimeout = setTimeout(() => { | ||
| const timeoutError = new Error('Connection timeout after 10 seconds'); | ||
| this.logCliEvent(flags, 'connectionTest', 'wsTimeout', timeoutError.message, { error: timeoutError.message }); | ||
| reject(timeoutError); | ||
| }, 10_000); | ||
|
|
||
| client.connection.once('connected', () => { | ||
| clearTimeout(connectionTimeout); | ||
| success = true; | ||
| this.logCliEvent(flags, 'connectionTest', 'wsSuccess', 'WebSocket connection successful', { connectionId: client.connection.id }); | ||
| if (!this.shouldOutputJson(flags)) { | ||
| this.log(`${chalk.green('✓')} WebSocket connection successful`); | ||
| this.log(` Connection ID: ${chalk.cyan(client.connection.id || 'unknown')}`); | ||
| } | ||
|
|
||
| resolve(); | ||
| }); | ||
|
|
||
| client.connection.once('failed', (stateChange) => { | ||
| clearTimeout(connectionTimeout); | ||
| errorResult = stateChange.reason || new Error('Connection failed'); | ||
| this.logCliEvent(flags, 'connectionTest', 'wsFailed', `WebSocket connection failed: ${errorResult.message}`, { error: errorResult.message, reason: stateChange.reason }); | ||
| if (!this.shouldOutputJson(flags)) { | ||
| this.log(`${chalk.red('✗')} WebSocket connection failed: ${errorResult.message}`); | ||
| } | ||
|
|
||
| resolve(); // Resolve even on failure to allow summary | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential multiple promise settlements
connection.once('failed', …) resolves the wrapper promise (via resolve()) after setting errorResult, but reject() has already been called if the timeout fired first. Although once handlers mitigate most races, there is still a small window where both callbacks fire, producing an UnhandledPromiseRejection.
A safer pattern is to store settled = false and early‑return in every handler if already settled, or remove the other listener once one fires.
| ...baseOptions, | ||
| transportParams: { | ||
| disallowXHR: true, | ||
| preferWebSockets: true | ||
| } | ||
| } | ||
| this.wsClient = new Ably.Realtime(wsOptions); | ||
| const client = this.wsClient; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Non‑standard Ably options – verify they are actually honoured
transportParams: { disallowXHR, preferWebSockets } is not part of the public Ably JS client options (it expects transports or close_on_unusable_transport). Unless you are on a fork, these flags will be ignored and the test may not exercise the intended transport.
Run a quick grep to see whether we patch Ably elsewhere:
If nothing appears, switch to the documented form:
const wsOptions = {
...baseOptions,
transports: ['web_socket'],
}🏁 Script executed:
#!/bin/bash
fd --extension js ably | xargs rg "disallowXHR|transportParams"Length of output: 247
Use documented Ably transport options
The transportParams object (with disallowXHR and preferWebSockets) isn’t part of the public Ably JS client API, so those flags will be ignored and your test won’t actually force WebSockets. Please replace it with the officially supported transports option:
• File: src/commands/connections/test.ts
• Lines 140–147
Suggested change:
- const wsOptions = {
- ...baseOptions,
- transportParams: {
- disallowXHR: true,
- preferWebSockets: true
- }
- }
+ const wsOptions = {
+ ...baseOptions,
+ transports: ['web_socket'],
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...baseOptions, | |
| transportParams: { | |
| disallowXHR: true, | |
| preferWebSockets: true | |
| } | |
| } | |
| this.wsClient = new Ably.Realtime(wsOptions); | |
| const client = this.wsClient; | |
| const wsOptions = { | |
| ...baseOptions, | |
| transports: ['web_socket'], | |
| } | |
| this.wsClient = new Ably.Realtime(wsOptions); | |
| const client = this.wsClient; |
| // Parse the message | ||
| let messageData: any | ||
| let messageData: MessageData | ||
| try { | ||
| messageData = JSON.parse(args.message) | ||
| } catch (error) { | ||
| } catch { | ||
| // If parsing fails, use the raw message as data | ||
| messageData = { data: args.message } | ||
| } |
There was a problem hiding this comment.
Unsafe access when parsed JSON is not an object
JSON.parse(args.message) can legally return a primitive ("hello", 42, null, …).
If that happens, the subsequent messageData.name and messageData.data property look‑ups will throw.
- messageData = JSON.parse(args.message)
+ const parsed = JSON.parse(args.message)
+ messageData =
+ parsed !== null && typeof parsed === 'object'
+ ? (parsed as MessageData)
+ : ({ data: parsed } as MessageData)This preserves the current fallback behaviour while guaranteeing messageData is an object.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Parse the message | |
| let messageData: any | |
| let messageData: MessageData | |
| try { | |
| messageData = JSON.parse(args.message) | |
| } catch (error) { | |
| } catch { | |
| // If parsing fails, use the raw message as data | |
| messageData = { data: args.message } | |
| } | |
| // Parse the message | |
| let messageData: MessageData | |
| try { | |
| const parsed = JSON.parse(args.message) | |
| messageData = | |
| parsed !== null && typeof parsed === 'object' | |
| ? (parsed as MessageData) | |
| : ({ data: parsed } as MessageData) | |
| } catch { | |
| // If parsing fails, use the raw message as data | |
| messageData = { data: args.message } | |
| } |
| private async setupCleanupHandler(channel: Ably.RealtimeChannel, client: Ably.Realtime, flags: Record<string, unknown>): Promise<void> { | ||
| return new Promise<void>((resolve, reject) => { | ||
| let cleanupInProgress = false; | ||
|
|
||
| this.logCliEvent(flags, 'presence', 'cleanupInitiated', 'Cleanup initiated (Ctrl+C pressed)'); | ||
| const cleanup = async (): Promise<void> => { | ||
| if (cleanupInProgress) return; | ||
| cleanupInProgress = true; | ||
|
|
||
| this.logCliEvent(flags, 'presence', 'cleanupInitiated', 'Cleanup initiated (Signal received)'); | ||
| if (!this.shouldOutputJson(flags)) { | ||
| this.log('\nLeaving presence and closing connection...'); | ||
| } | ||
|
|
||
| // Set a force exit timeout | ||
| const forceExitTimeout = setTimeout(() => { | ||
| this.logCliEvent(flags, 'presence', 'forceExit', 'Force exiting after timeout during cleanup'); | ||
| if (!this.shouldOutputJson(flags)) { | ||
| this.log(chalk.red('Force exiting after timeout...')); | ||
| } | ||
| process.exit(1) | ||
| }, 5000) | ||
|
|
||
| try { | ||
| reject(new Error('Cleanup timed out')); // Reject promise on timeout | ||
| }, 5000); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Possible double‑settlement of the promise in setupCleanupHandler
reject(new Error('Cleanup timed out')) (line 153) can fire before the asynchronous cleanup finishes; the subsequent finally always calls resolve().
If the timeout expires first the promise is rejected then resolved, leading to the Node warning:
(node:xxxx) Promise has already been settled
Consider resolving the race:
- reject(new Error('Cleanup timed out')); // Reject promise on timeout
+ reject(new Error('Cleanup timed out')); // Reject promise on timeout
+ // Ensure we don’t call resolve() afterwards
+ cleanupInProgress = true;or switch to Promise.race with an outer timeout.
Committable suggestion skipped: line range outside the PR's diff.
| function isValidToken(token: string): boolean { | ||
| log(`Placeholder token validation for: ${token.slice(0, 10)}...`); | ||
| // Replace with actual token validation logic! | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
Replace placeholder token validation with real auth logic
isValidToken() currently returns true for every token, which effectively disables authentication and lets anyone spin‑up costly Docker sessions on the host.
function isValidToken(token: string): boolean {
// Replace with actual token validation logic!
return true;
}At minimum, verify signature / expiry of the Ably token and enforce TLS‑only connections before calling it valid. Consider delegating the check to Ably’s Auth#requestToken or jwt.verify.
- return true;
+ try {
+ // Pseudo‑code – adapt to the auth scheme you use
+ const {validUntil} = jwt.verify(token, process.env.ABLY_JWT_SECRET);
+ return validUntil > Date.now();
+ } catch {
+ return false;
+ }
src/commands/bench/subscriber.ts
Outdated
| private addLogToBuffer(logMessage: string): void { | ||
| if (this.shouldOutputJson({})) return; // Don't buffer in JSON mode | ||
| this.messageLogBuffer.push(`[${new Date().toLocaleTimeString()}] ${logMessage}`); | ||
| if (this.messageLogBuffer.length > this.MAX_LOG_LINES) { | ||
| this.messageLogBuffer.shift(); // Remove the oldest log | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
shouldOutputJson({}) always returns false – pass real flags
Both addLogToBuffer() and updateStatusAndLogs() call this.shouldOutputJson({}), so JSON mode is silently ignored and logs flood the console when --json is used.
-private addLogToBuffer(logMessage: string): void {
- if (this.shouldOutputJson({})) return; // Don't buffer in JSON mode
+private addLogToBuffer(logMessage: string, flags: Record<string, unknown>): void {
+ if (this.shouldOutputJson(flags)) return;
…
}
-// call‑site example
-this.addLogToBuffer(logMsg);
+this.addLogToBuffer(logMsg, flags);Do the same for updateStatusAndLogs() and any other helper that needs to respect the output mode.
Failing to do so breaks scripted usage that relies on machine‑readable JSON.
Also applies to: 458-467
| import {randomUUID} from 'node:crypto' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Node 14 compatibility risk due to randomUUID usage
randomUUID() is only available from Node 16+. Your published package.json currently allows users on older LTS versions (e.g., 14) where this call will crash at start‑up.
Consider falling back to crypto.randomBytes when randomUUID is undefined, or bumping the declared Node engine requirement.
-import {randomUUID} from 'node:crypto'
+import {randomUUID as _uuid, randomBytes} from 'node:crypto'
+
+// Fallback for Node < 16
+const randomUUID = _uuid ?? (() => randomBytes(16).toString('hex'))
Extensive typescript lint fixes. Initial design violated a lot of linting rules (1k+). This PR resolves these issues so that we can get CI builds running with the CLI.
Before introducing automated builds and test suites, we need to ensure all linting is passing. This PR reduces the lint issues from 1k+ to 0.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
Documentation
Style
Tests