Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/commands/channels/inspect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Args, Flags } from "@oclif/core";
import chalk from "chalk";

import { AblyBaseCommand } from "../../base-command.js";
import openUrl from "../../utils/open-url.js";

export default class ChannelsInspect extends AblyBaseCommand {
static override args = {
channel: Args.string({
description: "The name of the channel to inspect in the Ably dashboard",
required: true,
}),
};

static override description =
"Open the Ably dashboard to inspect a specific channel";

static override examples = ["<%= config.bin %> <%= command.id %> my-channel"];

static override flags = {
...AblyBaseCommand.globalFlags,
app: Flags.string({
description: "App ID to use (uses current app if not specified)",
env: "ABLY_APP_ID",
}),
};

async run(): Promise<void> {
const { args, flags } = await this.parse(ChannelsInspect);

const currentAccount = this.configManager.getCurrentAccount();
const accountId = currentAccount?.accountId;
if (!accountId) {
this.error(
`No account configured. Please log in first with ${chalk.cyan('"ably accounts login"')}.`,
);
}

const appId = flags.app ?? this.configManager.getCurrentAppId();
if (!appId) {
this.error(
`No app selected. Please select an app first with ${chalk.cyan('"ably apps switch"')} or specify one with ${chalk.cyan("--app")}.`,
);
}

const url = `https://ably.com/accounts/${accountId}/apps/${appId}/channels/${encodeURIComponent(args.channel)}`;
Copy link

Choose a reason for hiding this comment

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

🛠️ 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 -100

Repository: 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 -20

Repository: 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 -150

Repository: 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 2

Repository: 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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


if (this.isWebCliMode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than if here, should we add the condition to openUrl?

this.log(`${chalk.cyan("Visit")} ${url}`);
} else {
await openUrl(url, this);
}
}
}
150 changes: 150 additions & 0 deletions test/unit/commands/channels/inspect.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { runCommand } from "@oclif/test";
import { getMockConfigManager } from "../../../helpers/mock-config-manager.js";

describe("channels:inspect command", () => {
const originalEnv = process.env.ABLY_WEB_CLI_MODE;

afterEach(() => {
if (originalEnv === undefined) {
delete process.env.ABLY_WEB_CLI_MODE;
} else {
process.env.ABLY_WEB_CLI_MODE = originalEnv;
}

vi.clearAllMocks();
});

describe("normal CLI mode", () => {
beforeEach(() => {
delete process.env.ABLY_WEB_CLI_MODE;
});

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(
["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`,
);
});

it("should URL-encode special characters in channel name", async () => {
const mockConfig = getMockConfigManager();
const accountId = mockConfig.getCurrentAccount()!.accountId!;
const appId = mockConfig.getCurrentAppId()!;

const { stdout } = await runCommand(
["channels:inspect", "my-channel/foo#bar"],
import.meta.url,
);

expect(stdout).toContain(
`https://ably.com/accounts/${accountId}/apps/${appId}/channels/my-channel%2Ffoo%23bar`,
);
});

it("should error when no account is configured", async () => {
const mockConfig = getMockConfigManager();
mockConfig.clearAccounts();

const { error } = await runCommand(
["channels:inspect", "my-channel"],
import.meta.url,
);

expect(error).toBeDefined();
expect(error?.message).toContain("No account configured");
expect(error?.message).toContain("ably accounts login");
});

it("should error when no app is selected", async () => {
const mockConfig = getMockConfigManager();
mockConfig.setCurrentAppIdForAccount(undefined);

const { error } = await runCommand(
["channels:inspect", "my-channel"],
import.meta.url,
);

expect(error).toBeDefined();
expect(error?.message).toContain("No app selected");
expect(error?.message).toContain("ably apps switch");
expect(error?.message).toContain("--app");
});

it("should use --app flag over current app", async () => {
const mockConfig = getMockConfigManager();
const accountId = mockConfig.getCurrentAccount()!.accountId!;

const { stdout } = await runCommand(
["channels:inspect", "my-channel", "--app", "custom-app-id"],
import.meta.url,
);

expect(stdout).toContain(
`https://ably.com/accounts/${accountId}/apps/custom-app-id/channels/my-channel`,
);
});

it("should use --app flag when no current app is set", async () => {
const mockConfig = getMockConfigManager();
const accountId = mockConfig.getCurrentAccount()!.accountId!;
mockConfig.setCurrentAppIdForAccount(undefined);

const { stdout } = await runCommand(
["channels:inspect", "my-channel", "--app", "override-app"],
import.meta.url,
);

expect(stdout).toContain(
`https://ably.com/accounts/${accountId}/apps/override-app/channels/my-channel`,
);
});
});

describe("web CLI mode", () => {
beforeEach(() => {
process.env.ABLY_WEB_CLI_MODE = "true";
});

it("should display URL without opening browser", async () => {
const mockConfig = getMockConfigManager();
const accountId = mockConfig.getCurrentAccount()!.accountId!;
const appId = mockConfig.getCurrentAppId()!;

const { stdout } = await runCommand(
["channels:inspect", "my-channel"],
import.meta.url,
);

expect(stdout).toContain("Visit");
expect(stdout).toContain(
`https://ably.com/accounts/${accountId}/apps/${appId}/channels/my-channel`,
);
expect(stdout).not.toContain("Opening");
expect(stdout).not.toContain("in your browser");
});
});

describe("help", () => {
it("should display help with --help flag", async () => {
const { stdout } = await runCommand(
["channels:inspect", "--help"],
import.meta.url,
);

expect(stdout).toContain("Open the Ably dashboard to inspect");
expect(stdout).toContain("USAGE");
expect(stdout).toContain("ARGUMENTS");
});
});
});
Loading