From eccc95579b8423152925dccfaa8ced7013ed9377 Mon Sep 17 00:00:00 2001 From: jzli Date: Mon, 21 Sep 2026 12:09:28 +0800 Subject: [PATCH 1/2] feat(server): OCX_PROBE_TIMEOUT_MS override for liveness probe ceilings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A host-level security layer (content filter / EDR network extension) can add a fixed per-connection cost to loopback TCP — measured at ~1s per connect on an affected macOS machine. The shipped 750ms single-probe default then aborts before a healthy proxy can answer, and every CLI liveness consumer (ocx health, ocx status, ocx account *, ocx login codex, ocx ready) reports the proxy as unreachable while a direct curl /healthz succeeds. Add an opt-in OCX_PROBE_TIMEOUT_MS escape hatch: parsed once at module load with strict validation (positive integer milliseconds; anything malformed is ignored), raising DEFAULT_PROBE_TIMEOUT_MS and the stop/start-ownership probe budgets together. Defaults are unchanged on unset or malformed values, so typical hosts see no behavior difference. Verified on the affected host: - bun test tests/server/proxy-liveness.test.ts tests/server/probe-timeout-env.test.ts -> 117 pass, 0 fail (9 new: strict parsing + module-load wiring via query-string module re-evaluation) - bun run typecheck -> clean - end-to-end contrast against the running 2.59.0 proxy: bun src/cli/index.ts status -> "health check failed ... timed out" OCX_PROBE_TIMEOUT_MS=5000 bun src/cli/index.ts status -> "Proxy: running ... healthz ok (live)" --- src/server/proxy-liveness.ts | 28 +++++++++++++++++-- tests/server/probe-timeout-env.test.ts | 38 ++++++++++++++++++++++++++ tests/server/proxy-liveness.test.ts | 33 ++++++++++++++++++++++ 3 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 tests/server/probe-timeout-env.test.ts diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index 594be867b02..e9bad52478a 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -60,12 +60,34 @@ export interface LivenessIo { nowFn?: () => number; } +/** + * Operator override for the per-probe fetch ceilings below: integer milliseconds > 0. + * + * For hosts where a security layer (content filter / EDR network extension) adds a + * fixed per-connection cost to loopback TCP — measured at ~1s per connect on an + * affected macOS machine — the shipped 750ms single-probe default aborts before a + * healthy proxy can answer, and every CLI liveness consumer (`ocx health`, + * `ocx status`, `ocx account *`, `ocx login codex`, `ocx ready`) then reports the + * proxy as unreachable while direct `curl /healthz` succeeds. Setting + * OCX_PROBE_TIMEOUT_MS=5000 restores correct verdicts on such hosts without + * changing behavior anywhere else. Parsed once at module load; malformed values + * are ignored so a typo can only fall back to the defaults, never break startup. + */ +export function parseProbeTimeoutOverrideMs(raw: string | undefined): number | undefined { + const trimmed = raw?.trim(); + if (!trimmed || !/^\d+$/.test(trimmed)) return undefined; + const n = Number(trimmed); + return n > 0 ? n : undefined; +} + +const probeTimeoutOverrideMs = parseProbeTimeoutOverrideMs(process.env.OCX_PROBE_TIMEOUT_MS); + /** Default per-probe fetch ceiling shared by liveness and readiness probes. */ -export const DEFAULT_PROBE_TIMEOUT_MS = 750; +export const DEFAULT_PROBE_TIMEOUT_MS = probeTimeoutOverrideMs ?? 750; /** Default probe options for service stop / orphan cleanup — a just-bound proxy can miss a single 750ms probe. */ export const SERVICE_STOP_LIVENESS: Pick = { - timeoutMs: 1500, + timeoutMs: probeTimeoutOverrideMs ?? 1500, attempts: 3, }; @@ -81,7 +103,7 @@ export const SERVICE_STOP_LIVENESS: Pick = * the stop path already uses for the mirror-image decision. */ export const START_OWNERSHIP_LIVENESS: Pick = { - timeoutMs: 1500, + timeoutMs: probeTimeoutOverrideMs ?? 1500, attempts: 3, }; diff --git a/tests/server/probe-timeout-env.test.ts b/tests/server/probe-timeout-env.test.ts new file mode 100644 index 00000000000..59f1b0988c4 --- /dev/null +++ b/tests/server/probe-timeout-env.test.ts @@ -0,0 +1,38 @@ +/** + * OCX_PROBE_TIMEOUT_MS wiring test. The probe ceilings are module-load constants, + * so their value depends on the environment at the moment proxy-liveness is first + * evaluated. Bun's test files can share one module registry, which makes in-process + * env mutation order-dependent — instead of spawning a child interpreter, each case + * imports the module through a distinct query string: a different specifier is a + * different module instance per ESM resolution rules, so the module body (and the + * env read at its top level) re-runs under the environment this test just set. + */ +import { describe, expect, test } from "bun:test"; + +describe("OCX_PROBE_TIMEOUT_MS override wiring", () => { + test("defaults load when the variable is unset", async () => { + delete process.env.OCX_PROBE_TIMEOUT_MS; + const mod = await import("../../src/server/proxy-liveness.ts?wiring=defaults"); + expect(mod.DEFAULT_PROBE_TIMEOUT_MS).toBe(750); + expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(1500); + expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(1500); + }); + + test("override raises every probe ceiling at module load", async () => { + process.env.OCX_PROBE_TIMEOUT_MS = "3210"; + const mod = await import("../../src/server/proxy-liveness.ts?wiring=override"); + expect(mod.DEFAULT_PROBE_TIMEOUT_MS).toBe(3210); + expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(3210); + expect(mod.SERVICE_STOP_LIVENESS.attempts).toBe(3); + expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(3210); + expect(mod.START_OWNERSHIP_LIVENESS.attempts).toBe(3); + }); + + test("a malformed override falls back to the defaults at module load", async () => { + process.env.OCX_PROBE_TIMEOUT_MS = "not-a-number"; + const mod = await import("../../src/server/proxy-liveness.ts?wiring=malformed"); + expect(mod.DEFAULT_PROBE_TIMEOUT_MS).toBe(750); + expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(1500); + expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(1500); + }); +}); diff --git a/tests/server/proxy-liveness.test.ts b/tests/server/proxy-liveness.test.ts index d675f04c8b8..f48f4ed2f74 100644 --- a/tests/server/proxy-liveness.test.ts +++ b/tests/server/proxy-liveness.test.ts @@ -8,10 +8,12 @@ import { findLiveProxy, isOpencodexHealthz, loopbackProbeHosts, + parseProbeTimeoutOverrideMs, probeHostname, probePortOwner, probeReadiness, proxyIdentityAt, + SERVICE_STOP_LIVENESS, START_OWNERSHIP_LIVENESS, validateReadyzBody, } from "../../src/server/proxy-liveness"; @@ -1040,3 +1042,34 @@ describe("client-role discrimination (#4662)", () => { expect(live).toEqual({ pid: 4242, port: 10100, hostname: undefined, source: "config", version: "2.6.17", role: "client" }); }); }); + +describe("parseProbeTimeoutOverrideMs", () => { + test("accepts positive integer milliseconds", () => { + expect(parseProbeTimeoutOverrideMs("5000")).toBe(5000); + expect(parseProbeTimeoutOverrideMs("1")).toBe(1); + }); + + test("trims surrounding whitespace", () => { + expect(parseProbeTimeoutOverrideMs(" 4321 ")).toBe(4321); + }); + + test("ignores absent, empty, and non-integer values", () => { + expect(parseProbeTimeoutOverrideMs(undefined)).toBeUndefined(); + expect(parseProbeTimeoutOverrideMs("")).toBeUndefined(); + expect(parseProbeTimeoutOverrideMs(" ")).toBeUndefined(); + expect(parseProbeTimeoutOverrideMs("abc")).toBeUndefined(); + expect(parseProbeTimeoutOverrideMs("1.5")).toBeUndefined(); + expect(parseProbeTimeoutOverrideMs("-5")).toBeUndefined(); + expect(parseProbeTimeoutOverrideMs("+7")).toBeUndefined(); + }); + + test("ignores zero so a typo cannot disable probe timeouts", () => { + expect(parseProbeTimeoutOverrideMs("0")).toBeUndefined(); + }); + + test("defaults stay untouched when no override parses", () => { + expect(DEFAULT_PROBE_TIMEOUT_MS).toBeGreaterThan(0); + expect(SERVICE_STOP_LIVENESS.timeoutMs).toBeGreaterThanOrEqual(DEFAULT_PROBE_TIMEOUT_MS); + expect(START_OWNERSHIP_LIVENESS.timeoutMs).toBeGreaterThanOrEqual(DEFAULT_PROBE_TIMEOUT_MS); + }); +}); From 0f8736c8864a9baef92ed555099958a168bfb6c7 Mon Sep 17 00:00:00 2001 From: jzli Date: Mon, 21 Sep 2026 12:41:01 +0800 Subject: [PATCH 2/2] fix(server): raises-only probe override, 32-bit ceiling, test hygiene, docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on PR #5409: - stop/start budgets keep their 1500ms floor: OCX_PROBE_TIMEOUT_MS may lengthen a ceiling but never shorten it below its shipped default, so a value like 1000 raises only the shared 750ms default probe and the budgets that guard against duplicate proxy starts (#764, #5004) stay put - values above 2147483647 are ignored: AbortSignal.timeout() only accepts a signed-32-bit delay, an out-of-range value throws in Bun, and the probe path would misread that as a dead proxy — the failure this override exists to fix - probe-timeout-env tests save and restore OCX_PROBE_TIMEOUT_MS around every case, and the defaults assertion in proxy-liveness tests is now exact (750/1500/1500) so a leaked override in the shared module registry cannot pass silently - document the variable in docs-site (reference/cli.md) --- docs-site/src/content/docs/reference/cli.md | 17 ++++++++ src/server/proxy-liveness.ts | 24 +++++++++--- tests/server/probe-timeout-env.test.ts | 43 +++++++++++++++++++-- tests/server/proxy-liveness.test.ts | 17 ++++++-- 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/docs-site/src/content/docs/reference/cli.md b/docs-site/src/content/docs/reference/cli.md index 7e32c2752f4..3e3cf906b4a 100644 --- a/docs-site/src/content/docs/reference/cli.md +++ b/docs-site/src/content/docs/reference/cli.md @@ -68,6 +68,23 @@ List or status is the default where unambiguous. Use `--json` for structured sna and other purely visual browser state have no CLI equivalent; Cloudflare Tunnel setup is outside this command set. +## Liveness probe ceiling override + +`ocx health`, `ocx status`, `ocx account *`, `ocx login codex`, and `ocx ready` locate the +running proxy through a short liveness probe (750ms per attempt by default, 1500ms with +retries for stop/start decisions). On hosts where a security layer adds a fixed +per-connection cost to loopback TCP — content filters and EDR-style network extensions, +measured at roughly one second per connect on an affected macOS machine — those ceilings +abort before a healthy proxy can answer, and every one of those commands reports the +proxy as down while a direct `curl http://127.0.0.1:10100/healthz` succeeds. + +Set `OCX_PROBE_TIMEOUT_MS` to raise the ceilings on such hosts, e.g. +`OCX_PROBE_TIMEOUT_MS=5000 ocx status`. The value is integer milliseconds in +`(0, 2147483647]`; anything else — unset, empty, fractional, negative, or out of range — +leaves the defaults in place. The override is raises-only: the 1500ms stop/start budgets +keep their floor, so a smaller value (say `1000`) lengthens the shared default probe +without ever shortening the budgets that guard against duplicate proxy starts. + ## Exit codes and confirmation Successful commands exit 0. Invalid usage, unknown commands or resources, failed API operations, diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index e9bad52478a..17012efa736 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -61,7 +61,8 @@ export interface LivenessIo { } /** - * Operator override for the per-probe fetch ceilings below: integer milliseconds > 0. + * Operator override for the per-probe fetch ceilings below: integer milliseconds in + * (0, 2147483647]. * * For hosts where a security layer (content filter / EDR network extension) adds a * fixed per-connection cost to loopback TCP — measured at ~1s per connect on an @@ -70,14 +71,25 @@ export interface LivenessIo { * `ocx status`, `ocx account *`, `ocx login codex`, `ocx ready`) then reports the * proxy as unreachable while direct `curl /healthz` succeeds. Setting * OCX_PROBE_TIMEOUT_MS=5000 restores correct verdicts on such hosts without - * changing behavior anywhere else. Parsed once at module load; malformed values - * are ignored so a typo can only fall back to the defaults, never break startup. + * changing behavior anywhere else. + * + * Raises-only: the stop/start-ownership budgets keep their 1500ms floor, because a + * value below it would shorten the very budgets that exist to catch a just-bound + * or shadowed proxy (#764, #5004) — an override may lengthen a ceiling, never + * shorten it below its shipped default. Values above the signed-32-bit ceiling are + * ignored: AbortSignal.timeout() only accepts that range, and an out-of-range + * delay throws in Bun, which the probe path would misread as a dead proxy — the + * exact failure this override exists to fix. Parsed once at module load; malformed + * values are ignored so a typo can only fall back to the defaults, never break + * startup. */ +export const MAX_PROBE_TIMEOUT_MS = 2_147_483_647; + export function parseProbeTimeoutOverrideMs(raw: string | undefined): number | undefined { const trimmed = raw?.trim(); if (!trimmed || !/^\d+$/.test(trimmed)) return undefined; const n = Number(trimmed); - return n > 0 ? n : undefined; + return n > 0 && n <= MAX_PROBE_TIMEOUT_MS ? n : undefined; } const probeTimeoutOverrideMs = parseProbeTimeoutOverrideMs(process.env.OCX_PROBE_TIMEOUT_MS); @@ -87,7 +99,7 @@ export const DEFAULT_PROBE_TIMEOUT_MS = probeTimeoutOverrideMs ?? 750; /** Default probe options for service stop / orphan cleanup — a just-bound proxy can miss a single 750ms probe. */ export const SERVICE_STOP_LIVENESS: Pick = { - timeoutMs: probeTimeoutOverrideMs ?? 1500, + timeoutMs: Math.max(probeTimeoutOverrideMs ?? 0, 1500), attempts: 3, }; @@ -103,7 +115,7 @@ export const SERVICE_STOP_LIVENESS: Pick = * the stop path already uses for the mirror-image decision. */ export const START_OWNERSHIP_LIVENESS: Pick = { - timeoutMs: probeTimeoutOverrideMs ?? 1500, + timeoutMs: Math.max(probeTimeoutOverrideMs ?? 0, 1500), attempts: 3, }; diff --git a/tests/server/probe-timeout-env.test.ts b/tests/server/probe-timeout-env.test.ts index 59f1b0988c4..12f1440b088 100644 --- a/tests/server/probe-timeout-env.test.ts +++ b/tests/server/probe-timeout-env.test.ts @@ -6,8 +6,17 @@ * imports the module through a distinct query string: a different specifier is a * different module instance per ESM resolution rules, so the module body (and the * env read at its top level) re-runs under the environment this test just set. + * The variable is saved and restored around every case so no other test in a shared + * registry can observe a leftover value at its own first module load. */ -import { describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, test } from "bun:test"; + +const previousOverride = process.env.OCX_PROBE_TIMEOUT_MS; + +afterEach(() => { + if (previousOverride === undefined) delete process.env.OCX_PROBE_TIMEOUT_MS; + else process.env.OCX_PROBE_TIMEOUT_MS = previousOverride; +}); describe("OCX_PROBE_TIMEOUT_MS override wiring", () => { test("defaults load when the variable is unset", async () => { @@ -18,9 +27,9 @@ describe("OCX_PROBE_TIMEOUT_MS override wiring", () => { expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(1500); }); - test("override raises every probe ceiling at module load", async () => { + test("an override above both defaults raises every ceiling", async () => { process.env.OCX_PROBE_TIMEOUT_MS = "3210"; - const mod = await import("../../src/server/proxy-liveness.ts?wiring=override"); + const mod = await import("../../src/server/proxy-liveness.ts?wiring=raise-all"); expect(mod.DEFAULT_PROBE_TIMEOUT_MS).toBe(3210); expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(3210); expect(mod.SERVICE_STOP_LIVENESS.attempts).toBe(3); @@ -28,6 +37,16 @@ describe("OCX_PROBE_TIMEOUT_MS override wiring", () => { expect(mod.START_OWNERSHIP_LIVENESS.attempts).toBe(3); }); + test("an override between the two defaults raises only the shared default", async () => { + // 1000 lengthens the 750ms default but must NOT shorten the 1500ms stop/start + // budgets — those exist to catch a just-bound or shadowed proxy (#764, #5004). + process.env.OCX_PROBE_TIMEOUT_MS = "1000"; + const mod = await import("../../src/server/proxy-liveness.ts?wiring=raise-default-only"); + expect(mod.DEFAULT_PROBE_TIMEOUT_MS).toBe(1000); + expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(1500); + expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(1500); + }); + test("a malformed override falls back to the defaults at module load", async () => { process.env.OCX_PROBE_TIMEOUT_MS = "not-a-number"; const mod = await import("../../src/server/proxy-liveness.ts?wiring=malformed"); @@ -35,4 +54,22 @@ describe("OCX_PROBE_TIMEOUT_MS override wiring", () => { expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(1500); expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(1500); }); + + test("a value beyond the signed-32-bit ceiling is ignored", async () => { + // AbortSignal.timeout() only accepts that range; an out-of-range delay throws + // in Bun and the probe path would misread it as a dead proxy. + process.env.OCX_PROBE_TIMEOUT_MS = "2147483648"; + const mod = await import("../../src/server/proxy-liveness.ts?wiring=overflow"); + expect(mod.DEFAULT_PROBE_TIMEOUT_MS).toBe(750); + expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(1500); + expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(1500); + }); + + test("the ceiling itself is accepted", async () => { + process.env.OCX_PROBE_TIMEOUT_MS = "2147483647"; + const mod = await import("../../src/server/proxy-liveness.ts?wiring=ceiling"); + expect(mod.DEFAULT_PROBE_TIMEOUT_MS).toBe(2_147_483_647); + expect(mod.SERVICE_STOP_LIVENESS.timeoutMs).toBe(2_147_483_647); + expect(mod.START_OWNERSHIP_LIVENESS.timeoutMs).toBe(2_147_483_647); + }); }); diff --git a/tests/server/proxy-liveness.test.ts b/tests/server/proxy-liveness.test.ts index f48f4ed2f74..4bb49414e6a 100644 --- a/tests/server/proxy-liveness.test.ts +++ b/tests/server/proxy-liveness.test.ts @@ -1067,9 +1067,18 @@ describe("parseProbeTimeoutOverrideMs", () => { expect(parseProbeTimeoutOverrideMs("0")).toBeUndefined(); }); - test("defaults stay untouched when no override parses", () => { - expect(DEFAULT_PROBE_TIMEOUT_MS).toBeGreaterThan(0); - expect(SERVICE_STOP_LIVENESS.timeoutMs).toBeGreaterThanOrEqual(DEFAULT_PROBE_TIMEOUT_MS); - expect(START_OWNERSHIP_LIVENESS.timeoutMs).toBeGreaterThanOrEqual(DEFAULT_PROBE_TIMEOUT_MS); + test("accepts values up to the signed-32-bit ceiling and rejects anything larger", () => { + expect(parseProbeTimeoutOverrideMs("2147483647")).toBe(2_147_483_647); + expect(parseProbeTimeoutOverrideMs("2147483648")).toBeUndefined(); + expect(parseProbeTimeoutOverrideMs("99999999999999999999")).toBeUndefined(); + }); + + test("defaults are exactly the shipped ceilings when no override is present", () => { + // Exact values, deliberately: a leftover OCX_PROBE_TIMEOUT_MS from another test + // file in this shared module registry would pin the constants to it, and a + // range check here would let that leak through silently. + expect(DEFAULT_PROBE_TIMEOUT_MS).toBe(750); + expect(SERVICE_STOP_LIVENESS.timeoutMs).toBe(1500); + expect(START_OWNERSHIP_LIVENESS.timeoutMs).toBe(1500); }); });