diff --git a/bin/cli/commands/doctor.mjs b/bin/cli/commands/doctor.mjs index 0a2b78db7475..087101b50da7 100644 --- a/bin/cli/commands/doctor.mjs +++ b/bin/cli/commands/doctor.mjs @@ -380,18 +380,61 @@ function resolveLivenessUrl(options = {}) { return `http://${formatHostForUrl(host || "127.0.0.1")}:${dashboardPort}/api/health/degradation`; } +async function probeUrl(url) { + try { + const response = await fetchWithTimeout(url); + return { ok: response.ok, status: response.status }; + } catch { + return { ok: false, status: 0 }; + } +} + async function checkServerLiveness(options = {}) { const url = resolveLivenessUrl(options); + // First attempt: configured health endpoint (may require auth token). + const primary = await probeUrl(url); + if (primary.ok) { + return ok("Server liveness", "Server health endpoint is reachable", { url, status: primary.status }); + } + + // #6162: /api/health and /api/health/degradation require a management token. + // When unauthenticated, fall back to probing a publicly served static asset + // (favicon.ico) to confirm the Next.js server is alive and reachable. + // Derive the fallback URL from the primary URL (preserving protocol/host/port) + // so custom liveness URL configurations are honored. Fall back to defaults + // only if the primary URL can't be parsed. + let fallbackUrl; try { - const response = await fetchWithTimeout(url); - if (!response.ok) { - return warn("Server liveness", `Server responded with HTTP ${response.status}`, { url }); - } - return ok("Server liveness", "Server health endpoint is reachable", { url }); + const parsed = new URL(url); + parsed.pathname = "/favicon.ico"; + parsed.search = ""; + parsed.hash = ""; + fallbackUrl = parsed.toString(); } catch { - return warn("Server liveness", "Server health endpoint is not reachable", { url }); + const port = parsePort(process.env.PORT || "20128", 20128); + const dashboardPort = parsePort(process.env.DASHBOARD_PORT || String(port), port); + const host = String(options.livenessHost || process.env.OMNIROUTE_DOCTOR_HOST || "127.0.0.1") + .trim() + .replace(/^https?:\/\//, "") + .replace(/\/.*$/, ""); + fallbackUrl = `http://${formatHostForUrl(host || "127.0.0.1")}:${dashboardPort}/favicon.ico`; } + const fallback = await probeUrl(fallbackUrl); + + if (fallback.ok) { + return ok( + "Server liveness", + `Server reachable (health endpoint returned ${primary.status}, likely requires MANAGEMENT_TOKEN)`, + { primaryUrl: url, primaryStatus: primary.status, fallbackUrl, fallbackStatus: fallback.status } + ); + } + + return warn( + "Server liveness", + `Server health endpoint returned HTTP ${primary.status || "no-response"} and fallback probe failed`, + { primaryUrl: url, primaryStatus: primary.status, fallbackUrl, fallbackStatus: fallback.status } + ); } export async function collectDoctorChecks(context = {}, options = {}) { diff --git a/src/lib/cli-helper/claudeProfileAutoSync.ts b/src/lib/cli-helper/claudeProfileAutoSync.ts index 19a515695af7..431f0a1013a0 100644 --- a/src/lib/cli-helper/claudeProfileAutoSync.ts +++ b/src/lib/cli-helper/claudeProfileAutoSync.ts @@ -1,7 +1,7 @@ import path from "node:path"; -import { ensureCliConfigWriteAllowed, getCliConfigPaths } from "@/shared/services/cliRuntime"; -import { getModelSyncInternalBaseUrl } from "@/shared/services/modelSyncScheduler"; -import { isFeatureFlagEnabled } from "@/shared/utils/featureFlags"; +import { ensureCliConfigWriteAllowed, getCliConfigPaths } from "../../shared/services/cliRuntime"; +import { getModelSyncInternalBaseUrl } from "../../shared/services/modelSyncScheduler"; +import { isFeatureFlagEnabled } from "../../shared/utils/featureFlags"; type SyncResult = | { diff --git a/src/lib/cli-helper/codexProfileAutoSync.ts b/src/lib/cli-helper/codexProfileAutoSync.ts index c3040ed09dd3..023da5d89903 100644 --- a/src/lib/cli-helper/codexProfileAutoSync.ts +++ b/src/lib/cli-helper/codexProfileAutoSync.ts @@ -1,7 +1,7 @@ import path from "node:path"; -import { ensureCliConfigWriteAllowed, getCliConfigPaths } from "@/shared/services/cliRuntime"; -import { getModelSyncInternalBaseUrl } from "@/shared/services/modelSyncScheduler"; -import { isFeatureFlagEnabled } from "@/shared/utils/featureFlags"; +import { ensureCliConfigWriteAllowed, getCliConfigPaths } from "../../shared/services/cliRuntime"; +import { getModelSyncInternalBaseUrl } from "../../shared/services/modelSyncScheduler"; +import { isFeatureFlagEnabled } from "../../shared/utils/featureFlags"; type SyncResult = | { diff --git a/src/lib/cli-helper/config-generator/opencode.ts b/src/lib/cli-helper/config-generator/opencode.ts index 265246e6d5de..4081cc32a2a9 100644 --- a/src/lib/cli-helper/config-generator/opencode.ts +++ b/src/lib/cli-helper/config-generator/opencode.ts @@ -5,7 +5,7 @@ import { parseOutboundUrl, isCloudMetadataHost, OutboundUrlGuardError, -} from "@/shared/network/outboundUrlGuard"; +} from "../../../shared/network/outboundUrlGuard"; const CONFIG_PATH = path.join(os.homedir(), ".config", "opencode", "opencode.json"); diff --git a/src/lib/cli-helper/tool-detector.ts b/src/lib/cli-helper/tool-detector.ts index 3dbfdb0ac703..b6463132ca82 100644 --- a/src/lib/cli-helper/tool-detector.ts +++ b/src/lib/cli-helper/tool-detector.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { getCurrentHermesAgentRoles } from "./config-generator/hermes-agent"; -import { getCachedLoginShellPath, mergeShellPath } from "@/shared/services/loginShellPath"; +import { getCachedLoginShellPath, mergeShellPath } from "../../shared/services/loginShellPath"; const execFileAsync = promisify(execFile); let execFileImpl = execFileAsync; diff --git a/tests/unit/cli-doctor-liveness-fallback-6162.test.ts b/tests/unit/cli-doctor-liveness-fallback-6162.test.ts new file mode 100644 index 000000000000..a77ec143e8c8 --- /dev/null +++ b/tests/unit/cli-doctor-liveness-fallback-6162.test.ts @@ -0,0 +1,66 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync, existsSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +// Regression test for #6162 (liveness portion): `omniroute doctor` reported +// "Server liveness: Server responded with HTTP 401" on healthy installs +// because /api/health and /api/health/degradation both require the management +// token. The doctor called them without auth → 401 → WARN, even when the +// Next.js server was clearly alive and listening. +// +// Fix: probe the configured health endpoint first; on 401/403, fall back to +// a publicly served static asset (/favicon.ico) to confirm the server is +// alive. WARN now only fires when both probes fail. +// +// This regression test asserts: +// 1. The current doctor.mjs source contains the fallback logic. +// 2. The fallback derives its URL from the primary URL via `new URL()` +// so custom host/port/protocol are preserved (Gemini code-assist review). + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); +const DOCTOR_SRC = join(ROOT, "bin/cli/commands/doctor.mjs"); + +test("doctor.mjs must exist", () => { + assert.ok(existsSync(DOCTOR_SRC), "bin/cli/commands/doctor.mjs should exist"); +}); + +test("doctor.mjs implements a /favicon.ico fallback for unauthenticated liveness (#6162)", () => { + const content = readFileSync(DOCTOR_SRC, "utf8"); + + assert.ok( + content.includes("/favicon.ico"), + "doctor.mjs must include /favicon.ico as a fallback probe (fix for #6162)" + ); + + // The probe order matters: try the configured health endpoint first, then + // fall back. Locking the order prevents future refactors from regressing + // back to "always report WARN on 401". + assert.ok( + /\bprimary\.ok\b/.test(content), + "doctor.mjs must branch on `primary.ok` to decide whether to fall back to /favicon.ico" + ); +}); + +test("doctor.mjs derives fallback URL from primary URL via new URL() (Gemini review)", () => { + const content = readFileSync(DOCTOR_SRC, "utf8"); + + assert.ok( + content.includes("new URL("), + "doctor.mjs must derive the fallback URL from the primary URL via `new URL()` to preserve protocol/host/port (Gemini review feedback on PR #6163)" + ); +}); + +test("doctor.mjs no longer reports WARN on HTTP 401/403 alone", () => { + const content = readFileSync(DOCTOR_SRC, "utf8"); + + // Old (buggy) line was: + // return warn("Server liveness", `Server responded with HTTP ${response.status}`, { url }); + // This asserted immediately on !response.ok without considering auth. + // The fix should NOT contain that exact phrase anymore. + assert.ok( + !content.includes("`Server responded with HTTP ${response.status}`"), + "doctor.mjs must not contain the buggy 'Server responded with HTTP ${response.status}' warn message (would be hit on auth-required 401)" + ); +}); \ No newline at end of file diff --git a/tests/unit/cli-helper-tool-detector-paths-6162.test.ts b/tests/unit/cli-helper-tool-detector-paths-6162.test.ts new file mode 100644 index 000000000000..e8810d8db7a8 --- /dev/null +++ b/tests/unit/cli-helper-tool-detector-paths-6162.test.ts @@ -0,0 +1,61 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync, existsSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +// Regression test for #6162: published `omniroute doctor` failed with +// "Could not run CLI tool checks: Cannot find package '@/shared'" because +// src/lib/cli-helper/*.ts files imported `@/shared/...` aliases that the +// CLI runtime (tsx + ESM `import()`) cannot resolve. Fix: replace +// `@/shared/...` with relative imports in the cli-helper files so they work +// in the published package without a compile step. +// +// Lock the fix by asserting that no cli-helper source file uses the +// `@/shared` alias any more, and that the runtime module load succeeds. + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); + +const CLI_HELPER_FILES = [ + "src/lib/cli-helper/tool-detector.ts", + "src/lib/cli-helper/claudeProfileAutoSync.ts", + "src/lib/cli-helper/codexProfileAutoSync.ts", + "src/lib/cli-helper/config-generator/opencode.ts", +]; + +for (const file of CLI_HELPER_FILES) { + test(`${file} must not use @/shared path alias (fix for #6162)`, () => { + const abs = join(ROOT, file); + assert.ok(existsSync(abs), `${file} should exist`); + const content = readFileSync(abs, "utf8"); + assert.ok( + !/@\/shared/.test(content), + `${file} must not import via "@/shared/..." alias — the published CLI runtime (tsx + ESM import) cannot resolve tsconfig path aliases. Use relative paths instead. See #6162.` + ); + }); +} + +test("tool-detector.ts is importable at runtime (regression for #6162)", async () => { + // This would have failed before the fix with + // "Cannot find package '@/shared' imported from .../tool-detector.ts". + const mod = await import("../../src/lib/cli-helper/tool-detector.ts"); + assert.equal(typeof mod.detectAllTools, "function"); + assert.equal(typeof mod.detectTool, "function"); +}); + +test("claudeProfileAutoSync.ts is importable at runtime (regression for #6162)", async () => { + const mod = await import("../../src/lib/cli-helper/claudeProfileAutoSync.ts"); + // The module exports at least the sync function; we don't care about its + // specific name, only that the import resolves without throwing. + assert.equal(typeof mod, "object"); +}); + +test("codexProfileAutoSync.ts is importable at runtime (regression for #6162)", async () => { + const mod = await import("../../src/lib/cli-helper/codexProfileAutoSync.ts"); + assert.equal(typeof mod, "object"); +}); + +test("config-generator/opencode.ts is importable at runtime (regression for #6162)", async () => { + const mod = await import("../../src/lib/cli-helper/config-generator/opencode.ts"); + assert.equal(typeof mod, "object"); +}); \ No newline at end of file