diff --git a/README.md b/README.md index 7be6787be2..044828741d 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, 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 | +| --- | --- | +| `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/docs-site/src/content/docs/ja/reference/cli/lifecycle.md b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md index 92b8fcfb2b..0905507adc 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..83586d5332 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..ef1f0f47a4 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 when it observes the terminal `failed` state. The +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. + ### `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..62dc3d5556 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..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 @@ -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 c74ba64752..77a50b5166 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -249,6 +249,19 @@ 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: [ + "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, timeout, and unreachable exit 1.", + ], + }, }; function packageVersion(): string { @@ -290,6 +303,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..049048fda7 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 } 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..de918600f4 --- /dev/null +++ b/src/cli/ready.ts @@ -0,0 +1,301 @@ +/** + * `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 { 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; +/** + * Cap a remaining deadline budget to a positive per-call IO timeout. Stays + * positive (>= 1ms) and never exceeds the logical remaining time nor the + * 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, DEFAULT_PROBE_TIMEOUT_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's + // deadlineAt budget 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. + // verifyPidFn is disabled: readiness is non-destructive, so the killable-pid + // OS command-line verification (WMIC/PowerShell, seconds on Windows) would + // run OUTSIDE the deadline budget for no readiness value. The /healthz + // identity marker and the strict /readyz contract validation remain. + const live = await findLiveProxy( + remainingMs === undefined + ? { verifyPidFn: () => null } + : { deadlineAt: Date.now() + remainingMs, timeoutMs: DEFAULT_PROBE_TIMEOUT_MS, verifyPidFn: () => null }, + ); + 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; + } + } else { + // Discovery stopped finding the proxy: it may have exited since the last + // probe. Clear the cached status/identity so a stale `pending` (or its + // pid/port) cannot mislead a supervisor at timeout — the honest answer is + // `unreachable` with no identity. + lastStatus = "unreachable"; + lastPid = null; + lastPort = null; + } + // 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..a5706a1a98 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; } /* @@ -388,7 +391,7 @@ export function consumeStartupCacheInvalidationWrite(): boolean { return wrote; } -export function startServer(port?: number, deps: StartServerDeps = {}) { +export function startServer(port?: number, deps: StartServerDeps = {}): Server { const localAttestationSecret = deps.localAttestationSecret ?? createLocalAttestationSecret(); const config = runAlibabaRegionStartupMigration(runOpenAiTierStartupMigration(loadConfig())); setLiveStateStoreConfig(config); @@ -523,18 +526,47 @@ 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}`); + // Readiness is exact-GET on the literal /readyz path. Compare the DECODED + // pathname 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). Malformed percent-sequences fall back to the raw pathname, + // which still cannot match the exact literal below. + let readyzPath: string | undefined; + try { + const decoded = decodeURIComponent(url.pathname); + if (decoded === "/readyz" || decoded === "/readyz/") readyzPath = decoded; + } catch { /* malformed encoding — not a readiness path */ } + if (req.method === "OPTIONS") { + // /readyz is exact-GET only; OPTIONS (like POST and the trailing-slash + // path) must answer the deterministic JSON 404, never the generic 204 + // preflight response that the SPA fallback would otherwise allow. + if (readyzPath !== undefined) { + return withCors(formatErrorResponse(404, "not_found", `Unknown endpoint: ${req.method} ${url.pathname}`), req, config); + } const managementPreflight = url.pathname.startsWith("/api/"); const allowed = managementPreflight ? isAllowedManagementOrigin(req, config) @@ -590,6 +622,42 @@ 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): answer them + // with a JSON 404 here so they can never be silently accepted by the GUI SPA + // fallback (which would serve index.html with HTTP 200 once gui/dist exists). + if (readyzPath !== undefined) { + if (readyzPath !== "/readyz" || req.method !== "GET") { + return withCors(formatErrorResponse(404, "not_found", `Unknown endpoint: ${req.method} ${url.pathname}`), req, config); + } + // A draining proxy must never advertise ready: every data-plane branch + // answers drainingResponse while isDraining() is set, but the one-shot + // readiness gate is not mutated on shutdown (it is owned by the startup + // sync). Report pending so `ocx ready --wait` and external supervisors + // keep polling instead of promoting a proxy that is draining. + const status = isDraining() ? "pending" : 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 +1390,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..e291a21c27 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -47,6 +47,9 @@ export interface LivenessIo { nowFn?: () => 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 @@ -138,6 +141,11 @@ 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 +168,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 ?? 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; + 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..fc7739f41c 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -34,6 +34,26 @@ deleting, or editing a provider's shape clears that per-provider cache; a disabl 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: +`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. `--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 +liveness contract. + ## Entry shape Routed entries keep Codex-required metadata such as reasoning levels, shell type, API support flags, diff --git a/tests/cli-catalog-prewarm.test.ts b/tests/cli-catalog-prewarm.test.ts index a423cb4d95..1b10ec3a12 100644 --- a/tests/cli-catalog-prewarm.test.ts +++ b/tests/cli-catalog-prewarm.test.ts @@ -53,7 +53,7 @@ describe("catalog prewarm on handleStart bind", () => { 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-subprocess.test.ts b/tests/cli-ready-subprocess.test.ts new file mode 100644 index 0000000000..557b1a671b --- /dev/null +++ b/tests/cli-ready-subprocess.test.ts @@ -0,0 +1,186 @@ +/** + * 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 }, + 10_000, + ); + + expect(healthzHits).toBe(1); + expect(readyzHits).toBe(1); + expect(result.timedOut).toBe(false); + expect(result.exitCode).toBe(1); + // 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", + 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", + }, + 10_000, + ); + + 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 }); + } + }); +}); diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts new file mode 100644 index 0000000000..329ae92118 --- /dev/null +++ b/tests/cli-ready.test.ts @@ -0,0 +1,815 @@ +/** + * 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), // First read is 100, so the deadline is 100 + 5000 = 5100; 100, 200, 300 … never crosses it. + 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), // First read is 500, so the deadline is 500 + 1000 = 1500; the after-find read of 1500 ends the loop. + 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), // First read is 1000, so the deadline is 1000 + 3000 = 4000; the after-probe read of 4000 ends the loop. + sleep: async () => {}, + }, + ); + expect(code).toBe(1); + 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; + 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(); + const seenNow: number[] = []; + let t = -1000; // so the first read (which establishes the deadline) is 0 + 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 () => {}, + }, + ); + // 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]); + }); +}); + +// ── 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) 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 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(); + + 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*\)/, + ); + expect(syncMatch, "syncCodexOnStartIfEnabled must receive readinessGate so the startup sync drives /readyz").not.toBeNull(); + + // Source order must be: create → startServer → startup sync. + 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 + startup sync. + 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 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 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("deadlineAt: Date.now() + remainingMs"); + // 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)", () => { + // 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. + // 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"); + }); + + 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"); + }); +}); 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..7f6066a665 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,470 @@ describe("findLiveProxy", () => { expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" }); }); }); + +// ── findLiveProxy single-deadline candidate gating ─────────────────────────── +// 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({ + deadlineAt: 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({ + deadlineAt: 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; // > deadlineAt (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) => { + 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({ + deadlineAt: 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) => { + 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({ + deadlineAt: 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 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) => { + 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, source: "config" }); + }); +}); + +// ── 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"); + // 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()); + }); +}); + +// ── 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..00399a17f6 100644 --- a/tests/server-live.test.ts +++ b/tests/server-live.test.ts @@ -2,14 +2,20 @@ * /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 { 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"; @@ -849,3 +855,322 @@ 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")).toBe("1"); + 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")).toBe("1"); + 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); + // 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); + 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); + } + }); + + 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); + } + }); +}); + +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(); + } + }); +}); 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);