diff --git a/bin/cli/commands/dashboard.mjs b/bin/cli/commands/dashboard.mjs index ff7d8c7b01..44d2da1df9 100644 --- a/bin/cli/commands/dashboard.mjs +++ b/bin/cli/commands/dashboard.mjs @@ -1,17 +1,22 @@ import { execFile } from "node:child_process"; import { t } from "../i18n.mjs"; +function parsePort(value, fallback) { + const parsed = parseInt(String(value), 10); + return Number.isFinite(parsed) && parsed > 0 && parsed <= 65535 ? parsed : fallback; +} + export function registerDashboard(program) { program .command("dashboard") .description(t("dashboard.description")) .option("--url", t("dashboard.urlOnly")) - .option("--port ", "Port the server is running on", "20128") + .option("--port ", "Port the server is running on") .option("--tui", t("dashboard.tui") || "Open interactive TUI dashboard (terminal UI)") .action(async (opts, cmd) => { if (opts.tui) { const globalOpts = cmd.optsWithGlobals(); - const port = opts.port ? parseInt(String(opts.port), 10) : 20128; + const port = parsePort(opts.port ?? process.env.PORT ?? "20128", 20128); const baseUrl = globalOpts.baseUrl ?? `http://localhost:${port}`; const apiKey = globalOpts.apiKey ?? null; const { startInteractiveTui } = await import("../tui/Dashboard.jsx"); @@ -24,7 +29,7 @@ export function registerDashboard(program) { } export async function runDashboardCommand(opts = {}) { - const port = opts.port ? parseInt(String(opts.port), 10) : 20128; + const port = parsePort(opts.port ?? process.env.PORT ?? "20128", 20128); const dashboardUrl = `http://localhost:${port}`; if (opts.url) { diff --git a/changelog.d/fixes/7049-dashboard-port-env-fallback.md b/changelog.d/fixes/7049-dashboard-port-env-fallback.md new file mode 100644 index 0000000000..b3c51761ea --- /dev/null +++ b/changelog.d/fixes/7049-dashboard-port-env-fallback.md @@ -0,0 +1 @@ +- **fix(cli):** `omniroute dashboard` (no `--port` flag) now respects `PORT` from the environment instead of always opening `localhost:20128`, matching `serve`/`launch` precedence (`--port` > `PORT` env > `20128` default) (#7049 — thanks @kaon0388v1). diff --git a/tests/unit/cli-dashboard-port.test.ts b/tests/unit/cli-dashboard-port.test.ts new file mode 100644 index 0000000000..75a22b2f2b --- /dev/null +++ b/tests/unit/cli-dashboard-port.test.ts @@ -0,0 +1,75 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +/** + * Replicate the parsePort + port resolution logic from bin/cli/commands/dashboard.mjs + * to verify that PORT env var is respected when --port is not passed (mirrors + * tests/unit/cli-serve-port.test.ts's convention for serve.mjs). + */ +function parsePort(value: string | undefined, fallback: number): number { + const parsed = parseInt(String(value), 10); + return Number.isFinite(parsed) && parsed > 0 && parsed <= 65535 ? parsed : fallback; +} + +function resolvePort(optsPort: string | undefined, envPort: string | undefined): number { + return parsePort(optsPort ?? envPort ?? "20128", 20128); +} + +test("dashboard port: uses --port flag when explicitly provided, overriding env", () => { + const port = resolvePort("3000", "9999"); + assert.equal(port, 3000); +}); + +test("dashboard port: falls back to PORT env var when --port is not provided", () => { + const port = resolvePort(undefined, "20129"); + assert.equal(port, 20129); +}); + +test("dashboard port: falls back to 20128 when neither --port nor PORT env var is set", () => { + const port = resolvePort(undefined, undefined); + assert.equal(port, 20128); +}); + +test("dashboard port: invalid --port (non-numeric) falls back to 20128", () => { + const port = resolvePort("abc", undefined); + assert.equal(port, 20128); +}); + +test("dashboard port: --port 0 (out of range) falls back to 20128", () => { + const port = resolvePort("0", undefined); + assert.equal(port, 20128); +}); + +test("dashboard port: --port 70000 (out of range) falls back to 20128", () => { + const port = resolvePort("70000", undefined); + assert.equal(port, 20128); +}); + +test("dashboard URL generation: http://localhost: built correctly for a custom port", () => { + const port = resolvePort(undefined, "31337"); + assert.equal(`http://localhost:${port}`, "http://localhost:31337"); +}); + +test("dashboard command: --port option has no Commander default", async () => { + const fs = await import("node:fs"); + const path = await import("node:path"); + const dashboardSource = fs.readFileSync( + path.resolve(import.meta.dirname, "../../bin/cli/commands/dashboard.mjs"), + "utf-8", + ); + // Ensure the option does NOT carry a baked-in Commander default (third arg). + assert.match( + dashboardSource, + /\.option\("--port ",\s*"Port the server is running on"\)/, + ); +}); + +test("dashboard command: source references process.env.PORT (env-fallback regression guard)", async () => { + const fs = await import("node:fs"); + const path = await import("node:path"); + const dashboardSource = fs.readFileSync( + path.resolve(import.meta.dirname, "../../bin/cli/commands/dashboard.mjs"), + "utf-8", + ); + assert.match(dashboardSource, /process\.env\.PORT/); +});