diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 8eafd7e26c..af840b950f 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -12,6 +12,7 @@ import { homedir } from "node:os"; import { dirname, join } from "node:path"; import { getConfigDir, getConfigPath, readConfigDiagnostics } from "../config"; import { readPid } from "../config/process-state"; +import { probeUncleanExitState } from "./status"; import { findLiveProxy, type LiveProxy } from "../server/proxy-liveness"; import { BUN_RUNTIME_SOURCES } from "../lib/bun-runtime"; import type { BunRuntimeSource } from "../lib/bun-runtime"; @@ -972,6 +973,11 @@ export function proxyDownRestartHint(input: { /** Absent means "unknown"; the hint then keeps its pre-repair wording. */ serviceInstalled?: boolean; serviceConflict?: boolean; + /** + * Persisted owner records outlived their process (#1419). Cause-neutral: what is on + * disk proves an unclean exit, not which signal caused it. + */ + staleProcessState?: boolean; }): string | null { if (input.proxyRunning) return null; // `serviceViable` alone conflates "no service at all" with "registered but stale or @@ -984,7 +990,10 @@ export function proxyDownRestartHint(input: { : installedButBroken ? "Restart it with 'ocx start', or refresh the installed service: 'ocx service repair'." : "Restart it with 'ocx start', or install the persistent service: 'ocx service install'."; - return `The ocx proxy is not running. Codex/Claude clients pinned to 127.0.0.1:${input.port} fail with errors like "error sending request for url (http://127.0.0.1:${input.port}/v1/responses)". ${restart}`; + const uncleanExit = input.staleProcessState === true + ? "Stale process records remain, so the previous run may have exited unexpectedly. " + : ""; + return `The ocx proxy is not running. ${uncleanExit}Codex/Claude clients pinned to 127.0.0.1:${input.port} fail with errors like "error sending request for url (http://127.0.0.1:${input.port}/v1/responses)". ${restart}`; } export async function runDoctor(args: string[] = []): Promise { @@ -1317,6 +1326,14 @@ export async function runDoctor(args: string[] = []): Promise { serviceViable: startup.serviceViable, serviceInstalled: startup.serviceInstalled, serviceConflict: startup.serviceConflict, + // Threaded through the same decision helper `ocx status` uses, so the two + // diagnostics cannot drift. A helper-only change would satisfy a unit test while + // real `ocx doctor` output never mentioned the crash (#1419). + staleProcessState: await probeUncleanExitState({ + live: Boolean(live), + port: doctorConfig.port, + hostname: doctorConfig.hostname, + }), }); if (proxyDown) hints.push(proxyDown); for (const row of providerApiKeys) { diff --git a/src/cli/index.ts b/src/cli/index.ts index 63ba2462dd..284a8b15d8 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -877,6 +877,17 @@ async function handleStatus() { // contradicted it in the same report, and install re-registers: UAC on Windows and a // possible WinSW-to-scheduler switch for someone who already has a service. const installed = status.json.startup.serviceInstalled && !status.json.startup.serviceConflict; + // #1419: the records outliving the process is the only evidence the user gets that a + // previous run ended without cleanup. Deliberately hedged and cause-neutral — cleanup + // ignores unlink failures and the records carry no session provenance, so this cannot + // prove a crash, only that the last run left state behind. The restart advice below is + // not repeated here; one recommendation per report. + if (status.json.proxy.staleProcessState) { + console.log(" Stale process records remain, so the previous run may have exited unexpectedly."); + if (!installed) { + console.log(" No background service was available to restart it."); + } + } console.log(installed ? " Restart with 'ocx start', or refresh the installed service: 'ocx service repair'." : " Restart with 'ocx start', or install the persistent service: 'ocx service install'."); diff --git a/src/cli/status.ts b/src/cli/status.ts index d47c55d56c..82d588eb92 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -1,12 +1,13 @@ import { durableBunRuntime } from "../lib/bun-runtime"; import { codexAutoStartEnabled, getConfigPath, readConfigDiagnostics } from "../config"; -import { getPidPath, readPid, readRuntimePort, type RuntimePortState } from "../config/process-state"; +import { getPidPath, readPid, readPidFileValue, readRuntimePort, type RuntimePortState } from "../config/process-state"; import { diagnoseCodexBundledPlugins, type CodexPluginsDiagnostic } from "../codex/plugins-doctor"; import { findLiveProxy, isOpencodexHealthz, probeHostname } from "../server/proxy-liveness"; import { directLocalHttpFetch } from "../server/direct-local-http"; import type { OcxConfig } from "../types"; import { diagnoseService, serviceLogPath } from "../service"; import { collectStartupHealth, type StartupHealth } from "../codex/autostart-health"; +import { isProcessAlive } from "../lib/process-control"; import { getCodexRoutingKind } from "../codex/inject"; import { diagnoseCodexShim } from "../codex/shim"; import { displayCodexRuntimePath, effortClampAppliesToRuntime, loadLastEffortClamp, resolveCodexRuntime } from "../codex/runtime"; @@ -21,6 +22,8 @@ type HealthCheck = { url: string; message: string; label: string; + /** True only for a connect-phase refusal: proof that nothing holds the port. */ + refused?: boolean; }; export type CliStatusJson = { @@ -28,6 +31,8 @@ export type CliStatusJson = { proxy: { running: boolean; pid: number | null; + /** Persisted owner records outlived their process: the last proxy did not exit cleanly. */ + staleProcessState: boolean; health: { ok: boolean; url: string; @@ -127,6 +132,69 @@ export function proxyHealthFailureReason(error: unknown, signal: AbortSignal): " : "unreachable"; } +/** + * "Nothing is listening" is narrower than "the probe failed". `unreachable` covers every + * non-abort failure, including a socket that was ACCEPTED and then reset — which is what + * an in-flight start looks like mid-bind. Only a connect-phase refusal proves the port is + * free, so this reads the underlying errno instead of the display string. + */ +export function isConnectionRefused(error: unknown): boolean { + for (let current: unknown = error, depth = 0; current instanceof Error && depth < 4; depth++) { + const code = (current as { code?: unknown }).code; + if (code === "ECONNREFUSED" || code === "ConnectionRefused") return true; + // Bun surfaces the refusal as a plain message on some platforms; the errno name is + // still the discriminator, not a substring of arbitrary prose. + if (typeof code === "string" && code.endsWith("ECONNREFUSED")) return true; + current = (current as { cause?: unknown }).cause; + } + return false; +} + +/** + * A proxy killed by a native trap or SIGKILL never runs the exit cleanup that removes + * `ocx.pid` and `runtime-port.json` (only SIGINT/SIGTERM/SIGHUP and normal exit are + * wired to it), so both records outlive it. That makes "crashed" and "never started" + * distinguishable — and #1419 is what it costs when we discard the distinction: the + * reporter's unsupervised `ocx gui` proxy died and every later command said only + * "not running", never that a previous process had exited or that a service would + * have restarted it. + * + * Two races have to stay closed, because a false "it crashed" is worse than a missing + * hint. `handleStart` binds the port BEFORE it publishes either record, so: + * + * - a start that publishes between two reads is caught by comparing the raw records + * observed before and after the probes (the same snapshot discipline + * `removePidIfValueIs` uses for deletion); + * - a start that has bound but not yet published leaves both snapshots identical, so + * records alone cannot see it. That one is excluded on the port instead: the probe + * must have been REFUSED at connect, which is the only outcome proving nothing holds + * the port. A socket that is accepted and then reset — an in-flight bind — is not a + * refusal, so review caught `unreachable` being too broad for this job. + * + * What this can and cannot prove: the records outliving their process establish that the + * previous run did not complete its cleanup. It does not establish a cause, and it cannot + * fully exclude a clean exit whose `unlinkSync` failed, because cleanup ignores that + * error (`src/cli/index.ts:324-325`) and the records carry no session provenance. The + * wording therefore says the records remain and the run MAY have exited unexpectedly. + */ +export function isUncleanExitEvidence(input: { + live: boolean; + healthOk: boolean; + healthRefused: boolean; + ownerPidAlive: boolean; + pidRecordBefore: number | null; + pidRecordAfter: number | null; + runtimePidBefore: number | null; + runtimePidAfter: number | null; +}): boolean { + if (input.live || input.healthOk) return false; + if (!input.healthRefused) return false; + if (input.ownerPidAlive) return false; + if (input.pidRecordBefore !== input.pidRecordAfter) return false; + if (input.runtimePidBefore !== input.runtimePidAfter) return false; + return input.pidRecordAfter !== null || input.runtimePidAfter !== null; +} + /** * `ocx status` greens on process liveness alone, so a proxy that answers * /healthz reads healthy even when Codex is not pointed at it and every routed @@ -171,12 +239,63 @@ async function checkProxyHealth(target: ListenTarget): Promise { return { ok: true, url, message, label: `${url} ${message}` }; } catch (error) { const reason = proxyHealthFailureReason(error, controller.signal); - return { ok: false, url, message: reason, label: `${url} ${reason}` }; + return { ok: false, url, message: reason, label: `${url} ${reason}`, refused: isConnectionRefused(error) }; } finally { clearTimeout(timer); } } +/** + * The ONE evidence gatherer for stale-process state, shared by `ocx status` and + * `ocx doctor`. + * + * It deliberately probes the port named by the STALE RECORD, not the configured display + * port. Review found the two commands disagreeing precisely here: a proxy that hopped to + * a fallback port, or a config whose port changed after the crash, left status probing + * the configured port while doctor probed the recorded one, so one reported a crash and + * the other did not. The question being asked is "is the process that wrote this record + * gone?", and only that record's own port can answer it. + * + * `live` short-circuits before the probe so a healthy install pays nothing. + */ +export async function probeUncleanExitState(input: { + live: boolean; + port?: number; + hostname?: string | null; +}): Promise { + if (input.live) return false; + const pidRecordBefore = readPidFileValue(); + const runtimeBefore = readRuntimePort(); + const runtimePidBefore = runtimeBefore?.pid ?? null; + if (pidRecordBefore === null && runtimePidBefore === null) return false; + const ownerPid = pidRecordBefore ?? runtimePidBefore; + if (ownerPid !== null && isProcessAlive(ownerPid)) return false; + // The recorded port is the evidence target. Fall back to the configured port only when + // no runtime record exists, which is the pid-file-only case. + const port = runtimeBefore?.port ?? input.port ?? 10100; + const hostname = runtimeBefore?.hostname ?? input.hostname ?? undefined; + const health = await checkProxyHealth({ + port, + hostname, + source: runtimeBefore ? "runtime" : "config", + healthUrl: `http://${probeHostname(hostname)}:${port}/healthz`, + dashboardUrl: `http://localhost:${port}/`, + }); + const pidRecordAfter = readPidFileValue(); + const runtimePidAfter = readRuntimePort()?.pid ?? null; + const ownerPidAfter = pidRecordAfter ?? runtimePidAfter; + return isUncleanExitEvidence({ + live: false, + healthOk: health.ok, + healthRefused: health.refused === true, + ownerPidAlive: ownerPidAfter !== null && isProcessAlive(ownerPidAfter), + pidRecordBefore, + pidRecordAfter, + runtimePidBefore, + runtimePidAfter, + }); +} + export async function collectStatus(): Promise { const configDiagnostics = readConfigDiagnostics(); const config = configDiagnostics.config; @@ -210,6 +329,13 @@ export async function collectStatus(): Promise { label: `${listen.healthUrl} ok (live)`, } : await checkProxyHealth(listen); + // Same gatherer `ocx doctor` uses, so the two commands cannot reach different verdicts + // about the same on-disk state (review found them diverging on fallback ports). + const staleProcessState = await probeUncleanExitState({ + live: Boolean(live), + port: config.port, + hostname: config.hostname, + }); const bunRuntime = durableBunRuntime(); const service = diagnoseService(); // A service can be registered and still not serve: the manager reports the job @@ -326,6 +452,7 @@ export async function collectStatus(): Promise { proxy: { running: Boolean(live) || Boolean(pid && health.ok), pid: live?.pid ?? pid, + staleProcessState, health: { ok: health.ok, url: health.url, diff --git a/tests/cli-status-json.test.ts b/tests/cli-status-json.test.ts index f959223342..d6bc9e93ab 100644 --- a/tests/cli-status-json.test.ts +++ b/tests/cli-status-json.test.ts @@ -1,10 +1,12 @@ -import { describe, expect, test } from "bun:test"; +import { beforeAll, describe, expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; +import { createServer } from "node:net"; +import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; -import { proxyHealthFailureReason, resolveStatusPid, selectListenTarget } from "../src/cli/status"; +import { isConnectionRefused, isUncleanExitEvidence, proxyHealthFailureReason, resolveStatusPid, selectListenTarget } from "../src/cli/status"; const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); const cliPath = join(repoRoot, "src", "cli", "index.ts"); @@ -326,3 +328,207 @@ describe("CLI status JSON", () => { expect(target.dashboardUrl).toBe("http://localhost:10100/"); }); }); + +/** + * #1419: an unsupervised proxy died from a native trap and every later command said + * only "not running". The persisted owner records are the one piece of evidence that + * separates a crash from a proxy that was never started, and status used to discard + * it. These cases pin the predicate, including the two false-positive shapes that a + * naive implementation gets wrong. + */ +describe("unclean prior exit evidence", () => { + const base = { + live: false, + healthOk: false, + healthRefused: true, + ownerPidAlive: false, + pidRecordBefore: 4242, + pidRecordAfter: 4242, + runtimePidBefore: 4242, + runtimePidAfter: 4242, + }; + + test("both records outliving a dead owner is an unclean exit", () => { + expect(isUncleanExitEvidence(base)).toBe(true); + }); + + // Blocker 5 from the plan audit: a fixture that always writes BOTH records cannot + // tell an AND from an OR, so each record must be sufficient on its own. + test("a pid record alone is sufficient", () => { + expect(isUncleanExitEvidence({ + ...base, + runtimePidBefore: null, + runtimePidAfter: null, + })).toBe(true); + }); + + test("a runtime-port record alone is sufficient", () => { + expect(isUncleanExitEvidence({ + ...base, + pidRecordBefore: null, + pidRecordAfter: null, + })).toBe(true); + }); + + test("a clean home reports nothing", () => { + expect(isUncleanExitEvidence({ + ...base, + pidRecordBefore: null, + pidRecordAfter: null, + runtimePidBefore: null, + runtimePidAfter: null, + })).toBe(false); + }); + + test("a live proxy or a healthy probe reports nothing", () => { + expect(isUncleanExitEvidence({ ...base, live: true })).toBe(false); + expect(isUncleanExitEvidence({ ...base, healthOk: true })).toBe(false); + }); + + // Re-audit blocker 2: without this case the owner-alive clause is never exercised, + // because every other fixture names a dead pid. + test("a live owner pid is a start in progress, not a crash", () => { + expect(isUncleanExitEvidence({ ...base, ownerPidAlive: true })).toBe(false); + }); + + // Re-audit blocker 1: `handleStart` binds the port before it publishes either + // record, so a start caught in that window leaves both snapshots identical. Only a + // refused connection proves nothing holds the port. + test("a held port is not a crash even when the records look stale", () => { + expect(isUncleanExitEvidence({ ...base, healthRefused: false })).toBe(false); + }); + + test("records published mid-probe suppress the report", () => { + expect(isUncleanExitEvidence({ ...base, pidRecordBefore: null })).toBe(false); + expect(isUncleanExitEvidence({ ...base, runtimePidAfter: 9999 })).toBe(false); + }); + + // Review blocker 2: `unreachable` covers every non-abort failure, including a socket + // that is ACCEPTED and then reset — which is what an in-flight bind looks like. Only a + // connect-phase refusal proves the port is free. + test("only a connect-phase refusal counts as nothing listening", () => { + const refused = Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:9"), { code: "ECONNREFUSED" }); + expect(isConnectionRefused(refused)).toBe(true); + + const nested = new Error("fetch failed", { cause: refused }); + expect(isConnectionRefused(nested)).toBe(true); + + const reset = Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); + expect(isConnectionRefused(reset)).toBe(false); + + // A message that merely mentions refusal without the errno is not evidence. + expect(isConnectionRefused(new Error("connection refused by policy"))).toBe(false); + expect(isConnectionRefused(undefined)).toBe(false); + }); +}); + +/** + * Command-level coverage. Review found the unit tests above were satisfiable by an + * implementation that never reported anything: replacing the returned + * `staleProcessState` with a constant `false` left every predicate test green. These + * drive the real CLI, so the field has to travel from disk to output. + */ +describe("status reports stale process records end to end", () => { + const seed = (home: string, opts: { pid?: number; runtime?: boolean; port: number }): void => { + writeFileSync(join(home, "config.json"), JSON.stringify({ port: opts.port, codexAutoStart: false }), "utf8"); + const pid = opts.pid ?? (process.pid === 4242 ? 4243 : 4242); + if (opts.pid !== 0) writeFileSync(join(home, "ocx.pid"), String(pid), "utf8"); + if (opts.runtime) { + writeFileSync(join(home, "runtime-port.json"), JSON.stringify({ pid, port: opts.port, hostname: "127.0.0.1" }), "utf8"); + } + }; + + /** + * A port that is genuinely free: bind an ephemeral port, read it, release it. The + * discard port 9 is conventionally unused but not guaranteed, and if anything answers + * on it the probe is accepted rather than refused and these fixtures invert. + */ + let freePort = 9; + beforeAll(async () => { + const probe = createServer(); + await new Promise(resolve => { probe.listen(0, "127.0.0.1", () => resolve()); }); + freePort = (probe.address() as AddressInfo).port; + await new Promise(resolve => { probe.close(() => resolve()); }); + }); + + test("a dead owner record surfaces in --json and in human output", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-stale-json-")); + try { + seed(home, { runtime: true, port: freePort }); + + const json = runStatusJson(home); + expect(json.status).toBe(0); + const parsed = JSON.parse(json.stdout) as { proxy?: { staleProcessState?: unknown } }; + expect(parsed.proxy?.staleProcessState).toBe(true); + + const human = spawnSync(process.execPath, [cliPath, "status"], { + cwd: repoRoot, + env: { ...process.env, OPENCODEX_HOME: home }, + encoding: "utf8", + }); + expect(human.stdout).toContain("may have exited unexpectedly"); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + test("a clean home reports false and says nothing about a previous run", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-stale-clean-")); + try { + writeFileSync(join(home, "config.json"), JSON.stringify({ port: freePort, codexAutoStart: false }), "utf8"); + + const json = runStatusJson(home); + const parsed = JSON.parse(json.stdout) as { proxy?: { staleProcessState?: unknown } }; + expect(parsed.proxy?.staleProcessState).toBe(false); + + const human = spawnSync(process.execPath, [cliPath, "status"], { + cwd: repoRoot, + env: { ...process.env, OPENCODEX_HOME: home }, + encoding: "utf8", + }); + expect(human.stdout).not.toContain("may have exited unexpectedly"); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + // Review blocker 3: a recycled pid must suppress rather than assert. This process is + // certainly alive, so recording it stands in for a reused pid. + test("a record naming a live pid is never reported as a stale exit", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-stale-livepid-")); + try { + seed(home, { pid: process.pid, runtime: true, port: freePort }); + + const parsed = JSON.parse(runStatusJson(home).stdout) as { proxy?: { staleProcessState?: unknown } }; + expect(parsed.proxy?.staleProcessState).toBe(false); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + // Review blocker 4: status and doctor disagreed when the recorded port differed from + // the configured one. Both now probe the recorded port, so both must agree. + // + // The configured port must be OCCUPIED for this to discriminate: if both ports are + // simply free, probing either one yields the same refusal and the test cannot tell the + // two implementations apart. A listener that accepts and resets is what an in-flight + // bind looks like, so a run that probed the configured port would suppress the report. + test("a fallback-port record is judged on the recorded port, not the configured one", async () => { + const home = mkdtempSync(join(tmpdir(), "ocx-stale-fallback-")); + const occupied = createServer(socket => { socket.destroy(); }); + await new Promise(resolve => { occupied.listen(0, "127.0.0.1", () => resolve()); }); + const occupiedPort = (occupied.address() as AddressInfo).port; + try { + const pid = process.pid === 4242 ? 4243 : 4242; + writeFileSync(join(home, "config.json"), JSON.stringify({ port: occupiedPort, codexAutoStart: false }), "utf8"); + writeFileSync(join(home, "ocx.pid"), String(pid), "utf8"); + writeFileSync(join(home, "runtime-port.json"), JSON.stringify({ pid, port: freePort, hostname: "127.0.0.1" }), "utf8"); + + const parsed = JSON.parse(runStatusJson(home).stdout) as { proxy?: { staleProcessState?: unknown } }; + expect(parsed.proxy?.staleProcessState).toBe(true); + } finally { + await new Promise(resolve => { occupied.close(() => resolve()); }); + rmSync(home, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index 75b642bba2..903220096c 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, rmSync, utimesSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { homedir, tmpdir } from "node:os"; import { @@ -656,6 +657,43 @@ describe("service memory section (#314 WP4)", () => { const conflict = proxyDownRestartHint({ proxyRunning: false, port: 10100, serviceViable: false, serviceInstalled: true, serviceConflict: true }); expect(conflict).toContain("ocx service install"); }); + + // #1419: the records outliving the process is the only signal the user gets that a + // proxy died rather than never started. Cause-neutral by design — SIGKILL, power + // loss and a native trap leave identical evidence. + test("an unclean prior exit is named before the restart path", () => { + const crashed = proxyDownRestartHint({ + proxyRunning: false, + port: 10100, + serviceViable: false, + serviceInstalled: false, + staleProcessState: true, + }); + expect(crashed).toContain("may have exited unexpectedly"); + expect(crashed).toContain("ocx service install"); + + // Absent or false must not invent a crash for a proxy that was never started. + const neverStarted = proxyDownRestartHint({ + proxyRunning: false, + port: 10100, + serviceViable: false, + serviceInstalled: false, + staleProcessState: false, + }); + expect(neverStarted).not.toContain("may have exited unexpectedly"); + }); + + test("the unclean-exit wording never asserts a cause", () => { + const hint = proxyDownRestartHint({ + proxyRunning: false, + port: 10100, + serviceViable: false, + staleProcessState: true, + }) ?? ""; + for (const forbidden of ["SIGTRAP", "SIGKILL", "Bun", "crash", "detached"]) { + expect(hint).not.toContain(forbidden); + } + }); }); describe("doctor abandoned response-state temps", () => { @@ -793,3 +831,67 @@ describe("doctor reclaim wiring (end to end)", () => { expect(logged.join("\n")).toContain("Unrecognized flag"); }); }); + +/** + * The wiring test, and the reason a helper-only assertion was rejected during plan + * review: `proxyDownRestartHint` can accept `staleProcessState` and stay green while + * `runDoctor` never passes it, leaving real `ocx doctor` output unchanged. This drives + * the actual command against a home holding a dead owner record. + */ +describe("doctor reports an unclean prior proxy exit", () => { + let tempHome: string; + let previousHome: string | undefined; + let logged: string[]; + const realLog = console.log; + + beforeEach(() => { + tempHome = mkdtempSync(join(tmpdir(), "ocx-doctor-unclean-")); + previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = tempHome; + logged = []; + console.log = (...args: unknown[]) => { logged.push(args.map(String).join(" ")); }; + }); + + afterEach(() => { + console.log = realLog; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + rmSync(tempHome, { recursive: true, force: true }); + }); + + /** + * A pid that is certainly dead: spawn a process, wait for it to exit, then reuse its + * number. A hardcoded constant can belong to an unrelated live process on a busy + * machine, which would silently invert this fixture. + */ + const deadPid = (): number => { + const spawned = spawnSync(process.execPath, ["-e", ""], { encoding: "utf8" }); + const pid = spawned.pid; + return typeof pid === "number" && pid > 0 ? pid : (process.pid === 4242 ? 4243 : 4242); + }; + + // Port 9 is the discard port: nothing listens, so the health probe is refused rather + // than timing out, which is what the predicate requires. + const seedConfig = (): void => { + writeFileSync(join(tempHome, "config.json"), JSON.stringify({ port: 9, codexAutoStart: false }), "utf8"); + }; + + test("a dead owner record surfaces the unclean-exit diagnosis", async () => { + seedConfig(); + const pid = deadPid(); + writeFileSync(join(tempHome, "ocx.pid"), String(pid), "utf8"); + writeFileSync(join(tempHome, "runtime-port.json"), JSON.stringify({ pid, port: 9, hostname: "127.0.0.1" }), "utf8"); + + await runDoctor([]); + + expect(logged.join("\n")).toContain("may have exited unexpectedly"); + }); + + test("a clean home never claims a prior crash", async () => { + seedConfig(); + + await runDoctor([]); + + expect(logged.join("\n")).not.toContain("may have exited unexpectedly"); + }); +});