From 1c116e0501abd9aad35922617b445b2e2dd813bd Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:27:45 -0300 Subject: [PATCH] fix(cli): merge node bin dir into CLI healthcheck PATH for codex detection (#8036) (#8156) checkRunnable() built the healthcheck spawn's minimalEnv.PATH from the caller's PATH only, never merging in this Node's own bin dir the way locateCommand's known-path search already does. npm-installed CLIs like codex are `#!/usr/bin/env node` shebang scripts, so when the server is launched with a minimal PATH (systemd/docker/PM2/Electron) lacking node's dir, the healthcheck spawn fails even though the binary was correctly located, and the tool shows as undetected. Extracted the merge into a new buildHealthcheckPath() helper (cliRuntimeHealthcheckPath.ts) to keep cliRuntime.ts within its frozen file-size ceiling. --- changelog.d/fixes/8036-codex-cli-detect.md | 1 + src/shared/services/cliRuntime.ts | 5 +- .../services/cliRuntimeHealthcheckPath.ts | 18 +++++ .../cliRuntime-codex-shebang-8036.test.ts | 73 +++++++++++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/8036-codex-cli-detect.md create mode 100644 src/shared/services/cliRuntimeHealthcheckPath.ts create mode 100644 tests/unit/cliRuntime-codex-shebang-8036.test.ts diff --git a/changelog.d/fixes/8036-codex-cli-detect.md b/changelog.d/fixes/8036-codex-cli-detect.md new file mode 100644 index 0000000000..467ecc0100 --- /dev/null +++ b/changelog.d/fixes/8036-codex-cli-detect.md @@ -0,0 +1 @@ +- fix(cli): merge Node's own bin dir into the CLI healthcheck spawn PATH so npm-shebang tools like codex resolve under a minimal launcher PATH (#8036) diff --git a/src/shared/services/cliRuntime.ts b/src/shared/services/cliRuntime.ts index 8538ec8878..2f20f3c05d 100644 --- a/src/shared/services/cliRuntime.ts +++ b/src/shared/services/cliRuntime.ts @@ -8,6 +8,7 @@ import { getCachedLoginShellPath, mergeShellPath } from "./loginShellPath"; import { withSettingsFallback } from "./cliInstallFallback"; import { GROK_BUILD_RUNTIME_ENTRY, AMP_RUNTIME_ENTRY } from "./cliRuntimeGrokBuild"; import { isLocationTrusted, findKnownPathMatch } from "./cliRuntimeKnownPath"; +import { buildHealthcheckPath } from "./cliRuntimeHealthcheckPath"; const VALID_RUNTIME_MODES = new Set(["auto", "host", "container"]); const FALSE_VALUES = new Set(["0", "false", "no", "off"]); @@ -907,7 +908,9 @@ const checkRunnable = async ( ) => { // Minimal environment to prevent credential leakage to potentially malicious binaries const minimalEnv: Record = { - PATH: env.PATH || env.Path, + // #8036: merge in this Node's own bin dir so `#!/usr/bin/env node` npm CLIs + // (e.g. codex) can resolve their interpreter under a minimal launcher PATH. + PATH: buildHealthcheckPath(env.PATH || env.Path || "", path.dirname(process.execPath)), HOME: env.HOME || env.USERPROFILE, USERPROFILE: env.USERPROFILE, // Windows needs this for os.homedir() APPDATA: env.APPDATA, // Many npm CLI tools rely on APPDATA diff --git a/src/shared/services/cliRuntimeHealthcheckPath.ts b/src/shared/services/cliRuntimeHealthcheckPath.ts new file mode 100644 index 0000000000..738a028705 --- /dev/null +++ b/src/shared/services/cliRuntimeHealthcheckPath.ts @@ -0,0 +1,18 @@ +import path from "path"; + +/** + * #8036: npm-installed CLIs (e.g. codex) are `#!/usr/bin/env node` shebang + * scripts — running them needs `node` resolvable on the child's PATH. A + * minimal launcher PATH (systemd/docker/PM2/Electron) may omit this Node's + * own bin dir even though the binary was correctly LOCATED via the + * PATH-independent known-path search (cliRuntime.ts's getKnownToolPaths() + * already merges it in for locating). Merge it into the healthcheck spawn's + * PATH too so the shebang can always resolve its interpreter. + */ +export const buildHealthcheckPath = (callerPath: string, nodeBinDir: string): string => { + const entries = callerPath.split(path.delimiter); + if (entries.includes(nodeBinDir)) { + return callerPath; + } + return [callerPath, nodeBinDir].filter(Boolean).join(path.delimiter); +}; diff --git a/tests/unit/cliRuntime-codex-shebang-8036.test.ts b/tests/unit/cliRuntime-codex-shebang-8036.test.ts new file mode 100644 index 0000000000..e31fd28321 --- /dev/null +++ b/tests/unit/cliRuntime-codex-shebang-8036.test.ts @@ -0,0 +1,73 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import fsp from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +// #8036: npm-installed CLIs (e.g. codex) are `#!/usr/bin/env node` shebang +// scripts. checkRunnable() in cliRuntime.ts builds a minimal spawn env whose +// PATH comes ONLY from the caller's PATH (getLookupEnv()), never merging in +// the running Node's own bin dir (path.dirname(process.execPath)) the way +// locateCommand's known-path search already does at :641. When the server is +// launched by a minimal-PATH launcher (systemd/docker/PM2) that lacks node's +// directory, `env node` inside the shebang can't resolve → healthcheck spawn +// fails → the CLI is (falsely) reported as not runnable even though it is +// correctly located. +// +// cliRuntime.ts computes EXPECTED_PARENT_PATHS from os.homedir() at MODULE +// LOAD time, so HOME must be redirected before the module is imported. +const sandboxHome = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-8036-home-")); +process.env.HOME = sandboxHome; +process.env.USERPROFILE = sandboxHome; +process.env.npm_config_prefix = path.join(sandboxHome, "npm-prefix-unused"); +delete process.env.CLI_CODEX_BIN; +delete process.env.CLI_EXTRA_PATHS; + +// Place the fake `codex` npm-shebang script at a KNOWN install location +// (home/.local/bin, within EXPECTED_PARENT_PATHS) so it's located via the +// PATH-independent known-path search — exactly like a real npm global +// install would be found — isolating the test from PATH-dependent `sh`/ +// `command -v` resolution and from this test-runner box's own PATH. +const localBinDir = path.join(sandboxHome, ".local", "bin"); +fs.mkdirSync(localBinDir, { recursive: true }); +const codexScriptPath = path.join(localBinDir, "codex"); +fs.writeFileSync( + codexScriptPath, + ["#!/usr/bin/env node", 'console.log("codex-cli 0.145.0-test");', ""].join("\n") +); +fs.chmodSync(codexScriptPath, 0o755); + +const nodeBinDir = path.dirname(process.execPath); + +const { getCliRuntimeStatus } = await import("../../src/shared/services/cliRuntime.ts"); + +test("#8036: codex is reported runnable even when the launcher PATH omits node's own bin dir", async () => { + const originalPath = process.env.PATH; + // Bogus PATH: no `sh`/`env`/system dirs, and — crucially — no node's own + // bin dir. The known-path match above never needs PATH to LOCATE codex; + // this isolates whether checkRunnable()'s healthcheck spawn can still + // RUN it (the `#!/usr/bin/env node` shebang needs `node` resolvable via + // the child's PATH). + process.env.PATH = path.join(sandboxHome, "nonexistent-launcher-path"); + try { + const status = await getCliRuntimeStatus("codex"); + assert.equal( + status.installed, + true, + `expected installed=true but got installed=${status.installed} reason=${status.reason}` + ); + assert.equal( + status.runnable, + true, + `expected runnable=true but got runnable=${status.runnable} reason=${status.reason} ` + + `(launcher PATH lacked node's bin dir ${nodeBinDir} — checkRunnable() must merge it in)` + ); + } finally { + process.env.PATH = originalPath; + } +}); + +test.after(async () => { + await fsp.rm(sandboxHome, { recursive: true, force: true }); +});