Add channels:inspect command to open dashboard#136
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 Use the checkbox below for a quick retry:
WalkthroughA new CLI command Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Cmd as ChannelsInspect
participant Config as Config Manager
participant Browser as Browser/Dashboard
User->>Cmd: channels:inspect [channel-name] --app [app-id]
Cmd->>Config: Get current account
alt Account missing
Cmd-->>User: Error: No account configured
else Account exists
Cmd->>Config: Get app (from --app flag or current)
alt App missing
Cmd-->>User: Error: No app selected
else App exists
Cmd->>Cmd: Build dashboard URL<br/>(account, app, channel)
alt Web CLI mode
Cmd-->>User: Display: Visit [URL]
else Normal mode
Cmd->>Browser: Open URL
Browser-->>User: Dashboard opened
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Opens the Ably dashboard channel page in the browser for a given channel name. Constructs the URL from the configured account ID and app ID, with URL-encoding for special characters in channel names. Supports --app flag to override the current app, web CLI mode (prints URL instead of opening browser), and validates that both account and app are configured before proceeding. This is change obviously falls short of a full channel inspector experience in the CLI, but at least the functionality is exposed to cli users, and provides a quick jump to the dashboard inspectors.
zknill
force-pushed
the
zak/channel-inspect-command
branch
from
cb427ae to
0b69485
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/unit/commands/channels/inspect.test.ts (1)
23-53: MockopenUrlto avoid triggering real browser-open syscalls and decouple assertions fromopenUrl's output format.The tests in this file execute the command in-process via
runCommand(), which calls the realopenUrl()utility. This causes actual browser-launch syscalls to fire in CI and couples the test assertions toopenUrl's internal print format — if that format ever changes, these tests fail even though the command logic is correct.Instead, mock
openUrland assert it was called with the expected URL:♻️ Suggested refactor
+import { vi } from "vitest"; + +vi.mock("../../../src/utils/open-url.js", () => ({ + default: vi.fn().mockResolvedValue(undefined), +})); + +import openUrl from "../../../src/utils/open-url.js"; it("should open browser with correct dashboard URL", async () => { const mockConfig = getMockConfigManager(); const accountId = mockConfig.getCurrentAccount()!.accountId!; const appId = mockConfig.getCurrentAppId()!; - const { stdout } = await runCommand( + await runCommand( ["channels:inspect", "my-channel"], import.meta.url, ); - expect(stdout).toContain("Opening"); - expect(stdout).toContain("in your browser"); - expect(stdout).toContain( - `https://ably.com/accounts/${accountId}/apps/${appId}/channels/my-channel`, - ); + expect(openUrl).toHaveBeenCalledWith( + `https://ably.com/accounts/${accountId}/apps/${appId}/channels/my-channel`, + expect.anything(), + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/commands/channels/inspect.test.ts` around lines 23 - 53, The tests currently call runCommand(...) which triggers the real openUrl() and causes browser syscalls and brittle output assertions; update the tests in inspect.test.ts to mock/stub the openUrl function (the openUrl export used by the channels:inspect command) before calling runCommand, assert openUrl was invoked once with the exact dashboard URL (including percent-encoding for "my-channel/foo#bar"), and remove or replace assertions that check for "Opening"/"in your browser" or rely on openUrl's stdout format; ensure you reference the openUrl symbol when creating the mock and keep other uses of runCommand the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/channels/inspect.ts`:
- Line 46: Replace the hardcoded dashboard base in the URL construction with a
configurable dashboardHost sourced from a new global flag or environment
variable; specifically, add a `dashboard-host` global flag to
AblyBaseCommand.globalFlags (or read process.env.ABLY_DASHBOARD_URL as a
fallback), then build the URL using that dashboardHost when forming the const
url that currently uses `https://ably.com` (keeping the rest: accountId, appId,
and encodeURIComponent(args.channel)); ensure the code uses
flags["dashboard-host"] || process.env.ABLY_DASHBOARD_URL || 'https://ably.com'
as the final host so staging/enterprise overrides work.
---
Nitpick comments:
In `@test/unit/commands/channels/inspect.test.ts`:
- Around line 23-53: The tests currently call runCommand(...) which triggers the
real openUrl() and causes browser syscalls and brittle output assertions; update
the tests in inspect.test.ts to mock/stub the openUrl function (the openUrl
export used by the channels:inspect command) before calling runCommand, assert
openUrl was invoked once with the exact dashboard URL (including
percent-encoding for "my-channel/foo#bar"), and remove or replace assertions
that check for "Opening"/"in your browser" or rely on openUrl's stdout format;
ensure you reference the openUrl symbol when creating the mock and keep other
uses of runCommand the same.
src/commands/channels/inspect.ts
Outdated
| ); | ||
| } | ||
|
|
||
| const url = `https://ably.com/accounts/${accountId}/apps/${appId}/channels/${encodeURIComponent(args.channel)}`; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Search for controlHost usage and other dashboard URL patterns
rg -n "ably\.com|dashboardHost|controlHost" src/commands/ --type ts -A 2 -B 2 | head -100Repository: ably/ably-cli
Length of output: 3771
🏁 Script executed:
# Check the base command and config for any dashboard host configuration
fd -e ts -e js src/ | xargs grep -l "dashboardHost\|dashboard.*url" 2>/dev/null | head -20Repository: ably/ably-cli
Length of output: 378
🏁 Script executed:
# Look at the base-command.ts to see what host configs are exposed
cat src/base-command.ts | head -150Repository: ably/ably-cli
Length of output: 5269
🏁 Script executed:
# Check if there are other commands that construct URLs to ably.com
rg "https://ably\.com" src/commands/ --type ts -B 5 -A 2Repository: ably/ably-cli
Length of output: 2654
Hardcoded dashboard base URL violates the "no hardcoded endpoint URLs" guideline.
Line 46 hardcodes https://ably.com as the dashboard base. This breaks for staging/enterprise environments and is not configurable. A dashboardHost flag or env-override (e.g., ABLY_DASHBOARD_URL) should supply the base, consistent with how control-host is used elsewhere in the codebase.
♻️ Suggested refactor
- const url = `https://ably.com/accounts/${accountId}/apps/${appId}/channels/${encodeURIComponent(args.channel)}`;
+ const dashboardHost = flags["dashboard-host"] ?? process.env.ABLY_DASHBOARD_URL ?? "https://ably.com";
+ const url = `${dashboardHost}/accounts/${accountId}/apps/${appId}/channels/${encodeURIComponent(args.channel)}`;Alternatively, add a dashboard-host global flag to AblyBaseCommand.globalFlags (similar to control-host) and use flags["dashboard-host"] here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/channels/inspect.ts` at line 46, Replace the hardcoded dashboard
base in the URL construction with a configurable dashboardHost sourced from a
new global flag or environment variable; specifically, add a `dashboard-host`
global flag to AblyBaseCommand.globalFlags (or read
process.env.ABLY_DASHBOARD_URL as a fallback), then build the URL using that
dashboardHost when forming the const url that currently uses `https://ably.com`
(keeping the rest: accountId, appId, and encodeURIComponent(args.channel));
ensure the code uses flags["dashboard-host"] || process.env.ABLY_DASHBOARD_URL
|| 'https://ably.com' as the final host so staging/enterprise overrides work.
src/commands/channels/inspect.ts
Outdated
|
|
||
| const url = `https://ably.com/accounts/${accountId}/apps/${appId}/channels/${encodeURIComponent(args.channel)}`; | ||
|
|
||
| if (this.isWebCliMode) { |
There was a problem hiding this comment.
Rather than if here, should we add the condition to openUrl?
src/commands/channels/inspect.ts
Outdated
| ); | ||
| } | ||
|
|
||
| const url = `https://ably.com/accounts/${accountId}/apps/${appId}/channels/${encodeURIComponent(args.channel)}`; |
There was a problem hiding this comment.
Ditto coderabbit, this needs to work on the staging sites so should be configurable. It'll also need to be added to the terminal server, because that's where the web CLI will be running
…penUrl Add a --dashboard-host global flag (env: ABLY_DASHBOARD_HOST) so the channels:inspect command can target staging or enterprise dashboard environments instead of the hardcoded https://ably.com. Move the web CLI mode check ("Visit <url>" instead of opening a browser) from individual commands into the openUrl utility so all callers get consistent behavior without duplicating the logic.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
When the --dashboard-host flag or ABLY_DASHBOARD_HOST env var is provided without a URL scheme, automatically prepend https:// so that the constructed dashboard URL is valid.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| this.error( | ||
| `No app selected. Please select an app first with ${chalk.cyan('"ably apps switch"')} or specify one with ${chalk.cyan("--app")}.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Web CLI mode may lack account config
Medium Severity
In web CLI mode, channels:inspect still requires this.configManager.getCurrentAccount() and getCurrentAppId() to be configured, and errors out if they aren’t. Since web CLI mode typically relies on environment-provided context rather than local config, this can make the new command unusable in the intended environment even though openUrl supports web CLI output.
2 participants
Opens the Ably dashboard channel page in the browser for a given channel name. Constructs the URL from the configured account ID and app ID, with URL-encoding for special characters in channel names. Supports --app flag to override the current app, web CLI mode (prints URL instead of opening browser), and validates that both account and app are configured before proceeding.
This is change obviously falls short of a full channel inspector experience in the CLI, but at least the functionality is exposed to cli users, and provides a quick jump to the dashboard inspectors.
Summary by CodeRabbit
New Features
channels:inspectCLI command for quickly accessing and reviewing channel details directly within the Ably dashboard. Supports optional app selection via the--appflag, which defaults to your current app selection. Automatically handles special characters in channel names.Note
Low Risk
Primarily adds a new command and a small behavior change in
openUrlfor web-CLI mode; minimal impact beyond link-opening flows.Overview
Adds a new
channels:inspectcommand that opens the Ably dashboard channel page for a given channel, using the configured account/app (with--appoverride) and URL-encoding the channel name.Introduces a hidden global
--dashboard-host/ABLY_DASHBOARD_HOSToverride for constructing dashboard links, and updatesopenUrlso web CLI mode printsVisit <url>instead of attempting to open a browser. Includes unit tests covering URL construction, overrides, and missing account/app errors.Written by Cursor Bugbot for commit f932b47. This will update automatically on new commits. Configure here.