diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 3905c739ac9..9b4372a2245 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -492,6 +492,7 @@ "codex-cli-installation-targets.test.ts": "codex-integration", "codex-cli-windows-installation-files.test.ts": "codex-integration", "codex-cli-update-launcher-policy.test.ts": "codex-integration", + "codex-cli-update-plan.test.ts": "codex-integration", "codex-cli-update-zero-effect.test.ts": "codex-integration", "codex-composed-acceptance.test.ts": "codex-integration", "codex-config-generation.test.ts": "codex-integration", diff --git a/skills/ocx/references/01_management_surface.md b/skills/ocx/references/01_management_surface.md index 5682c76ac71..905d8da8056 100644 --- a/skills/ocx/references/01_management_surface.md +++ b/skills/ocx/references/01_management_surface.md @@ -421,6 +421,25 @@ JSON mode: `envelope`. - selectionAttested, managed and applyAllowed remain false. The digest is an observation, not a durable update permit. - Does not run the named Codex/npm/Node files, query a registry, install software, control processes or persist state. +### `ocx system codex-cli-update plan` + +Dry-run a Codex CLI update and print the plan id that authorizes applying it. + +Drives no management route. + +| Flag | Value | Meaning | +|---|---|---| +| `--channel` | string | Registry channel to resolve. Only the stable latest channel is offered. | +| `--json` | boolean | Emit the plan as JSON. | + +JSON mode: `envelope`. + +- Adds the three inputs check leaves out: an exact registry version with its sha512 integrity, a fail-closed process-table read, and a decision. +- The registry evidence is pinned to the official npm registry with project/user npm configuration isolated, so a redirected .npmrc cannot supply the answer. +- Writes nothing and installs nothing. A refusal is a normal dry-run answer and still exits 0. +- The plan id is a digest of the evidence the decision rests on, not a stored job. There is no plan state on disk to expire, collide or clean up. +- An unreadable process table refuses rather than reading as no live session. + ### `ocx claude desktop status` Applied-vs-desired Claude Desktop state, including staleness, drift, and health. @@ -824,6 +843,25 @@ JSON mode: `payload`. - `policy set` never enables implicitly: omitting `--enabled` keeps the stored value. - `policy run` forces a run regardless of schedule, so it needs `--yes`. +### `ocx system codex-cli-update apply` + +Install the exact Codex CLI version bound into a plan id from a dry-run. + +Drives no management route. + +| Flag | Value | Meaning | +|---|---|---| +| `--plan` | string | Required: the plan id printed by a dry-run the operator read. | +| `--json` | boolean | Emit the apply result as JSON. | + +JSON mode: `envelope`. + +- --plan is mandatory because the operator must approve a target they have read. The plan is recomputed from live evidence and refused unless the id still matches. +- Resolves and packs only from the pinned official npm registry with project/user npm configuration isolated, verifies the tarball sha512 against the plan-bound integrity, and installs only that verified file; never stops, restarts or signals Codex, the app-server, the desktop app or the tray. +- Holds one cross-process update lease from the final session scan through the install and readback; a concurrent apply is refused, and Codex startup paths that observe the lease wait or refuse rather than load a half-replaced install. +- The outcome is classified from a fresh inspection rather than the installer exit code, and is never retried or rolled back automatically. +- Repairs the shim only when this installation owned a matched shim before the update. + ### `ocx system codex-restart` Restart the Codex desktop app and app-servers. @@ -930,6 +968,6 @@ JSON mode: `payload`. ## Counts -- declared capabilities: 50 -- of those, state-changing: 25 +- declared capabilities: 52 +- of those, state-changing: 26 - head-resolved invocations: 2 diff --git a/src/cli/capabilities.ts b/src/cli/capabilities.ts index cb117bcba05..eadf1305c9d 100644 --- a/src/cli/capabilities.ts +++ b/src/cli/capabilities.ts @@ -779,6 +779,42 @@ export const CAPABILITIES: readonly Capability[] = [ "Does not run the named Codex/npm/Node files, query a registry, install software, control processes or persist state.", ], }, + { + command: ["system", "codex-cli-update", "plan"], + summary: "Dry-run a Codex CLI update and print the plan id that authorizes applying it.", + routes: [], + flags: [ + { name: "--channel", value: "string", summary: "Registry channel to resolve. Only the stable latest channel is offered." }, + { name: "--json", value: "boolean", summary: "Emit the plan as JSON." }, + ], + mutates: false, + json: "envelope", + details: [ + "Adds the three inputs check leaves out: an exact registry version with its sha512 integrity, a fail-closed process-table read, and a decision.", + "The registry evidence is pinned to the official npm registry with project/user npm configuration isolated, so a redirected .npmrc cannot supply the answer.", + "Writes nothing and installs nothing. A refusal is a normal dry-run answer and still exits 0.", + "The plan id is a digest of the evidence the decision rests on, not a stored job. There is no plan state on disk to expire, collide or clean up.", + "An unreadable process table refuses rather than reading as no live session.", + ], + }, + { + command: ["system", "codex-cli-update", "apply"], + summary: "Install the exact Codex CLI version bound into a plan id from a dry-run.", + routes: [], + flags: [ + { name: "--plan", value: "string", required: true, summary: "Required: the plan id printed by a dry-run the operator read." }, + { name: "--json", value: "boolean", summary: "Emit the apply result as JSON." }, + ], + mutates: true, + json: "envelope", + details: [ + "--plan is mandatory because the operator must approve a target they have read. The plan is recomputed from live evidence and refused unless the id still matches.", + "Resolves and packs only from the pinned official npm registry with project/user npm configuration isolated, verifies the tarball sha512 against the plan-bound integrity, and installs only that verified file; never stops, restarts or signals Codex, the app-server, the desktop app or the tray.", + "Holds one cross-process update lease from the final session scan through the install and readback; a concurrent apply is refused, and Codex startup paths that observe the lease wait or refuse rather than load a half-replaced install.", + "The outcome is classified from a fresh inspection rather than the installer exit code, and is never retried or rolled back automatically.", + "Repairs the shim only when this installation owned a matched shim before the update.", + ], + }, { command: ["system", "codex-restart"], summary: "Restart the Codex desktop app and app-servers.", diff --git a/src/cli/codex-cli-update.ts b/src/cli/codex-cli-update.ts index 19a7f9d02ad..3fbd459aeff 100644 --- a/src/cli/codex-cli-update.ts +++ b/src/cli/codex-cli-update.ts @@ -11,20 +11,32 @@ import type { CodexCliInstallationSnapshot, CodexCliInstallationTargetDerivation, } from "../codex/cli-installation-targets"; +import { + applyCodexCliUpdatePlan, + createCodexCliUpdatePlan, + type CodexCliUpdateApplyDeps, + type CodexCliUpdateApplyResult, + type CodexCliUpdateChannel, + type CodexCliUpdatePlan, + type CodexCliUpdatePlanDeps, +} from "../codex/cli-update-plan"; import { CliUsageError, isJsonOption, printData, runCliAction } from "./runtime-api"; import { trustedNodeLauncherContext } from "./launcher-context"; export const CODEX_CLI_UPDATE_USAGE = `Usage: ocx system codex-cli-update check [--json] ocx system codex-cli-update attest [--json] - ocx system codex-cli-update attest --candidate --npm-prefix --npm-cli --node [--json]`; + ocx system codex-cli-update attest --candidate --npm-prefix --npm-cli --node [--json] + ocx system codex-cli-update plan [--channel latest] [--json] + ocx system codex-cli-update apply --plan [--json]`; + +const PLAN_ID_RE = /^[0-9a-f]{32}$/; -export type ParsedCodexCliUpdateArgs = Readonly<{ - json: boolean; -}> | Readonly<{ - json: boolean; - attest: CodexCliInstallationIdentityInput | "selected"; -}>; +export type ParsedCodexCliUpdateArgs = + | Readonly<{ action: "check"; json: boolean }> + | Readonly<{ action: "plan"; json: boolean; channel: CodexCliUpdateChannel }> + | Readonly<{ action: "apply"; json: boolean; planId: string }> + | Readonly<{ json: boolean; attest: CodexCliInstallationIdentityInput | "selected" }>; export interface CodexCliUpdateCommandDeps { readonly inspectInstall?: (deps: CodexCliInstallProvenanceDeps) => Promise; @@ -32,6 +44,8 @@ export interface CodexCliUpdateCommandDeps { readonly deriveInstallationInput?: ( snapshot: CodexCliInstallationSnapshot, ) => CodexCliInstallationTargetDerivation; + readonly createPlan?: (deps: CodexCliUpdatePlanDeps) => Promise; + readonly applyPlan?: (planId: string, deps: CodexCliUpdateApplyDeps) => Promise; } function identitySummary(report: CodexCliInstallationIdentityReport): string[] { @@ -68,24 +82,71 @@ function installSummary(report: CodexCliInstallReport): string[] { ]; } +function planSummary(plan: CodexCliUpdatePlan): string[] { + const lines = [ + `applicable: ${plan.applicable ? "yes" : "no"}`, + `reason: ${plan.refusal ?? "none"}`, + `provenance: ${plan.provenance}`, + `installed-version: ${plan.installedVersion ?? "unavailable"}`, + `version-evidence: ${plan.versionEvidence}`, + `channel: ${plan.channel}`, + `target-version: ${plan.targetVersion ?? "unresolved"}`, + `target-integrity: ${plan.targetIntegrity ?? "unresolved"}`, + `session: ${plan.session.state}${plan.session.matches === null ? "" : ` (${plan.session.matches})`}`, + ]; + if (plan.planId) lines.push(`plan: ${plan.planId}`); + if (plan.command) lines.push(`command: ${plan.command.join(" ")} (indicative — apply verifies the packed tarball and installs the verified file)`); + return lines; +} + +function applySummary(result: CodexCliUpdateApplyResult): string[] { + return [ + `status: ${result.status}`, + `reason: ${result.refusal ?? "none"}`, + `plan: ${result.planId ?? "unavailable"}`, + `target-version: ${result.targetVersion ?? "unavailable"}`, + `installed-before: ${result.installedVersionBefore ?? "unavailable"}`, + `installed-after: ${result.installedVersionAfter ?? "unavailable"}`, + `installer-exit: ${result.installerExitCode ?? "unavailable"}`, + ]; +} + +/** Read `--name value` and `--name=value` alike; both spellings reach this CLI. */ +function optionValue(tokens: readonly string[], index: number, name: string): { value: string; next: number } { + const token = tokens[index]!; + const inline = `--${name}=`; + if (token.startsWith(inline)) { + const value = token.slice(inline.length); + if (!value) throw new CliUsageError(`--${name} requires a value`, CODEX_CLI_UPDATE_USAGE); + return { value, next: index + 1 }; + } + const value = tokens[index + 1]; + if (value === undefined) throw new CliUsageError(`--${name} requires a value`, CODEX_CLI_UPDATE_USAGE); + return { value, next: index + 2 }; +} + +function isOption(token: string, name: string): boolean { + return token === `--${name}` || token.startsWith(`--${name}=`); +} + export function parseCodexCliUpdateArgs(argv: readonly string[]): ParsedCodexCliUpdateArgs { // `--json` is accepted in any argv position CLI-wide, so remove it before positional - // validation. Requiring `check` at index 0 first would reject `--json check`, which + // validation. Requiring the action at index 0 first would reject `--json check`, which // automation that puts output flags ahead of the subcommand legitimately produces. let json = false; - const positional: string[] = []; + const rest: string[] = []; for (const token of argv) { if (isJsonOption(token)) { if (json) throw new CliUsageError("--json may be specified only once", CODEX_CLI_UPDATE_USAGE); json = true; continue; } - positional.push(token); + rest.push(token); } - if (positional[0] === "attest") { + if (rest[0] === "attest") { // No options: attest the selected candidate identified from the proof-bound // launcher snapshot. The four explicit paths remain all-or-none. - if (positional.length === 1) { + if (rest.length === 1) { return Object.freeze({ json, attest: "selected" as const }); } const options = new Map([ @@ -93,12 +154,12 @@ export function parseCodexCliUpdateArgs(argv: readonly string[]): ParsedCodexCli ["--npm-cli", "npmCli"], ["--node", "node"], ]); const input: Partial> = {}; - for (let index = 1; index < positional.length; index += 2) { - const key = options.get(positional[index]!); + for (let index = 1; index < rest.length; index += 2) { + const key = options.get(rest[index]!); if (!key || input[key] !== undefined) { throw new CliUsageError("unsupported or duplicate attest option", CODEX_CLI_UPDATE_USAGE); } - const value = positional[index + 1]; + const value = rest[index + 1]; if (!value || !value.trim() || /[\0\r\n]/.test(value) || !(value.startsWith("/") || /^[a-z]:[\\/]/i.test(value))) { throw new CliUsageError("attest options require explicit absolute paths", CODEX_CLI_UPDATE_USAGE); @@ -112,13 +173,76 @@ export function parseCodexCliUpdateArgs(argv: readonly string[]): ParsedCodexCli candidate: input.candidate, npmPrefix: input.npmPrefix, npmCli: input.npmCli, node: input.node, }) }); } - if (positional[0] !== "check") { - throw new CliUsageError("codex-cli-update action must be check or attest", CODEX_CLI_UPDATE_USAGE); + const action = rest[0]; + if (action !== "check" && action !== "plan" && action !== "apply") { + throw new CliUsageError("codex-cli-update action must be check, attest, plan or apply", CODEX_CLI_UPDATE_USAGE); } - if (positional.length > 1) { - throw new CliUsageError("unsupported codex-cli-update argument", CODEX_CLI_UPDATE_USAGE); + + if (action === "check") { + if (rest.length > 1) throw new CliUsageError("unsupported codex-cli-update argument", CODEX_CLI_UPDATE_USAGE); + return Object.freeze({ action, json }); } - return Object.freeze({ json }); + + if (action === "plan") { + let channel: CodexCliUpdateChannel = "latest"; + let seen = false; + let index = 1; + while (index < rest.length) { + const token = rest[index]!; + if (!isOption(token, "channel")) { + throw new CliUsageError("unsupported codex-cli-update argument", CODEX_CLI_UPDATE_USAGE); + } + if (seen) throw new CliUsageError("--channel may be specified only once", CODEX_CLI_UPDATE_USAGE); + const read = optionValue(rest, index, "channel"); + // Only the stable channel is offered. A preview channel would need its own + // provenance story before it may install anything on the operator's behalf. + if (read.value !== "latest") throw new CliUsageError("--channel must be latest", CODEX_CLI_UPDATE_USAGE); + channel = read.value; + seen = true; + index = read.next; + } + return Object.freeze({ action, json, channel }); + } + + let planId: string | null = null; + let index = 1; + while (index < rest.length) { + const token = rest[index]!; + if (!isOption(token, "plan")) { + throw new CliUsageError("unsupported codex-cli-update argument", CODEX_CLI_UPDATE_USAGE); + } + if (planId !== null) throw new CliUsageError("--plan may be specified only once", CODEX_CLI_UPDATE_USAGE); + const read = optionValue(rest, index, "plan"); + if (!PLAN_ID_RE.test(read.value)) { + throw new CliUsageError("--plan must be a plan id from a dry-run", CODEX_CLI_UPDATE_USAGE); + } + planId = read.value; + index = read.next; + } + // Apply is never implicit: the operator quotes a plan id they read in a dry-run. + if (planId === null) throw new CliUsageError("apply requires --plan ", CODEX_CLI_UPDATE_USAGE); + return Object.freeze({ action, json, planId }); +} + +/** + * Inspection inputs for this one-shot CLI process. + * + * The published Node launcher supplies a proof-bound snapshot of configured candidate + * evidence, not selected-runtime admission. A direct Bun or source launch has no such + * proof, so nothing ambient or persisted is inspected at all. + */ +function inspectionDeps(): CodexCliInstallProvenanceDeps { + const trusted = trustedNodeLauncherContext()?.codexCliInspectionEnv; + if (!trusted || trusted.managerRoots === null) return { env: { PATH: "" }, configDir: "." }; + return { + env: { + ...trusted.managerRoots, + CODEX_CLI_PATH: trusted.codexCliPath ?? undefined, + PATH: trusted.path ?? undefined, + PATHEXT: trusted.pathExt ?? undefined, + }, + configDir: trusted.configDir, + }; } export async function handleCodexCliUpdateCommand( @@ -136,7 +260,10 @@ export async function handleCodexCliUpdateCommand( } throw error; } - return runCliAction(async () => { + // A refusal or an unapplied update is a legitimate answer rather than a crash, so the + // outcome exit code is decided here and only a thrown error is left to runCliAction. + let outcome = 0; + const code = await runCliAction(async () => { if ("attest" in parsed) { let report: CodexCliInstallationIdentityReport; try { @@ -167,25 +294,26 @@ export async function handleCodexCliUpdateCommand( printData(report, parsed.json, identitySummary(report)); return; } - const trustedInspectionEnv = trustedNodeLauncherContext()?.codexCliInspectionEnv; - const inspectionDeps: CodexCliInstallProvenanceDeps = trustedInspectionEnv - && trustedInspectionEnv.managerRoots !== null ? { - env: { - ...trustedInspectionEnv.managerRoots, - CODEX_CLI_PATH: trustedInspectionEnv.codexCliPath ?? undefined, - PATH: trustedInspectionEnv.path ?? undefined, - PATHEXT: trustedInspectionEnv.pathExt ?? undefined, - }, - configDir: trustedInspectionEnv.configDir, - // This is a fresh one-shot CLI process. Its proof-bound launcher snapshot - // supplies configured candidate evidence, not selected-runtime admission. - } : { - // Direct Bun/source launches have no pre-dotenv proof. Do not inspect - // ambient or persisted candidate state at all. - env: { PATH: "" }, - configDir: ".", - }; - const report = await (deps.inspectInstall ?? inspectCodexCliInstall)(inspectionDeps); - printData(report, parsed.json, installSummary(report)); + if (parsed.action === "check") { + const report = await (deps.inspectInstall ?? inspectCodexCliInstall)(inspectionDeps()); + printData(report, parsed.json, installSummary(report)); + return; + } + if (parsed.action === "plan") { + const plan = await (deps.createPlan ?? createCodexCliUpdatePlan)({ + channel: parsed.channel, + inspectionDeps: inspectionDeps(), + inspect: deps.inspectInstall, + }); + printData(plan, parsed.json, planSummary(plan)); + return; + } + const result = await (deps.applyPlan ?? applyCodexCliUpdatePlan)(parsed.planId, { + inspectionDeps: inspectionDeps(), + inspect: deps.inspectInstall, + }); + printData(result, parsed.json, applySummary(result)); + outcome = result.status === "applied" ? 0 : 1; }); + return code === 0 ? outcome : code; } diff --git a/src/cli/registry.ts b/src/cli/registry.ts index 6e4756c1664..6d0e970e4f2 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -419,13 +419,16 @@ export const CLI_COMMANDS: CliCommandEntry[] = [ { name: "system", usage: "ocx system ...", - summary: "Manage headless runtime settings, startup, sync, diagnostics, OpenCodex updates, and read-only Codex CLI inspection.", + summary: "Manage headless runtime settings, startup, sync, diagnostics, OpenCodex updates, and the Codex CLI update manager.", details: [ "system update manages OpenCodex itself.", "ocx system codex-cli-update check [--json]", "ocx system codex-cli-update attest [--json]", "ocx system codex-cli-update attest --candidate --npm-prefix --npm-cli --node [--json]", - "The Codex CLI inspection command makes no package-registry request, does not execute Codex or npm, install or repair software, control a process, or write configuration or cache state.", + "ocx system codex-cli-update plan [--channel latest] [--json]", + "ocx system codex-cli-update apply --plan [--json]", + "check and attest make no package-registry request, do not execute Codex or npm, and write nothing. plan adds a registry query and a process-table read and still writes nothing.", + "apply installs exactly the version bound into the plan id it is given, refuses a plan that no longer matches live evidence, and never stops or restarts Codex.", ], }, { diff --git a/src/cli/system-command.ts b/src/cli/system-command.ts index 4e08fcbc49c..fc5ddf955d6 100644 --- a/src/cli/system-command.ts +++ b/src/cli/system-command.ts @@ -23,6 +23,8 @@ const USAGE = `Usage: ocx system codex-cli-update check [--json] ocx system codex-cli-update attest [--json] ocx system codex-cli-update attest --candidate --npm-prefix --npm-cli --node [--json] + ocx system codex-cli-update plan [--channel latest] [--json] + ocx system codex-cli-update apply --plan [--json] ocx system update check [--channel ] [--json] ocx system update run [--channel ] [--restart ] --yes [--json] ocx system update status [--json] diff --git a/src/cli/version-skew.ts b/src/cli/version-skew.ts index 7acd5177b3c..c568959a502 100644 --- a/src/cli/version-skew.ts +++ b/src/cli/version-skew.ts @@ -9,7 +9,7 @@ * comparison instead of reimplementing it -- two diagnostics disagreeing about whether an * install is stale would be worse than neither reporting it. */ -import { parseStrictSemver, type StrictSemver } from "../lib/strict-semver"; +import { compareStrictSemver, parseStrictSemver } from "../lib/strict-semver"; /** Placeholder versions that mean "unknown", not "different". */ const PLACEHOLDERS = new Set(["unknown", "0.0.0"]); @@ -28,25 +28,6 @@ export function isConfirmedVersionMatch(skew: VersionSkew): boolean { return skew.proxyVersion === skew.cliVersion && !PLACEHOLDERS.has(skew.cliVersion); } -/** SemVer precedence ignores build metadata; raw equality is handled separately. */ -function compareVersions(cli: StrictSemver, proxy: StrictSemver): number { - for (let i = 0; i < cli.core.length; i++) { - if (cli.core[i]! !== proxy.core[i]!) return cli.core[i]! > proxy.core[i]! ? 1 : -1; - } - if (cli.prerelease.length === 0) return proxy.prerelease.length === 0 ? 0 : 1; - if (proxy.prerelease.length === 0) return -1; - for (let i = 0; i < Math.max(cli.prerelease.length, proxy.prerelease.length); i++) { - const left = cli.prerelease[i]; - const right = proxy.prerelease[i]; - if (left === right) continue; - if (left === undefined) return -1; - if (right === undefined) return 1; - if (typeof left !== typeof right) return typeof left === "bigint" ? -1 : 1; - return left > right ? 1 : -1; - } - return 0; -} - /** * Compare the running CLI against the live proxy. * @@ -63,7 +44,7 @@ export function computeVersionSkew(cliVersion: string, proxyVersion: string | un } const cliSemver = parseStrictSemver(cliVersion); const proxySemver = parseStrictSemver(proxy); - const order = cliSemver && proxySemver ? compareVersions(cliSemver, proxySemver) : 0; + const order = cliSemver && proxySemver ? compareStrictSemver(cliSemver, proxySemver) : 0; const advice = order > 0 ? "the running proxy is older than this CLI. Restart the proxy using the intended current installation. " // `restart`, not `repair`: a version skew leaves the service DEFINITION unchanged, and diff --git a/src/codex/app-server-processes.ts b/src/codex/app-server-processes.ts index a33a1091fd6..2a36fb5dd44 100644 --- a/src/codex/app-server-processes.ts +++ b/src/codex/app-server-processes.ts @@ -581,6 +581,45 @@ export function listCodexAppServerProcesses(io: CodexAppServerProcessIo = {}): C return matched; } +/** + * Enumeration outcome for callers that must DEFER on an unreadable process table. + * + * {@link listCodexAppServerProcesses} maps enumeration failure to an empty list because its + * kill contract must never signal a process it could not verify. The Codex CLI update + * workflow needs the opposite reading: "no matches" and "could not look" lead to different + * decisions, and only the first one may allow an install to proceed. + */ +export type CodexAppServerProcessScan = + | Readonly<{ kind: "observed"; processes: readonly CodexAppServerProcess[] }> + | Readonly<{ kind: "unavailable" }>; + +/** Same matcher and snapshot sources as {@link listCodexAppServerProcesses}, failing closed. */ +export function scanCodexAppServerProcesses(io: CodexAppServerProcessIo = {}): CodexAppServerProcessScan { + const platform = io.platform ?? process.platform; + const getuid = io.getuid ?? (() => { + try { + return typeof process.getuid === "function" ? process.getuid() : undefined; + } catch { + return undefined; + } + }); + let snapshots: ProcessSnapshot[]; + try { + snapshots = io.listSnapshots ? io.listSnapshots() : defaultListSnapshots(platform, getuid); + } catch { + return Object.freeze({ kind: "unavailable" as const }); + } + const seen = new Set(); + const matched: CodexAppServerProcess[] = []; + for (const snapshot of snapshots) { + if (seen.has(snapshot.pid)) continue; + if (!isCodexAppServerCommandLine(snapshot.commandLine, snapshot.executable)) continue; + seen.add(snapshot.pid); + matched.push({ pid: snapshot.pid, commandLine: snapshot.commandLine }); + } + return Object.freeze({ kind: "observed" as const, processes: Object.freeze(matched) }); +} + export function formatStaleCodexAppServerWarning( processes: readonly { pid: number }[], ): string { diff --git a/src/codex/cli-update-lease.ts b/src/codex/cli-update-lease.ts new file mode 100644 index 00000000000..4388383bc53 --- /dev/null +++ b/src/codex/cli-update-lease.ts @@ -0,0 +1,440 @@ +/** + * The one cross-process lease behind the Codex CLI update manager. + * + * A process scan is a snapshot, not mutual exclusion: an app-server can start after + * the plan reads the table and before "npm install -g" mutates the global prefix, and + * two apply commands can pass the same plan concurrently. This lockfile closes that + * window — apply holds it from before the final scan through the post-install + * readback, and the Codex startup paths this codebase controls (remote workspace + * app-server spawn, desktop-app relaunch) observe it and refuse or wait. + * + * The record is self-identifying: every holder publishes a random `token`, and every + * mutating transition (stale reclaim, release, heartbeat) compares that token against + * what is actually on disk via a compare-and-delete that links the current file to a + * tombstone before unlinking — a contender that observed record A can never unlink + * record B, and a late release can never delete a successor that recycled the path. + * + * Publication is atomic where the filesystem allows it: the record is written to a + * staging sibling and `linkSync`'d onto the contended path, so the lock file never + * exists in the empty/half-written state that `openSync("wx")` + `writeSync` leaves. + * Filesystems without hardlinks fall back to the O_EXCL create, and a corrupt record + * younger than PUBLISH_GRACE_MS reads as held rather than deletable, so even there a + * contender cannot steal the write window of an in-flight publisher. + * + * A live owner is never reaped by age alone: staleness needs a dead pid OR a + * heartbeat older than the bound (`startCodexCliUpdateLeaseHeartbeat` keeps it + * fresh for the length of a real install). A wedged holder whose heartbeats stopped + * is still reclaimable; one that is merely slow is not. + * + * Deliberately NOT own-pid reentrant: two applies in one process are still two + * contenders for one global install, and the second must see the lease as held. + */ +import { randomBytes } from "node:crypto"; +import { + closeSync, + linkSync, + mkdirSync, + openSync, + readFileSync, + renameSync, + statSync, + unlinkSync, + writeSync, +} from "node:fs"; +import { dirname, join } from "node:path"; + +import { getConfigDir } from "../config/paths"; + +/** + * A holder whose heartbeat is older than this is stale regardless of what its owner + * pid says. The bound exists for a live pid that no longer names an updater (a + * recycled pid, or a wedged holder); it is comfortably longer than the pack + + * install timeouts a real apply can legitimately hold the lease for, and a healthy + * holder heartbeats far below it. + */ +export const CODEX_CLI_UPDATE_LEASE_MAX_AGE_MS = 15 * 60_000; + +/** Startup paths poll a held lease for this long before refusing the launch. */ +export const CODEX_CLI_UPDATE_LEASE_WAIT_MS = 5_000; + +/** + * A record that exists on disk but fails to parse is treated as held for this long + * after its mtime. On filesystems without hardlinks the O_EXCL fallback leaves a + * brief empty window; deleting inside it would hand a second contender the lease + * while the first is still writing. + */ +const PUBLISH_GRACE_MS = 10_000; + +/** Heartbeat interval: comfortably below the staleness bound. */ +const HEARTBEAT_INTERVAL_MS = 60_000; + +export interface CodexCliUpdateLeaseRecord { + readonly version: 1; + readonly ownerPid: number; + readonly createdAtMs: number; + /** Self-identifying holder token; every mutating transition verifies it. */ + readonly token: string; + /** Last heartbeat; older than the staleness bound means wedged even when pid lives. */ + readonly heartbeatAtMs: number; + /** The plan id being applied. Diagnostics only; exclusion never reads it. */ + readonly planId: string | null; +} + +export interface CodexCliUpdateLeaseIo { + lockPath?: string; + isAlive?: (pid: number) => boolean; + now?: () => number; + pid?: number; + /** Test seam: force the O_EXCL publication fallback (hardlink-less filesystems). */ + forceExclusiveCreateFallback?: boolean; +} + +export interface CodexCliUpdateLeaseWaitIo extends CodexCliUpdateLeaseIo { + sleep?: (ms: number) => Promise; +} + +export type CodexCliUpdateLeaseAcquisition = + | { acquired: true; record: CodexCliUpdateLeaseRecord } + /** A live owner inside the staleness bound holds the lease. */ + | { acquired: false; reason: "held"; heldBy: number } + /** The lock path could not be created or observed at all. */ + | { acquired: false; reason: "unavailable" }; + +export function defaultCodexCliUpdateLeasePath(): string { + // getConfigDir owns OPENCODEX_HOME resolution; the lease sits beside + // desktop-restart.lock so every Codex lifecycle guard shares one directory. + return join(getConfigDir(), "codex-cli-update.lock"); +} + +function defaultIsAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function parseRecord(raw: string): CodexCliUpdateLeaseRecord | null { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + if (typeof parsed !== "object" || parsed === null) return null; + const view = parsed as Record; + const ownerPid = view.ownerPid; + const createdAtMs = view.createdAtMs; + const token = view.token; + const planId = view.planId; + if (view.version !== 1) return null; + if (typeof ownerPid !== "number" || !Number.isSafeInteger(ownerPid) || ownerPid <= 0) return null; + if (typeof createdAtMs !== "number" || !Number.isFinite(createdAtMs)) return null; + if (typeof token !== "string" || token.length === 0) return null; + const heartbeatAtMs = view.heartbeatAtMs; + return { + version: 1, + ownerPid, + createdAtMs, + token, + heartbeatAtMs: + typeof heartbeatAtMs === "number" && Number.isFinite(heartbeatAtMs) + ? heartbeatAtMs + : createdAtMs, + planId: typeof planId === "string" ? planId : null, + }; +} + +function readRecord(path: string): CodexCliUpdateLeaseRecord | null { + try { + return parseRecord(readFileSync(path, "utf-8")); + } catch { + return null; + } +} + +function newToken(): string { + return randomBytes(16).toString("hex"); +} + +/** + * Publish the record atomically: stage the full content on a unique sibling, then + * hardlink it onto the contended path. `linkSync` fails with EEXIST if the lock + * already exists, so the file is never observable without its full record. On a + * filesystem without hardlinks the caller's O_EXCL fallback applies. + */ +function publishViaLink(path: string, record: CodexCliUpdateLeaseRecord): boolean { + const staging = `${path}.staging-${process.pid}-${record.token}`; + try { + const fd = openSync(staging, "wx", 0o600); + try { + writeSync(fd, JSON.stringify(record)); + } finally { + closeSync(fd); + } + try { + linkSync(staging, path); + return true; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === "EEXIST") return false; + throw err; + } + } finally { + try { + unlinkSync(staging); + } catch { + /* staging already gone, or was never created */ + } + } +} + +/** O_EXCL fallback for filesystems that cannot hardlink (no atomic publish). */ +function publishViaExclusiveCreate(path: string, record: CodexCliUpdateLeaseRecord): boolean { + let fd: number; + try { + fd = openSync(path, "wx", 0o600); + } catch { + return false; + } + try { + writeSync(fd, JSON.stringify(record)); + } finally { + closeSync(fd); + } + return true; +} + +/** Exclusive publish on the contended path — the whole mutual exclusion. */ +function tryPublish( + path: string, + record: CodexCliUpdateLeaseRecord, + io: CodexCliUpdateLeaseIo, +): boolean { + mkdirSync(dirname(path), { recursive: true }); + if (io.forceExclusiveCreateFallback) return publishViaExclusiveCreate(path, record); + try { + return publishViaLink(path, record); + } catch { + return publishViaExclusiveCreate(path, record); + } +} + +/** Rewrite the lock in place. Only safe for the holder: it is not the contended path. */ +function rewriteRecord(path: string, record: CodexCliUpdateLeaseRecord): void { + const staging = `${path}.hb-${process.pid}-${record.token}`; + const fd = openSync(staging, "wx", 0o600); + try { + writeSync(fd, JSON.stringify(record)); + } finally { + closeSync(fd); + } + renameSync(staging, path); +} + +/** + * Compare-and-delete: unlink `path` only while it still names `expected.token`. + * + * Hardlink the current file to a tombstone first: the link binds the inode we + * verified, so an owner that rewrote its record (heartbeat) or a successor that + * never existed cannot be caught by the unlink. Without hardlinks we fall back to + * re-read + token compare immediately before unlink — the window narrows to syscall + * granularity, which is the best a lockfile can do there. + */ +function deleteIfToken(path: string, expected: CodexCliUpdateLeaseRecord): boolean { + const tombstone = `${path}.gone-${expected.token}`; + try { + linkSync(path, tombstone); + } catch { + // No link support or already gone: verify by re-read and accept the narrow race. + const current = readRecord(path); + if (!current || current.token !== expected.token) return false; + try { + unlinkSync(path); + return true; + } catch { + return false; + } + } + try { + const pathIno = statSync(path).ino; + const tombIno = statSync(tombstone).ino; + const tombRecord = readRecord(tombstone); + // A zero inode means the platform cannot express identity — use the token-only check. + const sameFile = pathIno !== 0 && tombIno !== 0 ? pathIno === tombIno : true; + if (sameFile && tombRecord && tombRecord.token === expected.token) { + unlinkSync(path); + return true; + } + return false; + } finally { + try { + unlinkSync(tombstone); + } catch { + /* tombstone already gone */ + } + } +} + +function holderIsStale(record: CodexCliUpdateLeaseRecord, io: CodexCliUpdateLeaseIo): boolean { + const isAlive = io.isAlive ?? defaultIsAlive; + const now = io.now ?? Date.now; + if (!isAlive(record.ownerPid)) return true; + // A live pid is held only while its heartbeat is fresh; age alone never reaps it. + return now() - record.heartbeatAtMs > CODEX_CLI_UPDATE_LEASE_MAX_AGE_MS; +} + +/** + * Take the update lease, or report why it could not be taken. + * + * Contended callers do not queue: a second apply reports "held" and refuses, which is + * the honest answer — queueing would run two global installs back to back against a + * plan the second caller read before the first one mutated the installation. + */ +export function acquireCodexCliUpdateLease( + io: CodexCliUpdateLeaseIo & { planId?: string } = {}, +): CodexCliUpdateLeaseAcquisition { + const path = io.lockPath ?? defaultCodexCliUpdateLeasePath(); + const now = io.now ?? Date.now; + const self = io.pid ?? process.pid; + + const existing = readRecord(path); + if (existing) { + if (!holderIsStale(existing, io)) { + return { acquired: false, reason: "held", heldBy: existing.ownerPid }; + } + // Stale. Remove it only while it still names the record we observed: two + // contenders can see the same stale lease, and an unconditional unlink would + // let the slower one delete the faster one's fresh lease. After compare-delete, + // the publish below decides which of us actually holds it. + if (!deleteIfToken(path, existing)) { + const successor = readRecord(path); + if (!successor) return { acquired: false, reason: "unavailable" }; + return holderIsStale(successor, io) + ? { acquired: false, reason: "unavailable" } + : { acquired: false, reason: "held", heldBy: successor.ownerPid }; + } + } + + const record: CodexCliUpdateLeaseRecord = { + version: 1, + ownerPid: self, + createdAtMs: now(), + token: newToken(), + heartbeatAtMs: now(), + planId: io.planId ?? null, + }; + if (tryPublish(path, record, io)) return { acquired: true, record }; + + const winner = readRecord(path); + if (winner) { + return holderIsStale(winner, io) + ? { acquired: false, reason: "unavailable" } + : { acquired: false, reason: "held", heldBy: winner.ownerPid }; + } + + // The file exists but names nobody: truncated or corrupt. If it is younger than + // the publish grace it is a contender's in-flight write on the O_EXCL fallback — + // held, not clearable. Past the grace it is a dead writer's debris: one ownerless + // file must not wedge every future update, so clear it (compare-delete on the + // path identity) and make exactly one more attempt. + try { + const stat = statSync(path); + if (now() - stat.mtimeMs < PUBLISH_GRACE_MS) { + return { acquired: false, reason: "unavailable" }; + } + } catch { + return { acquired: false, reason: "unavailable" }; + } + try { + unlinkSync(path); + } catch { + /* somebody else cleared it first */ + } + if (tryPublish(path, record, io)) return { acquired: true, record }; + const successor = readRecord(path); + return successor && !holderIsStale(successor, io) + ? { acquired: false, reason: "held", heldBy: successor.ownerPid } + : { acquired: false, reason: "unavailable" }; +} + +/** + * Compare-and-delete release bound to the holder's token — a late release can never + * unlink a successor's lease, and a release naming only a pid (recycled or not) + * cannot unlink a successor associated with the same pid edge case. + */ +export function releaseCodexCliUpdateLease( + io: CodexCliUpdateLeaseIo & { token?: string } = {}, +): void { + const path = io.lockPath ?? defaultCodexCliUpdateLeasePath(); + const self = io.pid ?? process.pid; + const existing = readRecord(path); + if (!existing || existing.ownerPid !== self) return; + if (io.token !== undefined && existing.token !== io.token) return; + deleteIfToken(path, existing); +} + +/** + * Keep a held lease's heartbeat fresh for the duration of a long install. Returns a + * stop function; the interval is unref'd so it never keeps a process alive. The + * heartbeat re-verifies its own token before rewriting, so a holder that was already + * reaped (or superseded) simply stops updating someone else's lease. + */ +export function startCodexCliUpdateLeaseHeartbeat( + record: CodexCliUpdateLeaseRecord, + io: CodexCliUpdateLeaseIo = {}, +): () => void { + const path = io.lockPath ?? defaultCodexCliUpdateLeasePath(); + const now = io.now ?? Date.now; + const timer = setInterval(() => { + const current = readRecord(path); + if (!current || current.token !== record.token) return; + try { + rewriteRecord(path, { ...current, heartbeatAtMs: now() }); + } catch { + /* a successor or a cleared lease: the next tick re-verifies and stays inert */ + } + }, HEARTBEAT_INTERVAL_MS); + timer.unref(); + return () => clearInterval(timer); +} + +export interface CodexCliUpdateLeaseObservation { + /** True only while a live owner with a fresh heartbeat holds the lease. */ + readonly held: boolean; + readonly ownerPid: number | null; +} + +/** + * Read-only observation for Codex startup paths. A stale or corrupt record reads as + * free — reclaiming it is the acquirer's job, and a dead file must not block Codex + * from ever starting again. + */ +export function observeCodexCliUpdateLease( + io: CodexCliUpdateLeaseIo = {}, +): CodexCliUpdateLeaseObservation { + const existing = readRecord(io.lockPath ?? defaultCodexCliUpdateLeasePath()); + if (!existing || holderIsStale(existing, io)) return { held: false, ownerPid: null }; + return { held: true, ownerPid: existing.ownerPid }; +} + +const WAIT_POLL_MS = 250; + +/** + * Bounded wait for the lease to clear, for startup paths that would rather wait out + * the tail of an install than refuse outright. Resolves true once the lease is free, + * false when the deadline passed with it still held. + */ +export async function waitForCodexCliUpdateLeaseRelease( + io: CodexCliUpdateLeaseWaitIo & { timeoutMs?: number } = {}, +): Promise { + const now = io.now ?? Date.now; + const sleep = io.sleep ?? (ms => new Promise(done => setTimeout(done, ms))); + const deadline = now() + (io.timeoutMs ?? CODEX_CLI_UPDATE_LEASE_WAIT_MS); + for (;;) { + if (!observeCodexCliUpdateLease(io).held) return true; + if (now() >= deadline) return false; + await sleep(Math.min(WAIT_POLL_MS, Math.max(1, deadline - now()))); + } +} diff --git a/src/codex/cli-update-plan.ts b/src/codex/cli-update-plan.ts new file mode 100644 index 00000000000..26efee767c8 --- /dev/null +++ b/src/codex/cli-update-plan.ts @@ -0,0 +1,639 @@ +import { spawnSync, type SpawnSyncReturns } from "node:child_process"; +import { createHash } from "node:crypto"; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { compareStrictSemver, parseStrictSemver } from "../lib/strict-semver"; +import { npmInvocation } from "../update/npm-invocation.mjs"; +import { + inspectCodexCliInstall, + type CodexCliInstallKind, + type CodexCliInstallProvenanceDeps, + type CodexCliInstallReport, +} from "./cli-install-provenance"; +import { + scanCodexAppServerProcesses, + type CodexAppServerProcessIo, + type CodexAppServerProcessScan, +} from "./app-server-processes"; +import { + acquireCodexCliUpdateLease, + releaseCodexCliUpdateLease, + startCodexCliUpdateLeaseHeartbeat, + type CodexCliUpdateLeaseIo, +} from "./cli-update-lease"; + +/** + * Phase 2 of the Codex CLI update manager: a deterministic dry-run plan and an explicit + * apply engine. + * + * Phase 1 ({@link inspectCodexCliInstall}) answers who owns the installation. It never + * queries a registry, never enumerates processes and never writes. This module adds the + * three inputs it deliberately left out — an exact resolved target with registry + * integrity, live session blockers, and a decision — and nothing else. + * + * Two invariants shape the whole module: + * + * - No persisted state. The plan is not stored anywhere; {@link CodexCliUpdatePlan.planId} + * is a digest over the evidence the decision rests on, and apply recomputes it from + * live evidence. A stale plan is refused, never silently regenerated. + * - Nothing is terminated or restarted. A live Codex session refuses the plan; it is + * never signalled, killed or waited on. + */ + +export const CODEX_CLI_PACKAGE = "@openai/codex"; +export const CODEX_CLI_UPDATE_SCHEMA_VERSION = 1 as const; + +/** + * The one registry this workflow is allowed to resolve, pack and install from. + * + * npm view/pack inherit the operator's npm configuration by default, so a project or + * user .npmrc — or an npm_config_* env var — can redirect "@openai/codex" to another + * registry whose version query, integrity query and tarball all agree with each + * other. Every npm call below pins this registry explicitly AND runs with the + * registry-affecting configuration isolated, so the pinned origin is the only source + * the evidence can come from. + */ +export const CODEX_CLI_REGISTRY = "https://registry.npmjs.org"; + +/** Only the stable channel is exposed; a moving dist-tag is resolved before it is used. */ +export type CodexCliUpdateChannel = "latest"; + +export type CodexCliUpdateRefusal = + | "windows_inspection_deferred" + | "not_managed" + | "installed_version_unverified" + | "target_unresolved" + | "already_current" + | "target_not_newer" + | "blocked_active_session" + | "blocked_process_state_unknown"; + +/** + * `not-evaluated` is not a weaker `unknown`: it records that the plan was already refused + * on ownership or target grounds, so the process table was never read. Only `unknown` + * means an enumeration attempt failed. + */ +export type CodexCliUpdateSessionState = "none" | "active" | "unknown" | "not-evaluated"; + +export interface CodexCliUpdateSession { + readonly state: CodexCliUpdateSessionState; + /** Match count only. Command lines and paths never leave the process scanner. */ + readonly matches: number | null; +} + +export type CodexCliUpdateTarget = + | Readonly<{ kind: "resolved"; version: string; integrity: string }> + | Readonly<{ kind: "unresolved"; reason: string }>; + +export interface CodexCliUpdatePlan { + readonly schemaVersion: typeof CODEX_CLI_UPDATE_SCHEMA_VERSION; + readonly package: typeof CODEX_CLI_PACKAGE; + readonly channel: CodexCliUpdateChannel; + readonly applicable: boolean; + readonly refusal: CodexCliUpdateRefusal | null; + /** Present only for an applicable plan; there is nothing to quote for a refusal. */ + readonly planId: string | null; + readonly provenance: CodexCliInstallKind; + readonly managed: boolean; + readonly installedVersion: string | null; + readonly versionEvidence: CodexCliInstallReport["versionEvidence"]["kind"]; + readonly location: string | null; + readonly targetVersion: string | null; + readonly targetIntegrity: string | null; + readonly session: CodexCliUpdateSession; + /** Indicative install argv for the operator to read before approving; apply actually runs `npm pack`, verifies the bound sha512, and installs the verified tarball. */ + readonly command: readonly string[] | null; +} + +export type CodexCliUpdateApplyStatus = + | "applied" + | "not_applied" + | "ambiguous" + | "refused"; + +export type CodexCliUpdateApplyRefusal = CodexCliUpdateRefusal + | "plan_stale" + | "plan_unknown" + | "integrity_mismatch" + | "update_in_progress" + | "update_lease_unavailable"; + +export interface CodexCliUpdateApplyResult { + readonly schemaVersion: typeof CODEX_CLI_UPDATE_SCHEMA_VERSION; + readonly status: CodexCliUpdateApplyStatus; + readonly refusal: CodexCliUpdateApplyRefusal | null; + readonly planId: string | null; + readonly targetVersion: string | null; + readonly installedVersionBefore: string | null; + readonly installedVersionAfter: string | null; + /** Evidence only. The readback classifies the outcome; the exit code never does. */ + readonly installerExitCode: number | null; +} + +export interface CodexCliUpdateInstallerResult { + /** `null` for a spawn failure or timeout, mirroring `spawnSync` status. */ + readonly exitCode: number | null; + /** True when the fetched artifact's sha512 did not match the plan-bound integrity. */ + readonly integrityMismatch?: boolean; +} + +export interface CodexCliUpdatePlanDeps { + readonly inspect?: (deps: CodexCliInstallProvenanceDeps) => Promise; + readonly inspectionDeps?: CodexCliInstallProvenanceDeps; + readonly platform?: NodeJS.Platform; + readonly channel?: CodexCliUpdateChannel; + readonly scanProcesses?: (io?: CodexAppServerProcessIo) => CodexAppServerProcessScan; + readonly processIo?: CodexAppServerProcessIo; + readonly resolveTarget?: (channel: CodexCliUpdateChannel) => CodexCliUpdateTarget; +} + +export interface CodexCliUpdateApplyDeps extends CodexCliUpdatePlanDeps { + readonly runInstaller?: (version: string, expectedIntegrity: string | null) => CodexCliUpdateInstallerResult; + /** Lease seam: a temp lockPath plus fake liveness drives the exclusion tests. */ + readonly leaseIo?: CodexCliUpdateLeaseIo; +} + +const PLAN_ID_LENGTH = 32; +const PLAN_ID_RE = /^[0-9a-f]{32}$/; +const REGISTRY_TIMEOUT_MS = 12_000; +const INSTALL_TIMEOUT_MS = 300_000; +const SHA512_INTEGRITY_RE = /^sha512-[A-Za-z0-9+/=]+$/; + +/** The one command apply is allowed to run, in argv form. */ +export function codexCliUpdateCommand(version: string): readonly string[] { + return Object.freeze(["npm", "install", "-g", `${CODEX_CLI_PACKAGE}@${version}`]); +} + +/** + * Digest over exactly the evidence the decision rests on. + * + * Session blockers are deliberately absent: a session that starts or ends between dry-run + * and apply must not invalidate an otherwise identical plan, and it is re-read at apply + * time where it can only ever refuse. Everything else — ownership, installed version, + * location, resolved target, integrity — is bound, so any drift produces + * a different id and `apply` refuses instead of installing something the operator did not + * read. + */ +export function codexCliUpdatePlanId(bound: { + readonly platform: NodeJS.Platform; + readonly provenance: CodexCliInstallKind; + readonly installedVersion: string; + readonly location: string | null; + readonly channel: CodexCliUpdateChannel; + readonly targetVersion: string; + readonly targetIntegrity: string; +}): string { + // Ordered pairs, not object key order: the digest must not depend on how a caller + // happened to build the record. + const fields: readonly (readonly [string, string])[] = [ + ["schemaVersion", String(CODEX_CLI_UPDATE_SCHEMA_VERSION)], + ["package", CODEX_CLI_PACKAGE], + ["platform", bound.platform], + ["provenance", bound.provenance], + ["installedVersion", bound.installedVersion], + ["location", bound.location ?? ""], + ["channel", bound.channel], + ["targetVersion", bound.targetVersion], + ["targetIntegrity", bound.targetIntegrity], + ]; + const hash = createHash("sha256"); + for (const [key, value] of fields) hash.update(`${key}=${value}\n`); + return hash.digest("hex").slice(0, PLAN_ID_LENGTH); +} + +interface NpmTarget { + readonly bin: string; + readonly args: string[]; + readonly options: { + readonly windowsVerbatimArguments?: boolean; + readonly cwd?: string; + readonly env?: NodeJS.ProcessEnv; + }; +} + +function npmTarget(args: readonly string[]): NpmTarget | null { + const invocation = npmInvocation(args); + if (!invocation) return null; + return { bin: invocation.file, args: invocation.args, options: invocation.options }; +} + +/** + * The npm environment with every registry-affecting config channel removed. + * + * npm maps any npm_config_* env var into its configuration, so NPM_CONFIG_REGISTRY or + * npm_config_@openai:registry would defeat the pinned --registry flag (a scoped + * registry beats the default for that scope). They are all stripped. The settings + * this codebase intentionally supports survive untouched: the standard proxy + * variables npm itself honors (HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY, any case) + * and NODE_EXTRA_CA_CERTS, which npm's own Node runtime reads for TLS. + */ +function codexCliUpdateNpmEnv(): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { ...process.env }; + for (const key of Object.keys(env)) { + if (key.toLowerCase().startsWith("npm_config_")) delete env[key]; + } + return env; +} + +interface NpmConfigIsolation { + /** Controlled cwd: contains the sentinel package.json, so npm's project-config walk stops here. */ + readonly dir: string; + /** Empty file substituted for both the user and the global npmrc. */ + readonly npmrc: string; + readonly env: NodeJS.ProcessEnv; +} + +/** + * A directory npm cannot read hostile configuration through. + * + * npm resolves project config from the nearest ancestor containing package.json (or + * node_modules/.git), so a bare temp dir is NOT enough — the walk would continue into + * the operator's real ancestors. The sentinel package.json anchors the walk here, + * where no .npmrc exists. --userconfig/--globalconfig replace the two file configs, + * and the env filter removes the env-var channel. What remains is exactly the pinned + * --registry flag plus deliberately supported proxy/CA env. + */ +function createNpmConfigIsolation(dir?: string): NpmConfigIsolation { + const root = dir ?? mkdtempSync(join(tmpdir(), "ocx-codex-cli-meta-")); + writeFileSync(join(root, "package.json"), "{}\n"); + const npmrc = join(root, "ocx-update.npmrc"); + writeFileSync(npmrc, ""); + return { dir: root, npmrc, env: codexCliUpdateNpmEnv() }; +} + +/** Registry-pinned, config-isolated argv for one npm call. */ +function isolatedNpmArgs(args: readonly string[], isolation: NpmConfigIsolation): readonly string[] { + return [ + ...args, + "--registry=" + CODEX_CLI_REGISTRY, + "--userconfig=" + isolation.npmrc, + "--globalconfig=" + isolation.npmrc, + ]; +} + +function isolatedNpmTarget(args: readonly string[], isolation: NpmConfigIsolation): NpmTarget | null { + const target = npmTarget(isolatedNpmArgs(args, isolation)); + if (!target) return null; + return { + bin: target.bin, + args: target.args, + options: { ...target.options, cwd: isolation.dir, env: isolation.env }, + }; +} + +/** + * Resolve the channel to one exact version plus its sha512 integrity token. + * + * OpenCodex's own updater treats a failed integrity query as `skipped` and proceeds + * best-effort. That is a defensible trade for the package we publish ourselves; it is not + * acceptable for a foreign package we are about to install on the operator's behalf, so + * every failure lane here is a refusal. + */ +export function resolveCodexCliUpdateTarget( + channel: CodexCliUpdateChannel, + spawn: typeof spawnSync = spawnSync, +): CodexCliUpdateTarget { + // The pinned registry and the isolation directory together are the boundary: the + // version, the integrity token and the tarball URL all come from npmjs or the + // target is unresolved — a redirected answer can never be the evidence. + const isolation = createNpmConfigIsolation(); + try { + const runView = (field: string, spec: string): SpawnSyncReturns | null => { + const target = isolatedNpmTarget(["view", spec, field], isolation); + if (!target) return null; + return spawn(target.bin, target.args, { + encoding: "utf8", + timeout: REGISTRY_TIMEOUT_MS, + windowsHide: true, + ...target.options, + }); + }; + + const versionRun = runView("version", `${CODEX_CLI_PACKAGE}@${channel}`); + if (!versionRun) return Object.freeze({ kind: "unresolved" as const, reason: "npm executable was not found on a trusted PATH entry" }); + // status === null covers a timeout as well as a spawn failure. + if (versionRun.status !== 0) { + return Object.freeze({ kind: "unresolved" as const, reason: `registry version query failed (status ${versionRun.status ?? "timeout"})` }); + } + const version = (versionRun.stdout ?? "").trim(); + if (!parseStrictSemver(version)) { + return Object.freeze({ kind: "unresolved" as const, reason: "registry returned no exact version" }); + } + + const integrityRun = runView("dist.integrity", `${CODEX_CLI_PACKAGE}@${version}`); + if (!integrityRun) return Object.freeze({ kind: "unresolved" as const, reason: "npm executable was not found on a trusted PATH entry" }); + if (integrityRun.status !== 0) { + return Object.freeze({ kind: "unresolved" as const, reason: `registry integrity query failed (status ${integrityRun.status ?? "timeout"})` }); + } + // `dist.integrity` may arrive quoted or as a space-separated multi-hash list. + const tokens = (integrityRun.stdout ?? "").replace(/["']/g, "").trim().split(/\s+/).filter(Boolean); + const integrity = tokens.find(token => SHA512_INTEGRITY_RE.test(token)); + if (!integrity) { + return Object.freeze({ kind: "unresolved" as const, reason: "registry returned no sha512 integrity token" }); + } + + // The digest binds the tarball's CONTENT; this binds its ORIGIN. A registry + // answer whose tarball lives outside the pinned registry is not the artifact + // the plan approved, even if its sha512 matched. + const tarballRun = runView("dist.tarball", `${CODEX_CLI_PACKAGE}@${version}`); + if (!tarballRun) return Object.freeze({ kind: "unresolved" as const, reason: "npm executable was not found on a trusted PATH entry" }); + if (tarballRun.status !== 0) { + return Object.freeze({ kind: "unresolved" as const, reason: `registry tarball query failed (status ${tarballRun.status ?? "timeout"})` }); + } + const tarball = (tarballRun.stdout ?? "").replace(/["']/g, "").trim(); + let tarballOrigin: string | null = null; + try { + tarballOrigin = new URL(tarball).origin; + } catch { + tarballOrigin = null; + } + if (tarballOrigin !== CODEX_CLI_REGISTRY) { + return Object.freeze({ kind: "unresolved" as const, reason: "registry returned a tarball outside the official registry origin" }); + } + return Object.freeze({ kind: "resolved" as const, version, integrity }); + } finally { + rmSync(isolation.dir, { recursive: true, force: true }); + } +} + +/** + * Install exactly `version`, verifying the fetched tarball against the plan-bound + * sha512 SRI first. `npm install @` re-resolves registry metadata at + * install time, so a registry or proxy answering differently after the plan check + * would go unnoticed. `npm pack` fetches the same tarball the install would use, the + * digest is compared with the planned integrity, and only the verified local file is + * installed. A mismatch fails closed before anything is written. + */ +function defaultRunInstaller(version: string, expectedIntegrity: string | null): CodexCliUpdateInstallerResult { + if (!expectedIntegrity) { + // Fail closed: a caller that cannot state the expected digest gets no + // unverified install, even though production plans always carry one. + return Object.freeze({ exitCode: null, integrityMismatch: true }); + } + const stage = mkdtempSync(join(tmpdir(), "ocx-codex-cli-update-")); + try { + // The same pinned registry + isolated config as the resolve: the pack must fetch + // from the origin the plan bound, and the install's dependency resolution must + // not consult a redirected registry either. + const isolation = createNpmConfigIsolation(stage); + const pack = isolatedNpmTarget(["pack", `${CODEX_CLI_PACKAGE}@${version}`, "--pack-destination", stage], isolation); + if (!pack) return Object.freeze({ exitCode: null }); + const packRun = spawnSync(pack.bin, pack.args, { + encoding: "utf8", + timeout: INSTALL_TIMEOUT_MS, + windowsHide: true, + stdio: "inherit", + ...pack.options, + }); + if (packRun.status !== 0) return Object.freeze({ exitCode: packRun.status }); + const tarballs = readdirSync(stage).filter(name => name.endsWith(".tgz")); + if (tarballs.length !== 1) return Object.freeze({ exitCode: null }); + const tarball = join(stage, tarballs[0]!); + const actual = `sha512-${createHash("sha512").update(readFileSync(tarball)).digest("base64")}`; + if (actual !== expectedIntegrity) return Object.freeze({ exitCode: null, integrityMismatch: true }); + const install = isolatedNpmTarget(["install", "-g", tarball], isolation); + if (!install) return Object.freeze({ exitCode: null }); + const run = spawnSync(install.bin, install.args, { + encoding: "utf8", + timeout: INSTALL_TIMEOUT_MS, + windowsHide: true, + stdio: "inherit", + ...install.options, + }); + return Object.freeze({ exitCode: run.status }); + } finally { + rmSync(stage, { recursive: true, force: true }); + } +} + +function refusedPlan( + refusal: CodexCliUpdateRefusal, + report: CodexCliInstallReport, + channel: CodexCliUpdateChannel, + target: CodexCliUpdateTarget | null, + session: CodexCliUpdateSession, +): CodexCliUpdatePlan { + return Object.freeze({ + schemaVersion: CODEX_CLI_UPDATE_SCHEMA_VERSION, + package: CODEX_CLI_PACKAGE, + channel, + applicable: false, + refusal, + planId: null, + provenance: report.provenance, + managed: report.managed, + installedVersion: report.packageVersion, + versionEvidence: report.versionEvidence.kind, + location: report.location, + targetVersion: target?.kind === "resolved" ? target.version : null, + targetIntegrity: target?.kind === "resolved" ? target.integrity : null, + session, + command: null, + }); +} + +const NOT_EVALUATED: CodexCliUpdateSession = Object.freeze({ state: "not-evaluated" as const, matches: null }); + +/** + * Build the dry-run plan. Reads only; nothing here writes, signals or installs. + * + * The refusal order is deliberate. Ownership and target questions are settled before the + * process table is read, so a machine that can never be updated by this workflow does not + * pay for an enumeration, and the reported refusal is the decisive one rather than + * whichever check happened to run first. + */ +export async function createCodexCliUpdatePlan(deps: CodexCliUpdatePlanDeps = {}): Promise { + const channel: CodexCliUpdateChannel = deps.channel ?? "latest"; + const platform = deps.platform ?? process.platform; + const inspect = deps.inspect ?? inspectCodexCliInstall; + const report = await inspect(deps.inspectionDeps ?? {}); + + // Phase 1 performs no candidate filesystem I/O at all on Windows, so there is no + // ownership evidence to build a plan on. This stays dormant until the handle-bound + // provenance layer exists; pretending otherwise would require exactly the reads phase 1 + // refused to perform. + if (platform === "win32" || report.reason === "windows_inspection_deferred") { + return refusedPlan("windows_inspection_deferred", report, channel, null, NOT_EVALUATED); + } + if (!report.managed || report.provenance !== "npm-global") { + return refusedPlan("not_managed", report, channel, null, NOT_EVALUATED); + } + // An advisory runtime string is what a candidate binary said about itself. Only + // package-manifest evidence states what is installed on disk, and only that can be + // compared with a registry version or read back after an install. + const installedVersion = report.versionEvidence.kind === "package-manifest" ? report.packageVersion : null; + const installedSemver = installedVersion ? parseStrictSemver(installedVersion) : null; + if (!installedVersion || !installedSemver) { + return refusedPlan("installed_version_unverified", report, channel, null, NOT_EVALUATED); + } + + const resolveTarget = deps.resolveTarget ?? (ch => resolveCodexCliUpdateTarget(ch)); + const target = resolveTarget(channel); + if (target.kind !== "resolved") { + return refusedPlan("target_unresolved", report, channel, target, NOT_EVALUATED); + } + // Raw equality is not the gate: a resolved target must advance the installed + // version by semver precedence. Equal versions are already current and lower + // ones are refused rather than applied as a silent downgrade. + const targetSemver = parseStrictSemver(target.version); + if (!targetSemver) { + return refusedPlan("target_unresolved", report, channel, target, NOT_EVALUATED); + } + const versionOrder = compareStrictSemver(targetSemver, installedSemver); + if (versionOrder === 0) { + return refusedPlan("already_current", report, channel, target, NOT_EVALUATED); + } + if (versionOrder < 0) { + return refusedPlan("target_not_newer", report, channel, target, NOT_EVALUATED); + } + + const scan = (deps.scanProcesses ?? scanCodexAppServerProcesses)(deps.processIo ?? {}); + if (scan.kind !== "observed") { + // An unreadable process table is not "no sessions". `listCodexAppServerProcesses` + // maps that failure to an empty list because its kill contract must never signal a + // process it could not verify; the update contract has to defer instead. + return refusedPlan("blocked_process_state_unknown", report, channel, target, Object.freeze({ state: "unknown" as const, matches: null })); + } + const matches = scan.processes.length; + if (matches > 0) { + return refusedPlan("blocked_active_session", report, channel, target, Object.freeze({ state: "active" as const, matches })); + } + + return Object.freeze({ + schemaVersion: CODEX_CLI_UPDATE_SCHEMA_VERSION, + package: CODEX_CLI_PACKAGE, + channel, + applicable: true, + refusal: null, + planId: codexCliUpdatePlanId({ + platform, + provenance: report.provenance, + installedVersion, + location: report.location, + channel, + targetVersion: target.version, + targetIntegrity: target.integrity, + }), + provenance: report.provenance, + managed: report.managed, + installedVersion, + versionEvidence: report.versionEvidence.kind, + location: report.location, + targetVersion: target.version, + targetIntegrity: target.integrity, + session: Object.freeze({ state: "none" as const, matches: 0 }), + command: codexCliUpdateCommand(target.version), + }); +} + +function refusedApply( + refusal: CodexCliUpdateApplyRefusal, + plan: CodexCliUpdatePlan | null, +): CodexCliUpdateApplyResult { + return Object.freeze({ + schemaVersion: CODEX_CLI_UPDATE_SCHEMA_VERSION, + status: "refused" as const, + refusal, + planId: plan?.planId ?? null, + targetVersion: plan?.targetVersion ?? null, + installedVersionBefore: plan?.installedVersion ?? null, + installedVersionAfter: null, + installerExitCode: null, + }); +} + +/** + * Apply a plan the operator has read. + * + * The plan is recomputed from live evidence and the id must match, so the operator + * approves the exact target that is about to be installed. The install itself is one + * command against an exact version; the outcome is classified from a fresh inspection, + * never from the installer exit code, and nothing is retried or rolled back automatically. + * + * The update lease is acquired BEFORE the recomputation: its process scan is the final + * scan this install will ever take, and a scan without the lease is a snapshot, not + * mutual exclusion. The lease is then held through the install and the readback, so a + * Codex startup that observes it cannot begin loading the package while it is being + * replaced — and a second apply of the same plan is refused rather than running a + * concurrent global install against the same prefix. + */ +export async function applyCodexCliUpdatePlan( + planId: string, + deps: CodexCliUpdateApplyDeps = {}, +): Promise { + if (!PLAN_ID_RE.test(planId)) return refusedApply("plan_unknown", null); + + const leaseIo = deps.leaseIo ?? {}; + const acquisition = acquireCodexCliUpdateLease({ ...leaseIo, planId }); + if (!acquisition.acquired) { + // Held by a live updater, or the lock could not be created at all — both refuse + // before any evidence is gathered. A lease that cannot be taken is not a softer + // "unknown": installing without it would reintroduce the race it exists to close. + return refusedApply( + acquisition.reason === "held" ? "update_in_progress" : "update_lease_unavailable", + null, + ); + } + // A real install can outlast any fixed age bound; the heartbeat keeps the lease + // self-identifying as live so a contender cannot reap it on age alone. + const stopHeartbeat = startCodexCliUpdateLeaseHeartbeat(acquisition.record, leaseIo); + try { + const plan = await createCodexCliUpdatePlan(deps); + if (!plan.applicable || !plan.planId || !plan.targetVersion || !plan.installedVersion) { + return refusedApply(plan.refusal ?? "plan_unknown", plan); + } + // Any drift in ownership, installed version, location or target changes the id. + // Refuse rather than regenerate: the operator would otherwise approve + // one plan and install another. + if (plan.planId !== planId) return refusedApply("plan_stale", plan); + + const targetVersion = plan.targetVersion; + const before = plan.installedVersion; + const installer = (deps.runInstaller ?? defaultRunInstaller)(targetVersion, plan.targetIntegrity); + // The fetched artifact failed the plan-bound digest; nothing was installed. + if (installer.integrityMismatch) return refusedApply("integrity_mismatch", plan); + + const inspect = deps.inspect ?? inspectCodexCliInstall; + let readback: CodexCliInstallReport | null = null; + try { + readback = await inspect(deps.inspectionDeps ?? {}); + } catch { + readback = null; + } + + const after = readback + && readback.provenance === "npm-global" + && readback.location === plan.location + && readback.versionEvidence.kind === "package-manifest" + ? readback.packageVersion + : null; + + const result = (status: CodexCliUpdateApplyStatus): CodexCliUpdateApplyResult => Object.freeze({ + schemaVersion: CODEX_CLI_UPDATE_SCHEMA_VERSION, + status, + refusal: null, + planId: plan.planId, + targetVersion, + installedVersionBefore: before, + installedVersionAfter: after, + installerExitCode: installer.exitCode, + }); + + if (after !== targetVersion) { + // A failed readback, an unchanged version and a third version are all reported + // as-is. None of them is retried, and none of them is rolled back. + if (after !== null && after === before) return result("not_applied"); + return result("ambiguous"); + } + + // A matched shim never reaches this point: the inspector reports a shim-owned + // candidate as standalone-unverified and the plan refuses `not_managed`, so an + // applied update never owned a shim npm could have replaced. + return result("applied"); + } finally { + stopHeartbeat(); + releaseCodexCliUpdateLease({ ...leaseIo, token: acquisition.record.token }); + } +} diff --git a/src/codex/desktop-app-restart.ts b/src/codex/desktop-app-restart.ts index 17bf83ed7de..b04df1ba35b 100644 --- a/src/codex/desktop-app-restart.ts +++ b/src/codex/desktop-app-restart.ts @@ -36,6 +36,7 @@ import { releaseDesktopRestartLock, type DesktopRestartLockIo, } from "./desktop-app/lock"; +import { observeCodexCliUpdateLease, type CodexCliUpdateLeaseIo } from "./cli-update-lease"; import { rootShells, type DesktopAppAdapter, type DesktopExec, type DesktopProcess } from "./desktop-app/types"; import { darwinDesktopAppAdapter, darwinDefaultExec } from "./desktop-app/darwin"; import { linuxDesktopAppAdapter, linuxDefaultExec } from "./desktop-app/linux"; @@ -77,6 +78,12 @@ export interface DesktopAppRestartIo { * after telling the operator it had been handed off. */ allowHandoff?: boolean; + /** + * Codex CLI update lease seam. While an apply holds it, relaunching the desktop app + * would start app-servers against a global install that is being replaced, so the + * restart skips rather than racing it. + */ + updateLease?: CodexCliUpdateLeaseIo; } export type DesktopAppRestartReason = @@ -86,6 +93,7 @@ export type DesktopAppRestartReason = | "no_targets" | "self_ancestry" | "restart_in_flight" + | "update_in_progress" | "handoff_started" | "targets_survived" | "relaunch_failed"; @@ -214,6 +222,12 @@ export function restartCodexDesktopApp(io: DesktopAppRestartIo = {}): DesktopApp const exec = io.execFile ?? selected?.exec; if (!adapter || !exec) return skipped("unsupported_platform"); + // Step -1. A Codex CLI update holds its lease across the install and readback; a + // relaunch under it would start app-servers against a half-replaced global + // install. Skip before touching the restart lock or signalling anything — the + // operator retries once the update finishes. + if (observeCodexCliUpdateLease(io.updateLease ?? {}).held) return skipped("update_in_progress"); + // Step 0. Two restarts at once are destructive rather than merely wasteful: the // first quits and relaunches, the second sees the freshly started shell as a target // and kills it. Own-pid reentrancy means the wp5 helper runs this same step and @@ -337,4 +351,3 @@ export function restartCodexDesktopApp(io: DesktopAppRestartIo = {}): DesktopApp if (!handedOff) releaseDesktopRestartLock(io.lock); } } - diff --git a/src/lib/strict-semver.ts b/src/lib/strict-semver.ts index 75e45b5fcbf..08b56da4072 100644 --- a/src/lib/strict-semver.ts +++ b/src/lib/strict-semver.ts @@ -45,3 +45,27 @@ export function parseStrictSemver(value: unknown, maxLength = 128): StrictSemver prerelease: Object.freeze(prereleaseParts.map(part => /^\d+$/.test(part) ? BigInt(part) : part)), }); } + +/** + * SemVer precedence per semver.org: the numeric core first, then prerelease + * identifiers (numeric identifiers sort below alphanumeric ones, and a shorter + * identifier set below a longer one sharing its prefix). Build metadata is not + * part of precedence, which is why StrictSemver does not retain it. + */ +export function compareStrictSemver(a: StrictSemver, b: StrictSemver): number { + for (let i = 0; i < a.core.length; i++) { + if (a.core[i]! !== b.core[i]!) return a.core[i]! > b.core[i]! ? 1 : -1; + } + if (a.prerelease.length === 0) return b.prerelease.length === 0 ? 0 : 1; + if (b.prerelease.length === 0) return -1; + for (let i = 0; i < Math.max(a.prerelease.length, b.prerelease.length); i++) { + const left = a.prerelease[i]; + const right = b.prerelease[i]; + if (left === right) continue; + if (left === undefined) return -1; + if (right === undefined) return 1; + if (typeof left !== typeof right) return typeof left === "bigint" ? -1 : 1; + return left > right ? 1 : -1; + } + return 0; +} diff --git a/src/remote-control/workspace-codex-runtime.ts b/src/remote-control/workspace-codex-runtime.ts index b064e3c9b8c..ea29e39e22a 100644 --- a/src/remote-control/workspace-codex-runtime.ts +++ b/src/remote-control/workspace-codex-runtime.ts @@ -2,6 +2,10 @@ import { chmodSync, linkSync, mkdirSync, mkdtempSync, realpathSync, symlinkSync import { tmpdir } from "node:os"; import { dirname, isAbsolute, join } from "node:path"; import { resolveCodexRuntime } from "../codex/runtime"; +import { + waitForCodexCliUpdateLeaseRelease, + type CodexCliUpdateLeaseWaitIo, +} from "../codex/cli-update-lease"; import { remoteWorkspaceThreadStartParams } from "./workspace-coordinator"; import { startRemoteWorkspaceToolBridge } from "./workspace-tool-bridge"; import { truncateRemoteWorkspaceUtf8 } from "./workspace-utf8"; @@ -288,6 +292,12 @@ export interface CodexRemoteWorkspaceRuntimeOptions { command?: readonly string[]; env?: Record; version?: string; + /** + * Codex CLI update lease seam. Production reads the real lockfile; while an apply + * holds it, the npm-global Codex this spawn would load is being replaced, so the + * start waits briefly for the tail of the install and then refuses. + */ + updateLease?: CodexCliUpdateLeaseWaitIo & { timeoutMs?: number }; } export class CodexRemoteWorkspaceRuntimeFactory implements RemoteWorkspaceRuntimeFactory { @@ -308,6 +318,12 @@ export class CodexRemoteWorkspaceRuntimeFactory implements RemoteWorkspaceRuntim } async start(options: Parameters[0]): Promise { + // The update lease is the mutual exclusion the process scan cannot give: without + // it an apply could be mid-install while this app-server loads the package. + const leaseFree = await waitForCodexCliUpdateLeaseRelease(this.options.updateLease ?? {}); + if (!leaseFree) { + throw new Error("a Codex CLI update is in progress; refusing to start a Codex app-server mid-install"); + } const command = this.options.command ? [...this.options.command] : [resolveCodexRuntime().runtime.command]; diff --git a/tests/cli/cli-codex-cli-update.test.ts b/tests/cli/cli-codex-cli-update.test.ts index e31c675a32b..eab1b95de6a 100644 --- a/tests/cli/cli-codex-cli-update.test.ts +++ b/tests/cli/cli-codex-cli-update.test.ts @@ -198,9 +198,9 @@ describe("Codex CLI update CLI", () => { }); test("parses the shared JSON flag spellings within the exact check grammar", () => { - expect(parseCodexCliUpdateArgs(["check"])).toEqual({ json: false }); + expect(parseCodexCliUpdateArgs(["check"])).toEqual({ action: "check", json: false }); for (const flag of ["--json", "--json=true", "-json", "—json"]) { - expect(parseCodexCliUpdateArgs(["check", flag])).toEqual({ json: true }); + expect(parseCodexCliUpdateArgs(["check", flag])).toEqual({ action: "check", json: true }); } for (const args of [ ["check", "--channel", "latest"], @@ -217,7 +217,7 @@ describe("Codex CLI update CLI", () => { */ test("the JSON flag is accepted before the check action", () => { for (const flag of ["--json", "--json=true", "-json", "—json"]) { - expect(parseCodexCliUpdateArgs([flag, "check"])).toEqual({ json: true }); + expect(parseCodexCliUpdateArgs([flag, "check"])).toEqual({ action: "check", json: true }); } // Duplicate detection and positional validation still hold in that order. expect(() => parseCodexCliUpdateArgs(["--json", "check", "--json"])).toThrow(); @@ -376,3 +376,87 @@ describe("Codex CLI update CLI", () => { } }); }); + +/** + * Phase 2 verbs. The parser is the authorization boundary here: apply must never be + * reachable without a plan id the operator read in a dry-run. + */ +describe("Codex CLI update plan and apply grammar", () => { + const PLAN_ID = "0123456789abcdef0123456789abcdef"; + + test("plan defaults to the stable channel and accepts both option spellings", () => { + expect(parseCodexCliUpdateArgs(["plan"])).toEqual({ action: "plan", json: false, channel: "latest" }); + expect(parseCodexCliUpdateArgs(["plan", "--channel", "latest"])).toEqual({ action: "plan", json: false, channel: "latest" }); + expect(parseCodexCliUpdateArgs(["plan", "--channel=latest", "--json"])).toEqual({ action: "plan", json: true, channel: "latest" }); + }); + + test("apply requires a well-formed plan id", () => { + expect(parseCodexCliUpdateArgs(["apply", "--plan", PLAN_ID])).toEqual({ action: "apply", json: false, planId: PLAN_ID }); + expect(parseCodexCliUpdateArgs(["--json", "apply", "--plan=" + PLAN_ID])).toEqual({ action: "apply", json: true, planId: PLAN_ID }); + for (const args of [ + ["apply"], + ["apply", "--plan"], + ["apply", "--plan", "short"], + ["apply", "--plan", PLAN_ID.toUpperCase()], + ["apply", "--plan", PLAN_ID, "--plan", PLAN_ID], + ["apply", "--channel", "latest"], + ["plan", "--plan", PLAN_ID], + ["plan", "--channel", "preview"], + ["plan", "extra"], + ]) expect(() => parseCodexCliUpdateArgs(args)).toThrow(); + }); + + test("a refused dry-run is a normal answer and still exits 0", async () => { + let inspected = 0; + const code = await handleCodexCliUpdateCommand(["plan", "--json"], { + inspectInstall: async () => { inspected += 1; return report; }, + createPlan: async () => ({ + schemaVersion: 1, package: "@openai/codex", channel: "latest", + applicable: false, refusal: "not_managed", planId: null, + provenance: "app-bundle", managed: false, installedVersion: null, + versionEvidence: "unavailable", location: null, + targetVersion: null, targetIntegrity: null, + session: { state: "not-evaluated", matches: null }, command: null, + }), + }); + expect(code).toBe(0); + // The plan engine owns inspection; the command must not run a second one. + expect(inspected).toBe(0); + }); + + test("apply passes the operator's plan id through and reports a refusal as nonzero", async () => { + const seen: string[] = []; + const code = await handleCodexCliUpdateCommand(["apply", "--plan", PLAN_ID, "--json"], { + applyPlan: async planId => { + seen.push(planId); + return { + schemaVersion: 1, status: "refused", refusal: "plan_stale", planId: null, + targetVersion: null, installedVersionBefore: null, installedVersionAfter: null, + installerExitCode: null, + }; + }, + }); + expect(seen).toEqual([PLAN_ID]); + expect(code).toBe(1); + }); + + test("a completed apply exits 0", async () => { + const code = await handleCodexCliUpdateCommand(["apply", "--plan", PLAN_ID], { + applyPlan: async () => ({ + schemaVersion: 1, status: "applied", refusal: null, planId: PLAN_ID, + targetVersion: "1.1.0", installedVersionBefore: "1.0.0", installedVersionAfter: "1.1.0", + installerExitCode: 0, + }), + }); + expect(code).toBe(0); + }); + + test("a malformed plan id is rejected before anything is applied", async () => { + let applies = 0; + const code = await handleCodexCliUpdateCommand(["apply", "--plan", "nope"], { + applyPlan: async () => { applies += 1; throw new Error("unreachable"); }, + }); + expect(code).toBe(2); + expect(applies).toBe(0); + }); +}); diff --git a/tests/cli/cli-registry.test.ts b/tests/cli/cli-registry.test.ts index 27084cecae9..c8d4c83a33c 100644 --- a/tests/cli/cli-registry.test.ts +++ b/tests/cli/cli-registry.test.ts @@ -99,10 +99,13 @@ describe("CLI command registry parity", () => { expect(new Set(names).size).toBe(names.length); }); - test("system help exposes the exact Codex CLI inspection grammar", () => { + test("system help exposes the exact Codex CLI update grammar", () => { const details = findCommand("system")?.details ?? []; expect(details).toContain("ocx system codex-cli-update check [--json]"); - expect(details.some(line => line.includes("dry-run"))).toBe(false); + expect(details).toContain("ocx system codex-cli-update plan [--channel latest] [--json]"); + // apply must never render as an argument-free verb: the plan id IS the authorization. + expect(details).toContain("ocx system codex-cli-update apply --plan [--json]"); + expect(details.some(line => line.startsWith("ocx system codex-cli-update apply") && !line.includes("--plan"))).toBe(false); }); test("GUI registry usage documents explicit-origin single-use pairing", () => { diff --git a/tests/clients/desktop-app-restart.test.ts b/tests/clients/desktop-app-restart.test.ts index fd7ae597dcd..4f638d97e94 100644 --- a/tests/clients/desktop-app-restart.test.ts +++ b/tests/clients/desktop-app-restart.test.ts @@ -4,6 +4,7 @@ import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { restartCodexDesktopApp, type DesktopAppRestartIo } from "../../src/codex/desktop-app-restart"; +import { acquireCodexCliUpdateLease } from "../../src/codex/cli-update-lease"; import { windowsDesktopAppAdapter } from "../../src/codex/desktop-app/windows"; import { setTrustedWindowsElevationExecutablesForTests } from "../../src/lib/windows-elevation"; @@ -158,6 +159,24 @@ describe("Codex desktop app restart (#2292)", () => { expect(calls).toEqual([]); }); + test("a held Codex CLI update lease skips the restart before probing anything", () => { + // Relaunching the desktop app mid-install would start app-servers against the + // half-replaced global package; the lease makes the restart defer instead. + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + expect(acquireCodexCliUpdateLease({ lockPath, pid: 4_242 }).acquired).toBe(true); + const calls: Call[] = []; + const result = restartCodexDesktopApp({ + ...scriptedIo({ discovery: DISCOVERY, calls }), + updateLease: { lockPath, isAlive: () => true }, + }); + expect(result).toEqual({ + attempted: false, stopped: [], surviving: [], relaunch: "skipped", reason: "update_in_progress", + }); + // The lease check precedes discovery and the restart lock: nothing ran. + expect(calls).toEqual([]); + }); + test("fails closed when the package cannot be identified, killing nothing", () => { const calls: Call[] = []; const result = withTrustedExes(() => restartCodexDesktopApp(scriptedIo({ discovery: "MISS", calls }))); @@ -447,4 +466,3 @@ describe("#2557 a failed probe is not an absent app", () => { expect(script).not.toContain("SilentlyContinue' $root"); }); }); - diff --git a/tests/clients/remote-workspace-codex-runtime.test.ts b/tests/clients/remote-workspace-codex-runtime.test.ts index b9d259c2ebb..cd96856ddca 100644 --- a/tests/clients/remote-workspace-codex-runtime.test.ts +++ b/tests/clients/remote-workspace-codex-runtime.test.ts @@ -1,6 +1,9 @@ import { repoPath } from "../helpers/repo-root"; import { expect, test } from "bun:test"; -import { resolve } from "node:path"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { acquireCodexCliUpdateLease } from "../../src/codex/cli-update-lease"; import { CodexRemoteWorkspaceRuntimeFactory, RemoteWorkspaceCoordinator, @@ -8,6 +11,66 @@ import { type RemoteWorkspaceTransport, } from "../../src/remote-control"; +function startOptions(coordinator: RemoteWorkspaceCoordinator) { + return { + sessionId: "session-lease", + deviceId: "device-2", + deviceName: "Computer 2", + rootId: "root-2", + rootLabel: "Project", + capabilities: ["workspace.read"], + tools: ["list_directory", "read_file"], + coordinator, + emit: () => {}, + }; +} + +function leaseCoordinator(): RemoteWorkspaceCoordinator { + return new RemoteWorkspaceCoordinator({ + isOnline: () => true, + async invoke() { return { ok: true }; }, + }); +} + +test("a held Codex CLI update lease refuses the app-server start before spawn", async () => { + // The lease is the mutual exclusion the process scan cannot give: an app-server + // that starts while an apply holds it would load a half-replaced global install. + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + const held = acquireCodexCliUpdateLease({ lockPath, pid: 4_242, planId: "a".repeat(32) }); + expect(held.acquired).toBe(true); + + const factory = new CodexRemoteWorkspaceRuntimeFactory({ + command: [process.execPath, repoPath("tests", "fake-codex-server.ts")], + updateLease: { lockPath, isAlive: () => true, timeoutMs: 0 }, + }); + await expect(factory.start(startOptions(leaseCoordinator()))) + .rejects.toThrow("Codex CLI update is in progress"); +}); + +test("the app-server start waits out the lease and proceeds once it clears", async () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + const held = acquireCodexCliUpdateLease({ lockPath, pid: 4_242, planId: "a".repeat(32) }); + expect(held.acquired).toBe(true); + + // The owner is alive on the first observation and gone by the second — the tail + // of an install releasing, which a startup may wait out rather than refuse. + let livenessChecks = 0; + const factory = new CodexRemoteWorkspaceRuntimeFactory({ + command: [process.execPath, repoPath("tests", "fake-codex-server.ts")], + env: { FAKE_CODEX_SCRIPT: JSON.stringify({ turns: [] }) }, + updateLease: { + lockPath, + isAlive: () => livenessChecks++ === 0, + sleep: async () => {}, + timeoutMs: 1_000, + }, + }); + const handle = await factory.start(startOptions(leaseCoordinator())); + await handle.stop(); +}); + test("Codex Remote Workspace runtime owns the model process on the Hub", async () => { const events: Array<{ type: RemoteWorkspaceSessionEvent["type"]; text: string }> = []; const transport: RemoteWorkspaceTransport = { diff --git a/tests/codex-integration/codex-cli-update-plan.test.ts b/tests/codex-integration/codex-cli-update-plan.test.ts new file mode 100644 index 00000000000..f45602fd441 --- /dev/null +++ b/tests/codex-integration/codex-cli-update-plan.test.ts @@ -0,0 +1,733 @@ +import { describe, expect, test } from "bun:test"; + +import { existsSync, mkdtempSync, readFileSync, utimesSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { scanCodexAppServerProcesses } from "../../src/codex/app-server-processes"; +import type { CodexCliInstallReport } from "../../src/codex/cli-install-provenance"; +import { + acquireCodexCliUpdateLease, + observeCodexCliUpdateLease, + releaseCodexCliUpdateLease, + type CodexCliUpdateLeaseIo, +} from "../../src/codex/cli-update-lease"; +import { + applyCodexCliUpdatePlan, + CODEX_CLI_REGISTRY, + codexCliUpdatePlanId, + createCodexCliUpdatePlan, + resolveCodexCliUpdateTarget, + type CodexCliUpdateApplyDeps, + type CodexCliUpdatePlan, + type CodexCliUpdatePlanDeps, + type CodexCliUpdateTarget, +} from "../../src/codex/cli-update-plan"; + +/** + * Phase 2 of the Codex CLI update manager. + * + * Every case here is about one of three refusals to be casual: adopting an installation + * we do not own, installing a target the registry did not pin, and reading an unreadable + * process table as "nothing is running". + */ + +const MANAGED: CodexCliInstallReport = Object.freeze({ + schemaVersion: 1, + candidateAvailable: true, + candidateVersion: "1.0.0", + candidateSource: "environment", + selectionAttested: true, + versionEvidence: { kind: "package-manifest" }, + provenance: "npm-global", + managed: true, + reason: "npm_global_unverified", + location: "/@openai/codex", + packageVersion: "1.0.0", + shim: { status: "not-tracked", backingKind: null }, + evidence: ["package_manifest", "global_npm_layout"], +}); + +function report(overrides: Partial = {}): CodexCliInstallReport { + return { ...MANAGED, ...overrides } as CodexCliInstallReport; +} + +const RESOLVED: CodexCliUpdateTarget = Object.freeze({ + kind: "resolved", + version: "1.1.0", + integrity: "sha512-AAAA", +}); + +function planDeps(overrides: Partial = {}): CodexCliUpdatePlanDeps { + return { + platform: "linux", + inspect: async () => report(), + resolveTarget: () => RESOLVED, + scanProcesses: () => ({ kind: "observed", processes: [] }), + ...overrides, + }; +} + +async function applicablePlan(overrides: Partial = {}): Promise { + const plan = await createCodexCliUpdatePlan(planDeps(overrides)); + expect(plan.applicable).toBe(true); + return plan; +} + +describe("Codex CLI update dry-run plan", () => { + test("an applicable plan pins the exact resolved version and quotes the command it would run", async () => { + const plan = await applicablePlan(); + expect(plan.refusal).toBeNull(); + expect(plan.targetVersion).toBe("1.1.0"); + expect(plan.targetIntegrity).toBe("sha512-AAAA"); + expect(plan.installedVersion).toBe("1.0.0"); + expect(plan.session).toEqual({ state: "none", matches: 0 }); + // The dist-tag is resolved once and bound; the install can never widen back to it. + expect(plan.command).toEqual(["npm", "install", "-g", "@openai/codex@1.1.0"]); + expect(plan.planId).toMatch(/^[0-9a-f]{32}$/); + }); + + test("Windows defers without querying the registry or the process table", async () => { + let queries = 0; + let scans = 0; + const plan = await createCodexCliUpdatePlan(planDeps({ + platform: "win32", + inspect: async () => report({ reason: "windows_inspection_deferred", managed: false, provenance: "unknown" }), + resolveTarget: () => { queries += 1; return RESOLVED; }, + scanProcesses: () => { scans += 1; return { kind: "observed", processes: [] }; }, + })); + expect(plan.applicable).toBe(false); + expect(plan.refusal).toBe("windows_inspection_deferred"); + // Phase 1 reads nothing on Windows, so there is no ownership evidence to spend a + // registry request or a process enumeration on. + expect(queries).toBe(0); + expect(scans).toBe(0); + expect(plan.session.state).toBe("not-evaluated"); + }); + + test("an installation we do not own is never adopted", async () => { + for (const owned of [ + report({ managed: false, provenance: "app-bundle", reason: "app_bundle" }), + report({ managed: false, provenance: "version-manager", reason: "version_manager_owned" }), + report({ managed: false, provenance: "standalone-unverified", reason: "unverified_standalone" }), + report({ managed: true, provenance: "version-manager", reason: "version_manager_owned" }), + ]) { + const plan = await createCodexCliUpdatePlan(planDeps({ inspect: async () => owned })); + expect(plan.refusal).toBe("not_managed"); + expect(plan.planId).toBeNull(); + expect(plan.command).toBeNull(); + } + }); + + test("an advisory runtime version is not evidence of what is installed", async () => { + // A candidate binary reporting its own version cannot be compared with a registry + // version or read back after an install; only the package manifest can. + const plan = await createCodexCliUpdatePlan(planDeps({ + inspect: async () => report({ versionEvidence: { kind: "advisory-runtime" } }), + })); + expect(plan.refusal).toBe("installed_version_unverified"); + }); + + test("a version the registry did not pin with integrity is refused, not installed best-effort", async () => { + const plan = await createCodexCliUpdatePlan(planDeps({ + resolveTarget: () => ({ kind: "unresolved", reason: "registry integrity query failed (status timeout)" }), + })); + expect(plan.refusal).toBe("target_unresolved"); + expect(plan.targetVersion).toBeNull(); + expect(plan.command).toBeNull(); + }); + + test("an already current installation has nothing to apply", async () => { + const plan = await createCodexCliUpdatePlan(planDeps({ + resolveTarget: () => ({ kind: "resolved", version: "1.0.0", integrity: "sha512-AAAA" }), + })); + expect(plan.refusal).toBe("already_current"); + }); + + test("a resolved target lower than the install is refused, not applied as a downgrade", async () => { + const plan = await createCodexCliUpdatePlan(planDeps({ + resolveTarget: () => ({ kind: "resolved", version: "0.9.0", integrity: "sha512-AAAA" }), + })); + expect(plan.refusal).toBe("target_not_newer"); + expect(plan.planId).toBeNull(); + }); + + test("an unreadable process table defers instead of reading as no live session", async () => { + const plan = await createCodexCliUpdatePlan(planDeps({ scanProcesses: () => ({ kind: "unavailable" }) })); + expect(plan.refusal).toBe("blocked_process_state_unknown"); + expect(plan.session).toEqual({ state: "unknown", matches: null }); + }); + + test("a live Codex session refuses the plan and is never signalled", async () => { + const plan = await createCodexCliUpdatePlan(planDeps({ + scanProcesses: () => ({ kind: "observed", processes: [{ pid: 4242, commandLine: "node app-server --secret" }] }), + })); + expect(plan.refusal).toBe("blocked_active_session"); + expect(plan.session).toEqual({ state: "active", matches: 1 }); + // Only the count crosses the boundary. Command lines carry paths and arguments. + expect(JSON.stringify(plan)).not.toContain("secret"); + expect(JSON.stringify(plan)).not.toContain("4242"); + }); +}); + +describe("Codex CLI update plan identity", () => { + test("every bound field changes the id", async () => { + const base = await applicablePlan(); + const variants: Partial[] = [ + { resolveTarget: () => ({ kind: "resolved", version: "1.2.0", integrity: "sha512-AAAA" }) }, + { resolveTarget: () => ({ kind: "resolved", version: "1.1.0", integrity: "sha512-BBBB" }) }, + { inspect: async () => report({ packageVersion: "1.0.1", candidateVersion: "1.0.1" }) }, + { inspect: async () => report({ location: "/other/@openai/codex" }) }, + ]; + for (const variant of variants) { + const plan = await applicablePlan(variant); + expect(plan.planId).not.toBe(base.planId); + } + }); + + test("a session starting or ending between dry-run and apply does not invalidate the plan", async () => { + // Blockers are re-read at apply time where they can only refuse. Binding them into + // the id would expire a plan the operator read correctly, for a reason that cannot + // make the install wrong. + const first = await applicablePlan(); + const second = await applicablePlan({ + scanProcesses: () => ({ kind: "observed", processes: [] }), + }); + expect(second.planId).toBe(first.planId); + }); + + test("the id is a digest of the bound evidence, not a random handle", () => { + const bound = { + platform: "linux" as NodeJS.Platform, + provenance: "npm-global" as const, + installedVersion: "1.0.0", + location: "/@openai/codex", + channel: "latest" as const, + targetVersion: "1.1.0", + targetIntegrity: "sha512-AAAA", + }; + expect(codexCliUpdatePlanId(bound)).toBe(codexCliUpdatePlanId(bound)); + }); +}); + +function applyDeps( + overrides: Partial = {}, + installs: string[] = [], +): CodexCliUpdateApplyDeps { + return { + ...planDeps(), + runInstaller: version => { installs.push(version); return { exitCode: 0 }; }, + ...overrides, + }; +} + +describe("Codex CLI update apply", () => { + test("a stale or unknown plan id installs nothing", async () => { + const installs: string[] = []; + const plan = await applicablePlan(); + + const unknown = await applyCodexCliUpdatePlan("not-a-plan-id", applyDeps({}, installs)); + expect(unknown.status).toBe("refused"); + expect(unknown.refusal).toBe("plan_unknown"); + + const stale = await applyCodexCliUpdatePlan("0".repeat(32), applyDeps({}, installs)); + expect(stale.status).toBe("refused"); + expect(stale.refusal).toBe("plan_stale"); + expect(stale.planId).toBe(plan.planId); + + expect(installs).toEqual([]); + }); + + test("drift between dry-run and apply refuses rather than regenerating the plan", async () => { + const installs: string[] = []; + const plan = await applicablePlan(); + // The operator read a plan for 1.1.0; the registry has since moved on. + const drifted = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + resolveTarget: () => ({ kind: "resolved", version: "1.3.0", integrity: "sha512-CCCC" }), + }, installs)); + expect(drifted.status).toBe("refused"); + expect(drifted.refusal).toBe("plan_stale"); + expect(installs).toEqual([]); + }); + + test("a session that appeared after the dry-run refuses the apply", async () => { + const installs: string[] = []; + const plan = await applicablePlan(); + const blocked = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + scanProcesses: () => ({ kind: "observed", processes: [{ pid: 7, commandLine: "codex app-server" }] }), + }, installs)); + expect(blocked.status).toBe("refused"); + expect(blocked.refusal).toBe("blocked_active_session"); + expect(installs).toEqual([]); + }); + + test("an artifact digest mismatch fails closed before install", async () => { + const installs: string[] = []; + const plan = await applicablePlan(); + let seenIntegrity: string | null | undefined; + const result = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + runInstaller: (version, expectedIntegrity) => { + installs.push(version); + seenIntegrity = expectedIntegrity; + return { exitCode: null, integrityMismatch: true }; + }, + }, installs)); + // The plan-bound sha512 reaches the installer seam, and a mismatch refuses + // rather than reporting an ambiguous post-install state. + expect(seenIntegrity).toBe(plan.targetIntegrity); + expect(result.status).toBe("refused"); + expect(result.refusal).toBe("integrity_mismatch"); + expect(result.installerExitCode).toBeNull(); + }); + + test("the readback classifies the result, and installs exactly the pinned version", async () => { + const installs: string[] = []; + const plan = await applicablePlan(); + let inspections = 0; + const result = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + inspect: async () => { + inspections += 1; + return inspections === 1 ? report() : report({ packageVersion: "1.1.0", candidateVersion: "1.1.0" }); + }, + }, installs)); + expect(installs).toEqual(["1.1.0"]); + expect(result.status).toBe("applied"); + expect(result.installedVersionBefore).toBe("1.0.0"); + expect(result.installedVersionAfter).toBe("1.1.0"); + }); + + test("a nonzero installer exit never overrides a readback that shows the target", async () => { + const plan = await applicablePlan(); + let inspections = 0; + const result = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + runInstaller: () => ({ exitCode: 1 }), + inspect: async () => { + inspections += 1; + return inspections === 1 ? report() : report({ packageVersion: "1.1.0" }); + }, + })); + expect(result.status).toBe("applied"); + expect(result.installerExitCode).toBe(1); + }); + + test("an unchanged version is not applied, and is not retried", async () => { + const installs: string[] = []; + const plan = await applicablePlan(); + const result = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + runInstaller: version => { installs.push(version); return { exitCode: 1 }; }, + }, installs)); + expect(result.status).toBe("not_applied"); + expect(result.installedVersionAfter).toBe("1.0.0"); + expect(installs).toEqual(["1.1.0"]); + }); + + test("a failed readback, a third version or changed provenance is ambiguous", async () => { + const plan = await applicablePlan(); + const cases: [Partial, string | null][] = [ + [{ inspect: async () => { throw new Error("readback failed"); } }, null], + [{ inspect: async () => report({ packageVersion: "9.9.9" }) }, "9.9.9"], + [{ inspect: async () => report({ provenance: "version-manager" }) }, null], + [{ inspect: async () => report({ versionEvidence: { kind: "advisory-runtime" } }) }, null], + // A different npm-global candidate at the target version is not the planned install. + [{ inspect: async () => report({ location: "/other/@openai/codex", packageVersion: "1.1.0" }) }, null], + ]; + for (const [override, after] of cases) { + // The first inspection builds the plan, so these must still produce a plan id; + // drive them through a plan whose deps differ only in the readback. + const first = { ...applyDeps(), ...override } as CodexCliUpdateApplyDeps; + let calls = 0; + const result = await applyCodexCliUpdatePlan(plan.planId!, { + ...first, + inspect: async deps => { + calls += 1; + if (calls === 1) return report(); + return await (override.inspect ?? (async () => report()))(deps); + }, + }); + expect(result.status).toBe("ambiguous"); + expect(result.installedVersionAfter).toBe(after); + } + }); +}); + +describe("strict Codex app-server process scan", () => { + test("an enumeration failure is unavailable, not an empty list", () => { + const scan = scanCodexAppServerProcesses({ + platform: "linux", + listSnapshots: () => { throw new Error("procfs unreadable"); }, + }); + expect(scan).toEqual({ kind: "unavailable" }); + }); + + test("a readable but empty process table is observed with no matches", () => { + const scan = scanCodexAppServerProcesses({ platform: "linux", listSnapshots: () => [] }); + expect(scan).toEqual({ kind: "observed", processes: [] }); + }); + + test("the same matcher and de-duplication as the kill-path lister", () => { + const snapshot = { pid: 11, commandLine: "codex app-server" }; + const scan = scanCodexAppServerProcesses({ + platform: "linux", + listSnapshots: () => [snapshot, { ...snapshot }, { pid: 12, commandLine: "vim notes.txt" }], + }); + expect(scan.kind).toBe("observed"); + if (scan.kind !== "observed") return; + expect(scan.processes.map(p => p.pid)).toEqual([11]); + }); +}); + +describe("registry target resolution", () => { + function spawnStub(outputs: { status: number | null; stdout: string }[]) { + let call = 0; + return (() => { + const next = outputs[call++] ?? { status: 1, stdout: "" }; + return { status: next.status, stdout: next.stdout, stderr: "" }; + }) as never; + } + + test("an exact version with a sha512 token resolves", () => { + const target = resolveCodexCliUpdateTarget("latest", spawnStub([ + { status: 0, stdout: "1.4.2\n" }, + { status: 0, stdout: "'sha512-abc/DEF+123=' sha1-old\n" }, + { status: 0, stdout: "https://registry.npmjs.org/@openai/codex/-/codex-1.4.2.tgz\n" }, + ])); + expect(target).toEqual({ kind: "resolved", version: "1.4.2", integrity: "sha512-abc/DEF+123=" }); + }); + + test("a missing integrity token refuses instead of proceeding best-effort", () => { + const target = resolveCodexCliUpdateTarget("latest", spawnStub([ + { status: 0, stdout: "1.4.2\n" }, + { status: 0, stdout: "sha1-onlythis\n" }, + ])); + expect(target.kind).toBe("unresolved"); + }); + + test("a registry timeout refuses", () => { + const target = resolveCodexCliUpdateTarget("latest", spawnStub([{ status: null, stdout: "" }])); + expect(target.kind).toBe("unresolved"); + }); + + test("a non-version answer is not treated as a version", () => { + const target = resolveCodexCliUpdateTarget("latest", spawnStub([ + { status: 0, stdout: "latest\n" }, + ])); + expect(target.kind).toBe("unresolved"); + }); + + test("a tarball outside the pinned registry origin is refused even with a valid digest", () => { + const target = resolveCodexCliUpdateTarget("latest", spawnStub([ + { status: 0, stdout: "1.4.2\n" }, + { status: 0, stdout: "sha512-abc/DEF+123=\n" }, + { status: 0, stdout: "https://evil.invalid/@openai/codex/-/codex-1.4.2.tgz\n" }, + ])); + expect(target.kind).toBe("unresolved"); + }); + + test("a tarball answer that is not a URL at all is refused", () => { + const target = resolveCodexCliUpdateTarget("latest", spawnStub([ + { status: 0, stdout: "1.4.2\n" }, + { status: 0, stdout: "sha512-abc/DEF+123=\n" }, + { status: 0, stdout: "not-a-url\n" }, + ])); + expect(target.kind).toBe("unresolved"); + }); +}); + +describe("registry configuration isolation", () => { + interface CapturedCall { + bin: string; + args: string[]; + options: Record; + /** Snapshot taken while the call was live; the isolation dir is gone by return. */ + cwdHadSentinel: boolean; + cwdHadNpmrc: boolean; + } + + /** The npm argv, whether it reached spawn bare (POSIX) or inside a cmd /c line (Windows). */ + function argvLine(call: CapturedCall): string { + return call.args.join(" "); + } + + /** Which registry field this invocation queried, read off the argv line. */ + function queriedField(call: CapturedCall): string { + const line = argvLine(call); + if (line.includes("dist.tarball")) return "dist.tarball"; + if (line.includes("dist.integrity")) return "dist.integrity"; + return "version"; + } + + function capturingSpawn(outputs: Record): { calls: CapturedCall[]; spawn: never } { + const calls: CapturedCall[] = []; + const spawn = ((bin: string, args: string[], options: Record) => { + const cwd = options.cwd as string; + const call: CapturedCall = { + bin, + args, + options, + cwdHadSentinel: existsSync(join(cwd, "package.json")), + cwdHadNpmrc: existsSync(join(cwd, "ocx-update.npmrc")), + }; + calls.push(call); + const field = queriedField(call); + return { status: 0, stdout: outputs[field] ?? "", stderr: "" }; + }) as never; + return { calls, spawn }; + } + + const RESOLVE_OUTPUTS = { + version: "1.4.2\n", + "dist.integrity": "sha512-abc/DEF+123=\n", + "dist.tarball": "https://registry.npmjs.org/@openai/codex/-/codex-1.4.2.tgz\n", + }; + + test("every query pins the official registry and substitutes a controlled npmrc", () => { + const { calls, spawn } = capturingSpawn(RESOLVE_OUTPUTS); + const target = resolveCodexCliUpdateTarget("latest", spawn); + expect(target.kind).toBe("resolved"); + expect(calls.length).toBe(3); + for (const call of calls) { + expect(argvLine(call)).toContain("--registry=" + CODEX_CLI_REGISTRY); + // Both file configs are substituted with the controlled empty npmrc inside the + // isolation dir — a user ~/.npmrc or a global $PREFIX/etc/npmrc cannot answer. + expect(argvLine(call)).toContain("--userconfig="); + expect(argvLine(call)).toContain("--globalconfig="); + expect(argvLine(call)).toContain("ocx-update.npmrc"); + expect(call.cwdHadSentinel).toBe(true); + expect(call.cwdHadNpmrc).toBe(true); + } + // The isolation directory is cleaned up after the resolve. + expect(existsSync(calls[0]!.options.cwd as string)).toBe(false); + }); + + test("a hostile npm_config_* env cannot reach the spawned npm", () => { + const prior = process.env.npm_config_registry; + const priorScoped = process.env["npm_config_@openai:registry"]; + process.env.npm_config_registry = "https://evil.invalid"; + process.env["npm_config_@openai:registry"] = "https://evil.invalid/"; + try { + const { calls, spawn } = capturingSpawn(RESOLVE_OUTPUTS); + const target = resolveCodexCliUpdateTarget("latest", spawn); + expect(target.kind).toBe("resolved"); + for (const call of calls) { + const env = call.options.env as NodeJS.ProcessEnv; + expect(Object.keys(env).filter(key => key.toLowerCase().startsWith("npm_config_"))).toEqual([]); + } + } finally { + if (prior === undefined) delete process.env.npm_config_registry; + else process.env.npm_config_registry = prior; + if (priorScoped === undefined) delete process.env["npm_config_@openai:registry"]; + else process.env["npm_config_@openai:registry"] = priorScoped; + } + }); + + test("the controlled cwd never is the operator's project directory", () => { + // A project .npmrc in the launch directory would otherwise redirect the scope; + // the query must run from the sentinel-anchored isolation dir instead. + const { calls, spawn } = capturingSpawn(RESOLVE_OUTPUTS); + resolveCodexCliUpdateTarget("latest", spawn); + for (const call of calls) { + const cwd = call.options.cwd as string; + expect(cwd).not.toBe(process.cwd()); + expect(cwd.startsWith(tmpdir())).toBe(true); + } + }); +}); + +describe("Codex CLI update lease", () => { + function leaseIo(dir: string, pid: number, alive: ReadonlySet): CodexCliUpdateLeaseIo { + return { + lockPath: join(dir, "codex-cli-update.lock"), + pid, + isAlive: candidate => alive.has(candidate), + }; + } + + function heldLease(dir: string, holderPid: number): CodexCliUpdateLeaseIo { + const io = leaseIo(dir, holderPid, new Set([holderPid])); + const acquisition = acquireCodexCliUpdateLease({ ...io, planId: "a".repeat(32) }); + expect(acquisition.acquired).toBe(true); + return io; + } + + test("a live holder refuses a second apply before any evidence is gathered", async () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const holder = heldLease(dir, 4_242); + const installs: string[] = []; + let inspected = 0; + const result = await applyCodexCliUpdatePlan("b".repeat(32), applyDeps({ + leaseIo: { lockPath: holder.lockPath, isAlive: pid => pid === 4_242 }, + inspect: async () => { inspected += 1; return report(); }, + }, installs)); + expect(result.status).toBe("refused"); + expect(result.refusal).toBe("update_in_progress"); + // The refusal happens before the plan is recomputed: no inspection, no install. + expect(inspected).toBe(0); + expect(installs).toEqual([]); + }); + + test("two concurrent applies cannot pass the same plan", async () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + const installs: string[] = []; + const plan = await applicablePlan(); + + // The first apply holds the lease while its inspection is still in flight. + let releaseInspection: (value: CodexCliInstallReport) => void = () => {}; + const gate = new Promise(done => { releaseInspection = done; }); + let firstCalls = 0; + const first = applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + leaseIo: { lockPath }, + inspect: async () => { + firstCalls += 1; + return firstCalls === 1 ? await gate : report({ packageVersion: "1.1.0" }); + }, + }, installs)); + // Let the first apply reach its gated inspection while holding the lease. + await new Promise(done => setTimeout(done, 10)); + + const second = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + leaseIo: { lockPath }, + }, installs)); + expect(second.status).toBe("refused"); + expect(second.refusal).toBe("update_in_progress"); + + releaseInspection(report()); + const firstResult = await first; + expect(firstResult.status).toBe("applied"); + expect(installs).toEqual(["1.1.0"]); + // The holder released the lease when it finished. + expect(existsSync(lockPath)).toBe(false); + }); + + test("a dead holder's lease is reclaimed instead of blocking forever", async () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + // A holder that died without releasing: dead pid, stale or not. + const stale = acquireCodexCliUpdateLease({ lockPath, pid: 9_999, planId: "c".repeat(32) }); + expect(stale.acquired).toBe(true); + + const installs: string[] = []; + const plan = await applicablePlan(); + let inspections = 0; + const result = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + leaseIo: { lockPath, isAlive: () => false }, + inspect: async () => { + inspections += 1; + return inspections === 1 ? report() : report({ packageVersion: "1.1.0" }); + }, + }, installs)); + expect(result.status).toBe("applied"); + expect(installs).toEqual(["1.1.0"]); + expect(existsSync(lockPath)).toBe(false); + }); + + test("a refused apply still releases the lease it took", async () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + const plan = await applicablePlan(); + const result = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + leaseIo: { lockPath }, + // Drift the target so the recomputed plan is refused as stale. + resolveTarget: () => Object.freeze({ kind: "resolved" as const, version: "9.9.9", integrity: "sha512-ZZZZ" }), + })); + expect(result.status).toBe("refused"); + expect(result.refusal).toBe("plan_stale"); + expect(existsSync(lockPath)).toBe(false); + }); + + test("a corrupt leftover lease cannot wedge every future update", async () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + writeFileSync(lockPath, "{not json"); + // Backdate it past the publish grace: a fresh unparseable file is a contender's + // in-flight write, not debris, and must not be cleared under it. + utimesSync(lockPath, 0, 0); + const installs: string[] = []; + const plan = await applicablePlan(); + let inspections = 0; + const result = await applyCodexCliUpdatePlan(plan.planId!, applyDeps({ + leaseIo: { lockPath }, + inspect: async () => { + inspections += 1; + return inspections === 1 ? report() : report({ packageVersion: "1.1.0" }); + }, + }, installs)); + expect(result.status).toBe("applied"); + expect(existsSync(lockPath)).toBe(false); + }); + + test("a fresh unparseable record is a publish in flight, not debris", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + // The O_EXCL fallback leaves a create-before-record window: a contender that + // sees it must report contention/unavailable, never unlink the publisher's file. + writeFileSync(lockPath, ""); + const attempt = acquireCodexCliUpdateLease({ lockPath, pid: 7_777 }); + expect(attempt.acquired).toBe(false); + expect(existsSync(lockPath)).toBe(true); + // Once the file is old it is genuinely dead debris and reclaims. + utimesSync(lockPath, 0, 0); + const retry = acquireCodexCliUpdateLease({ lockPath, pid: 7_777 }); + expect(retry.acquired).toBe(true); + }); + + test("a contender that observed a stale record cannot delete the successor's lease", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + // A holds a dead-pid lease; B observes it stale and takes over. The record A + // published must never let a slower contender (or A's own late release) delete B's. + const a = acquireCodexCliUpdateLease({ lockPath, pid: 1_111, planId: "a".repeat(32) }); + expect(a.acquired).toBe(true); + if (!a.acquired) return; + const b = acquireCodexCliUpdateLease({ lockPath, pid: 2_222, isAlive: () => false }); + expect(b.acquired).toBe(true); + if (!b.acquired) return; + // The late release names the superseded token: B's live lease survives. + releaseCodexCliUpdateLease({ lockPath, pid: 1_111, token: a.record.token }); + const surviving = readFileSync(lockPath, "utf-8"); + expect(JSON.parse(surviving).token).toBe(b.record.token); + // A release naming only the pid still cannot touch a successor's record either: + // the token the caller never had is required inside compare-and-delete. + releaseCodexCliUpdateLease({ lockPath, pid: 1_111 }); + expect(JSON.parse(readFileSync(lockPath, "utf-8")).token).toBe(b.record.token); + }); + + test("a live owner is never reaped by age while its heartbeat is fresh", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + const held = acquireCodexCliUpdateLease({ lockPath, pid: 3_333 }); + expect(held.acquired).toBe(true); + if (!held.acquired) return; + // Age the record far past the bound but keep the heartbeat fresh: a long install + // must stay held, not be deliberately broken. + writeFileSync(lockPath, JSON.stringify({ + ...held.record, + createdAtMs: 0, + heartbeatAtMs: Date.now(), + })); + const contender = acquireCodexCliUpdateLease({ lockPath, pid: 4_444, isAlive: () => true }); + expect(contender.acquired).toBe(false); + if (!contender.acquired) { + expect(contender.reason).toBe("held"); + } + // The same live pid with a heartbeat that stopped is a wedged holder: reclaimable. + writeFileSync(lockPath, JSON.stringify({ + ...held.record, + createdAtMs: 0, + heartbeatAtMs: 0, + })); + const reaper = acquireCodexCliUpdateLease({ lockPath, pid: 4_444, isAlive: () => true }); + expect(reaper.acquired).toBe(true); + }); + + test("startup observation reads a held lease and ignores a stale one", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-update-lease-")); + const lockPath = join(dir, "codex-cli-update.lock"); + const held = acquireCodexCliUpdateLease({ lockPath, pid: 5_555 }); + expect(held.acquired).toBe(true); + // Observation is read-only and liveness-bound: the same file is "held" while the + // owner lives and "free" once it is gone, without the observer mutating anything. + expect(observeCodexCliUpdateLease({ lockPath, isAlive: () => true }).held).toBe(true); + expect(observeCodexCliUpdateLease({ lockPath, isAlive: () => false }).held).toBe(false); + }); +}); diff --git a/tests/codex-integration/codex-cli-update-zero-effect.test.ts b/tests/codex-integration/codex-cli-update-zero-effect.test.ts index 11ca23c8add..49ba8c44581 100644 --- a/tests/codex-integration/codex-cli-update-zero-effect.test.ts +++ b/tests/codex-integration/codex-cli-update-zero-effect.test.ts @@ -195,7 +195,7 @@ describe("Codex CLI updater zero-effect boundary", () => { }); expect(result.error).toBeUndefined(); expect(result.status).toBe(2); - expect(result.stderr).toContain("codex-cli-update action must be check"); + expect(result.stderr).toContain("codex-cli-update action must be check, attest, plan or apply"); expect(readFileSync(statePath)).toEqual(before); expect(existsSync(marker)).toBe(false); }); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 38e3a733c72..5486f01ce74 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -323,6 +323,7 @@ "codex-cli-installation-targets.test.ts": "codex-integration", "codex-cli-windows-installation-files.test.ts": "codex-integration", "codex-cli-update-launcher-policy.test.ts": "codex-integration", + "codex-cli-update-plan.test.ts": "codex-integration", "codex-cli-update-zero-effect.test.ts": "codex-integration", "codex-composed-acceptance.test.ts": "codex-integration", "codex-config-generation.test.ts": "codex-integration",