From bb6d26beaae777638a951d870aa74b95ff81c4b5 Mon Sep 17 00:00:00 2001 From: Diego Cantarero <71346275+diegocantarero@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:50:17 -0600 Subject: [PATCH 01/11] Add post-sync readiness endpoint and bounded wait --- README.md | 23 + src/cli/help.ts | 12 + src/cli/index.ts | 52 +- src/cli/ready.ts | 293 ++++++++++ src/codex/desired-state.ts | 20 +- src/server/index.ts | 52 +- src/server/proxy-liveness.ts | 125 ++++- src/server/readiness.ts | 99 ++++ structure/03_catalog-and-subagents.md | 23 +- tests/cli-ready.test.ts | 778 ++++++++++++++++++++++++++ tests/cli-restart-health.test.ts | 110 +++- tests/proxy-liveness.test.ts | 476 +++++++++++++++- tests/server-live.test.ts | 262 ++++++++- tests/update-notify.test.ts | 5 +- 14 files changed, 2285 insertions(+), 45 deletions(-) create mode 100644 src/cli/ready.ts create mode 100644 src/server/readiness.ts create mode 100644 tests/cli-ready.test.ts diff --git a/README.md b/README.md index 7be6787be2..7a7c0836c2 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,8 @@ ocx start [--port 10100] # start the proxy in the foreground ocx stop # stop + restore native Codex ocx service [install|start|stop|status|uninstall|remove] # background service ocx codex-shim install # start the proxy on demand whenever `codex` launches +ocx health [--json] # check immediate proxy liveness +ocx ready [--json] [--wait [--timeout ]] # check post-sync readiness ocx status # is the proxy running? ocx gui # open the web dashboard ocx provider <...> # manage providers (list/add/edit/test/remove) @@ -151,6 +153,27 @@ ocx update [--tag preview] # update opencodex Unpinned starts may pick another free port if the preferred one is busy; an explicit `--port` never hops. Full reference: [CLI docs](https://opencodex.me/reference/cli/). +### Health and readiness + +`GET /healthz` reports immediate proxy liveness. The unauthenticated `GET /readyz` endpoint reports +post-sync readiness with the sanitized JSON identity `{service, version, uptime, pid, port, status}`. +It returns `200` when `status` is `ready`; `pending` and terminal `failed` return `503` with +`Retry-After: 1`. + +`ocx ready [--json] [--wait [--timeout ]]` performs one probe by default. `--wait` polls +for up to 45 seconds by default, but exits immediately when it observes terminal `failed`; +`--timeout ` sets a 1–300 second limit and requires `--wait`. CLI `--json` output is +`{ready, status, pid, port}`, where `status` is `ready`, `pending`, `failed`, or `unreachable`. + +| Exit | Result | +| --- | --- | +| `0` | Ready | +| `1` | Not ready: pending, failed, timeout, or unreachable | +| `64` | Invalid arguments | + +An older proxy without `/readyz` fails closed as `unreachable` with exit 1, while `ocx health` +remains compatible. + ### Autostart: service vs shim Use the **service** (`ocx service`) for an always-on proxy that restarts on crash. Use the diff --git a/src/cli/help.ts b/src/cli/help.ts index c74ba64752..dc12bc1bf0 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -249,6 +249,17 @@ const helpEntries: Record = { summary: "Check proxy health. Exits 0 if healthy, 1 otherwise.", details: ["Use --json for structured output: {ok, pid, port}."], }, + ready: { + usage: "ocx ready [--json] [--wait [--timeout ]]", + summary: "Check post-sync readiness. Exits 0 only when ready.", + details: [ + "Default is a single identity-checked /readyz probe.", + "--wait polls until ready or the timeout elapses (default 45s, max 300s).", + "--timeout requires --wait and accepts a positive integer (1..300).", + "--json emits {ready, status, pid, port}; status is one of ready|pending|failed|unreachable.", + "Invalid or unknown arguments exit 64. Not-ready, pending, failed, and timeout exit nonzero.", + ], + }, }; function packageVersion(): string { @@ -290,6 +301,7 @@ Usage: ocx restart Stop and restart the proxy ocx v2 multi_agent_v2 surface (status|on|off|mode|threads) ocx health [--json] Check proxy health (exit 0=healthy, 1=not) + ocx ready [--json] [--wait [--timeout ]] Check post-sync readiness (exit 0 only when ready) ocx provider Providers, connectivity, quota, and selected models ocx account Accounts, login/reauth, key pools, and quota controls ocx models Live/custom models, visibility, context, and shadow calls diff --git a/src/cli/index.ts b/src/cli/index.ts index 1865864365..fa40b839a2 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -26,6 +26,8 @@ import { installCrashGuards } from "../lib/crash-guard"; import { hasHelpFlag, printSubcommandUsage, printUsage, printVersion } from "./help"; import { findAvailablePort, isAddrInUse, PortUnavailableError, shouldPersistSelectedPort, waitForPortAvailable } from "../server/ports"; import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-liveness"; +import { createReadinessGate, runStartupReadinessSync } from "../server/readiness"; +import { parseReadyArgs, runReady, type ReadyArgs } from "./ready"; import { stopProxy } from "../lib/process-control"; import { loadServiceTokenFromFile } from "../lib/service-secrets"; import { diagnoseService, isServiceOwnershipError, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalled, uninstallServiceIfInstalled } from "../service"; @@ -69,6 +71,23 @@ if (command !== undefined && command !== "help" && hasHelpFlag(args.slice(1))) { process.exit(0); } +// P1: pre-parse `ocx ready` and reject invalid arguments with exit 64 BEFORE +// maybeAutoRestoreCodexShim (or any discovery/probe/filesystem-capable global +// preflight) runs. `ready --help` / `help ready` already exited above, so this +// only sees ready args without a help flag. Valid args are stashed so the +// switch dispatch can call runReady without a second parse. +let readyArgs: ReadyArgs | undefined; +if (command === "ready") { + const parsed = parseReadyArgs(args.slice(1)); + if (!parsed.ok) { + console.error("Usage: ocx ready [--json] [--wait [--timeout ]]"); + console.error(" --timeout requires --wait; must be a positive integer (1..300)."); + console.error(" Default wait timeout is 45 seconds."); + process.exit(parsed.code); + } + readyArgs = parsed.args; +} + maybeAutoRestoreCodexShim(command, args); function parsePortOption(): number | undefined { @@ -197,11 +216,16 @@ async function handleStart(options: { block?: boolean } = {}) { // between the probe and Bun.serve. Soft starts may re-pick; hard-pinned `--port` retries // the same port only (never hop — that was the remaining PR #152 gap). let port = await chooseListenPort(requestedPort); + // One private readiness gate for this startServer invocation, captured by the + // listener's closure. handleStart owns it and transitions it after the + // post-startup sync settles. A second startServer in the same process would + // get its own gate and could never reset/mutate this one. + const readinessGate = createReadinessGate(); let server: ReturnType; const localAttestationSecret = createLocalAttestationSecret(); for (let attempt = 0; ; attempt++) { try { - server = startServer(port, { localAttestationSecret }); + server = startServer(port, { localAttestationSecret, readinessGate }); // Prewarm the live provider model cache as soon as the port is bound so the // first GUI /v1/models (and syncModelsToCodex below) share one discovery flight // instead of racing duplicate upstream /models fetches. @@ -321,7 +345,12 @@ async function handleStart(options: { block?: boolean } = {}) { installShellHook(); await maybeShowStarPrompt(); // once-only Yes/No GitHub-star prompt on first interactive start - const startupSync = await syncCodexOnStartIfEnabled(port, config); + // Post-startup sync drives the readiness gate AND the #1046 stale app-server + // warning. `syncCodexOnStartIfEnabled` respects the Codex integration toggle + // (OFF → no sync) and reports whether anything was written; the readiness gate + // observes the real sync outcome (ok/warning) so /readyz never advertises a + // half-synced proxy as ready while /healthz stays live. + const startupSync = await syncCodexOnStartIfEnabled(port, config, undefined, readinessGate); // #1046: one warning per startup, after BOTH writes. The server's cache // invalidation happens first and the catalog sync second, so the mtime is only // final here — and neither write site warns on its own, or a boot that hits @@ -757,6 +786,17 @@ async function handleRecoverHistory() { console.log(`Recovered ${r.rows} legacy thread(s) to openai (${r.files} rollout file(s) updated).`); } +/** + * `ocx ready` — arguments are pre-parsed above (before + * maybeAutoRestoreCodexShim) so invalid usage exits 64 before any global + * preflight. This handler only runs the dependency-injected runner in ./ready + * and exits with the returned code; it performs no parsing and no I/O of its + * own. The full behavior is unit-testable without spawning a subprocess. + */ +async function handleReady(args: ReadyArgs): Promise { + process.exit(await runReady(args)); +} + switch (command) { case "init": case "setup": { @@ -1016,6 +1056,14 @@ switch (command) { } process.exit(live ? 0 : 1); } + case "ready": + // Fail-closed impossible-state guard: readyArgs is populated by the + // preparse block before maybeAutoRestoreCodexShim, so reaching here + // without it means dispatch diverged. Refuse with code 64 and perform + // NO I/O (no discovery/probe). process.exit is `never`, narrowing below. + if (!readyArgs) process.exit(64); + await handleReady(readyArgs); + break; case "provider": { const { handleProviderCommand } = await import("./provider"); await handleProviderCommand(args.slice(1)); diff --git a/src/cli/ready.ts b/src/cli/ready.ts new file mode 100644 index 0000000000..a19aa0645d --- /dev/null +++ b/src/cli/ready.ts @@ -0,0 +1,293 @@ +/** + * `ocx ready` — pure parser + dependency-injected runner. + * + * Lives outside cli/index.ts (which dispatches argv at module top level) so the + * full behavior is unit-testable without spawning a subprocess, opening a + * loopback socket, or touching the real HOME/CODEX_HOME. The runner returns an + * exit code; cli/index.ts only calls it and exits with the returned code. + * + * Contract (per P1 review): + * - Single wait loop. `--wait` waits for proxy discovery AND readiness within + * one bounded deadline (default 45s, max 300s). Without `--wait` a single + * identity-checked probe runs. + * - Invalid arguments return exit code 64 BEFORE any discovery/network work. + * - Output is sanitized: only the fixed `ready|pending|failed|unreachable` + * vocabulary, plus pid/port; never carries sync message, warning, path, + * provider, account, or error data. + */ +import { findLiveProxy, probeReadiness } from "../server/proxy-liveness"; + +/** Default --wait deadline (45s). */ +export const DEFAULT_READY_WAIT_TIMEOUT_SECONDS = 45; +/** Maximum allowed --wait deadline (300s). */ +export const MAX_READY_WAIT_TIMEOUT_SECONDS = 300; +const POLL_INTERVAL_MS = 500; +/** + * Per-call fetch ceiling for the production discovery/probe defaults. Each + * find/probe fetch is bounded both by the single wait deadline (so no single + * fetch outlives it) and by this ceiling (so a nearly-full 45s deadline does + * not become a 45-second hang on one request). Matches the underlying + * findLiveProxy/probeReadiness default of 750ms for the non-wait path. + */ +const IO_TIMEOUT_CAP_MS = 750; +/** + * Cap a remaining deadline budget to a positive per-call IO timeout. Stays + * positive (>= 1ms) and never exceeds the logical remaining time nor the + * per-call ceiling. Passed by the production defaults to findLiveProxy's / + * probeReadiness's timeoutMs so their fetch waits are bounded by the deadline. + */ +function capIoTimeout(remainingMs: number): number { + return Math.max(1, Math.min(remainingMs, IO_TIMEOUT_CAP_MS)); +} + +/** Fixed sanitized CLI status vocabulary. */ +export type CliReadinessStatus = "ready" | "pending" | "failed" | "unreachable"; + +export interface ReadyArgs { + json: boolean; + wait: boolean; + timeoutSeconds: number; +} + +export type ReadyParseResult = + | { ok: true; args: ReadyArgs } + | { ok: false; code: 64 }; + +/** + * Pure argument parser. Accepts `--json`, `--wait`, and + * `--wait --timeout ` (positive integer 1..300). Any unknown flag, + * positional argument, missing/invalid --timeout value, or `--timeout` without + * `--wait` is a usage error that must surface exit code 64. + */ +export function parseReadyArgs(argv: string[]): ReadyParseResult { + let json = false; + let wait = false; + let timeoutSeconds: number | undefined; + for (let i = 0; i < argv.length; i++) { + const flag = argv[i]; + if (flag === "--json") { + json = true; + continue; + } + if (flag === "--wait") { + wait = true; + continue; + } + if (flag === "--timeout") { + const raw = argv[i + 1]; + // Positive finite integer seconds only; /^[0-9]+$/ rejects negatives, decimals, hex, and "". + if (raw === undefined || !/^[0-9]+$/.test(raw)) return { ok: false, code: 64 }; + const n = Number(raw); + if (!Number.isInteger(n) || n < 1 || n > MAX_READY_WAIT_TIMEOUT_SECONDS) return { ok: false, code: 64 }; + timeoutSeconds = n; + i++; + continue; + } + // Any unknown flag or positional argument is a usage error. + return { ok: false, code: 64 }; + } + // --timeout only applies to --wait; pairing it with a single probe is a usage error. + if (timeoutSeconds !== undefined && !wait) return { ok: false, code: 64 }; + return { + ok: true, + args: { + json, + wait, + timeoutSeconds: timeoutSeconds ?? DEFAULT_READY_WAIT_TIMEOUT_SECONDS, + }, + }; +} + +export interface ReadyLive { + pid: number | null; + port: number; + hostname?: string; +} + +export interface ReadyProbe { + ready: boolean; + status: "ready" | "pending" | "failed" | null; + pid: number | null; + port: number | null; +} + +export interface ReadyIo { + /** + * Injected proxy discovery (defaults to the identity-checked findLiveProxy). + * Receives the positive remaining deadline budget (ms) in the --wait path so + * the production default can bound its fetch by the single deadline; receives + * `undefined` in the non-wait path so the default keeps its built-in timeout. + */ + findLive?: (remainingMs: number | undefined) => Promise; + /** + * Injected readiness probe (defaults to the strict probeReadiness). The third + * argument is the positive remaining deadline budget (ms) in the --wait path, + * and `undefined` in the non-wait path (default probe timeout preserved). + */ + probe?: ( + port: number, + opts: { hostname?: string; expectedPid?: number }, + remainingMs: number | undefined, + ) => Promise; + /** Injected sleep so tests can poll without real timers. */ + sleep?: (ms: number) => Promise; + /** Injected clock so the deadline is deterministic without real time. */ + now?: () => number; + /** Injected stdout (only `.log` is used). */ + stdout?: { log: (s: string) => void }; +} + +function sanitizeProbeStatus(status: ReadyProbe["status"]): Exclude { + if (status === "failed") return "failed"; + if (status === "pending") return "pending"; + return "unreachable"; +} + +function report( + args: ReadyArgs, + ready: boolean, + status: CliReadinessStatus, + pid: number | null, + port: number | null, + stdout: { log: (s: string) => void }, +): void { + if (args.json) { + stdout.log(JSON.stringify({ ready, status, pid, port })); + return; + } + switch (status) { + case "ready": + stdout.log(`Proxy ready (PID ${pid ?? "?"}, port ${port ?? "?"})`); + return; + case "pending": + stdout.log("Proxy running but not ready yet (pending)."); + return; + case "failed": + stdout.log("Proxy running but not ready (sync failed)."); + return; + case "unreachable": + stdout.log("Proxy not reachable or readiness unavailable."); + return; + } +} + +/** + * Run `ocx ready` over injected I/O. Returns the exit code (0 only when ready, + * 1 for not-ready/timeout, 64 for usage errors — though usage errors are + * normally caught by parseReadyArgs before this runs). The runner performs NO + * real subprocess/network work when the io injections are supplied. + */ +export async function runReady(args: ReadyArgs, io: ReadyIo = {}): Promise { + const find = io.findLive ?? (async (remainingMs: number | undefined) => { + // In the --wait path, forward an ABSOLUTE deadline derived from the real + // wall clock (Date.now, NOT the injected logical now) plus the remaining + // budget: the per-probe AbortSignal timeout is real wall-clock time, so the + // injected test clock must not govern the network deadline. findLiveProxy + // recomputes remaining before each candidate probe and bounds each fetch by + // it (plus the per-probe cap below). Non-wait path keeps the built-in default. + const live = await findLiveProxy( + remainingMs === undefined ? {} : { deadlineAt: Date.now() + remainingMs, timeoutMs: IO_TIMEOUT_CAP_MS }, + ); + return live ? { pid: live.pid, port: live.port, hostname: live.hostname } : null; + }); + const probe = io.probe ?? (async (port, opts, remainingMs) => + probeReadiness(port, opts, remainingMs === undefined ? {} : { timeoutMs: capIoTimeout(remainingMs) })); + const sleep = io.sleep ?? (ms => new Promise(resolve => setTimeout(resolve, ms))); + const now = io.now ?? Date.now; + const stdout = io.stdout ?? console; + + if (!args.wait) { + // Default: exactly one identity-checked readiness probe. No wait deadline — + // bounded only by the probe's own default timeout (remainingMs=undefined so + // the production defaults keep their built-in 750ms ceiling; no semantic + // regression vs. the single-probe behavior). + const live = await find(undefined); + if (!live) { + report(args, false, "unreachable", null, null, stdout); + return 1; + } + const p = await probe(live.port, { hostname: live.hostname, expectedPid: live.pid ?? undefined }, undefined); + if (p?.ready) { + report(args, true, "ready", p.pid, live.port, stdout); + return 0; + } + const status: CliReadinessStatus = sanitizeProbeStatus(p?.status ?? null); + report(args, false, status, p?.pid ?? live.pid ?? null, live.port, stdout); + return 1; + } + + // --wait: ONE loop bounded by a single deadline (default 45s, max 300s). The + // deadline is the single source of truth: + // - Before EVERY discovery and EVERY probe we compute remaining = deadline - + // now(); if it is non-positive we return code 1 WITHOUT starting that I/O. + // - After each awaited discovery/probe we re-read the clock; reached/exceeded + // means timeout wins (code 1), so a ready probe resolving at/after the + // deadline does NOT promote to ready. + // - No second discovery/probe may start once the deadline is reached. + // - Each sleep is capped to the positive remaining time. The remaining + // budget is also forwarded to the production discovery/probe defaults so + // their individual fetch waits are bounded by this same deadline. + const deadline = now() + args.timeoutSeconds * 1000; + let lastStatus: CliReadinessStatus = "unreachable"; + let lastPid: number | null = null; + let lastPort: number | null = null; + for (;;) { + // Before discovery: refuse to start I/O when the deadline has elapsed. + const remainingBeforeFind = deadline - now(); + if (remainingBeforeFind <= 0) { + report(args, false, lastStatus, lastPid, lastPort, stdout); + return 1; + } + const live = await find(remainingBeforeFind); + // After discovery: re-read the clock. Reached/exceeded → timeout wins and + // no probe is started (no second discovery/probe after the deadline). + let clock = now(); + if (clock >= deadline) { + report(args, false, lastStatus, lastPid, lastPort, stdout); + return 1; + } + if (live) { + lastPort = live.port; + // Before probe: remaining derived from the post-discovery reading + // (guaranteed positive because clock < deadline above). This bounds the + // probe's fetch wait by the single deadline. + const remainingBeforeProbe = deadline - clock; + const p = await probe( + live.port, + { hostname: live.hostname, expectedPid: live.pid ?? undefined }, + remainingBeforeProbe, + ); + lastStatus = sanitizeProbeStatus(p?.status ?? null); + lastPid = p?.pid ?? live.pid ?? null; + // After every awaited probe: re-read the clock BEFORE terminal-failed or + // ready handling. Reached/exceeded → timeout wins (code 1) with the last + // sanitized status — a failed/ready probe resolving at/after the deadline + // must not take the terminal-failed shortcut or promote to ready. + clock = now(); + if (clock >= deadline) { + report(args, false, lastStatus, lastPid, live.port, stdout); + return 1; + } + // `failed` is terminal only while still before the deadline: the startup + // sync has settled unsuccessfully, so waiting cannot change this gate. + // Report it immediately instead of consuming the rest of the timeout. + if (p?.status === "failed") { + report(args, false, "failed", lastPid, live.port, stdout); + return 1; + } + if (p?.ready) { + report(args, true, "ready", p.pid, live.port, stdout); + return 0; + } + } + // Cap the sleep to the positive remaining time so we never sleep past the + // deadline. `clock` is the latest reading (post-discovery when no live + // proxy was found, post-probe otherwise) and is known to be < deadline. + const remainingForSleep = deadline - clock; + if (remainingForSleep <= 0) { + report(args, false, lastStatus, lastPid, lastPort, stdout); + return 1; + } + await sleep(Math.min(POLL_INTERVAL_MS, remainingForSleep)); + } +} diff --git a/src/codex/desired-state.ts b/src/codex/desired-state.ts index f75f261b31..6862796c16 100644 --- a/src/codex/desired-state.ts +++ b/src/codex/desired-state.ts @@ -22,6 +22,7 @@ */ import { loadConfig, mutatePersistedConfig } from "../config"; import type { OcxClientIntegrationsConfig, OcxConfig } from "../types"; +import { runStartupReadinessSync, type ReadinessGate, type SyncOutcomeLike } from "../server/readiness"; /** Clients whose durable intent this module owns. */ export type DurableIntentClientId = keyof OcxClientIntegrationsConfig; @@ -36,8 +37,11 @@ export type DurableIntentClientId = keyof OcxClientIntegrationsConfig; export interface CodexStartupSyncOutcome { catalogWritten: boolean; cacheSynced: boolean; + /** Readiness-observable fields carried by the raw startup sync. */ + ok?: boolean; + warning?: string; } -export type CodexStartupSync = (port: number) => Promise; +export type CodexStartupSync = (port: number) => Promise; export type CodexDesiredStateResult = | { readonly ok: true; readonly status: "committed" | "unchanged"; readonly enabled: boolean } @@ -161,13 +165,23 @@ export async function syncCodexOnStartIfEnabled( port: number, config: Pick, sync: CodexStartupSync = defaultStartupSync, + readinessGate?: ReadinessGate, ): Promise<{ ran: boolean; catalogWritten: boolean; cacheSynced: boolean }> { if (!codexIntegrationEnabled(config)) { + // The user explicitly turned Codex off: there is nothing to sync, so the + // proxy is ready as soon as it is up. The gate is driven here so /readyz + // does not stay pending forever for a deployment that deliberately disabled + // the native Codex integration. + readinessGate?.markReady(); return { ran: false, catalogWritten: false, cacheSynced: false }; } // The `.catch` is deliberate and stays: a failure to APPLY must not stop the - // proxy from coming up. A failed sync simply reports no writes. - const outcome = await sync(port).catch(() => undefined); + // proxy from coming up. A failed sync simply reports no writes. The readiness + // gate observes the real outcome so /readyz reflects the sync state exactly as + // the PR contract defines (ready only on ok=true with no warning). + const outcome = readinessGate + ? await runStartupReadinessSync(readinessGate, async () => (await sync(port)) ?? null) + : await sync(port).catch(() => undefined); return { ran: true, catalogWritten: outcome?.catalogWritten === true, diff --git a/src/server/index.ts b/src/server/index.ts index 7d08fc422c..173f98d2c9 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -180,6 +180,7 @@ import { createLocalAttestationProof, createLocalAttestationSecret, } from "../lib/local-management-attestation"; +import { createReadinessGate, type ReadinessGate } from "./readiness"; const MAX_WS_FRAME_BYTES = 50 * 1024 * 1024; const WEBSOCKET_IDLE_TIMEOUT_SECONDS = 0; @@ -365,6 +366,8 @@ export interface StartServerDeps { liveSidebandWebSocketFactory?: LiveSidebandWebSocketFactory; /** Test-only seam; production derives a fresh local-attestation secret per process. */ localAttestationSecret?: string; + /** Optional readiness gate; a fresh pending gate is created when omitted. */ + readinessGate?: ReadinessGate; } /* @@ -390,6 +393,8 @@ export function consumeStartupCacheInvalidationWrite(): boolean { export function startServer(port?: number, deps: StartServerDeps = {}) { const localAttestationSecret = deps.localAttestationSecret ?? createLocalAttestationSecret(); +export function startServer(port?: number, deps: StartServerDeps = {}): Server { + const localAttestationSecret = deps.localAttestationSecret ?? createLocalAttestationSecret(); const config = runAlibabaRegionStartupMigration(runOpenAiTierStartupMigration(loadConfig())); setLiveStateStoreConfig(config); applyProxyEnv(config); @@ -523,14 +528,25 @@ export function startServer(port?: number, deps: StartServerDeps = {}) { return response; } + // Readiness gate: one PRIVATE controller per startServer invocation, captured + // by this listener's closure. Starting/failing a second server in the same + // process can never reset or mutate this gate. handleStart creates the gate, + // passes it in, and transitions it after the post-startup sync settles. When + // no gate is supplied (tests, ad-hoc starts) a fresh pending gate is created. + const readinessGate = deps.readinessGate ?? createReadinessGate(); + // Actual bound port, filled in after Bun.serve binds so /readyz reports the + // real ephemeral port for startServer(0). /healthz keeps its existing port + // field (the requested listenPort) byte-for-byte. + let boundPort: number | null = null; + const nativeMainLifecycle = startNativeMainStartupLifecycle(deps.nativeMainStartup); let server: Server; try { server = Bun.serve({ - port: listenPort, - hostname: bindHost, - idleTimeout: 255, - async fetch(req, requestServer): Promise { + port: listenPort, + hostname: bindHost, + idleTimeout: 255, + async fetch(req, requestServer): Promise { const url = new URL(req.url); markActivity(`${req.method} ${url.pathname}`); @@ -590,6 +606,33 @@ export function startServer(port?: number, deps: StartServerDeps = {}) { return response; } + // Readiness: like /healthz this is exact GET and unauthenticated (so a client can + // back off BEFORE knowing the admission token), but stricter than liveness. The + // body carries only sanitized identity + the fixed status enum; the sync message, + // warning text, catalog path, provider output, and account data are never exposed. + // POST or "/readyz/" must NOT match (exact pathname + GET method) so they fall + // through to the JSON 404 guard rather than being silently accepted. + if (url.pathname === "/readyz" && req.method === "GET") { + const status = readinessGate.getStatus(); + const body = { + service: "opencodex", + version: VERSION, + uptime: process.uptime(), + pid: process.pid, + port: boundPort ?? listenPort, + status, + }; + if (status === "ready") { + return jsonResponse(body, 200, req, config); + } + // Pending/failed: 503 with a conservative Retry-After so well-behaved clients + // (and `ocx ready --wait`) back off instead of hot-looping. + const resp = jsonResponse(body, 503, req, config); + const headers = new Headers(resp.headers); + headers.set("Retry-After", "1"); + return new Response(resp.body, { status: 503, headers }); + } + if (url.pathname.startsWith("/api/")) { const apiAuthError = requireManagementAuth(req, managementAuth, config); if (apiAuthError) return withManagementCors(apiAuthError, req, config); @@ -1322,6 +1365,7 @@ export function startServer(port?: number, deps: StartServerDeps = {}) { }); setServerRef(server); const actualPort = server.port ?? listenPort; + boundPort = actualPort; setCorsOrigin(actualPort); console.log(`🚀 opencodex proxy running on http://localhost:${actualPort}`); diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index 367ec7b1f2..bcd474826b 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -138,6 +138,12 @@ export async function findLiveProxy(io: LivenessIo = {}): Promise + deadlineAt !== undefined && nowFn() >= deadlineAt; // The cheap pid is discovery-only. Before it can appear in a returned (killable) result // it must pass the full identity check AND the verifier must echo the exact candidate — @@ -160,8 +166,9 @@ export async function findLiveProxy(io: LivenessIo = {}): Promise(["ready", "pending", "failed"]); + +/** + * Validate a parsed /readyz body against the strict contract. Returns the + * sanitized probe result, or `null` when the body is foreign, legacy, + * malformed, or fails the pid/port checks. Pure (no I/O) so it is fully + * deterministic and unit-testable. + */ +export function validateReadyzBody( + body: unknown, + port: number, + opts: { expectedPid?: number } = {}, +): ReadinessProbeResult | null { + if (!body || typeof body !== "object") return null; + const b = body as ReadyzBody; + if (b.service !== "opencodex") return null; + if (typeof b.version !== "string" || b.version.length === 0) return null; + if (typeof b.uptime !== "number" || !Number.isFinite(b.uptime) || b.uptime < 0) return null; + if (typeof b.pid !== "number" || !Number.isInteger(b.pid) || b.pid <= 0) return null; + if ( + typeof b.port !== "number" + || !Number.isInteger(b.port) + || b.port < 1 + || b.port > 65535 + || b.port !== port + ) return null; + if (typeof b.status !== "string" || !READYZ_STATUS_VALUES.has(b.status as "ready" | "pending" | "failed")) return null; + const status = b.status as "ready" | "pending" | "failed"; + if (opts.expectedPid !== undefined && b.pid !== opts.expectedPid) return null; + return { ready: status === "ready", status, pid: b.pid, port: b.port }; +} + +/** + * Identity- and contract-checked /readyz probe. Returns `null` when the + * endpoint is unreachable or the body fails the strict contract (foreign 200, + * legacy health-only body, non-JSON, missing/malformed/mismatched fields, + * wrong port/pid, or an HTTP/body-status inconsistency). Returns + * `{ready:false, ...}` when the body is ours but pending or failed. Returns + * `{ready:true, ...}` ONLY for a valid 200 body with `status:"ready"` and (when + * requested) a matching pid. + */ +export async function probeReadiness( + port: number, + opts: { hostname?: string; expectedPid?: number } = {}, + io: ReadinessProbeIo = {}, +): Promise { + const fetchFn = io.fetchFn ?? fetch; + try { + const res = await fetchFn(`http://${probeHostname(opts.hostname)}:${port}/readyz`, { + signal: AbortSignal.timeout(io.timeoutMs ?? 750), + }); + // Parse even on 503: /readyz returns JSON with a sanitized status while pending. + const body = (await res.json().catch(() => null)) as unknown; + const parsed = validateReadyzBody(body, port, opts); + if (!parsed) return null; + // HTTP/body-status consistency: ready requires 200; pending/failed require 503. + if (parsed.status === "ready" && res.status !== 200) return null; + if (parsed.status !== "ready" && res.status !== 503) return null; + return parsed; + } catch { + return null; + } +} diff --git a/src/server/readiness.ts b/src/server/readiness.ts new file mode 100644 index 0000000000..5dc3c2021f --- /dev/null +++ b/src/server/readiness.ts @@ -0,0 +1,99 @@ +/** + * Per-server readiness gate for the opencodex proxy. + * + * `GET /healthz` answers "is the process alive and serving HTTP?" the instant the + * listener binds. Readiness is stricter: the proxy is "ready" only after the + * post-startup Codex catalog/config sync (`syncModelsToCodex`) has settled with + * `ok=true` and no catalog-sync warning. Until then the process is live (Codex can + * open a socket) but not ready (a request would race the sync or hit a stale + * catalog), so clients should back off. + * + * Design contract (per P1 review): + * - NO module-global mutable state. Each `startServer` invocation gets its own + * private gate via `createReadinessGate()`, captured by that listener's + * closure. Starting/failing a second server in the same process can never + * reset or mutate the first server's gate. + * - Only the fixed sanitized status enum `pending | ready | failed` is stored + * and exposed. There is no `changedAt`, no free-form failure reason, no sync + * message, no warning text, no catalog path, no provider output, and no + * account data — those are private diagnostic data and are never exposed by + * `/readyz`. + */ + +/** Sanitized readiness state. Exactly these three values, nothing else. */ +export type ReadinessStatus = "pending" | "ready" | "failed"; + +/** + * Private per-server readiness controller. The status starts at `pending` and + * transitions at most once (to `ready` or `failed`) when the post-startup sync + * settles. The gate is owned by the listener closure that requested it. + */ +export interface ReadinessGate { + /** Current sanitized status. */ + getStatus(): ReadinessStatus; + /** Mark the proxy ready (post-startup sync settled cleanly). */ + markReady(): void; + /** Mark the proxy failed. No reason is stored or exposed. */ + markFailed(): void; +} + +/** + * Create a fresh private gate for one `startServer` invocation. The returned + * gate is the only way to read or mutate this server's readiness. + */ +export function createReadinessGate(): ReadinessGate { + let status: ReadinessStatus = "pending"; + return { + getStatus: () => status, + markReady: () => { + if (status === "pending") status = "ready"; + }, + markFailed: () => { + if (status === "pending") status = "failed"; + }, + }; +} + +/** Minimal shape of the post-startup sync outcome the gate cares about. */ +export interface SyncOutcomeLike { + ok?: boolean; + warning?: string; + /** #1046: whether the sync actually rewrote the on-disk catalog/cache. */ + catalogWritten?: boolean; + cacheSynced?: boolean; +} + +/** + * Drive the gate from the post-startup sync. Awaits `syncFn`; the gate goes to + * `ready` ONLY on `ok=true` with no nonempty warning. A throw, `null`, `ok=false`, + * or a nonempty warning transitions to `failed`. Used directly by `handleStart` + * so the startup transition is unit-testable without spawning the proxy. Returns + * the raw sync outcome so a caller that also needs the #1046 write flags (did the + * sync actually write the catalog/cache?) can keep them without a second call. + */ +export async function runStartupReadinessSync( + gate: ReadinessGate, + syncFn: () => Promise, +): Promise { + let result: SyncOutcomeLike | null; + try { + result = await syncFn(); + } catch { + gate.markFailed(); + return null; + } + if (result === null) { + gate.markFailed(); + return null; + } + if (result.ok !== true) { + gate.markFailed(); + return result; + } + if (result.warning !== undefined && result.warning !== "") { + gate.markFailed(); + return result; + } + gate.markReady(); + return result; +} diff --git a/structure/03_catalog-and-subagents.md b/structure/03_catalog-and-subagents.md index ea34fa164a..ba4ab300bf 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -29,10 +29,25 @@ bundled catalog never contains those rows. Codex App model picker visibility comes from this shared catalog, not from patching the App. -Provider live-model lists are cached with a configured TTL (`src/codex/model-cache.ts`). Adding, -deleting, or editing a provider's shape clears that per-provider cache; a disabled-only change -deliberately does not, because a disabled provider is already excluded from the catalog gather -instead. Codex's own `models_cache.json` is a different cache, invalidated by catalog refresh. + Provider live-model lists are cached with a configured TTL (`src/codex/model-cache.ts`). Adding, + deleting, or editing a provider's shape clears that per-provider cache; a disabled-only change + deliberately does not, because a disabled provider is already excluded from the catalog gather + instead. Codex's own `models_cache.json` is a different cache, invalidated by catalog refresh. + + ## Startup readiness + +Each `startServer` invocation owns a private, one-shot readiness gate created before the listener +binds. `handleStart` supplies its gate and transitions it after the shared catalog sync settles. +Calls without a supplied gate receive a fresh private gate that intentionally remains pending. Only +`ok: true` with no nonempty warning becomes ready; `null`, a throw, `ok !== true`, or a nonempty +warning becomes failed. State is isolated per server instance. + +Exact unauthenticated `GET /readyz` returns sanitized identity fields plus pending, ready, or failed. +The `ocx ready` probe validates the service, version, uptime, PID, port, status, and HTTP/status +pairing. With `--wait`, it applies one absolute deadline across discovery, readiness probes, polling, +and sleeps; the single-probe path preserves the existing per-call timeout without a wait deadline. +Older proxies without `/readyz` fail closed as unreachable. `/healthz` remains the separate liveness +contract. ## Entry shape diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts new file mode 100644 index 0000000000..036663e073 --- /dev/null +++ b/tests/cli-ready.test.ts @@ -0,0 +1,778 @@ +/** + * Injected tests for `ocx ready` (parseReadyArgs + runReady). + * + * These REPLACE the prior subprocess/network/no-proxy tests for the ready + * command. Everything is driven over injected findLive / probe / sleep / now / + * stdout stubs, so the suite never opens a real loopback socket, spawns a + * subprocess, or touches the real HOME/CODEX_HOME. + */ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { + DEFAULT_READY_WAIT_TIMEOUT_SECONDS, + MAX_READY_WAIT_TIMEOUT_SECONDS, + parseReadyArgs, + runReady, + type ReadyArgs, + type ReadyIo, + type ReadyLive, + type ReadyProbe, +} from "../src/cli/ready"; + +// ── parseReadyArgs ──────────────────────────────────────────────────────────── + +describe("parseReadyArgs", () => { + function ok(args: ReadyArgs, json: boolean, wait: boolean, timeoutSeconds: number): void { + expect(args.json).toBe(json); + expect(args.wait).toBe(wait); + expect(args.timeoutSeconds).toBe(timeoutSeconds); + } + + test("empty argv → single probe, default json/wait, default timeout", () => { + const r = parseReadyArgs([]); + expect(r.ok).toBe(true); + if (r.ok) ok(r.args, false, false, DEFAULT_READY_WAIT_TIMEOUT_SECONDS); + }); + + test("--json alone", () => { + const r = parseReadyArgs(["--json"]); + expect(r.ok).toBe(true); + if (r.ok) ok(r.args, true, false, DEFAULT_READY_WAIT_TIMEOUT_SECONDS); + }); + + test("--wait alone uses default timeout", () => { + const r = parseReadyArgs(["--wait"]); + expect(r.ok).toBe(true); + if (r.ok) ok(r.args, false, true, DEFAULT_READY_WAIT_TIMEOUT_SECONDS); + }); + + test("--wait --json --timeout N", () => { + const r = parseReadyArgs(["--wait", "--json", "--timeout", "10"]); + expect(r.ok).toBe(true); + if (r.ok) ok(r.args, true, true, 10); + }); + + test("default timeout is 45s", () => { + expect(DEFAULT_READY_WAIT_TIMEOUT_SECONDS).toBe(45); + }); + + test("max timeout is 300s", () => { + expect(MAX_READY_WAIT_TIMEOUT_SECONDS).toBe(300); + }); + + test("--timeout 300 is accepted (upper bound)", () => { + const r = parseReadyArgs(["--wait", "--timeout", "300"]); + expect(r.ok).toBe(true); + if (r.ok) expect(r.args.timeoutSeconds).toBe(300); + }); + + test("--timeout without --wait is a usage error (code 64)", () => { + expect(parseReadyArgs(["--timeout", "5"])).toEqual({ ok: false, code: 64 }); + }); + + test("unknown flag is a usage error", () => { + expect(parseReadyArgs(["--nope"])).toEqual({ ok: false, code: 64 }); + }); + + test("positional argument is a usage error", () => { + expect(parseReadyArgs(["now"])).toEqual({ ok: false, code: 64 }); + }); + + test("--timeout with non-numeric value is a usage error", () => { + expect(parseReadyArgs(["--wait", "--timeout", "abc"])).toEqual({ ok: false, code: 64 }); + }); + + test("--timeout with missing value is a usage error", () => { + expect(parseReadyArgs(["--wait", "--timeout"])).toEqual({ ok: false, code: 64 }); + }); + + test("--timeout zero is a usage error (must be positive)", () => { + expect(parseReadyArgs(["--wait", "--timeout", "0"])).toEqual({ ok: false, code: 64 }); + }); + + test("--timeout above 300 is a usage error (max enforced)", () => { + expect(parseReadyArgs(["--wait", "--timeout", "301"])).toEqual({ ok: false, code: 64 }); + }); + + test("--timeout negative is a usage error", () => { + expect(parseReadyArgs(["--wait", "--timeout", "-5"])).toEqual({ ok: false, code: 64 }); + }); + + test("--timeout decimal is a usage error", () => { + expect(parseReadyArgs(["--wait", "--timeout", "1.5"])).toEqual({ ok: false, code: 64 }); + }); +}); + +// ── runReady over injected io ───────────────────────────────────────────────── + +function captureIo(): { io: ReadyIo; out: string[] } { + const out: string[] = []; + const io: ReadyIo = { + stdout: { log: (s: string) => { out.push(s); } }, + }; + return { io, out }; +} + +const LIVE: ReadyLive = { pid: 4242, port: 10100, hostname: undefined }; +const READY_PROBE: ReadyProbe = { ready: true, status: "ready", pid: 4242, port: 10100 }; +const PENDING_PROBE: ReadyProbe = { ready: false, status: "pending", pid: 4242, port: 10100 }; +const FAILED_PROBE: ReadyProbe = { ready: false, status: "failed", pid: 4242, port: 10100 }; + +describe("runReady single probe (no --wait)", () => { + test("ready probe exits 0 and prints the ready plain line", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: false, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { ...io, findLive: async () => LIVE, probe: async () => READY_PROBE }, + ); + expect(code).toBe(0); + expect(out.join("")).toContain("Proxy ready (PID 4242, port 10100)"); + }); + + test("ready probe --json emits sanitized JSON and exits 0", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: true, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { ...io, findLive: async () => LIVE, probe: async () => READY_PROBE }, + ); + expect(code).toBe(0); + const parsed = JSON.parse(out.join("")); + expect(parsed).toEqual({ ready: true, status: "ready", pid: 4242, port: 10100 }); + // Sanitized: no urls/paths/errors/provider data. + expect(JSON.stringify(parsed)).not.toContain("http"); + expect(JSON.stringify(parsed)).not.toContain("error"); + }); + + test("pending probe exits 1 with the pending line", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: false, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { ...io, findLive: async () => LIVE, probe: async () => PENDING_PROBE }, + ); + expect(code).toBe(1); + expect(out.join("")).toContain("not ready yet (pending)"); + }); + + test("failed probe exits 1 with the failed line", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: false, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { ...io, findLive: async () => LIVE, probe: async () => FAILED_PROBE }, + ); + expect(code).toBe(1); + expect(out.join("")).toContain("sync failed"); + }); + + test("no live proxy exits 1 with unreachable (--json sanitized)", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: true, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { ...io, findLive: async () => null, probe: async () => READY_PROBE }, + ); + expect(code).toBe(1); + const parsed = JSON.parse(out.join("")); + expect(parsed).toEqual({ ready: false, status: "unreachable", pid: null, port: null }); + expect(JSON.stringify(parsed)).not.toContain("http"); + expect(JSON.stringify(parsed)).not.toContain("error"); + }); + + test("foreign/invalid probe body (null) counts as unreachable, exits 1", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: false, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { ...io, findLive: async () => LIVE, probe: async () => null }, + ); + expect(code).toBe(1); + expect(out.join("")).toContain("not reachable"); + }); + + test("the probe receives expectedPid from the discovered live pid", async () => { + const { io } = captureIo(); + const seen: Array<{ expectedPid?: number }> = []; + const code = await runReady( + { json: false, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { + ...io, + findLive: async () => LIVE, + probe: async (_port, opts) => { seen.push(opts); return READY_PROBE; }, + }, + ); + expect(code).toBe(0); + expect(seen).toEqual([{ hostname: undefined, expectedPid: 4242 }]); + }); +}); + +describe("runReady --wait (single bounded loop, deterministic)", () => { + test("transitions discovery → ready within the deadline, exits 0", async () => { + const { io, out } = captureIo(); + let t = 0; + let findCalls = 0; + const probeBodies = [null, PENDING_PROBE, READY_PROBE]; + let probeCalls = 0; + const code = await runReady( + { json: false, wait: true, timeoutSeconds: 5 }, + { + ...io, + // First discovery returns null (proxy not up yet); second returns LIVE. + findLive: async () => { findCalls++; return findCalls < 2 ? null : LIVE; }, + probe: async () => probeBodies[Math.min(probeCalls++, probeBodies.length - 1)]!, + now: () => (t += 100), // 100, 200, 300 … never crosses the 5000ms deadline. + sleep: async () => {}, + }, + ); + expect(code).toBe(0); + expect(out.join("")).toContain("Proxy ready"); + expect(findCalls).toBeGreaterThanOrEqual(2); + expect(probeCalls).toBeGreaterThanOrEqual(2); + }); + + test("times out when the proxy never appears, exits 1 (--json)", async () => { + const { io, out } = captureIo(); + let t = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => null, + probe: async () => READY_PROBE, + now: () => (t += 500), // 500, 1000, 1500 … crosses 1000 after the 2nd iteration. + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + const parsed = JSON.parse(out.join("")); + expect(parsed).toEqual({ ready: false, status: "unreachable", pid: null, port: null }); + }); + + test("times out when the body stays pending, exits 1 with pending", async () => { + const { io, out } = captureIo(); + let t = 0; + const code = await runReady( + { json: false, wait: true, timeoutSeconds: 3 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => PENDING_PROBE, + now: () => (t += 1000), // 1000, 2000, 3000, 4000 crosses the 3000 deadline. + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + expect(out.join("")).toContain("not ready yet (pending)"); + }); + + test("failed is terminal: exits 1 immediately without polling or consuming timeout", async () => { + const { io, out } = captureIo(); + let findCalls = 0; + let probeCalls = 0; + let sleepCalls = 0; + let nowCalls = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 300 }, + { + ...io, + findLive: async () => { findCalls++; return LIVE; }, + probe: async () => { probeCalls++; return FAILED_PROBE; }, + now: () => { nowCalls++; return 0; }, + sleep: async () => { sleepCalls++; }, + }, + ); + expect(code).toBe(1); + expect(JSON.parse(out.join(""))).toEqual({ ready: false, status: "failed", pid: 4242, port: 10100 }); + expect(findCalls).toBe(1); + expect(probeCalls).toBe(1); + expect(sleepCalls).toBe(0); + // The hard-deadline loop reads the clock at each checkpoint (init, before + // find, after find, after probe) so timeout can still win at/after the + // deadline; the terminal failed path then returns BEFORE any sleep/poll, + // so the 300s timeout is never consumed waiting after a before-deadline + // failure. + expect(nowCalls).toBe(4); + }); + + test("never counts a foreign/invalid probe as ready, then times out", async () => { + const { io, out } = captureIo(); + let t = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => null, // foreign / invalid contract every time + now: () => (t += 500), + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + const parsed = JSON.parse(out.join("")); + // Foreign/invalid bodies never promote to pending; status stays unreachable. + expect(parsed.status).toBe("unreachable"); + expect(parsed.ready).toBe(false); + }); + + test("uses the configured --timeout value as the single deadline", async () => { + const { io } = captureIo(); + let t = 0; + const seenNow: number[] = []; + await runReady( + { json: false, wait: true, timeoutSeconds: 7 }, + { + ...io, + findLive: async () => null, + probe: async () => READY_PROBE, + now: () => { t += 1000; seenNow.push(t); return t; }, + sleep: async () => {}, + }, + ); + // The deadline is 7000ms; the loop stops once now() >= 7000. + const firstOverDeadline = seenNow.find(n => n >= 7000); + expect(firstOverDeadline).toBeGreaterThanOrEqual(7000); + expect(firstOverDeadline).toBeLessThan(7000 + 1000); + }); +}); + +// ── deadline correctness: timeout wins at/after the deadline ────────────────── +// Deterministic regression for the deadline contract: a ready probe that +// resolves AT OR AFTER the deadline must NOT win — the bounded wait already +// expired, so the exit code is 1. A ready probe that resolves strictly before +// the deadline wins (code 0). The clock is a queued stub so the exact +// post-probe reading is pinned, not a side effect of real elapsed time. +describe("runReady --wait deadline correctness", () => { + // Queue-based clock: returns values[i], then holds the last value. This lets + // the test pin the exact reading after the awaited discovery/probe instead of + // relying on real elapsed time. + function seqNow(values: number[]): () => number { + let i = 0; + return () => { + const v = values[Math.min(i, values.length - 1)]; + i++; + return v; + }; + } + + test("timeout=1000ms: a ready probe resolving at 1501ms (past deadline) → code 1", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => READY_PROBE, + // deadline = 0 + 1000 = 1000. + // checkpoints: init=0 → before-find=0 (starts find) → after-find=0 (<1000, + // starts probe) → after-probe=1501 (≥ deadline, ready does NOT win). + now: seqNow([0, 0, 0, 1501]), + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + // Timeout won: the ready signal is not advertised. + const parsed = JSON.parse(out.join("")); + expect(parsed.ready).toBe(false); + }); + + test("timeout=1000ms: a ready probe resolving at 999ms (strictly before deadline) → code 0", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: false, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => READY_PROBE, + // deadline = 0 + 1000 = 1000; the post-probe clock reads 999 < 1000 + // (and 999 is held for every subsequent read). + now: seqNow([0, 999]), + sleep: async () => {}, + }, + ); + expect(code).toBe(0); + expect(out.join("")).toContain("Proxy ready"); + }); + + test("exact deadline: a ready probe resolving AT the deadline (now === deadline) → code 1 (timeout wins)", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => READY_PROBE, + // deadline = 0 + 1000 = 1000. + // checkpoints: init=0 → before-find=0 → after-find=0 → after-probe=1000. + // The contract is "reached/exceeded → timeout wins", so now === deadline + // does NOT count as before-deadline. + now: seqNow([0, 0, 0, 1000]), + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + const parsed = JSON.parse(out.join("")); + expect(parsed.ready).toBe(false); + }); + + test("exact deadline: a failed probe resolving AT the deadline → code 1 (timeout wins over terminal failed)", async () => { + const { io, out } = captureIo(); + let sleepCalls = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => FAILED_PROBE, + // deadline = 0 + 1000 = 1000. + // checkpoints: init=0 → before-find=0 → after-find=0 → after-probe=1000. + // Post-probe clock is checked BEFORE terminal-failed handling, so + // now === deadline → timeout wins (code 1, sanitized last status). + now: seqNow([0, 0, 0, 1000]), + sleep: async () => { sleepCalls++; }, + }, + ); + expect(code).toBe(1); + expect(sleepCalls).toBe(0); + const parsed = JSON.parse(out.join("")); + expect(parsed).toEqual({ ready: false, status: "failed", pid: 4242, port: 10100 }); + expect(JSON.stringify(parsed)).not.toContain("http"); + expect(JSON.stringify(parsed)).not.toContain("error"); + }); + + test("past deadline: a failed probe resolving after the deadline → code 1 (timeout wins over terminal failed)", async () => { + const { io, out } = captureIo(); + let sleepCalls = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => FAILED_PROBE, + // deadline = 0 + 1000 = 1000. + // checkpoints: init=0 → before-find=0 → after-find=0 → after-probe=1501. + // Past-deadline failed must not take the terminal-failed shortcut. + now: seqNow([0, 0, 0, 1501]), + sleep: async () => { sleepCalls++; }, + }, + ); + expect(code).toBe(1); + expect(sleepCalls).toBe(0); + const parsed = JSON.parse(out.join("")); + expect(parsed).toEqual({ ready: false, status: "failed", pid: 4242, port: 10100 }); + expect(JSON.stringify(parsed)).not.toContain("http"); + expect(JSON.stringify(parsed)).not.toContain("error"); + }); + + test("every sleep is capped to the positive remaining time (never past the deadline, never negative)", async () => { + const { io } = captureIo(); + const sleeps: number[] = []; + const code = await runReady( + { json: false, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => PENDING_PROBE, + // deadline = 0 + 1000 = 1000. + // iter1: init=0 → before-find=0 → after-find=0 → after-probe=800. + // remaining for sleep = 1000-800 = 200 → sleep capped to 200 (not 500). + // iter2: before-find=1600 → remaining=-600 ≤ 0 → return 1, no sleep. + now: seqNow([0, 0, 0, 800, 1600]), + sleep: async (ms) => { sleeps.push(ms); }, + }, + ); + expect(code).toBe(1); + // The single sleep was capped to the remaining 200ms, never the full 500ms + // poll interval, and never negative. + expect(sleeps).toEqual([200]); + for (const ms of sleeps) { + expect(ms).toBeGreaterThan(0); + expect(ms).toBeLessThanOrEqual(500); + } + }); + + // ── hard-deadline I/O gating (P1) ──────────────────────────────────────────── + // The deadline must gate I/O, not just sleeps. Before EVERY discovery and + // EVERY probe the loop computes remaining = deadline - now() and refuses to + // start that I/O when it is non-positive; after each awaited discovery/probe + // it re-reads the clock and a reached/exceeded deadline wins (code 1). No + // second discovery/probe may start once the deadline is reached. + test("deadline already expired before first I/O → finds=0, probes=0, code 1", async () => { + const { io, out } = captureIo(); + let findCalls = 0; + let probeCalls = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => { findCalls++; return LIVE; }, + probe: async () => { probeCalls++; return READY_PROBE; }, + // deadline = 0 + 1000 = 1000; before-find reads 1001 ≥ 1000 → no I/O. + now: seqNow([0, 1001]), + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + expect(findCalls).toBe(0); + expect(probeCalls).toBe(0); + const parsed = JSON.parse(out.join("")); + expect(parsed.ready).toBe(false); + }); + + test("first find crosses deadline → finds=1, probes=0, no second find, code 1", async () => { + const { io, out } = captureIo(); + let findCalls = 0; + let probeCalls = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => { findCalls++; return LIVE; }, + probe: async () => { probeCalls++; return READY_PROBE; }, + // deadline=1000; before-find=0 (find starts, finds=1), after-find=1000 + // (≥ deadline → no probe, return 1). + now: seqNow([0, 0, 1000]), + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + expect(findCalls).toBe(1); + expect(probeCalls).toBe(0); + const parsed = JSON.parse(out.join("")); + expect(parsed.ready).toBe(false); + }); + + test("find before deadline but probe crosses it → finds=1, probes=1, no second find, code 1", async () => { + const { io, out } = captureIo(); + let findCalls = 0; + let probeCalls = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 1 }, + { + ...io, + findLive: async () => { findCalls++; return LIVE; }, + probe: async () => { probeCalls++; return READY_PROBE; }, + // deadline=1000; before-find=0 → after-find=0 (<1000, probe starts, + // probes=1) → after-probe=1000 (≥ deadline, ready does NOT win, return 1). + now: seqNow([0, 0, 0, 1000]), + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + expect(findCalls).toBe(1); + expect(probeCalls).toBe(1); + const parsed = JSON.parse(out.join("")); + expect(parsed.ready).toBe(false); + }); + + test("injected remainingMs equals logical remaining time, stays positive, never exceeds it", async () => { + const { io } = captureIo(); + const findRemaining: number[] = []; + const probeRemaining: number[] = []; + // deadline = 0 + 1000 = 1000; every find/probe starts while the clock reads + // 0, so the injected remaining equals deadline-0 = 1000 exactly. The + // after-probe reading then crosses the deadline to terminate the loop. + await runReady( + { json: false, wait: true, timeoutSeconds: 1 }, + { + ...io, + now: seqNow([0, 0, 0, 1001]), + findLive: async (remainingMs) => { findRemaining.push(remainingMs ?? -1); return LIVE; }, + probe: async (_port, _opts, remainingMs) => { probeRemaining.push(remainingMs ?? -1); return PENDING_PROBE; }, + sleep: async () => {}, + }, + ); + // Both discovery and probe received the logical remaining = 1000ms: positive + // and equal to deadline-now_at_call (cannot exceed the remaining budget). + expect(findRemaining).toEqual([1000]); + expect(probeRemaining).toEqual([1000]); + for (const r of [...findRemaining, ...probeRemaining]) { + expect(r).toBeGreaterThan(0); + expect(r).toBeLessThanOrEqual(1000); + } + }); + + test("ready strictly before deadline still exits 0 (no regression)", async () => { + const { io, out } = captureIo(); + const code = await runReady( + { json: false, wait: true, timeoutSeconds: 2 }, + { + ...io, + findLive: async () => LIVE, + probe: async () => READY_PROBE, + // deadline = 0 + 2000 = 2000; post-probe reads 500 < 2000 → ready wins. + now: seqNow([0, 0, 0, 500]), + sleep: async () => {}, + }, + ); + expect(code).toBe(0); + expect(out.join("")).toContain("Proxy ready"); + }); +}); + +// ── handleStart readinessGate wiring (source-level integration guard) ───────── +// A bounded source-level assertion reading ONLY src/cli/index.ts. It verifies +// that the SAME identifier `readinessGate` is (1) created in handleStart via +// createReadinessGate(), (2) passed to startServer in the retry path, and +// (3) passed to runStartupReadinessSync wrapping syncModelsToCodex(port), in +// that order. This catches a regression where the gate is wired to only one of +// the two call sites. It complements the executable runStartupReadinessSync +// outcome tests in tests/proxy-liveness.test.ts (no architecture refactor). +describe("handleStart readinessGate wiring (source-level)", () => { + const cliSource = readFileSync(join(import.meta.dir, "../src/cli/index.ts"), "utf8"); + + test("readinessGate is created, threaded into startServer, and into runStartupReadinessSync — in order", () => { + const createMatch = cliSource.match(/const\s+readinessGate\s*=\s*createReadinessGate\(\)/); + expect(createMatch, "handleStart must create readinessGate via createReadinessGate()").not.toBeNull(); + + const startMatch = cliSource.match(/startServer\s*\(\s*port\s*,\s*\{\s*readinessGate\s*\}\s*\)/); + expect(startMatch, "startServer must be called with { readinessGate } in the retry path").not.toBeNull(); + + const syncMatch = cliSource.match( + /runStartupReadinessSync\s*\(\s*readinessGate\s*,\s*\(\s*\)\s*=>\s*syncModelsToCodex\s*\(\s*port\s*\)\s*\)/, + ); + expect(syncMatch, "runStartupReadinessSync must wrap () => syncModelsToCodex(port) with readinessGate").not.toBeNull(); + + // Source order must be: create → startServer → runStartupReadinessSync. + const createIdx = createMatch!.index!; + const startIdx = startMatch!.index!; + const syncIdx = syncMatch!.index!; + expect(createIdx).toBeLessThan(startIdx); + expect(startIdx).toBeLessThan(syncIdx); + }); + + test("the readinessGate identifier is the SAME symbol at all three call sites", () => { + // Exactly one declaration of readinessGate in handleStart's scope; every + // call site references that identifier (no shadowing, no second local). + const declarations = cliSource.match(/\breadinessGate\s*=/g); + expect(declarations, "readinessGate must be assigned exactly once").toHaveLength(1); + // Three references total: one declaration + startServer + runStartupReadinessSync. + const references = cliSource.match(/\breadinessGate\b/g); + expect(references?.length ?? 0).toBeGreaterThanOrEqual(3); + }); +}); + +// ── P1: ready pre-parse before maybeAutoRestoreCodexShim (source-level) ──────── +// `ocx ready` must reject invalid arguments with exit 64 BEFORE the global +// maybeAutoRestoreCodexShim preflight (or any discovery/probe/filesystem-capable +// step) runs. These source-level guards pin that ordering and the single-parse +// contract so a future edit cannot silently move parsing back into handleReady +// or after auto-restore. No subprocess/network/HOME is used. +describe("ready pre-parse before maybeAutoRestoreCodexShim (source-level, P1)", () => { + const cliSource = readFileSync(join(import.meta.dir, "../src/cli/index.ts"), "utf8"); + + test("ready pre-parse call runs BEFORE maybeAutoRestoreCodexShim", () => { + const preparseIdx = cliSource.indexOf("parseReadyArgs(args.slice(1))"); + expect(preparseIdx, "pre-parse must call parseReadyArgs(args.slice(1))").toBeGreaterThanOrEqual(0); + const autoIdx = cliSource.indexOf("maybeAutoRestoreCodexShim(command, args)"); + expect(autoIdx, "maybeAutoRestoreCodexShim(command, args) call must be present").toBeGreaterThanOrEqual(0); + expect(preparseIdx, "ready pre-parse must precede maybeAutoRestoreCodexShim").toBeLessThan(autoIdx); + }); + + test("invalid ready exits 64 inside the pre-parse block, before auto-restore", () => { + const autoIdx = cliSource.indexOf("maybeAutoRestoreCodexShim(command, args)"); + const beforeAuto = cliSource.slice(0, autoIdx); + expect(beforeAuto).toContain('command === "ready"'); + expect(beforeAuto).toContain("parseReadyArgs(args.slice(1))"); + expect(beforeAuto).toContain("process.exit(parsed.code)"); + }); + + test("exactly one runtime parseReadyArgs(args.slice(1)) call site in cli/index.ts", () => { + const matches = cliSource.match(/parseReadyArgs\(args\.slice\(1\)\)/g); + expect(matches, "parseReadyArgs(args.slice(1)) must appear exactly once (no re-parse)").toHaveLength(1); + }); + + test("handleReady accepts pre-parsed ReadyArgs and never re-parses", () => { + const sig = cliSource.match(/async\s+function\s+handleReady\s*\(\s*\w+\s*:\s*ReadyArgs\s*\)\s*:\s*Promise/); + expect(sig, "handleReady(args: ReadyArgs): Promise signature must exist").not.toBeNull(); + // The handleReady body (up to the next top-level function/switch) must not + // call parseReadyArgs and must call runReady with the passed args. + const start = sig!.index!; + const rest = cliSource.slice(start); + const bodyEnd = rest.search(/\n(?:async function |function |switch \(command\))/); + const body = rest.slice(0, bodyEnd === -1 ? undefined : bodyEnd); + expect(body).not.toContain("parseReadyArgs"); + expect(body).toContain("runReady"); + }); + + test("valid ready dispatch reaches handleReady AFTER maybeAutoRestoreCodexShim, with fail-closed guard", () => { + const autoIdx = cliSource.indexOf("maybeAutoRestoreCodexShim(command, args)"); + const readyCaseIdx = cliSource.indexOf('case "ready":'); + expect(readyCaseIdx, 'a "ready" switch case must exist').toBeGreaterThanOrEqual(0); + expect(autoIdx).toBeLessThan(readyCaseIdx); + // Slice the whole ready case body (up to the next case), not a fixed width. + const nextCaseIdx = cliSource.indexOf("case ", readyCaseIdx + 1); + const caseBody = cliSource.slice(readyCaseIdx, nextCaseIdx === -1 ? undefined : nextCaseIdx); + // Passes the stashed readyArgs; fail-closed guard exits 64 with NO I/O if + // the impossible state (missing pre-parsed args) ever occurs. + expect(caseBody).toContain("readyArgs"); + expect(caseBody).toContain("handleReady"); + expect(caseBody).toContain("process.exit(64)"); + }); +}); + +// ── P1: invalid matrices never reach discovery/probe (counters) ─────────────── +// parseReadyArgs is pure (no I/O), and runReady only accepts already-valid +// ReadyArgs. So an invalid matrix exits 64 in the pre-parse block and can never +// reach runReady's findLive/probe. The valid counterpart below exercises +// find/probe exactly once, confirming the ONLY path to discovery/probe is +// valid-args → runReady. No subprocess/network/HOME. +describe("invalid ready matrices never invoke findLive/probe (P1 counters)", () => { + const invalidMatrices: string[][] = [ + ["--timeout", "5"], + ["--nope"], + ["now"], + ["--wait", "--timeout", "abc"], + ["--wait", "--timeout"], + ["--wait", "--timeout", "0"], + ["--wait", "--timeout", "301"], + ["--wait", "--timeout", "-5"], + ["--wait", "--timeout", "1.5"], + ]; + + test("every invalid matrix returns exit code 64 (pure parser, no I/O)", () => { + for (const argv of invalidMatrices) { + expect(parseReadyArgs(argv)).toEqual({ ok: false, code: 64 }); + } + }); + + test("valid counterpart reaches runReady and calls find/probe exactly once (single probe)", async () => { + let findCalls = 0; + let probeCalls = 0; + const code = await runReady( + { json: false, wait: false, timeoutSeconds: DEFAULT_READY_WAIT_TIMEOUT_SECONDS }, + { + stdout: { log: () => {} }, + findLive: async () => { findCalls++; return LIVE; }, + probe: async () => { probeCalls++; return READY_PROBE; }, + }, + ); + expect(code).toBe(0); + expect(findCalls).toBe(1); + expect(probeCalls).toBe(1); + }); +}); + +// ── production findLiveProxy deadline wiring (source-level) ─────────────────── +// The production default find must forward an ABSOLUTE deadline (real wall +// clock + remaining budget) into findLiveProxy in the --wait path, so the +// sequential candidate probes inside findLiveProxy are bounded by the single +// wait deadline. It must NOT use the injected logical now: AbortSignal time is +// real wall-clock time, so Date.now is authoritative for the network deadline. +// The non-wait path keeps findLiveProxy's built-in default (no deadlineMs). +describe("runReady production findLiveProxy deadline wiring (source-level)", () => { + const readySource = readFileSync(join(import.meta.dir, "../src/cli/ready.ts"), "utf8"); + + test("the --wait path derives deadlineMs from Date.now() + remainingMs (not the injected now)", () => { + // Date.now (real wall clock) is authoritative for the AbortSignal deadline; + // the injected logical now must not govern the network timeout. + expect(readySource).toContain("deadlineMs: Date.now() + remainingMs"); + // The per-probe cap is forwarded alongside the absolute deadline. + expect(readySource).toContain("timeoutMs: IO_TIMEOUT_CAP_MS"); + }); + + test("the non-wait path keeps findLiveProxy's built-in default (no deadlineMs)", () => { + // The default find forwards {} when remainingMs is undefined so the + // built-in per-probe timeout (no deadline) is preserved for the single probe. + expect(readySource).toContain("remainingMs === undefined ? {}"); + // deadlineMs is only ever passed conditionally (in the wait branch), never + // as an unconditional findLiveProxy({ deadlineMs: ... }). + expect(readySource).not.toContain("findLiveProxy({ deadlineMs"); + }); +}); diff --git a/tests/cli-restart-health.test.ts b/tests/cli-restart-health.test.ts index 218bb1a1ff..69f592e438 100644 --- a/tests/cli-restart-health.test.ts +++ b/tests/cli-restart-health.test.ts @@ -8,6 +8,13 @@ import { fileURLToPath } from "node:url"; const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); const cliPath = join(repoRoot, "src", "cli", "index.ts"); +/** + * Every subprocess in this file runs against a private temp OPENCODEX_HOME so no + * check can ever discover/inspect/mutate the operator's real proxy state. The + * ready describe keeps ONLY the help-routing subprocess checks: the + * network/no-proxy/argument-validation ready tests live as injected tests in + * tests/cli-ready.test.ts (no real loopback/home). + */ function runCli(args: string[], env: Record = {}) { return spawnSync(process.execPath, [cliPath, ...args], { cwd: repoRoot, @@ -17,41 +24,69 @@ function runCli(args: string[], env: Record = {}) { }); } +function isolatedHome(prefix: string): string { + return mkdtempSync(join(tmpdir(), prefix)); +} + +function writeIsolatedConfig(dir: string): void { + writeFileSync(join(dir, "config.json"), JSON.stringify({ + port: 19999, + providers: { openai: { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", authMode: "forward" } }, + defaultProvider: "openai", + codexAutoStart: false, + }), "utf8"); +} + describe("ocx restart", () => { test("restart --help prints usage", () => { - const result = runCli(["restart", "--help"]); - expect(result.status).toBe(0); - expect(result.stdout).toContain("ocx restart"); + const dir = isolatedHome("ocx-restart-help-"); + try { + const result = runCli(["restart", "--help"], { OPENCODEX_HOME: dir }); + expect(result.status).toBe(0); + expect(result.stdout).toContain("ocx restart"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } }); test("help restart shows restart help entry", () => { - const result = runCli(["help", "restart"]); - expect(result.status).toBe(0); - expect(result.stdout).toContain("Stop the proxy and restart"); + const dir = isolatedHome("ocx-restart-help-entry-"); + try { + const result = runCli(["help", "restart"], { OPENCODEX_HOME: dir }); + expect(result.status).toBe(0); + expect(result.stdout).toContain("Stop the proxy and restart"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } }); }); describe("ocx health", () => { test("health --help prints usage", () => { - const result = runCli(["health", "--help"]); - expect(result.status).toBe(0); - expect(result.stdout).toContain("ocx health"); + const dir = isolatedHome("ocx-health-help-"); + try { + const result = runCli(["health", "--help"], { OPENCODEX_HOME: dir }); + expect(result.status).toBe(0); + expect(result.stdout).toContain("ocx health"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } }); test("help health shows health help entry", () => { - const result = runCli(["help", "health"]); - expect(result.status).toBe(0); - expect(result.stdout).toContain("Check proxy health"); + const dir = isolatedHome("ocx-health-help-entry-"); + try { + const result = runCli(["help", "health"], { OPENCODEX_HOME: dir }); + expect(result.status).toBe(0); + expect(result.stdout).toContain("Check proxy health"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } }); test("health exits 1 with no proxy running (isolated home)", () => { - const dir = mkdtempSync(join(tmpdir(), "ocx-health-")); - writeFileSync(join(dir, "config.json"), JSON.stringify({ - port: 19999, - providers: { openai: { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", authMode: "forward" } }, - defaultProvider: "openai", - codexAutoStart: false, - }), "utf8"); + const dir = isolatedHome("ocx-health-"); + writeIsolatedConfig(dir); try { const result = runCli(["health"], { OPENCODEX_HOME: dir }); expect(result.status).toBe(1); @@ -62,13 +97,8 @@ describe("ocx health", () => { }); test("health --json exits 1 with valid JSON when no proxy", () => { - const dir = mkdtempSync(join(tmpdir(), "ocx-health-json-")); - writeFileSync(join(dir, "config.json"), JSON.stringify({ - port: 19999, - providers: { openai: { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", authMode: "forward" } }, - defaultProvider: "openai", - codexAutoStart: false, - }), "utf8"); + const dir = isolatedHome("ocx-health-json-"); + writeIsolatedConfig(dir); try { const result = runCli(["health", "--json"], { OPENCODEX_HOME: dir }); expect(result.status).toBe(1); @@ -80,3 +110,31 @@ describe("ocx health", () => { } }); }); + +describe("ocx ready", () => { + // Only the help-routing subprocess checks live here. The default-probe, + // --json, --wait, --timeout, and argument-validation cases are injected tests + // in tests/cli-ready.test.ts (no real loopback/home). + test("ready --help prints usage (exit 0)", () => { + const dir = isolatedHome("ocx-ready-help-"); + try { + const result = runCli(["ready", "--help"], { OPENCODEX_HOME: dir }); + expect(result.status).toBe(0); + expect(result.stdout).toContain("ocx ready"); + expect(result.stdout).toContain("--wait"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("help ready shows the ready help entry", () => { + const dir = isolatedHome("ocx-ready-help-entry-"); + try { + const result = runCli(["help", "ready"], { OPENCODEX_HOME: dir }); + expect(result.status).toBe(0); + expect(result.stdout).toContain("post-sync readiness"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/proxy-liveness.test.ts b/tests/proxy-liveness.test.ts index 9e8498eac1..9e5e653cf7 100644 --- a/tests/proxy-liveness.test.ts +++ b/tests/proxy-liveness.test.ts @@ -1,5 +1,16 @@ import { describe, expect, test } from "bun:test"; -import { findLiveProxy, isOpencodexHealthz, probeHostname, proxyIdentityAt } from "../src/server/proxy-liveness"; +import { + createReadinessGate, + runStartupReadinessSync, +} from "../src/server/readiness"; +import { + findLiveProxy, + isOpencodexHealthz, + probeHostname, + probeReadiness, + proxyIdentityAt, + validateReadyzBody, +} from "../src/server/proxy-liveness"; function healthz(body: unknown, status = 200): Response { return new Response(JSON.stringify(body), { status }); @@ -258,3 +269,466 @@ describe("findLiveProxy", () => { expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" }); }); }); + +// ── findLiveProxy single-deadline candidate gating ─────────────────────────── +// The absolute deadlineMs must gate EVERY internal candidate probe, not just +// the first. Before each proxyIdentityAt the budget is recomputed against the +// injected nowFn: a non-positive remaining terminates discovery (returns null) +// so no later candidate fetch starts, and each probe's timeoutMs is capped to +// min(existing per-probe cap, positive remaining). Existing no-deadline callers +// keep their current semantics (no deadlineMs → unbounded per-probe default). +describe("findLiveProxy single-deadline candidate gating", () => { + test("deadline already expired before the first probe → no fetch, returns null", async () => { + let fetchCalls = 0; + const live = await findLiveProxy({ + deadlineMs: 1000, + nowFn: () => 2000, // already past the deadline before any probe + readPidFn: () => 4242, + readRuntimeFn: pid => (pid === 4242 ? { port: 58195 } : null), + configFn: () => ({ port: 10100 }), + fetchFn: (async () => { fetchCalls++; return healthz(OURS); }) as typeof fetch, + }); + expect(live).toBeNull(); + expect(fetchCalls).toBe(0); + }); + + test("first candidate fetch crosses the deadline → fetch count 1, no later candidate", async () => { + let clock = 1000; + const urls: string[] = []; + const live = await findLiveProxy({ + deadlineMs: 2000, + nowFn: () => clock, + // Two sequential candidates wired: pid path → port 58195; config fallback → 10100. + readPidFn: () => 4242, + readRuntimeFn: pid => (pid === 4242 ? { port: 58195 } : null), + configFn: () => ({ port: 10100 }), + fetchFn: (async (url: string | URL | Request, init?: RequestInit) => { + urls.push(String(url)); + // The first (and only) probe receives a real AbortSignal that is + // positive and not yet aborted (safe injected signal state). + const sig = init?.signal as AbortSignal | undefined; + expect(sig).toBeTruthy(); + expect(sig?.aborted).toBe(false); + // Advance the clock PAST the deadline inside the first fetch so the + // next candidate's budget check terminates discovery. + clock = 3000; // > deadlineMs (2000) + // Foreign body → identity null → would normally fall through to config. + return healthz({ status: "ok" }); + }) as typeof fetch, + }); + expect(live).toBeNull(); + // Only the runtime-record probe ran; the config-fallback fetch never started. + expect(urls).toEqual(["http://127.0.0.1:58195/healthz"]); + expect(urls).toHaveLength(1); + }); + + test("per-probe timeoutMs is capped to the positive remaining (min of cap and remaining)", async () => { + // Spy on AbortSignal.timeout to read the exact ms passed to proxyIdentityAt. + // Scoped and restored in finally; no real timer is consumed. + const calls: number[] = []; + const originalTimeout = AbortSignal.timeout; + AbortSignal.timeout = ((ms: number) => { + calls.push(ms); + return originalTimeout(ms); + }) as typeof AbortSignal.timeout; + try { + let clock = 1000; + await findLiveProxy({ + deadlineMs: 1300, // remaining = 300 at the first probe (< 750 cap) + nowFn: () => clock, + readPidFn: () => 4242, + readRuntimeFn: () => ({ port: 58195 }), + configFn: () => ({ port: 10100 }), + fetchFn: (async () => { + clock = 2000; // cross deadline so no second probe + return healthz({ status: "ok" }); + }) as typeof fetch, + }); + } finally { + AbortSignal.timeout = originalTimeout; + } + // timeoutMs = min(750 default cap, 300 remaining) = 300, exactly once. + expect(calls).toEqual([300]); + }); + + test("per-probe timeoutMs never exceeds the cap even when remaining is large", async () => { + const calls: number[] = []; + const originalTimeout = AbortSignal.timeout; + AbortSignal.timeout = ((ms: number) => { + calls.push(ms); + return originalTimeout(ms); + }) as typeof AbortSignal.timeout; + try { + let clock = 1000; + await findLiveProxy({ + deadlineMs: 100000, // remaining ≈ 99000 ≫ 750 cap + timeoutMs: 750, // explicit per-probe cap, mirroring the production wiring + nowFn: () => clock, + readPidFn: () => 4242, + readRuntimeFn: () => ({ port: 58195 }), + configFn: () => ({ port: 10100 }), + fetchFn: (async () => { + clock = 100001; // cross deadline so no second probe + return healthz({ status: "ok" }); + }) as typeof fetch, + }); + } finally { + AbortSignal.timeout = originalTimeout; + } + // timeoutMs capped at 750 even though remaining is ~99000. + expect(calls).toEqual([750]); + }); + + test("no deadlineMs retains the existing multi-candidate fallback behavior (two fetches)", async () => { + // Without deadlineMs the pid path failing falls through to config, which + // succeeds — proving the deadline gate is inert when deadlineMs is unset. + const urls: string[] = []; + const live = await findLiveProxy({ + readPidFn: () => 4242, + readRuntimeFn: pid => (pid === 4242 ? { port: 58195 } : null), + configFn: () => ({ port: 10100 }), + fetchFn: (async (url: string | URL | Request) => { + urls.push(String(url)); + return String(url).includes("58195") + ? healthz({ ...OURS, pid: 9999 }) // mismatched pid → identity null on runtime probe + : healthz(OURS); // config fallback adopts the reported pid + }) as typeof fetch, + }); + expect(urls).toEqual(["http://127.0.0.1:58195/healthz", "http://127.0.0.1:10100/healthz"]); + expect(urls).toHaveLength(2); + expect(live).toEqual({ pid: 4242, port: 10100, hostname: undefined }); + }); +}); + +// ── per-server readiness gate (closure, no module-global state) ──────────────── + +describe("createReadinessGate", () => { + test("a fresh gate starts pending", () => { + const gate = createReadinessGate(); + expect(gate.getStatus()).toBe("pending"); + }); + + test("markReady transitions pending → ready", () => { + const gate = createReadinessGate(); + gate.markReady(); + expect(gate.getStatus()).toBe("ready"); + }); + + test("markFailed transitions pending → failed", () => { + const gate = createReadinessGate(); + gate.markFailed(); + expect(gate.getStatus()).toBe("failed"); + }); + + test("two gates are fully independent (no shared module-global state)", () => { + const a = createReadinessGate(); + const b = createReadinessGate(); + a.markReady(); + b.markFailed(); + expect(a.getStatus()).toBe("ready"); + expect(b.getStatus()).toBe("failed"); + }); + + test("status transitions at most once (ready then failed stays ready)", () => { + const gate = createReadinessGate(); + gate.markReady(); + gate.markFailed(); + expect(gate.getStatus()).toBe("ready"); + }); + + test("status transitions at most once (failed then ready stays failed)", () => { + const gate = createReadinessGate(); + gate.markFailed(); + gate.markReady(); + expect(gate.getStatus()).toBe("failed"); + }); + + test("the gate exposes only the fixed status enum (no reason/changedAt payload)", () => { + const gate = createReadinessGate(); + gate.markFailed(); + // Only the sanitized status enum is reachable; the interface surface is fixed. + expect(gate.getStatus()).toBe("failed"); + const serialized = JSON.stringify(gate); + expect(serialized).not.toContain("reason"); + expect(serialized).not.toContain("changedAt"); + expect(serialized).not.toContain("warning"); + expect(serialized).not.toContain("path"); + }); +}); + +// ── runStartupReadinessSync drives the gate from the sync outcome ────────────── + +describe("runStartupReadinessSync", () => { + test("ok=true with no warning → ready", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => ({ ok: true })); + expect(gate.getStatus()).toBe("ready"); + }); + + test("ok=true with non-warning extras (external-provider short circuit) → ready", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => ({ ok: true, added: 0, message: "injected" })); + expect(gate.getStatus()).toBe("ready"); + }); + + test("ok=true with empty-string warning → ready", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => ({ ok: true, warning: "" })); + expect(gate.getStatus()).toBe("ready"); + }); + + test("ok=false → failed", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => ({ ok: false, message: "x" })); + expect(gate.getStatus()).toBe("failed"); + }); + + test("ok=true with nonempty warning → failed", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => ({ ok: true, warning: "catalog sync skipped: no source" })); + expect(gate.getStatus()).toBe("failed"); + }); + + test("null result → failed", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => null); + expect(gate.getStatus()).toBe("failed"); + }); + + test("syncFn that throws → failed (never rejects)", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => { throw new Error("boom"); }); + expect(gate.getStatus()).toBe("failed"); + }); + + test("transition is one-shot: a later failing sync does not flip a ready gate", async () => { + const gate = createReadinessGate(); + await runStartupReadinessSync(gate, async () => ({ ok: true })); + await runStartupReadinessSync(gate, async () => { throw new Error("late"); }); + expect(gate.getStatus()).toBe("ready"); + }); + + test("syncFn is awaited exactly once", async () => { + const gate = createReadinessGate(); + let calls = 0; + await runStartupReadinessSync(gate, async () => { calls++; return { ok: true }; }); + expect(calls).toBe(1); + expect(gate.getStatus()).toBe("ready"); + }); +}); + +// ── validateReadyzBody: strict contract (pure) ───────────────────────────────── + +const VALID_BODY = { service: "opencodex", version: "2.6.17", uptime: 12, pid: 4242, port: 10100, status: "ready" as const }; + +describe("validateReadyzBody strict contract", () => { + test("accepts a well-formed ready body and echoes the sanitized fields", () => { + expect(validateReadyzBody(VALID_BODY, 10100)).toEqual({ ready: true, status: "ready", pid: 4242, port: 10100 }); + }); + + test("accepts pending/failed bodies as not-ready with the same fixed status", () => { + expect(validateReadyzBody({ ...VALID_BODY, status: "pending" }, 10100)).toEqual({ ready: false, status: "pending", pid: 4242, port: 10100 }); + expect(validateReadyzBody({ ...VALID_BODY, status: "failed" }, 10100)).toEqual({ ready: false, status: "failed", pid: 4242, port: 10100 }); + }); + + test("rejects a foreign service marker", () => { + expect(validateReadyzBody({ ...VALID_BODY, service: "other-app" }, 10100)).toBeNull(); + }); + + test("rejects a legacy health-only body (status ok, no service marker)", () => { + expect(validateReadyzBody({ status: "ok", version: "2.6.16", uptime: 5, pid: 4242, port: 10100 }, 10100)).toBeNull(); + }); + + test("rejects missing version", () => { + expect(validateReadyzBody({ service: "opencodex", uptime: 1, pid: 4242, port: 10100, status: "ready" }, 10100)).toBeNull(); + }); + + test("rejects empty-string version", () => { + expect(validateReadyzBody({ ...VALID_BODY, version: "" }, 10100)).toBeNull(); + }); + + test("rejects non-string version", () => { + expect(validateReadyzBody({ ...VALID_BODY, version: 1 }, 10100)).toBeNull(); + }); + + test("rejects missing pid", () => { + expect(validateReadyzBody({ service: "opencodex", version: "v", uptime: 1, port: 10100, status: "ready" }, 10100)).toBeNull(); + }); + + test("rejects non-integer / non-positive pid", () => { + expect(validateReadyzBody({ ...VALID_BODY, pid: 0 }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, pid: -1 }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, pid: 1.5 }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, pid: "4242" }, 10100)).toBeNull(); + }); + + test("rejects missing port", () => { + expect(validateReadyzBody({ service: "opencodex", version: "v", uptime: 1, pid: 4242, status: "ready" }, 10100)).toBeNull(); + }); + + test("rejects out-of-range / non-integer port", () => { + expect(validateReadyzBody({ ...VALID_BODY, port: 0 }, 0)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, port: 65536 }, 65536)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, port: 1.5 }, 1)).toBeNull(); + }); + + test("rejects a port that does not equal the probed port", () => { + expect(validateReadyzBody(VALID_BODY, 9999)).toBeNull(); + }); + + test("rejects missing/malformed uptime (negative, NaN, Infinity, non-number)", () => { + expect(validateReadyzBody({ ...VALID_BODY, uptime: -1 }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, uptime: Number.NaN }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, uptime: Number.POSITIVE_INFINITY }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, uptime: "12" }, 10100)).toBeNull(); + }); + + test("rejects missing/malformed status (not a fixed enum value)", () => { + expect(validateReadyzBody({ service: "opencodex", version: "v", uptime: 1, pid: 4242, port: 10100 }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, status: "ok" }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, status: "READY" }, 10100)).toBeNull(); + expect(validateReadyzBody({ ...VALID_BODY, status: 1 }, 10100)).toBeNull(); + }); + + test("rejects an expectedPid mismatch", () => { + expect(validateReadyzBody(VALID_BODY, 10100, { expectedPid: 1 })).toBeNull(); + }); + + test("accepts the expectedPid when it matches", () => { + expect(validateReadyzBody(VALID_BODY, 10100, { expectedPid: 4242 })?.ready).toBe(true); + }); + + test("uptime zero is allowed (just-started proxy)", () => { + expect(validateReadyzBody({ ...VALID_BODY, uptime: 0 }, 10100)?.ready).toBe(true); + }); + + test("non-object / null bodies are rejected", () => { + expect(validateReadyzBody(null, 10100)).toBeNull(); + expect(validateReadyzBody("opencodex", 10100)).toBeNull(); + expect(validateReadyzBody(undefined, 10100)).toBeNull(); + }); +}); + +// ── probeReadiness: strict HTTP + contract enforcement ───────────────────────── + +function readyz(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } }); +} + +const READY_BODY = { service: "opencodex", version: "2.6.17", uptime: 12, pid: 4242, port: 10100, status: "ready" }; +const PENDING_BODY = { service: "opencodex", version: "2.6.17", uptime: 1, pid: 4242, port: 10100, status: "pending" }; +const FAILED_BODY = { service: "opencodex", version: "2.6.17", uptime: 1, pid: 4242, port: 10100, status: "failed" }; + +describe("probeReadiness happy path", () => { + test("accepts a correct 200 + ready body", async () => { + const probe = await probeReadiness(10100, { expectedPid: 4242 }, { fetchFn: (async () => readyz(READY_BODY, 200)) as typeof fetch }); + expect(probe).toEqual({ ready: true, status: "ready", pid: 4242, port: 10100 }); + }); + + test("returns ready=false with status for a pending 503 body", async () => { + const probe = await probeReadiness(10100, {}, { fetchFn: (async () => readyz(PENDING_BODY, 503)) as typeof fetch }); + expect(probe).toEqual({ ready: false, status: "pending", pid: 4242, port: 10100 }); + }); + + test("returns ready=false with status for a failed 503 body", async () => { + const probe = await probeReadiness(10100, {}, { fetchFn: (async () => readyz(FAILED_BODY, 503)) as typeof fetch }); + expect(probe).toEqual({ ready: false, status: "failed", pid: 4242, port: 10100 }); + }); +}); + +describe("probeReadiness adversarial contract (never counts ready)", () => { + test("rejects 503 + ready (HTTP/body-status inconsistency)", async () => { + const probe = await probeReadiness(10100, {}, { fetchFn: (async () => readyz(READY_BODY, 503)) as typeof fetch }); + expect(probe).toBeNull(); + }); + + test("rejects 200 + pending (HTTP/body-status inconsistency)", async () => { + const probe = await probeReadiness(10100, {}, { fetchFn: (async () => readyz(PENDING_BODY, 200)) as typeof fetch }); + expect(probe).toBeNull(); + }); + + test("rejects 200 + failed (HTTP/body-status inconsistency)", async () => { + const probe = await probeReadiness(10100, {}, { fetchFn: (async () => readyz(FAILED_BODY, 200)) as typeof fetch }); + expect(probe).toBeNull(); + }); + + test("rejects a foreign 200 body (service mismatch)", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ service: "other-app", status: "ready", version: "v", uptime: 1, pid: 4242, port: 10100 }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("rejects a legacy health-only body (status ok, no service marker)", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ status: "ok", version: "2.6.16", uptime: 5, pid: 4242, port: 10100 }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("rejects a body missing version", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ service: "opencodex", uptime: 1, pid: 4242, port: 10100, status: "ready" }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("rejects a body missing pid", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ service: "opencodex", version: "v", uptime: 1, port: 10100, status: "ready" }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("rejects a body missing port", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ service: "opencodex", version: "v", uptime: 1, pid: 4242, status: "ready" }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("rejects a body whose port does not equal the probed port", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ ...READY_BODY, port: 9999 }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("rejects a body whose pid does not match expectedPid", async () => { + const probe = await probeReadiness(10100, { expectedPid: 1 }, { fetchFn: (async () => readyz(READY_BODY, 200)) as typeof fetch }); + expect(probe).toBeNull(); + }); + + test("rejects malformed uptime (negative)", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ ...READY_BODY, uptime: -1 }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("rejects malformed status (not a fixed enum value)", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz({ ...READY_BODY, status: "ok" }, 200)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("returns null when unreachable", async () => { + const probe = await probeReadiness(10100, {}, { fetchFn: (async () => { throw new Error("refused"); }) as typeof fetch }); + expect(probe).toBeNull(); + }); + + test("returns null for non-JSON body", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => new Response("not-json", { status: 200 })) as typeof fetch, + }); + expect(probe).toBeNull(); + }); + + test("returns null for a 500 body (neither 200 nor 503)", async () => { + const probe = await probeReadiness(10100, {}, { + fetchFn: (async () => readyz(READY_BODY, 500)) as typeof fetch, + }); + expect(probe).toBeNull(); + }); +}); diff --git a/tests/server-live.test.ts b/tests/server-live.test.ts index 21f665c98e..990c06a141 100644 --- a/tests/server-live.test.ts +++ b/tests/server-live.test.ts @@ -2,13 +2,18 @@ * /v1/live relay: Codex App / ChatGPT voice POSTs call-create against the injected base_url, * so the proxy must relay it to an OpenAI upstream instead of the /v1/* JSON-404 guard. */ -import { afterEach, beforeEach, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { existsSync, mkdirSync, readFileSync, rmSync } from "node:fs"; import { join } from "node:path"; import { saveCodexAccountCredential } from "../src/codex/account-store"; import { clearAccountNeedsReauth, clearAccountQuota } from "../src/codex/auth-api"; import { clearCodexUpstreamHealth, clearThreadAccountMap } from "../src/codex/routing"; import { saveConfig } from "../src/config"; +import { + createReadinessGate, + runStartupReadinessSync, + type ReadinessGate, +} from "../src/server/readiness"; import { startServer } from "../src/server"; import type { OcxConfig } from "../src/types"; import { fakeChatGptJwt } from "./helpers/fake-chatgpt-jwt"; @@ -849,3 +854,258 @@ test("sideband frame log records direction, kind, and U+FFFD context without ful await upstream.stop(true); } }); + +// ── /readyz: per-server readiness gate ──────────────────────────────────────── +// /healthz stays byte-for-byte the same immediate liveness signal; /readyz is the +// stricter gate that reflects the post-startup Codex sync outcome. It is exact-GET +// and unauthenticated (like /healthz), returns a sanitized body, 503+Retry-After +// while pending/failed, and 200 only when ready. Each startServer gets its own +// PRIVATE gate via createReadinessGate(); starting/failing a second server in the +// same process can never reset or mutate the first server's gate. +describe("GET /readyz", () => { + test("controlled startup sync drives the server gate to ready or failed", async () => { + saveConfig(forwardConfig()); + const cases = [ + { outcome: { ok: true }, expectedStatus: "ready", expectedHttp: 200 }, + { outcome: { ok: true, warning: "catalog sync blocked" }, expectedStatus: "failed", expectedHttp: 503 }, + ] as const; + + for (const { outcome, expectedStatus, expectedHttp } of cases) { + const gate = createReadinessGate(); + const server = startServer(0, { readinessGate: gate }); + try { + const pending = await fetch(new URL("/readyz", server.url)); + expect(pending.status).toBe(503); + expect(((await pending.json()) as { status: string }).status).toBe("pending"); + + await runStartupReadinessSync(gate, async () => outcome); + + const settled = await fetch(new URL("/readyz", server.url)); + expect(settled.status).toBe(expectedHttp); + expect(((await settled.json()) as { status: string }).status).toBe(expectedStatus); + } finally { + await server.stop(true); + } + } + }); + + test("fresh gate is pending; /healthz 200 while /readyz 503 with Retry-After", async () => { + saveConfig(forwardConfig()); + const gate = createReadinessGate(); + const server = startServer(0, { readinessGate: gate }); + try { + const base = server.url; + // Liveness is immediate and unchanged. + const healthzRes = await fetch(new URL("/healthz", base)); + expect(healthzRes.status).toBe(200); + const healthzBody = (await healthzRes.json()) as { status: string; service: string; pid: number }; + expect(healthzBody.status).toBe("ok"); + expect(healthzBody.service).toBe("opencodex"); + + // Readiness is pending right after bind: 503 + Retry-After, sanitized body. + const readyzRes = await fetch(new URL("/readyz", base)); + expect(readyzRes.status).toBe(503); + expect(readyzRes.headers.get("retry-after")).toBeTruthy(); + const readyzBody = (await readyzRes.json()) as Record; + expect(readyzBody.service).toBe("opencodex"); + expect(readyzBody.status).toBe("pending"); + expect(typeof readyzBody.version).toBe("string"); + expect(typeof readyzBody.uptime).toBe("number"); + expect(typeof readyzBody.pid).toBe("number"); + expect(typeof readyzBody.port).toBe("number"); + // Sanitization: the body must never carry sync diagnostics, paths, or warnings. + expect(Object.keys(readyzBody).sort()).toEqual(["pid", "port", "service", "status", "uptime", "version"]); + expect(JSON.stringify(readyzBody)).not.toContain("warning"); + expect(JSON.stringify(readyzBody)).not.toContain("path"); + } finally { + await server.stop(true); + } + }); + + test("/readyz is 200 with status ready only after gate.markReady()", async () => { + saveConfig(forwardConfig()); + const gate = createReadinessGate(); + const server = startServer(0, { readinessGate: gate }); + try { + const base = server.url; + expect(((await (await fetch(new URL("/readyz", base))).json()) as { status: string }).status).toBe("pending"); + gate.markReady(); + const res = await fetch(new URL("/readyz", base)); + expect(res.status).toBe(200); + expect(res.headers.get("retry-after")).toBeNull(); + const body = (await res.json()) as { status: string; service: string }; + expect(body.status).toBe("ready"); + expect(body.service).toBe("opencodex"); + } finally { + await server.stop(true); + } + }); + + test("/readyz is 503 with Retry-After when the gate is failed", async () => { + saveConfig(forwardConfig()); + const gate = createReadinessGate(); + gate.markFailed(); + const server = startServer(0, { readinessGate: gate }); + try { + const res = await fetch(new URL("/readyz", server.url)); + expect(res.status).toBe(503); + expect(res.headers.get("retry-after")).toBeTruthy(); + expect(((await res.json()) as { status: string }).status).toBe("failed"); + } finally { + await server.stop(true); + } + }); + + test("exact method/path: POST /readyz and GET /readyz/ are NOT matched", async () => { + saveConfig(forwardConfig()); + const gate = createReadinessGate(); + const server = startServer(0, { readinessGate: gate }); + try { + const base = server.url; + // POST must not be accepted as readiness (falls through to the JSON 404 guard). + const postRes = await fetch(new URL("/readyz", base), { method: "POST" }); + expect(postRes.status).toBe(404); + // Trailing slash is a distinct path and must not match either. + const slashRes = await fetch(new URL("/readyz/", base)); + expect(slashRes.status).toBe(404); + } finally { + await server.stop(true); + } + }); + + test("/readyz answers without an admission token (unauthenticated, like /healthz)", async () => { + saveConfig(forwardConfig()); + const gate = createReadinessGate(); + const server = startServer(0, { readinessGate: gate }); + try { + // No Authorization header, no API auth token in env (beforeEach deletes it). + const res = await fetch(new URL("/readyz", server.url), { headers: { Authorization: "" } }); + // Pending → 503, but it answered (did NOT demand auth → would be 401). + expect([200, 503]).toContain(res.status); + const body = (await res.json()) as { service?: string }; + expect(body.service).toBe("opencodex"); + } finally { + await server.stop(true); + } + }); + + test("ephemeral startServer(0): /readyz reports the actual bound port (not the requested 0)", async () => { + saveConfig(forwardConfig()); + const gate = createReadinessGate(); + const server = startServer(0, { readinessGate: gate }); + try { + const boundPort = new URL(server.url).port; + // The bound port is a real ephemeral port, not the requested 0. + expect(Number(boundPort)).toBeGreaterThan(0); + const res = await fetch(new URL("/readyz", server.url)); + const body = (await res.json()) as { port: number }; + // /readyz must report the ACTUAL bound port so the strict probe passes. + expect(body.port).toBe(Number(boundPort)); + } finally { + await server.stop(true); + } + }); +}); + +// ── /readyz per-server gate isolation ───────────────────────────────────────── +// Two simultaneous startServer invocations in the same process get INDEPENDENT +// gates. Marking one ready cannot flip the other; a second server whose bind +// fails cannot mutate the first server's gate; the first stays ready while the +// second is pending/failed. +describe("per-server readiness gate isolation", () => { + test("two simultaneous servers keep independent gates (first ready while second pending)", async () => { + saveConfig(forwardConfig()); + const gateA: ReadinessGate = createReadinessGate(); + const gateB: ReadinessGate = createReadinessGate(); + const serverA = startServer(0, { readinessGate: gateA }); + const serverB = startServer(0, { readinessGate: gateB }); + try { + gateA.markReady(); + // gateB stays pending. + + const resA = await fetch(new URL("/readyz", serverA.url)); + expect(resA.status).toBe(200); + expect(((await resA.json()) as { status: string }).status).toBe("ready"); + + const resB = await fetch(new URL("/readyz", serverB.url)); + expect(resB.status).toBe(503); + expect(((await resB.json()) as { status: string }).status).toBe("pending"); + } finally { + await serverA.stop(true); + await serverB.stop(true); + } + }); + + test("marking one gate failed does not mutate the other gate", async () => { + saveConfig(forwardConfig()); + const gateA: ReadinessGate = createReadinessGate(); + const gateB: ReadinessGate = createReadinessGate(); + const serverA = startServer(0, { readinessGate: gateA }); + const serverB = startServer(0, { readinessGate: gateB }); + try { + gateA.markReady(); + gateB.markFailed(); + + const resA = await fetch(new URL("/readyz", serverA.url)); + expect(resA.status).toBe(200); + expect(((await resA.json()) as { status: string }).status).toBe("ready"); + + const resB = await fetch(new URL("/readyz", serverB.url)); + expect(resB.status).toBe(503); + expect(((await resB.json()) as { status: string }).status).toBe("failed"); + } finally { + await serverA.stop(true); + await serverB.stop(true); + } + }); + + test("a server with no gate passed in still serves pending /readyz (fresh private gate)", async () => { + saveConfig(forwardConfig()); + // No gate supplied → startServer creates a fresh pending private gate. + const server = startServer(0); + try { + const res = await fetch(new URL("/readyz", server.url)); + expect(res.status).toBe(503); + expect(((await res.json()) as { status: string }).status).toBe("pending"); + } finally { + await server.stop(true); + } + }); + + test("a bind failure on server A's actual occupied port cannot mutate A's gate (A stays ready, B stays pending)", async () => { + saveConfig(forwardConfig()); + const gateA: ReadinessGate = createReadinessGate(); + const gateB: ReadinessGate = createReadinessGate(); + // Ephemeral start: A binds a real kernel-assigned port (NOT a fixed user port). + const serverA = startServer(0, { readinessGate: gateA }); + try { + gateA.markReady(); + const occupiedPort = Number(new URL(serverA.url).port); + // The bound port is a real ephemeral port — the second startServer targets + // the SAME actual port, which is already held by A. + expect(occupiedPort).toBeGreaterThan(0); + expect(Number.isInteger(occupiedPort)).toBe(true); + + // Binding the occupied port with a separate gate must throw (EADDRINUSE). + // It must never silently share A's listener or reset A's gate. + expect(() => startServer(occupiedPort, { readinessGate: gateB })).toThrow(); + + // A is unaffected by the failed bind: still HTTP 200 / ready. + const resA = await fetch(new URL("/readyz", serverA.url)); + expect(resA.status).toBe(200); + expect(((await resA.json()) as { status: string }).status).toBe("ready"); + expect(gateA.getStatus()).toBe("ready"); + + // B never got past Bun.serve, so its gate was never marked: it stays + // pending (not failed, not ready) — exactly the isolation contract. + expect(gateB.getStatus()).toBe("pending"); + + // /healthz on A is unchanged too (liveness and readiness are independent). + const healthzRes = await fetch(new URL("/healthz", serverA.url)); + expect(healthzRes.status).toBe(200); + } finally { + // Clean up ONLY A — B never bound, so there is no B server to stop. + await serverA.stop(true); + } + }); +}); diff --git a/tests/update-notify.test.ts b/tests/update-notify.test.ts index aa2f7d9fcd..5976c69452 100644 --- a/tests/update-notify.test.ts +++ b/tests/update-notify.test.ts @@ -124,7 +124,10 @@ describe("cli wiring", () => { const cli = await readText("src/cli/index.ts"); const promptIndex = cli.indexOf("await maybeShowUpdatePrompt()"); const portIndex = cli.indexOf("let port = await chooseListenPort"); - const serverIndex = cli.indexOf("startServer(port, { localAttestationSecret })"); + const serverIndex = cli.search(/\bstartServer\s*\(\s*port\b/); + // A -1 from `search` would compare "before" every real index and turn the + // ordering assertion below into a silent pass. + expect(serverIndex).toBeGreaterThanOrEqual(0); expect(promptIndex).toBeGreaterThan(-1); expect(portIndex).toBeGreaterThan(-1); expect(promptIndex).toBeLessThan(portIndex); From fae0b4cb162dbb2bb6385b72003fd71fa8ac6b92 Mon Sep 17 00:00:00 2001 From: Diego Cantarero <71346275+diegocantarero@users.noreply.github.com> Date: Mon, 27 Jul 2026 15:17:26 -0600 Subject: [PATCH 02/11] docs(ready): align /readyz contract docs and add subprocess coverage Document the sanitized HTTP identity separately from the CLI --json shape. Clarify failed-immediate wait behavior and the default 45s timeout. Keep the readiness contract aligned across supported locales. Add real subprocess coverage for terminal failed readiness. --- .../docs/ja/reference/cli/lifecycle.md | 13 ++ .../docs/ko/reference/cli/lifecycle.md | 12 ++ .../content/docs/reference/cli/lifecycle.md | 12 ++ .../docs/ru/reference/cli/lifecycle.md | 13 ++ .../docs/zh-cn/reference/cli/lifecycle.md | 10 + src/cli/help.ts | 8 +- structure/03_catalog-and-subagents.md | 16 +- tests/cli-ready-subprocess.test.ts | 183 ++++++++++++++++++ 8 files changed, 258 insertions(+), 9 deletions(-) create mode 100644 tests/cli-ready-subprocess.test.ts diff --git a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md index 92b8fcfb2b..e5c96905b0 100644 --- a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md @@ -115,6 +115,19 @@ ocx status --json 稼働中のプロキシの ID を確認します。ヒューマン出力は PID/ポートをレポートします。 `--json` は `{ok, pid, port}` を出力します。このコマンドは正常な場合のみ 0 で終了し、それ以外の場合は 1 で終了するため、サービス プローブに適しています。 +### `ocx ready [--json] [--wait [--timeout ]]` + +認証不要の `GET /readyz` エンドポイントで同期後の準備状態を確認します。準備完了時は `200`、 +`pending` または終端状態の `failed` では `Retry-After: 1` とともに `503` を返します。HTTP の +サニタイズ済み識別フィールドは `{service, version, uptime, pid, port, status}` です。`/readyz` がない +旧プロキシは `unreachable` として fail-closed し、`/healthz` は readiness ではなく別の liveness 確認です。 +デフォルトでは 1 回だけ probe します。`--wait` は準備完了または timeout まで polling しますが、 +終端 `failed` を確認すると即座に終了します。デフォルト timeout は 45 秒で、`--timeout ` には +`--wait` が必要です（1〜300 秒の範囲）。CLI JSON は +`{ready, status, pid, port}` を出力し、`status` は `ready`、`pending`、`failed`、`unreachable` の +いずれかです。終了コードは ready が 0、not-ready/pending/failed/timeout/unreachable が 1、 +不正な引数が 64 です。 + ### `ocx doctor` 読み取り専用環境と接続の診断を実行します: 状態パスとファイル システム タイプ、WSL デュアル インストール、プロキシ環境/構成、ChatGPT の到達可能性、Codex プラグインとプロジェクト設定の警告、保留中の履歴の移行。 Codex のアプリとホームのターゲット設定セクションでは、Windows Orca ランタイムとホームの狭い不一致も検出し、該当する場合はサービスの移行について説明します。この診断によって表示されるパスでは、OS ユーザー名が編集されます。医師は修復ヒントを出力しますが、適用しません。 diff --git a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md index 3d1ee4023d..f02c0032a9 100644 --- a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md @@ -143,6 +143,18 @@ ocx status --json `{ok, pid, port}`를 내보냅니다. 이 명령은 정상일 때만 종료 코드 0을, 그렇지 않으면 1을 반환하므로 서비스 프로브에 적합합니다. +### `ocx ready [--json] [--wait [--timeout ]]` + +인증이 필요 없는 `GET /readyz` 엔드포인트로 동기화 후 준비 상태를 확인합니다. 준비되면 `200`, +`pending` 또는 종단 상태인 `failed`이면 `Retry-After: 1`과 함께 `503`을 반환합니다. HTTP의 정제된 +식별 필드는 `{service, version, uptime, pid, port, status}`입니다. `/readyz`가 없는 이전 프록시는 +`unreachable`로 fail-closed하며, `/healthz`는 준비 상태가 아닌 별도의 liveness 확인입니다. 기본값은 한 번의 +probe이며, `--wait`는 준비 또는 timeout까지 polling하지만 종단 `failed`를 확인하면 즉시 종료합니다. +기본 timeout은 45초이며, `--timeout `는 `--wait`와 함께 써야 하고 1~300초 범위를 받습니다. CLI JSON은 +`{ready, status, pid, port}`를 출력하며 `status`는 `ready`, `pending`, `failed`, +`unreachable` 중 하나입니다. 종료 코드는 ready가 0, not-ready/pending/failed/timeout/unreachable이 +1, 잘못된 인수가 64입니다. + ### `ocx doctor` 읽기 전용 환경 및 연결 진단을 실행합니다. 상태 경로와 파일시스템 유형, WSL 이중 설치, 프록시 diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index 0fab5b4d43..6b750f304c 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -142,6 +142,18 @@ tokens, authorization headers, request content, emails, and account identities. Identity-check the live proxy. Human output reports PID/port; `--json` emits `{ok, pid, port}`. The command exits 0 only when healthy and 1 otherwise, making it suitable for service probes. +### `ocx ready [--json] [--wait [--timeout ]]` + +Check post-sync readiness through the unauthenticated `GET /readyz` endpoint. It returns `200` when +ready, or `503` with `Retry-After: 1` for `pending` and terminal `failed`. Its sanitized HTTP identity +is `{service, version, uptime, pid, port, status}`. Old proxies without `/readyz` fail closed as +`unreachable`; `/healthz` is separate liveness, not readiness. The command performs one probe by +default; `--wait` polls until ready or timeout, but exits immediately on terminal `failed`. The +default timeout is 45 seconds; `--timeout ` requires `--wait` and accepts 1–300 seconds. +CLI JSON emits `{ready, status, pid, port}`, where `status` is `ready`, `pending`, `failed`, or +`unreachable`. Exit codes are 0 for ready; 1 for not-ready, pending, failed, timeout, or +unreachable; and 64 for invalid arguments. + ### `ocx doctor` Run read-only environment and connectivity diagnostics: state paths and filesystem type, WSL dual diff --git a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md index e082593874..52eceb8a1d 100644 --- a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md @@ -153,6 +153,19 @@ Identity-check живого прокси. Текстовый вывод сооб `{ok, pid, port}`. Команда завершается кодом 0 только когда прокси здоров, и 1 во всех остальных случаях, поэтому подходит для service probe. +### `ocx ready [--json] [--wait [--timeout ]]` + +Проверяет готовность после синхронизации через не требующий аутентификации `GET /readyz`. При +готовности возвращается `200`; для `pending` и терминального `failed` возвращается `503` с +`Retry-After: 1`. Санитизированные поля HTTP-ответа: `{service, version, uptime, pid, port, status}`. +Старые прокси без `/readyz` fail-closed как `unreachable`; `/healthz` — отдельная проверка liveness, +а не готовности. По умолчанию команда выполняет одну пробу. `--wait` опрашивает до готовности или +тайм-аута, но при терминальном `failed` завершается немедленно. Тайм-аут по умолчанию — 45 секунд; +`--timeout ` требует `--wait` и принимает значения 1–300 секунд. CLI JSON выдаёт +`{ready, status, pid, port}`, где `status` — `ready`, `pending`, `failed` или +`unreachable`. Коды завершения: 0 — готово; 1 — не готово, pending, failed, тайм-аут или +недоступность; 64 — недопустимые аргументы. + ### `ocx doctor` Запускает read-only диагностику среды и связности: пути состояний и тип файловой системы, diff --git a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md index 1800d12a71..ca8595996e 100644 --- a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md @@ -115,6 +115,16 @@ ocx status --json 对正在运行的代理做身份校验。人类可读输出报告 PID/端口；`--json` 输出 `{ok, pid, port}`。只有在健康时该命令才以 0 退出，否则以 1 退出，因此适合用作服务探针。 +### `ocx ready [--json] [--wait [--timeout ]]` + +通过无需认证的 `GET /readyz` 端点检查同步后的就绪状态。就绪时返回 `200`；状态为 `pending` 或 +终态 `failed` 时返回 `503`，并带有 `Retry-After: 1`。HTTP 仅返回经脱敏的身份字段 +`{service, version, uptime, pid, port, status}`。不支持 `/readyz` 的旧代理会按 `unreachable` 失败关闭； +`/healthz` 是独立的存活检查，不是就绪检查。默认只探测一次；`--wait` 会轮询到就绪或超时，但遇到终态 +`failed` 会立即退出。默认超时为 45 秒；`--timeout ` 必须与 `--wait` 一起使用，取值范围为 1–300 秒。CLI JSON +输出 `{ready, status, pid, port}`，其中 `status` 为 `ready`、`pending`、`failed` 或 +`unreachable`。退出码：就绪为 0；未就绪、pending、failed、超时或无法连接为 1；参数无效为 64。 + ### `ocx doctor` 运行只读的环境与连通性诊断：状态路径和文件系统类型、WSL 双重安装、代理环境/配置、ChatGPT 可达性、Codex 插件和项目配置警告，以及待处理的历史迁移。Codex app-home 定位部分也会检测狭义的 Windows Orca 运行时 home 不匹配，并在适用时解释服务迁移。此诊断展示的路径会对操作系统用户名进行脱敏。Doctor 会输出修复提示，但不会自动应用。 diff --git a/src/cli/help.ts b/src/cli/help.ts index dc12bc1bf0..77a50b5166 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -253,11 +253,13 @@ const helpEntries: Record = { usage: "ocx ready [--json] [--wait [--timeout ]]", summary: "Check post-sync readiness. Exits 0 only when ready.", details: [ - "Default is a single identity-checked /readyz probe.", - "--wait polls until ready or the timeout elapses (default 45s, max 300s).", + "Exact unauthenticated GET /readyz returns HTTP 200 when ready, or 503 with Retry-After: 1 for pending or failed.", + "Its sanitized HTTP identity is {service, version, uptime, pid, port, status}; /healthz is separate liveness, not readiness.", + "Default is a single identity-checked /readyz probe; old proxies without /readyz fail closed as unreachable.", + "--wait polls until ready or timeout, but exits immediately on terminal failed (default 45s, max 300s).", "--timeout requires --wait and accepts a positive integer (1..300).", "--json emits {ready, status, pid, port}; status is one of ready|pending|failed|unreachable.", - "Invalid or unknown arguments exit 64. Not-ready, pending, failed, and timeout exit nonzero.", + "Invalid or unknown arguments exit 64. Not-ready, pending, failed, timeout, and unreachable exit 1.", ], }, }; diff --git a/structure/03_catalog-and-subagents.md b/structure/03_catalog-and-subagents.md index ba4ab300bf..da535fc20f 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -42,12 +42,16 @@ Calls without a supplied gate receive a fresh private gate that intentionally re `ok: true` with no nonempty warning becomes ready; `null`, a throw, `ok !== true`, or a nonempty warning becomes failed. State is isolated per server instance. -Exact unauthenticated `GET /readyz` returns sanitized identity fields plus pending, ready, or failed. -The `ocx ready` probe validates the service, version, uptime, PID, port, status, and HTTP/status -pairing. With `--wait`, it applies one absolute deadline across discovery, readiness probes, polling, -and sleeps; the single-probe path preserves the existing per-call timeout without a wait deadline. -Older proxies without `/readyz` fail closed as unreachable. `/healthz` remains the separate liveness -contract. +Exact unauthenticated `GET /readyz` returns sanitized identity fields plus pending, ready, or failed: +`200` for ready, or `503` with `Retry-After: 1` for pending and terminal failed. The full CLI syntax +is `ocx ready [--json] [--wait [--timeout ]]`. The probe validates the service, version, +uptime, PID, port, status, and HTTP/status pairing. The default is one probe. With `--wait`, it +applies one absolute deadline (45 seconds by default) across discovery, readiness probes, polling, +and sleeps, but exits immediately on terminal failed. CLI `--json` emits +`{ready, status, pid, port}`, with status in `ready|pending|failed|unreachable`. Exit 0 means ready; +exit 1 covers not-ready, pending, failed, timeout, and unreachable; exit 64 means invalid arguments. +Older proxies without `/readyz` fail closed as unreachable. `/healthz` remains the separate +liveness contract. ## Entry shape diff --git a/tests/cli-ready-subprocess.test.ts b/tests/cli-ready-subprocess.test.ts new file mode 100644 index 0000000000..7c2bd5bbd8 --- /dev/null +++ b/tests/cli-ready-subprocess.test.ts @@ -0,0 +1,183 @@ +/** + * Real subprocess/loopback coverage for ocx ready dispatch boundaries. + * + * Keep tests/cli-ready.test.ts injected-only. These focused integration tests + * prove that the top-level CLI preserves terminal-failed and pre-parse behavior + * with isolated homes and an actual discovered proxy fixture. + */ +import { describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); +const cliPath = join(repoRoot, "src", "cli", "index.ts"); + +interface CliResult { + exitCode: number; + stdout: string; + stderr: string; + elapsedMs: number; + timedOut: boolean; +} + +async function runCli( + args: string[], + env: Record, + killAfterMs = 3_000, +): Promise { + const startedAt = performance.now(); + const child = Bun.spawn([process.execPath, cliPath, ...args], { + cwd: repoRoot, + env: { ...process.env, ...env }, + stdout: "pipe", + stderr: "pipe", + }); + const stdoutPromise = new Response(child.stdout).text(); + const stderrPromise = new Response(child.stderr).text(); + let timedOut = false; + let timer: ReturnType | undefined; + const timeout = new Promise(resolve => { + timer = setTimeout(() => { + timedOut = true; + child.kill(); + void child.exited.then(resolve); + }, killAfterMs); + }); + const exitCode = await Promise.race([child.exited, timeout]); + if (timer !== undefined) clearTimeout(timer); + const [stdout, stderr] = await Promise.all([stdoutPromise, stderrPromise]); + return { exitCode, stdout, stderr, elapsedMs: performance.now() - startedAt, timedOut }; +} + +function isolatedHomes(prefix: string): { root: string; opencodexHome: string; codexHome: string } { + const root = mkdtempSync(join(tmpdir(), prefix)); + const opencodexHome = join(root, "opencodex"); + const codexHome = join(root, "codex"); + mkdirSync(opencodexHome, { recursive: true }); + mkdirSync(codexHome, { recursive: true }); + return { root, opencodexHome, codexHome }; +} + +function writeRuntimePort(opencodexHome: string, port: number, pid: number): void { + writeFileSync( + join(opencodexHome, "runtime-port.json"), + JSON.stringify({ pid, port, hostname: "127.0.0.1" }) + "\n", + "utf8", + ); +} + +describe("ocx ready real subprocess", () => { + test("ready --wait exits immediately on terminal failed readiness", async () => { + const homes = isolatedHomes("ocx-ready-subprocess-failed-"); + const fixturePid = process.pid; + let healthzHits = 0; + let readyzHits = 0; + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch(request) { + const path = new URL(request.url).pathname; + if (path === "/healthz") { + healthzHits++; + return Response.json({ + service: "opencodex", + status: "ok", + version: "test", + uptime: 1, + pid: fixturePid, + }); + } + if (path === "/readyz") { + readyzHits++; + return Response.json( + { + service: "opencodex", + version: "test", + uptime: 1, + status: "failed", + pid: fixturePid, + port: server.port, + }, + { status: 503 }, + ); + } + return new Response("not found", { status: 404 }); + }, + }); + writeRuntimePort(homes.opencodexHome, server.port, fixturePid); + + try { + const result = await runCli( + ["ready", "--wait", "--timeout", "300", "--json"], + { OPENCODEX_HOME: homes.opencodexHome, CODEX_HOME: homes.codexHome }, + ); + + expect(healthzHits).toBe(1); + expect(readyzHits).toBe(1); + expect(result.timedOut).toBe(false); + expect(result.exitCode).toBe(1); + expect(result.elapsedMs).toBeLessThan(2_000); + expect(JSON.parse(result.stdout.trim())).toEqual({ + ready: false, + status: "failed", + pid: fixturePid, + port: server.port, + }); + expect(result.stderr).toBe(""); + } finally { + server.stop(true); + rmSync(homes.root, { recursive: true, force: true }); + } + }); + + test("invalid --timeout exits 64 before discovery and auto-restore", async () => { + const homes = isolatedHomes("ocx-ready-subprocess-invalid-"); + const fixturePid = process.pid; + let healthzHits = 0; + let readyzHits = 0; + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch(request) { + const path = new URL(request.url).pathname; + if (path === "/healthz") healthzHits++; + if (path === "/readyz") readyzHits++; + return Response.json({ + service: "opencodex", + status: "ok", + version: "test", + uptime: 1, + pid: fixturePid, + }); + }, + }); + writeRuntimePort(homes.opencodexHome, server.port, fixturePid); + // If the global auto-restore preflight runs, this non-file state emits an + // auto-restore warning. Invalid ready args must exit before inspecting it. + mkdirSync(join(homes.opencodexHome, "codex-shim.json")); + + try { + const result = await runCli( + ["ready", "--timeout", "5"], + { + OPENCODEX_HOME: homes.opencodexHome, + CODEX_HOME: homes.codexHome, + OPENCODEX_CODEX_SHIM_AUTO_RESTORE: "1", + }, + ); + + expect(result.timedOut).toBe(false); + expect(result.exitCode).toBe(64); + expect(result.stdout).toBe(""); + expect(result.stderr).toContain("Usage: ocx ready"); + expect(result.stderr).not.toContain("auto-restore"); + expect(healthzHits).toBe(0); + expect(readyzHits).toBe(0); + } finally { + server.stop(true); + rmSync(homes.root, { recursive: true, force: true }); + } + }); +}); From 2331ff4c3c377f80aae49fd6062e4cc196e0051d Mon Sep 17 00:00:00 2001 From: Diego Cantarero <71346275+diegocantarero@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:14:24 -0600 Subject: [PATCH 03/11] test(readyz): assert Retry-After is exactly 1 --- tests/server-live.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/server-live.test.ts b/tests/server-live.test.ts index 990c06a141..7f6ee210d8 100644 --- a/tests/server-live.test.ts +++ b/tests/server-live.test.ts @@ -905,7 +905,7 @@ describe("GET /readyz", () => { // Readiness is pending right after bind: 503 + Retry-After, sanitized body. const readyzRes = await fetch(new URL("/readyz", base)); expect(readyzRes.status).toBe(503); - expect(readyzRes.headers.get("retry-after")).toBeTruthy(); + expect(readyzRes.headers.get("retry-after")).toBe("1"); const readyzBody = (await readyzRes.json()) as Record; expect(readyzBody.service).toBe("opencodex"); expect(readyzBody.status).toBe("pending"); @@ -949,7 +949,7 @@ describe("GET /readyz", () => { try { const res = await fetch(new URL("/readyz", server.url)); expect(res.status).toBe(503); - expect(res.headers.get("retry-after")).toBeTruthy(); + expect(res.headers.get("retry-after")).toBe("1"); expect(((await res.json()) as { status: string }).status).toBe("failed"); } finally { await server.stop(true); From 1a26fc571363c41fad0eb2928e33e7318fe09b62 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 06:42:46 +0200 Subject: [PATCH 04/11] fix(ready): rebase onto dev and harden the exact-method contract - Rebase the readiness PR onto current dev: adopt the existing deadlineAt discovery budget in proxy-liveness instead of the duplicate deadlineMs/ probeBudget mechanism, keeping dev's attempts/source discriminator. - Answer POST /readyz and GET /readyz/ with a deterministic JSON 404 so the exact-GET contract never depends on whether gui/dist exists (the GUI SPA fallback previously served index.html with 200 for those paths). - Re-target the ready docs to reference/cli/lifecycle.md across all locales and document the 1-300 second --timeout range. - Fix the deadline test off-by-one and AbortSignal stub ordering; adapt the catalog-prewarm source-order test to startServer(port, { readinessGate }). --- src/cli/ready.ts | 7 +++--- src/server/index.ts | 10 +++++--- tests/cli-catalog-prewarm.test.ts | 2 +- tests/cli-ready.test.ts | 25 ++++++++++--------- tests/proxy-liveness.test.ts | 41 +++++++++++++++++-------------- 5 files changed, 47 insertions(+), 38 deletions(-) diff --git a/src/cli/ready.ts b/src/cli/ready.ts index a19aa0645d..e8868d3238 100644 --- a/src/cli/ready.ts +++ b/src/cli/ready.ts @@ -182,9 +182,10 @@ export async function runReady(args: ReadyArgs, io: ReadyIo = {}): Promise { test("handleStart schedules catalog prewarm immediately after a successful bind", async () => { const cli = (await readText("src/cli/index.ts")).replace(/\r\n/g, "\n"); - const bindIdx = cli.indexOf("server = startServer(port, { localAttestationSecret });"); + const bindIdx = cli.indexOf("server = startServer(port"); const prewarmIdx = cli.indexOf("scheduleCatalogPrewarm()"); const breakIdx = cli.indexOf("\n break;", bindIdx); diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index 036663e073..cdd332aaf4 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -313,8 +313,8 @@ describe("runReady --wait (single bounded loop, deterministic)", () => { test("uses the configured --timeout value as the single deadline", async () => { const { io } = captureIo(); - let t = 0; const seenNow: number[] = []; + let t = -1000; // so the first read (which establishes the deadline) is 0 await runReady( { json: false, wait: true, timeoutSeconds: 7 }, { @@ -325,10 +325,11 @@ describe("runReady --wait (single bounded loop, deterministic)", () => { sleep: async () => {}, }, ); - // The deadline is 7000ms; the loop stops once now() >= 7000. - const firstOverDeadline = seenNow.find(n => n >= 7000); - expect(firstOverDeadline).toBeGreaterThanOrEqual(7000); - expect(firstOverDeadline).toBeLessThan(7000 + 1000); + // First read is 0, so the deadline is exactly 0 + 7*1000 = 7000. The loop + // must stop on the FIRST reading at/after it, i.e. the last value read is + // 7000 — proving timeoutSeconds actually controls the deadline. + expect(seenNow.at(-1)).toBe(7000); + expect(seenNow.filter(n => n >= 7000)).toEqual([7000]); }); }); @@ -755,24 +756,24 @@ describe("invalid ready matrices never invoke findLive/probe (P1 counters)", () // sequential candidate probes inside findLiveProxy are bounded by the single // wait deadline. It must NOT use the injected logical now: AbortSignal time is // real wall-clock time, so Date.now is authoritative for the network deadline. -// The non-wait path keeps findLiveProxy's built-in default (no deadlineMs). +// The non-wait path keeps findLiveProxy's built-in default (no deadlineAt). describe("runReady production findLiveProxy deadline wiring (source-level)", () => { const readySource = readFileSync(join(import.meta.dir, "../src/cli/ready.ts"), "utf8"); - test("the --wait path derives deadlineMs from Date.now() + remainingMs (not the injected now)", () => { + test("the --wait path derives deadlineAt from Date.now() + remainingMs (not the injected now)", () => { // Date.now (real wall clock) is authoritative for the AbortSignal deadline; // the injected logical now must not govern the network timeout. - expect(readySource).toContain("deadlineMs: Date.now() + remainingMs"); + expect(readySource).toContain("deadlineAt: Date.now() + remainingMs"); // The per-probe cap is forwarded alongside the absolute deadline. expect(readySource).toContain("timeoutMs: IO_TIMEOUT_CAP_MS"); }); - test("the non-wait path keeps findLiveProxy's built-in default (no deadlineMs)", () => { + test("the non-wait path keeps findLiveProxy's built-in default (no deadlineAt)", () => { // The default find forwards {} when remainingMs is undefined so the // built-in per-probe timeout (no deadline) is preserved for the single probe. expect(readySource).toContain("remainingMs === undefined ? {}"); - // deadlineMs is only ever passed conditionally (in the wait branch), never - // as an unconditional findLiveProxy({ deadlineMs: ... }). - expect(readySource).not.toContain("findLiveProxy({ deadlineMs"); + // deadlineAt is only ever passed conditionally (in the wait branch), never + // as an unconditional findLiveProxy({ deadlineAt: ... }). + expect(readySource).not.toContain("findLiveProxy({ deadlineAt"); }); }); diff --git a/tests/proxy-liveness.test.ts b/tests/proxy-liveness.test.ts index 9e5e653cf7..5b25e305a0 100644 --- a/tests/proxy-liveness.test.ts +++ b/tests/proxy-liveness.test.ts @@ -271,17 +271,17 @@ describe("findLiveProxy", () => { }); // ── findLiveProxy single-deadline candidate gating ─────────────────────────── -// The absolute deadlineMs must gate EVERY internal candidate probe, not just -// the first. Before each proxyIdentityAt the budget is recomputed against the -// injected nowFn: a non-positive remaining terminates discovery (returns null) -// so no later candidate fetch starts, and each probe's timeoutMs is capped to -// min(existing per-probe cap, positive remaining). Existing no-deadline callers -// keep their current semantics (no deadlineMs → unbounded per-probe default). +// The absolute deadlineAt must gate EVERY internal candidate probe, not just +// the first. proxyIdentityAt recomputes the remaining budget against the +// injected nowFn before each attempt: a non-positive remaining aborts without +// fetching, and each probe's timeoutMs is capped to min(existing per-probe cap, +// positive remaining). Existing no-deadline callers keep their current +// semantics (no deadlineAt → unbounded per-probe default). describe("findLiveProxy single-deadline candidate gating", () => { test("deadline already expired before the first probe → no fetch, returns null", async () => { let fetchCalls = 0; const live = await findLiveProxy({ - deadlineMs: 1000, + deadlineAt: 1000, nowFn: () => 2000, // already past the deadline before any probe readPidFn: () => 4242, readRuntimeFn: pid => (pid === 4242 ? { port: 58195 } : null), @@ -296,7 +296,7 @@ describe("findLiveProxy single-deadline candidate gating", () => { let clock = 1000; const urls: string[] = []; const live = await findLiveProxy({ - deadlineMs: 2000, + deadlineAt: 2000, nowFn: () => clock, // Two sequential candidates wired: pid path → port 58195; config fallback → 10100. readPidFn: () => 4242, @@ -311,7 +311,7 @@ describe("findLiveProxy single-deadline candidate gating", () => { expect(sig?.aborted).toBe(false); // Advance the clock PAST the deadline inside the first fetch so the // next candidate's budget check terminates discovery. - clock = 3000; // > deadlineMs (2000) + clock = 3000; // > deadlineAt (2000) // Foreign body → identity null → would normally fall through to config. return healthz({ status: "ok" }); }) as typeof fetch, @@ -328,13 +328,14 @@ describe("findLiveProxy single-deadline candidate gating", () => { const calls: number[] = []; const originalTimeout = AbortSignal.timeout; AbortSignal.timeout = ((ms: number) => { - calls.push(ms); - return originalTimeout(ms); + const signal = originalTimeout(ms); + calls.push(ms); // record only after the signal was actually constructed + return signal; }) as typeof AbortSignal.timeout; try { let clock = 1000; await findLiveProxy({ - deadlineMs: 1300, // remaining = 300 at the first probe (< 750 cap) + deadlineAt: 1300, // remaining = 300 at the first probe (< 750 cap) nowFn: () => clock, readPidFn: () => 4242, readRuntimeFn: () => ({ port: 58195 }), @@ -355,13 +356,14 @@ describe("findLiveProxy single-deadline candidate gating", () => { const calls: number[] = []; const originalTimeout = AbortSignal.timeout; AbortSignal.timeout = ((ms: number) => { - calls.push(ms); - return originalTimeout(ms); + const signal = originalTimeout(ms); + calls.push(ms); // record only after the signal was actually constructed + return signal; }) as typeof AbortSignal.timeout; try { let clock = 1000; await findLiveProxy({ - deadlineMs: 100000, // remaining ≈ 99000 ≫ 750 cap + deadlineAt: 100000, // remaining ≈ 99000 ≫ 750 cap timeoutMs: 750, // explicit per-probe cap, mirroring the production wiring nowFn: () => clock, readPidFn: () => 4242, @@ -379,12 +381,13 @@ describe("findLiveProxy single-deadline candidate gating", () => { expect(calls).toEqual([750]); }); - test("no deadlineMs retains the existing multi-candidate fallback behavior (two fetches)", async () => { - // Without deadlineMs the pid path failing falls through to config, which - // succeeds — proving the deadline gate is inert when deadlineMs is unset. + test("no deadlineAt retains the existing multi-candidate fallback behavior (two fetches)", async () => { + // Without deadlineAt the pid path failing falls through to config, which + // succeeds — proving the deadline gate is inert when deadlineAt is unset. const urls: string[] = []; const live = await findLiveProxy({ readPidFn: () => 4242, + verifyPidFn: candidate => candidate, readRuntimeFn: pid => (pid === 4242 ? { port: 58195 } : null), configFn: () => ({ port: 10100 }), fetchFn: (async (url: string | URL | Request) => { @@ -396,7 +399,7 @@ describe("findLiveProxy single-deadline candidate gating", () => { }); expect(urls).toEqual(["http://127.0.0.1:58195/healthz", "http://127.0.0.1:10100/healthz"]); expect(urls).toHaveLength(2); - expect(live).toEqual({ pid: 4242, port: 10100, hostname: undefined }); + expect(live).toEqual({ pid: 4242, port: 10100, hostname: undefined, source: "config" }); }); }); From 5c32a3aa38fa654558b6c9b9cb639f82342db91f Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 06:48:55 +0200 Subject: [PATCH 05/11] refactor(ready): share the default probe timeout constant Extract the 750ms per-probe ceiling as DEFAULT_PROBE_TIMEOUT_MS in proxy-liveness and import it from the ready CLI instead of redeclaring IO_TIMEOUT_CAP_MS, so liveness and readiness defaults cannot diverge. --- src/cli/ready.ts | 19 ++++++------------- src/server/proxy-liveness.ts | 7 +++++-- tests/cli-ready.test.ts | 4 ++-- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/cli/ready.ts b/src/cli/ready.ts index e8868d3238..f7d6a4f573 100644 --- a/src/cli/ready.ts +++ b/src/cli/ready.ts @@ -15,29 +15,22 @@ * vocabulary, plus pid/port; never carries sync message, warning, path, * provider, account, or error data. */ -import { findLiveProxy, probeReadiness } from "../server/proxy-liveness"; +import { DEFAULT_PROBE_TIMEOUT_MS, findLiveProxy, probeReadiness } from "../server/proxy-liveness"; /** Default --wait deadline (45s). */ export const DEFAULT_READY_WAIT_TIMEOUT_SECONDS = 45; /** Maximum allowed --wait deadline (300s). */ export const MAX_READY_WAIT_TIMEOUT_SECONDS = 300; const POLL_INTERVAL_MS = 500; -/** - * Per-call fetch ceiling for the production discovery/probe defaults. Each - * find/probe fetch is bounded both by the single wait deadline (so no single - * fetch outlives it) and by this ceiling (so a nearly-full 45s deadline does - * not become a 45-second hang on one request). Matches the underlying - * findLiveProxy/probeReadiness default of 750ms for the non-wait path. - */ -const IO_TIMEOUT_CAP_MS = 750; /** * Cap a remaining deadline budget to a positive per-call IO timeout. Stays * positive (>= 1ms) and never exceeds the logical remaining time nor the - * per-call ceiling. Passed by the production defaults to findLiveProxy's / - * probeReadiness's timeoutMs so their fetch waits are bounded by the deadline. + * shared per-call ceiling (DEFAULT_PROBE_TIMEOUT_MS). Passed by the production + * defaults to findLiveProxy's / probeReadiness's timeoutMs so their fetch waits + * are bounded by the deadline. */ function capIoTimeout(remainingMs: number): number { - return Math.max(1, Math.min(remainingMs, IO_TIMEOUT_CAP_MS)); + return Math.max(1, Math.min(remainingMs, DEFAULT_PROBE_TIMEOUT_MS)); } /** Fixed sanitized CLI status vocabulary. */ @@ -187,7 +180,7 @@ export async function runReady(args: ReadyArgs, io: ReadyIo = {}): Promise number; } +/** Default per-probe fetch ceiling shared by liveness and readiness probes. */ +export const DEFAULT_PROBE_TIMEOUT_MS = 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, @@ -95,7 +98,7 @@ export async function proxyIdentityAt( const fetchFn = io.fetchFn ?? fetch; const sleepFn = io.sleepFn ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); const nowFn = io.nowFn ?? Date.now; - const baseTimeoutMs = io.timeoutMs ?? 750; + const baseTimeoutMs = io.timeoutMs ?? DEFAULT_PROBE_TIMEOUT_MS; const requestedAttempts = Math.trunc(io.attempts ?? 1); const attempts = Number.isNaN(requestedAttempts) ? 1 @@ -304,7 +307,7 @@ export async function probeReadiness( const fetchFn = io.fetchFn ?? fetch; try { const res = await fetchFn(`http://${probeHostname(opts.hostname)}:${port}/readyz`, { - signal: AbortSignal.timeout(io.timeoutMs ?? 750), + signal: AbortSignal.timeout(io.timeoutMs ?? DEFAULT_PROBE_TIMEOUT_MS), }); // Parse even on 503: /readyz returns JSON with a sanitized status while pending. const body = (await res.json().catch(() => null)) as unknown; diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index cdd332aaf4..16b24c806d 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -764,8 +764,8 @@ describe("runReady production findLiveProxy deadline wiring (source-level)", () // Date.now (real wall clock) is authoritative for the AbortSignal deadline; // the injected logical now must not govern the network timeout. expect(readySource).toContain("deadlineAt: Date.now() + remainingMs"); - // The per-probe cap is forwarded alongside the absolute deadline. - expect(readySource).toContain("timeoutMs: IO_TIMEOUT_CAP_MS"); + // The shared per-probe cap is forwarded alongside the absolute deadline. + expect(readySource).toContain("timeoutMs: DEFAULT_PROBE_TIMEOUT_MS"); }); test("the non-wait path keeps findLiveProxy's built-in default (no deadlineAt)", () => { From 356ae7b0a7afc7f5318f6a1a3036cebb963b6588 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Mon, 3 Aug 2026 06:59:46 +0200 Subject: [PATCH 06/11] fix(ready): keep readiness discovery free of killable-pid OS verification verifyPidIdentity spawns WMIC/PowerShell (up to seconds on Windows) and is only needed for kill targets. runReady's production discovery now passes verifyPidFn: () => null so no OS command-line check runs outside the wait deadline; the /healthz identity marker and strict /readyz contract validation are unchanged. --- src/cli/ready.ts | 8 +++++++- tests/cli-ready.test.ts | 14 +++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/cli/ready.ts b/src/cli/ready.ts index f7d6a4f573..3b1b49eaf5 100644 --- a/src/cli/ready.ts +++ b/src/cli/ready.ts @@ -179,8 +179,14 @@ export async function runReady(args: ReadyArgs, io: ReadyIo = {}): Promise null } + : { deadlineAt: Date.now() + remainingMs, timeoutMs: DEFAULT_PROBE_TIMEOUT_MS, verifyPidFn: () => null }, ); return live ? { pid: live.pid, port: live.port, hostname: live.hostname } : null; }); diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index 16b24c806d..e0dccad08e 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -769,11 +769,19 @@ describe("runReady production findLiveProxy deadline wiring (source-level)", () }); test("the non-wait path keeps findLiveProxy's built-in default (no deadlineAt)", () => { - // The default find forwards {} when remainingMs is undefined so the - // built-in per-probe timeout (no deadline) is preserved for the single probe. - expect(readySource).toContain("remainingMs === undefined ? {}"); + // The default find forwards only verifyPidFn: () => null when remainingMs is + // undefined so the built-in per-probe timeout (no deadline) is preserved for + // the single probe and no OS pid verification runs outside a deadline. + expect(readySource).toContain("remainingMs === undefined\n ? { verifyPidFn: () => null }"); // deadlineAt is only ever passed conditionally (in the wait branch), never // as an unconditional findLiveProxy({ deadlineAt: ... }). expect(readySource).not.toContain("findLiveProxy({ deadlineAt"); }); + + test("readiness discovery never runs killable-pid OS verification (deadline-bounded, non-destructive)", () => { + // verifyPidIdentity spawns WMIC/PowerShell (up to seconds on Windows) and is + // only needed for kill targets. Readiness must not run it: the check would + // be unbounded by the wait deadline. + expect(readySource).toContain("verifyPidFn: () => null"); + }); }); From 4b1b9d4f6ac5a548fd8c5417bf082e36b6c17540 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Thu, 6 Aug 2026 05:37:10 +0200 Subject: [PATCH 07/11] fix(ready): integrate readiness gate with dev startup sync (overtake) Rebase the readiness PR onto current dev and resolve the two conflict surfaces against dev's modern startup path: - src/cli/index.ts: keep syncCodexOnStartIfEnabled (Codex toggle + #1046 stale app-server warning) and thread the readiness gate into it as the fourth argument, so /readyz reflects the real sync outcome without bypassing the toggle or the write-tracking contract. - src/codex/desired-state.ts: accept an optional readiness gate; mark ready immediately when the Codex integration is explicitly OFF (nothing to sync), otherwise drive the gate via runStartupReadinessSync from the raw sync outcome (ready only on ok=true with no nonempty warning). - src/server/readiness.ts: runStartupReadinessSync returns the raw sync outcome so callers keep the #1046 write flags without a second call. - structure/03_catalog-and-subagents.md: un-indent the pre-existing cache paragraph and fix the Startup readiness heading to top-level markdown. - tests/cli-ready.test.ts: update the source-level wiring guard to assert the new syncCodexOnStartIfEnabled(port, config, undefined, readinessGate) contract instead of the pre-rebase inline wiring. --- src/cli/index.ts | 2 +- structure/03_catalog-and-subagents.md | 10 +++++----- tests/cli-ready.test.ts | 20 +++++++++++--------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index fa40b839a2..049048fda7 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -26,7 +26,7 @@ import { installCrashGuards } from "../lib/crash-guard"; import { hasHelpFlag, printSubcommandUsage, printUsage, printVersion } from "./help"; import { findAvailablePort, isAddrInUse, PortUnavailableError, shouldPersistSelectedPort, waitForPortAvailable } from "../server/ports"; import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-liveness"; -import { createReadinessGate, runStartupReadinessSync } from "../server/readiness"; +import { createReadinessGate } from "../server/readiness"; import { parseReadyArgs, runReady, type ReadyArgs } from "./ready"; import { stopProxy } from "../lib/process-control"; import { loadServiceTokenFromFile } from "../lib/service-secrets"; diff --git a/structure/03_catalog-and-subagents.md b/structure/03_catalog-and-subagents.md index da535fc20f..0b48f54886 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -29,12 +29,12 @@ bundled catalog never contains those rows. Codex App model picker visibility comes from this shared catalog, not from patching the App. - Provider live-model lists are cached with a configured TTL (`src/codex/model-cache.ts`). Adding, - deleting, or editing a provider's shape clears that per-provider cache; a disabled-only change - deliberately does not, because a disabled provider is already excluded from the catalog gather - instead. Codex's own `models_cache.json` is a different cache, invalidated by catalog refresh. +Provider live-model lists are cached with a configured TTL (`src/codex/model-cache.ts`). Adding, +deleting, or editing a provider's shape clears that per-provider cache; a disabled-only change +deliberately does not, because a disabled provider is already excluded from the catalog gather +instead. Codex's own `models_cache.json` is a different cache, invalidated by catalog refresh. - ## Startup readiness +## Startup readiness Each `startServer` invocation owns a private, one-shot readiness gate created before the listener binds. `handleStart` supplies its gate and transitions it after the shared catalog sync settles. diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index e0dccad08e..ef5800264a 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -611,14 +611,16 @@ describe("runReady --wait deadline correctness", () => { // A bounded source-level assertion reading ONLY src/cli/index.ts. It verifies // that the SAME identifier `readinessGate` is (1) created in handleStart via // createReadinessGate(), (2) passed to startServer in the retry path, and -// (3) passed to runStartupReadinessSync wrapping syncModelsToCodex(port), in -// that order. This catches a regression where the gate is wired to only one of -// the two call sites. It complements the executable runStartupReadinessSync -// outcome tests in tests/proxy-liveness.test.ts (no architecture refactor). +// (3) threaded into the startup sync via syncCodexOnStartIfEnabled(..., gate), +// in that order. The gate-drive itself lives in syncCodexOnStartIfEnabled (the +// modern dev startup path, which respects the Codex integration toggle and the +// #1046 write-tracking contract); this catches a regression where the gate is +// wired to only one of the two call sites. It complements the executable +// runStartupReadinessSync outcome tests in tests/proxy-liveness.test.ts. describe("handleStart readinessGate wiring (source-level)", () => { const cliSource = readFileSync(join(import.meta.dir, "../src/cli/index.ts"), "utf8"); - test("readinessGate is created, threaded into startServer, and into runStartupReadinessSync — in order", () => { + test("readinessGate is created, threaded into startServer, and into the startup sync — in order", () => { const createMatch = cliSource.match(/const\s+readinessGate\s*=\s*createReadinessGate\(\)/); expect(createMatch, "handleStart must create readinessGate via createReadinessGate()").not.toBeNull(); @@ -626,11 +628,11 @@ describe("handleStart readinessGate wiring (source-level)", () => { expect(startMatch, "startServer must be called with { readinessGate } in the retry path").not.toBeNull(); const syncMatch = cliSource.match( - /runStartupReadinessSync\s*\(\s*readinessGate\s*,\s*\(\s*\)\s*=>\s*syncModelsToCodex\s*\(\s*port\s*\)\s*\)/, + /syncCodexOnStartIfEnabled\s*\(\s*port\s*,\s*config\s*,\s*undefined\s*,\s*readinessGate\s*\)/, ); - expect(syncMatch, "runStartupReadinessSync must wrap () => syncModelsToCodex(port) with readinessGate").not.toBeNull(); + expect(syncMatch, "syncCodexOnStartIfEnabled must receive readinessGate so the startup sync drives /readyz").not.toBeNull(); - // Source order must be: create → startServer → runStartupReadinessSync. + // Source order must be: create → startServer → startup sync. const createIdx = createMatch!.index!; const startIdx = startMatch!.index!; const syncIdx = syncMatch!.index!; @@ -643,7 +645,7 @@ describe("handleStart readinessGate wiring (source-level)", () => { // call site references that identifier (no shadowing, no second local). const declarations = cliSource.match(/\breadinessGate\s*=/g); expect(declarations, "readinessGate must be assigned exactly once").toHaveLength(1); - // Three references total: one declaration + startServer + runStartupReadinessSync. + // Three references total: one declaration + startServer + startup sync. const references = cliSource.match(/\breadinessGate\b/g); expect(references?.length ?? 0).toBeGreaterThanOrEqual(3); }); From 25b8dd88c6b415a0374912475bca255401e769e0 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Thu, 6 Aug 2026 06:07:11 +0200 Subject: [PATCH 08/11] fix(ready): address CodeRabbit findings on the rebased head - src/server/index.ts: /readyz reports pending (503) while the listener is draining instead of advertising ready; the one-shot readiness gate is not mutated on shutdown (startup-sync ownership preserved). - src/server/proxy-liveness.ts: drop the untyped deadlineMs alias in findLiveProxy; no caller supplies it and LivenessIo only declares deadlineAt. - tests/server-live.test.ts: add a draining /readyz regression test. - tests/proxy-liveness.test.ts: replace the vacuous JSON.stringify privacy assertion with a direct own-property surface check. - tests/cli-ready-subprocess.test.ts: raise the subprocess timing ceiling so cold Bun startup on CI is not mistaken for readiness timeout behavior. - tests/cli-ready.test.ts: correct off-by-one-tick deadline comments and make the non-wait wiring assertion whitespace-tolerant. - tests/update-notify.test.ts: fail loudly when startServer(port) is not found instead of silently passing via -1. - docs-site/.../lifecycle.md: clarify the terminal failed wording. --- .../content/docs/reference/cli/lifecycle.md | 2 +- src/server/index.ts | 7 +++- src/server/proxy-liveness.ts | 5 +-- tests/cli-ready-subprocess.test.ts | 4 +- tests/cli-ready.test.ts | 9 +++-- tests/proxy-liveness.test.ts | 11 ++--- tests/server-live.test.ts | 40 +++++++++++++++++++ 7 files changed, 63 insertions(+), 15 deletions(-) diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index 6b750f304c..b5f949c3cb 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -148,7 +148,7 @@ Check post-sync readiness through the unauthenticated `GET /readyz` endpoint. It ready, or `503` with `Retry-After: 1` for `pending` and terminal `failed`. Its sanitized HTTP identity is `{service, version, uptime, pid, port, status}`. Old proxies without `/readyz` fail closed as `unreachable`; `/healthz` is separate liveness, not readiness. The command performs one probe by -default; `--wait` polls until ready or timeout, but exits immediately on terminal `failed`. The +default; `--wait` polls until ready or timeout, but exits immediately when it observes the terminal `failed` state. The default timeout is 45 seconds; `--timeout ` requires `--wait` and accepts 1–300 seconds. CLI JSON emits `{ready, status, pid, port}`, where `status` is `ready`, `pending`, `failed`, or `unreachable`. Exit codes are 0 for ready; 1 for not-ready, pending, failed, timeout, or diff --git a/src/server/index.ts b/src/server/index.ts index 518b624d7e..389c06dd5e 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -617,7 +617,12 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server deadlineAt !== undefined && nowFn() >= deadlineAt; diff --git a/tests/cli-ready-subprocess.test.ts b/tests/cli-ready-subprocess.test.ts index 7c2bd5bbd8..7a90c4830a 100644 --- a/tests/cli-ready-subprocess.test.ts +++ b/tests/cli-ready-subprocess.test.ts @@ -112,13 +112,15 @@ describe("ocx ready real subprocess", () => { const result = await runCli( ["ready", "--wait", "--timeout", "300", "--json"], { OPENCODEX_HOME: homes.opencodexHome, CODEX_HOME: homes.codexHome }, + 10_000, ); expect(healthzHits).toBe(1); expect(readyzHits).toBe(1); expect(result.timedOut).toBe(false); expect(result.exitCode).toBe(1); - expect(result.elapsedMs).toBeLessThan(2_000); + // Far below the 300s --timeout, but tolerant of cold Bun startup on CI. + expect(result.elapsedMs).toBeLessThan(9_000); expect(JSON.parse(result.stdout.trim())).toEqual({ ready: false, status: "failed", diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index ef5800264a..47a98ea3c1 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -217,7 +217,7 @@ describe("runReady --wait (single bounded loop, deterministic)", () => { // First discovery returns null (proxy not up yet); second returns LIVE. findLive: async () => { findCalls++; return findCalls < 2 ? null : LIVE; }, probe: async () => probeBodies[Math.min(probeCalls++, probeBodies.length - 1)]!, - now: () => (t += 100), // 100, 200, 300 … never crosses the 5000ms deadline. + now: () => (t += 100), // First read is 100, so the deadline is 100 + 5000 = 5100; 100, 200, 300 … never crosses it. sleep: async () => {}, }, ); @@ -236,7 +236,7 @@ describe("runReady --wait (single bounded loop, deterministic)", () => { ...io, findLive: async () => null, probe: async () => READY_PROBE, - now: () => (t += 500), // 500, 1000, 1500 … crosses 1000 after the 2nd iteration. + now: () => (t += 500), // First read is 500, so the deadline is 500 + 1000 = 1500; the after-find read of 1500 ends the loop. sleep: async () => {}, }, ); @@ -254,7 +254,7 @@ describe("runReady --wait (single bounded loop, deterministic)", () => { ...io, findLive: async () => LIVE, probe: async () => PENDING_PROBE, - now: () => (t += 1000), // 1000, 2000, 3000, 4000 crosses the 3000 deadline. + now: () => (t += 1000), // First read is 1000, so the deadline is 1000 + 3000 = 4000; the after-probe read of 4000 ends the loop. sleep: async () => {}, }, ); @@ -774,7 +774,8 @@ describe("runReady production findLiveProxy deadline wiring (source-level)", () // The default find forwards only verifyPidFn: () => null when remainingMs is // undefined so the built-in per-probe timeout (no deadline) is preserved for // the single probe and no OS pid verification runs outside a deadline. - expect(readySource).toContain("remainingMs === undefined\n ? { verifyPidFn: () => null }"); + // Whitespace-tolerant so the assertion survives reformatting of the ternary. + expect(readySource).toMatch(/remainingMs === undefined\s*\?\s*\{ verifyPidFn: \(\) => null \}/); // deadlineAt is only ever passed conditionally (in the wait branch), never // as an unconditional findLiveProxy({ deadlineAt: ... }). expect(readySource).not.toContain("findLiveProxy({ deadlineAt"); diff --git a/tests/proxy-liveness.test.ts b/tests/proxy-liveness.test.ts index 5b25e305a0..7f6066a665 100644 --- a/tests/proxy-liveness.test.ts +++ b/tests/proxy-liveness.test.ts @@ -451,11 +451,12 @@ describe("createReadinessGate", () => { gate.markFailed(); // Only the sanitized status enum is reachable; the interface surface is fixed. expect(gate.getStatus()).toBe("failed"); - const serialized = JSON.stringify(gate); - expect(serialized).not.toContain("reason"); - expect(serialized).not.toContain("changedAt"); - expect(serialized).not.toContain("warning"); - expect(serialized).not.toContain("path"); + // JSON.stringify of a method-only object returns "{}", so it cannot prove + // the absence of closure-held diagnostic fields. Assert the own-property + // surface directly: exactly the three control methods and no data field. + expect(Object.keys(gate).sort()).toEqual(["getStatus", "markFailed", "markReady"]); + // The only readable value is the fixed sanitized enum. + expect(["pending", "ready", "failed"]).toContain(gate.getStatus()); }); }); diff --git a/tests/server-live.test.ts b/tests/server-live.test.ts index 7f6ee210d8..cc4b5113a6 100644 --- a/tests/server-live.test.ts +++ b/tests/server-live.test.ts @@ -15,6 +15,7 @@ import { type ReadinessGate, } from "../src/server/readiness"; import { startServer } from "../src/server"; +import { beginShutdownDrain, isDraining, resetLifecycleDrainStateForTests } from "../src/server/lifecycle"; import type { OcxConfig } from "../src/types"; import { fakeChatGptJwt } from "./helpers/fake-chatgpt-jwt"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; @@ -1109,3 +1110,42 @@ describe("per-server readiness gate isolation", () => { } }); }); + +describe("GET /readyz while draining", () => { + afterEach(() => { + resetLifecycleDrainStateForTests(); + }); + + test("a ready gate reports pending (503) once the listener starts draining", async () => { + saveConfig(forwardConfig()); + const gate = createReadinessGate(); + const server = startServer(0, { readinessGate: gate }); + try { + gate.markReady(); + const base = server.url; + + // Before drain: ready → 200. + const readyRes = await fetch(new URL("/readyz", base)); + expect(readyRes.status).toBe(200); + expect(((await readyRes.json()) as { status: string }).status).toBe("ready"); + + // Begin draining (as shutdown would). The one-shot gate is NOT mutated, + // but /readyz must stop advertising ready while the listener drains. + expect(isDraining()).toBe(false); + expect(beginShutdownDrain()).toBe(true); + expect(isDraining()).toBe(true); + + const drainRes = await fetch(new URL("/readyz", base)); + expect(drainRes.status).toBe(503); + expect(drainRes.headers.get("retry-after")).toBe("1"); + expect(((await drainRes.json()) as { status: string }).status).toBe("pending"); + + // The gate itself is untouched — draining is a listener state, not a gate + // transition, so the startup-sync ownership contract is preserved. + expect(gate.getStatus()).toBe("ready"); + } finally { + await server.stop(true); + resetLifecycleDrainStateForTests(); + } + }); +}); From 640488cc352b2cd955f1a0dfead2457e8cb19b2c Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Thu, 6 Aug 2026 06:16:37 +0200 Subject: [PATCH 09/11] fix(ready): reject OPTIONS /readyz with JSON 404 before the preflight branch The generic OPTIONS handler ran before the /readyz exact-GET route, so OPTIONS /readyz (and /readyz/) answered 204 instead of the deterministic JSON 404 the exact-method contract promises. Reject both paths in the OPTIONS branch and add regression coverage. --- src/server/index.ts | 6 ++++++ tests/server-live.test.ts | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/server/index.ts b/src/server/index.ts index 389c06dd5e..ce4cc234e0 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -551,6 +551,12 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { // Trailing slash is a distinct path and must not match either. const slashRes = await fetch(new URL("/readyz/", base)); expect(slashRes.status).toBe(404); + // OPTIONS is a non-GET method and must not be accepted as a readiness + // preflight (the generic OPTIONS branch would otherwise answer 204). + const optionsRes = await fetch(new URL("/readyz", base), { method: "OPTIONS" }); + expect(optionsRes.status).toBe(404); + const optionsSlashRes = await fetch(new URL("/readyz/", base), { method: "OPTIONS" }); + expect(optionsSlashRes.status).toBe(404); } finally { await server.stop(true); } From bb1aa2e358dfe80356aca88a1e029b3cead2520f Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Thu, 6 Aug 2026 06:36:18 +0200 Subject: [PATCH 10/11] fix(ready): reject encoded /readyz variants and assert the JSON 404 contract Compare the DECODED pathname for /readyz routing so an encoded variant like /readyz%2F (which decodes to /readyz/) cannot bypass the exact-path rejection and reach the GUI fallback (serveGuiFile decodes the pathname and would serve index.html with 200). GET, POST, and OPTIONS on encoded variants all answer the deterministic JSON 404. Strengthen the OPTIONS regression test to assert the JSON 404 body (content-type, error.type, and the path-specific error.message) and add encoded-path coverage for GET, POST, and OPTIONS. --- src/server/index.ts | 20 +++++++++++++++----- tests/cli-ready.test.ts | 4 ++-- tests/server-live.test.ts | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/server/index.ts b/src/server/index.ts index ce4cc234e0..a5706a1a98 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -391,8 +391,6 @@ export function consumeStartupCacheInvalidationWrite(): boolean { return wrote; } -export function startServer(port?: number, deps: StartServerDeps = {}) { - const localAttestationSecret = deps.localAttestationSecret ?? createLocalAttestationSecret(); export function startServer(port?: number, deps: StartServerDeps = {}): Server { const localAttestationSecret = deps.localAttestationSecret ?? createLocalAttestationSecret(); const config = runAlibabaRegionStartupMigration(runOpenAiTierStartupMigration(loadConfig())); @@ -550,11 +548,23 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { const createMatch = cliSource.match(/const\s+readinessGate\s*=\s*createReadinessGate\(\)/); expect(createMatch, "handleStart must create readinessGate via createReadinessGate()").not.toBeNull(); - const startMatch = cliSource.match(/startServer\s*\(\s*port\s*,\s*\{\s*readinessGate\s*\}\s*\)/); - expect(startMatch, "startServer must be called with { readinessGate } in the retry path").not.toBeNull(); + const startMatch = cliSource.match(/startServer\s*\(\s*port\s*,\s*\{\s*[^}]*readinessGate[^}]*\}\s*\)/); + expect(startMatch, "startServer must be called with readinessGate among its deps in the retry path").not.toBeNull(); const syncMatch = cliSource.match( /syncCodexOnStartIfEnabled\s*\(\s*port\s*,\s*config\s*,\s*undefined\s*,\s*readinessGate\s*\)/, diff --git a/tests/server-live.test.ts b/tests/server-live.test.ts index 8f4737b2df..00399a17f6 100644 --- a/tests/server-live.test.ts +++ b/tests/server-live.test.ts @@ -973,8 +973,27 @@ describe("GET /readyz", () => { // preflight (the generic OPTIONS branch would otherwise answer 204). const optionsRes = await fetch(new URL("/readyz", base), { method: "OPTIONS" }); expect(optionsRes.status).toBe(404); + expect(optionsRes.headers.get("content-type")).toContain("application/json"); + expect(((await optionsRes.json()) as { error?: { type?: string; message?: string } }).error).toMatchObject({ + type: "not_found", + message: "Unknown endpoint: OPTIONS /readyz", + }); const optionsSlashRes = await fetch(new URL("/readyz/", base), { method: "OPTIONS" }); expect(optionsSlashRes.status).toBe(404); + expect(optionsSlashRes.headers.get("content-type")).toContain("application/json"); + expect(((await optionsSlashRes.json()) as { error?: { type?: string; message?: string } }).error).toMatchObject({ + type: "not_found", + message: "Unknown endpoint: OPTIONS /readyz/", + }); + // Encoded variants (e.g. /readyz%2F, which decodes to /readyz/) must NOT + // bypass the exact-path rejection and reach the GUI fallback (which would + // serve index.html with 200). GET, POST, and OPTIONS all answer the JSON 404. + const encodedGetRes = await fetch(`${base}/readyz%2F`); + expect(encodedGetRes.status).toBe(404); + const encodedPostRes = await fetch(`${base}/readyz%2F`, { method: "POST" }); + expect(encodedPostRes.status).toBe(404); + const encodedOptionsRes = await fetch(`${base}/readyz%2F`, { method: "OPTIONS" }); + expect(encodedOptionsRes.status).toBe(404); } finally { await server.stop(true); } From a93fc4ac8773de2533707c4a08ee8fc1fcec69de Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Thu, 6 Aug 2026 07:14:33 +0200 Subject: [PATCH 11/11] fix(ready): address CodeRabbit round-2 findings - src/cli/ready.ts: clear the cached status/pid/port when discovery stops finding the proxy, so a proxy that reported pending and then exited is reported as unreachable at timeout instead of stale pending (P2). - src/server/proxy-liveness.ts: narrow ReadinessProbeResult to non-null members (a foreign/unreadable body yields a null RESULT, never a null member); the producers only ever emit valid values. - tests/cli-ready.test.ts: regression test for the stale-pending fix. - tests/cli-ready-subprocess.test.ts: enlarge the second subprocess kill budget to 10s (CI cold-start tolerance, matching the sibling test). - README, lifecycle docs (en + 4 locales), structure doc: document that --timeout accepts positive integer seconds from 1-300. --- README.md | 2 +- .../docs/ja/reference/cli/lifecycle.md | 2 +- .../docs/ko/reference/cli/lifecycle.md | 2 +- .../content/docs/reference/cli/lifecycle.md | 2 +- .../docs/ru/reference/cli/lifecycle.md | 2 +- .../docs/zh-cn/reference/cli/lifecycle.md | 2 +- src/cli/ready.ts | 8 ++++++ src/server/proxy-liveness.ts | 10 +++++--- structure/03_catalog-and-subagents.md | 3 ++- tests/cli-ready-subprocess.test.ts | 1 + tests/cli-ready.test.ts | 25 +++++++++++++++++++ 11 files changed, 48 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 7a7c0836c2..044828741d 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ It returns `200` when `status` is `ready`; `pending` and terminal `failed` retur `ocx ready [--json] [--wait [--timeout ]]` performs one probe by default. `--wait` polls for up to 45 seconds by default, but exits immediately when it observes terminal `failed`; -`--timeout ` sets a 1–300 second limit and requires `--wait`. CLI `--json` output is +`--timeout ` sets a 1–300 second limit, requires `--wait`, and accepts only positive integers. CLI `--json` output is `{ready, status, pid, port}`, where `status` is `ready`, `pending`, `failed`, or `unreachable`. | Exit | Result | diff --git a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md index e5c96905b0..0905507adc 100644 --- a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md @@ -123,7 +123,7 @@ ocx status --json 旧プロキシは `unreachable` として fail-closed し、`/healthz` は readiness ではなく別の liveness 確認です。 デフォルトでは 1 回だけ probe します。`--wait` は準備完了または timeout まで polling しますが、 終端 `failed` を確認すると即座に終了します。デフォルト timeout は 45 秒で、`--timeout ` には -`--wait` が必要です（1〜300 秒の範囲）。CLI JSON は +`--wait` が必要です（1〜300 秒の正の整数）。CLI JSON は `{ready, status, pid, port}` を出力し、`status` は `ready`、`pending`、`failed`、`unreachable` の いずれかです。終了コードは ready が 0、not-ready/pending/failed/timeout/unreachable が 1、 不正な引数が 64 です。 diff --git a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md index f02c0032a9..83586d5332 100644 --- a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md @@ -150,7 +150,7 @@ ocx status --json 식별 필드는 `{service, version, uptime, pid, port, status}`입니다. `/readyz`가 없는 이전 프록시는 `unreachable`로 fail-closed하며, `/healthz`는 준비 상태가 아닌 별도의 liveness 확인입니다. 기본값은 한 번의 probe이며, `--wait`는 준비 또는 timeout까지 polling하지만 종단 `failed`를 확인하면 즉시 종료합니다. -기본 timeout은 45초이며, `--timeout `는 `--wait`와 함께 써야 하고 1~300초 범위를 받습니다. CLI JSON은 +기본 timeout은 45초이며, `--timeout `는 `--wait`와 함께 써야 하고 양의 정수인 1~300초 범위를 받습니다. CLI JSON은 `{ready, status, pid, port}`를 출력하며 `status`는 `ready`, `pending`, `failed`, `unreachable` 중 하나입니다. 종료 코드는 ready가 0, not-ready/pending/failed/timeout/unreachable이 1, 잘못된 인수가 64입니다. diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index b5f949c3cb..ef1f0f47a4 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -149,7 +149,7 @@ ready, or `503` with `Retry-After: 1` for `pending` and terminal `failed`. Its s is `{service, version, uptime, pid, port, status}`. Old proxies without `/readyz` fail closed as `unreachable`; `/healthz` is separate liveness, not readiness. The command performs one probe by default; `--wait` polls until ready or timeout, but exits immediately when it observes the terminal `failed` state. The -default timeout is 45 seconds; `--timeout ` requires `--wait` and accepts 1–300 seconds. +default timeout is 45 seconds; `--timeout ` requires `--wait` and accepts positive integer seconds from 1–300. CLI JSON emits `{ready, status, pid, port}`, where `status` is `ready`, `pending`, `failed`, or `unreachable`. Exit codes are 0 for ready; 1 for not-ready, pending, failed, timeout, or unreachable; and 64 for invalid arguments. diff --git a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md index 52eceb8a1d..62dc3d5556 100644 --- a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md @@ -161,7 +161,7 @@ Identity-check живого прокси. Текстовый вывод сооб Старые прокси без `/readyz` fail-closed как `unreachable`; `/healthz` — отдельная проверка liveness, а не готовности. По умолчанию команда выполняет одну пробу. `--wait` опрашивает до готовности или тайм-аута, но при терминальном `failed` завершается немедленно. Тайм-аут по умолчанию — 45 секунд; -`--timeout ` требует `--wait` и принимает значения 1–300 секунд. CLI JSON выдаёт +`--timeout ` требует `--wait` и принимает целые положительные значения 1–300 секунд. CLI JSON выдаёт `{ready, status, pid, port}`, где `status` — `ready`, `pending`, `failed` или `unreachable`. Коды завершения: 0 — готово; 1 — не готово, pending, failed, тайм-аут или недоступность; 64 — недопустимые аргументы. diff --git a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md index ca8595996e..124a36c053 100644 --- a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md @@ -121,7 +121,7 @@ ocx status --json 终态 `failed` 时返回 `503`，并带有 `Retry-After: 1`。HTTP 仅返回经脱敏的身份字段 `{service, version, uptime, pid, port, status}`。不支持 `/readyz` 的旧代理会按 `unreachable` 失败关闭； `/healthz` 是独立的存活检查，不是就绪检查。默认只探测一次；`--wait` 会轮询到就绪或超时，但遇到终态 -`failed` 会立即退出。默认超时为 45 秒；`--timeout ` 必须与 `--wait` 一起使用，取值范围为 1–300 秒。CLI JSON +`failed` 会立即退出。默认超时为 45 秒；`--timeout ` 必须与 `--wait` 一起使用，取值范围为 1–300 秒的正整数。CLI JSON 输出 `{ready, status, pid, port}`，其中 `status` 为 `ready`、`pending`、`failed` 或 `unreachable`。退出码：就绪为 0；未就绪、pending、failed、超时或无法连接为 1；参数无效为 64。 diff --git a/src/cli/ready.ts b/src/cli/ready.ts index 3b1b49eaf5..de918600f4 100644 --- a/src/cli/ready.ts +++ b/src/cli/ready.ts @@ -279,6 +279,14 @@ export async function runReady(args: ReadyArgs, io: ReadyIo = {}): Promise]]`. The probe validates the service, version, uptime, PID, port, status, and HTTP/status pairing. The default is one probe. With `--wait`, it applies one absolute deadline (45 seconds by default) across discovery, readiness probes, polling, -and sleeps, but exits immediately on terminal failed. CLI `--json` emits +and sleeps, but exits immediately on terminal failed. `--timeout ` requires `--wait` and +accepts positive integer seconds from 1–300. CLI `--json` emits `{ready, status, pid, port}`, with status in `ready|pending|failed|unreachable`. Exit 0 means ready; exit 1 covers not-ready, pending, failed, timeout, and unreachable; exit 64 means invalid arguments. Older proxies without `/readyz` fail closed as unreachable. `/healthz` remains the separate diff --git a/tests/cli-ready-subprocess.test.ts b/tests/cli-ready-subprocess.test.ts index 7a90c4830a..557b1a671b 100644 --- a/tests/cli-ready-subprocess.test.ts +++ b/tests/cli-ready-subprocess.test.ts @@ -168,6 +168,7 @@ describe("ocx ready real subprocess", () => { CODEX_HOME: homes.codexHome, OPENCODEX_CODEX_SHIM_AUTO_RESTORE: "1", }, + 10_000, ); expect(result.timedOut).toBe(false); diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index 91278d2512..329ae92118 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -262,6 +262,31 @@ describe("runReady --wait (single bounded loop, deterministic)", () => { expect(out.join("")).toContain("not ready yet (pending)"); }); + test("a proxy that reported pending and then exits reports unreachable, not stale pending", async () => { + const { io, out } = captureIo(); + let t = 0; + let discoveryCount = 0; + const code = await runReady( + { json: true, wait: true, timeoutSeconds: 8 }, + { + ...io, + // First discovery finds the proxy (pending), then it vanishes: every + // later discovery returns null. The cached pending status must not + // survive to the timeout report — the honest answer is unreachable. + findLive: async () => (++discoveryCount === 1 ? LIVE : null), + probe: async () => PENDING_PROBE, + // 500ms steps: first read sets the deadline, then each find/probe/sleep + // advances past a null discovery well before the deadline so the + // timeout report carries the cleared unreachable state. + now: () => (t += 500), + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + const parsed = JSON.parse(out.join("")) as { ready: boolean; status: string; pid: unknown; port: unknown }; + expect(parsed).toEqual({ ready: false, status: "unreachable", pid: null, port: null }); + }); + test("failed is terminal: exits 1 immediately without polling or consuming timeout", async () => { const { io, out } = captureIo(); let findCalls = 0;