From 7f3d07dd2ddf0d2b3d4e3382a198e5a6648dc390 Mon Sep 17 00:00:00 2001 From: Aris Sunandar Date: Sat, 4 Jul 2026 14:30:20 +0700 Subject: [PATCH 1/4] fix(doctor): resolve two false-positive WARNs (#6162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `omniroute doctor` command reported two warnings on healthy installs even though the underlying checks actually passed. Both came from the doctor probing state that already worked; they looked like bugs but users couldn't tell without manual digging. Issue 1 — Server liveness HTTP 401 /api/health and /api/health/degradation both require the management token. 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. Issue 2 — CLI Tools '@/shared' import tool-detector.ts (and 3 other cli-helper files) import @/shared/... aliases that resolve via tsconfig.json paths. The CLI ships raw TS source (no compile step) and runs through tsx, but tsx does not honor tsconfig paths at runtime, and tsconfig-paths only hooks CJS Module._resolveFilename while doctor uses ESM `import()`. Fix: replace @/shared/... with relative imports in the 4 cli-helper files. This is the same pattern these files already use for ./config- generator/* imports. No new dependency, no architectural change, and the fix doesn't regress Next.js itself which keeps using @/shared. Verified on v3.8.43 (Node v24.17, Windows 11): Before: 7 ok, 2 warning(s), 0 failure(s) After: 8 ok, N warning(s), 0 failure(s) where N accurately reflects which CLI tools are installed and configured for OmniRoute (e.g. Hermes Agent installed but not pointed at 20128 → 2 real warnings, not 1 false-positive). Refs #6162 --- bin/cli/commands/doctor.mjs | 47 +++++++++++++++---- src/lib/cli-helper/claudeProfileAutoSync.ts | 6 +-- src/lib/cli-helper/codexProfileAutoSync.ts | 6 +-- .../cli-helper/config-generator/opencode.ts | 2 +- src/lib/cli-helper/tool-detector.ts | 2 +- 5 files changed, 47 insertions(+), 16 deletions(-) diff --git a/bin/cli/commands/doctor.mjs b/bin/cli/commands/doctor.mjs index 0a2b78db7475..2c8fc0f4e104 100644 --- a/bin/cli/commands/doctor.mjs +++ b/bin/cli/commands/doctor.mjs @@ -380,18 +380,49 @@ function resolveLivenessUrl(options = {}) { return `http://${formatHostForUrl(host || "127.0.0.1")}:${dashboardPort}/api/health/degradation`; } -async function checkServerLiveness(options = {}) { - const url = resolveLivenessUrl(options); - +async function probeUrl(url) { 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 }); + return { ok: response.ok, status: response.status }; } catch { - return warn("Server liveness", "Server health endpoint is not reachable", { url }); + 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. + 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(/\/.*$/, ""); + const 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 on :${dashboardPort} (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; From ac80831f1883521b854fca882d1cd91c103e1100 Mon Sep 17 00:00:00 2001 From: Aris Sunandar Date: Sat, 4 Jul 2026 15:07:33 +0700 Subject: [PATCH 2/4] fix(doctor): derive fallback URL from primary URL via new URL() Per Gemini code-assist review feedback: the previous fallback constructed the /favicon.ico URL from defaults (127.0.0.1:PORT) which ignored custom host/port/protocol configurations supplied via: - OMNIROUTE_DOCTOR_LIVENESS_URL - OMNIROUTE_DOCTOR_HOST - --liveness-url / --host CLI flags Parse the primary URL with new URL() to preserve protocol, host, port, and subpaths. The previous default-based fallback remains as a catch-all for invalid primary URLs. --- bin/cli/commands/doctor.mjs | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/bin/cli/commands/doctor.mjs b/bin/cli/commands/doctor.mjs index 2c8fc0f4e104..087101b50da7 100644 --- a/bin/cli/commands/doctor.mjs +++ b/bin/cli/commands/doctor.mjs @@ -401,19 +401,31 @@ async function checkServerLiveness(options = {}) { // #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. - 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(/\/.*$/, ""); - const fallbackUrl = `http://${formatHostForUrl(host || "127.0.0.1")}:${dashboardPort}/favicon.ico`; + // 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 parsed = new URL(url); + parsed.pathname = "/favicon.ico"; + parsed.search = ""; + parsed.hash = ""; + fallbackUrl = parsed.toString(); + } catch { + 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 on :${dashboardPort} (health endpoint returned ${primary.status}, likely requires MANAGEMENT_TOKEN)`, + `Server reachable (health endpoint returned ${primary.status}, likely requires MANAGEMENT_TOKEN)`, { primaryUrl: url, primaryStatus: primary.status, fallbackUrl, fallbackStatus: fallback.status } ); } From adc9cfa095c8c8a27881db222f2fb417f7ea76a1 Mon Sep 17 00:00:00 2001 From: Aris Sunandar Date: Sat, 4 Jul 2026 16:45:23 +0700 Subject: [PATCH 3/4] test(doctor): add regression tests for #6162 fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new test files lock the fix and satisfy the PR Test Policy gate ("production code change without tests"): - tests/unit/cli-helper-tool-detector-paths-6162.test.ts Locks the @/shared → relative imports fix across all 4 cli-helper files. Asserts (a) no @/shared alias remains in the cli-helper sources, and (b) each file is importable at runtime via tsx/ESM, which would have thrown "Cannot find package '@/shared'" before the fix. - tests/unit/cli-doctor-liveness-fallback-6162.test.ts Locks the /favicon.ico fallback in doctor.mjs. Asserts the fallback probe exists, derives its URL from the primary URL via new URL() (per Gemini review feedback), and that the buggy 'Server responded with HTTP 401' WARN path is gone. Both tests use only node:test + node:assert/strict so they slot into the existing 'test' and 'test:unit' scripts with no extra config. --- .../cli-doctor-liveness-fallback-6162.test.ts | 64 +++++++++++++++++++ ...li-helper-tool-detector-paths-6162.test.ts | 61 ++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 tests/unit/cli-doctor-liveness-fallback-6162.test.ts create mode 100644 tests/unit/cli-helper-tool-detector-paths-6162.test.ts 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..25232d23f292 --- /dev/null +++ b/tests/unit/cli-doctor-liveness-fallback-6162.test.ts @@ -0,0 +1,64 @@ +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". + const primaryMatch = content.match(/primary\.ok\s*\?/); + assert.ok(primaryMatch, "doctor.mjs must check `primary.ok` to decide whether to fall back"); +}); + +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 From d93d7f14ee81efb2a5cbe650ab1675d8356f4aef Mon Sep 17 00:00:00 2001 From: Aris Sunandar Date: Sat, 4 Jul 2026 17:23:15 +0700 Subject: [PATCH 4/4] test(doctor): fix primary.ok regex in fallback test The earlier regex /primary\.ok\s*\?/ required a '?' immediately after, but the actual doctor.mjs code uses a multi-line if-block: if (primary.ok) { return ok(...); } Use /\bprimary\.ok\b/ instead so the assertion matches the existing branching. --- tests/unit/cli-doctor-liveness-fallback-6162.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/cli-doctor-liveness-fallback-6162.test.ts b/tests/unit/cli-doctor-liveness-fallback-6162.test.ts index 25232d23f292..a77ec143e8c8 100644 --- a/tests/unit/cli-doctor-liveness-fallback-6162.test.ts +++ b/tests/unit/cli-doctor-liveness-fallback-6162.test.ts @@ -37,8 +37,10 @@ test("doctor.mjs implements a /favicon.ico fallback for unauthenticated liveness // 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". - const primaryMatch = content.match(/primary\.ok\s*\?/); - assert.ok(primaryMatch, "doctor.mjs must check `primary.ok` to decide whether to fall back"); + 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)", () => {