Skip to content
Merged
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
55 changes: 49 additions & 6 deletions bin/cli/commands/doctor.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}) {
Expand Down
6 changes: 3 additions & 3 deletions src/lib/cli-helper/claudeProfileAutoSync.ts
Original file line number Diff line number Diff line change
@@ -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 =
| {
Expand Down
6 changes: 3 additions & 3 deletions src/lib/cli-helper/codexProfileAutoSync.ts
Original file line number Diff line number Diff line change
@@ -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 =
| {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/cli-helper/config-generator/opencode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
2 changes: 1 addition & 1 deletion src/lib/cli-helper/tool-detector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
66 changes: 66 additions & 0 deletions tests/unit/cli-doctor-liveness-fallback-6162.test.ts
Original file line number Diff line number Diff line change
@@ -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)"
);
});
61 changes: 61 additions & 0 deletions tests/unit/cli-helper-tool-detector-paths-6162.test.ts
Original file line number Diff line number Diff line change
@@ -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");
});
Loading