From 9324b6e1a9441254b30ac4085b5b04b9c72db9cb Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:12:48 +0200 Subject: [PATCH] fix(cli): status/doctor use identity-verified live proxy (#642) Fixes #618. Prefer findLiveProxy over ocx.pid alone; keep status/doctor read-only on malformed config; preserve authoritative null live pids and runtime vs config listen source. --- src/cli/doctor.ts | 28 +++++++++------- src/cli/status.ts | 60 +++++++++++++++++++++++++++-------- src/server/proxy-liveness.ts | 17 ++++++++-- tests/cli-status-json.test.ts | 9 +++++- tests/proxy-liveness.test.ts | 16 +++++----- 5 files changed, 94 insertions(+), 36 deletions(-) diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index f4a28361a..b3bcf7c02 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -11,6 +11,7 @@ import { accessSync, constants, existsSync, readFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join } from "node:path"; import { getConfigDir, getConfigPath, readConfigDiagnostics, readPid, readRuntimePort, resolveEnvValue } from "../config"; +import { findLiveProxy } from "../server/proxy-liveness"; import { gracefulStopHost } from "../lib/process-control"; import { maskAccountId } from "../lib/privacy"; import { loadServiceTokenFromFile } from "../lib/service-secrets"; @@ -718,9 +719,21 @@ export async function runDoctor(args: string[] = []): Promise { } } + // #618: identity-verified liveness first so pid-file absence does not hide a live service. + // Reuse the diagnostics config already loaded above so doctor stays read-only on malformed JSON. + const live = await findLiveProxy({ + configFn: () => ({ port: doctorConfig.port, hostname: doctorConfig.hostname }), + }); + const livePid = live ? live.pid : readPid(); + const liveRuntime = live + ? { pid: live.pid ?? 0, port: live.port, hostname: live.hostname } + : (livePid ? readRuntimePort(livePid) : null); + const currentProxyEnv = collectProxyEnv(); const configuredProxy = collectConfiguredProxy(); - const runningProxyEnv = collectRunningProxyEnv(); + const runningProxyEnv = collectRunningProxyEnv({ + readPidFn: () => (live ? live.pid : readPid()), + }); console.log("\nCurrent doctor process proxy env (presence only)"); for (const row of currentProxyEnv) { @@ -742,17 +755,10 @@ export async function runDoctor(args: string[] = []): Promise { } } - // #314: service-process memory/runtime identity via the authed management - // endpoint. readPid() FIRST (liveness), then the pid-scoped runtime record — - // readRuntimePort alone can serve a stale file pointing at a foreign port. - // Hoisted out of the block below: the Hints section reuses the same liveness pair - // for the proxy-down restart hint. - const livePid = readPid(); - const liveRuntime = livePid ? readRuntimePort(livePid) : null; console.log("\nMemory / runtime"); { const runtime = liveRuntime; - if (!runtime) { + if (!runtime || !live) { console.log(` -- doctor process Bun ${Bun.version} (this is NOT the service process)`); console.log(" -- no running ocx proxy found (no live pid/runtime record)"); } else { @@ -815,8 +821,8 @@ export async function runDoctor(args: string[] = []): Promise { // Hints, not fixes. const hints: string[] = []; const proxyDown = proxyDownRestartHint({ - proxyRunning: Boolean(livePid && liveRuntime), - port: doctorConfig.port ?? 10100, + proxyRunning: Boolean(live), + port: live?.port ?? doctorConfig.port ?? 10100, serviceViable: startup.serviceViable, }); if (proxyDown) hints.push(proxyDown); diff --git a/src/cli/status.ts b/src/cli/status.ts index a9686db01..76d61d7f8 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -1,7 +1,7 @@ import { durableBunRuntime } from "../lib/bun-runtime"; import { codexAutoStartEnabled, getConfigPath, getPidPath, readConfigDiagnostics, readPid, readRuntimePort, type RuntimePortState } from "../config"; import { diagnoseCodexBundledPlugins, type CodexPluginsDiagnostic } from "../codex/plugins-doctor"; -import { isOpencodexHealthz, probeHostname } from "../server/proxy-liveness"; +import { findLiveProxy, isOpencodexHealthz, probeHostname } from "../server/proxy-liveness"; import type { OcxConfig } from "../types"; import { diagnoseService } from "../service"; import { collectStartupHealth, type StartupHealth } from "../codex/autostart-health"; @@ -102,6 +102,14 @@ export function selectListenTarget( }; } +/** Prefer live result (including authoritative null pid) over the on-disk pid file. */ +export function resolveStatusPid( + live: { pid: number | null } | null, + pidFile: number | null, +): number | null { + return live ? live.pid : pidFile; +} + async function checkProxyHealth(target: ListenTarget): Promise { const url = target.healthUrl; const controller = new AbortController(); @@ -132,9 +140,33 @@ async function checkProxyHealth(target: ListenTarget): Promise { export async function collectStatus(): Promise { const configDiagnostics = readConfigDiagnostics(); const config = configDiagnostics.config; - const pid = readPid(); - const listen = selectListenTarget(config, pid, pid ? readRuntimePort(pid) : null); - const health = await checkProxyHealth(listen); + // Prefer identity-verified liveness (runtime-port + /healthz) over ocx.pid alone (#618). + // Pass the already-resolved diagnostics config so findLiveProxy does not re-load and + // warn on malformed config.json (status --json must stay stderr-clean). + const live = await findLiveProxy({ + configFn: () => ({ port: config.port, hostname: config.hostname }), + }); + const pidFile = readPid(); + // Preserve an authoritative null from orphan/legacy liveness — do not restore pidFile. + const pid = resolveStatusPid(live, pidFile); + const listen = live + ? { + port: live.port, + hostname: live.hostname, + source: live.source, + healthUrl: `http://${probeHostname(live.hostname)}:${live.port}/healthz`, + dashboardUrl: `http://localhost:${live.port}/`, + } + : selectListenTarget(config, pidFile, pidFile ? readRuntimePort(pidFile) : null); + // findLiveProxy already identity-probed /healthz; avoid a second fetch that can race. + const health = live + ? { + ok: true, + url: listen.healthUrl, + message: `ok (pid ${live.pid ?? "unknown"})`, + label: `${listen.healthUrl} ok (live)`, + } + : await checkProxyHealth(listen); const bunRuntime = durableBunRuntime(); const service = diagnoseService(); const serviceSummary = service.summary; @@ -228,13 +260,15 @@ export async function collectStatus(): Promise { runtimeVersion: clampActive ? (lastClamp?.runtimeVersion ?? null) : null, }, }; - const proxyLabel = pid && health.ok - ? `running (PID ${pid})` - : pid - ? `PID file points to PID ${pid}, but health check failed` - : health.ok - ? "reachable, but PID file is missing or stale" - : "not running"; + const proxyLabel = live + ? `running (PID ${live.pid ?? pid ?? "unknown"})` + : pid && health.ok + ? `running (PID ${pid})` + : pid + ? `PID file points to PID ${pid}, but health check failed` + : health.ok + ? "reachable, but PID file is missing or stale" + : "not running"; return { proxyLabel, @@ -242,8 +276,8 @@ export async function collectStatus(): Promise { json: { schemaVersion: 1, proxy: { - running: Boolean(pid && health.ok), - pid, + running: Boolean(live) || Boolean(pid && health.ok), + pid: live?.pid ?? pid, health: { ok: health.ok, url: health.url, diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index 67ff35667..0ed7e6bbf 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -37,6 +37,8 @@ export interface LiveProxy { port: number; /** Raw bind hostname the probe succeeded against; compose URLs via `probeHostname`. */ hostname?: string; + /** Whether the successful probe used runtime-port metadata or the configured listen port. */ + source: "runtime" | "config"; } /** @@ -118,7 +120,7 @@ export async function findLiveProxy(io: LivenessIo = {}): Promise { expect(target.dashboardUrl).toBe("http://localhost:58195/"); }); + test("resolveStatusPid preserves an authoritative null from live orphan checks", () => { + expect(resolveStatusPid({ pid: null }, 4242)).toBeNull(); + expect(resolveStatusPid({ pid: 1111 }, 4242)).toBe(1111); + expect(resolveStatusPid(null, 4242)).toBe(4242); + expect(resolveStatusPid(null, null)).toBeNull(); + }); + test("listen target brackets raw IPv6 hostnames in the health URL", () => { const target = selectListenTarget( { port: 10100, hostname: "::1" }, diff --git a/tests/proxy-liveness.test.ts b/tests/proxy-liveness.test.ts index cdb787fb0..66c09b4fd 100644 --- a/tests/proxy-liveness.test.ts +++ b/tests/proxy-liveness.test.ts @@ -66,7 +66,7 @@ describe("findLiveProxy", () => { }) as typeof fetch, }); - expect(live).toEqual({ pid: 4242, port: 58195 }); + expect(live).toEqual({ pid: 4242, port: 58195, source: "runtime" }); expect(urls).toEqual(["http://127.0.0.1:58195/healthz"]); }); @@ -78,7 +78,7 @@ describe("findLiveProxy", () => { fetchFn: (async () => healthz(OURS)) as typeof fetch, }); - expect(live).toEqual({ pid: 4242, port: 10100 }); + expect(live).toEqual({ pid: 4242, port: 10100, source: "config" }); }); test("a foreign listener on the configured port is not treated as our proxy", async () => { @@ -104,7 +104,7 @@ describe("findLiveProxy", () => { }) as typeof fetch, }); - expect(live).toEqual({ pid: 4242, port: 58195, hostname: "::1" }); + expect(live).toEqual({ pid: 4242, port: 58195, hostname: "::1", source: "runtime" }); expect(urls).toEqual(["http://[::1]:58195/healthz"]); }); @@ -120,7 +120,7 @@ describe("findLiveProxy", () => { // The record's pid 1111 may be dead/reused — synthesizing it would let `ocx stop` // kill an unrelated process via the taskkill/kill fallback. - expect(live).toEqual({ pid: null, port: 58195, hostname: undefined }); + expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" }); }); test("an orphaned record whose healthz pid mismatches is rejected (config fallback still runs)", async () => { @@ -145,7 +145,7 @@ describe("findLiveProxy", () => { // The runtime probe fails the pid check; the config fallback probes the same port // without a pid expectation and adopts the reported live pid instead. - expect(live).toEqual({ pid: 9999, port: 58195 }); + expect(live).toEqual({ pid: 9999, port: 58195, source: "config" }); }); test("a pidless legacy healthz never promotes an unverified cheap pid to a kill target", async () => { @@ -158,7 +158,7 @@ describe("findLiveProxy", () => { fetchFn: (async () => healthz(legacyBody)) as typeof fetch, }); - expect(live).toEqual({ pid: null, port: 58195, hostname: undefined }); + expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" }); }); test("a pidless legacy healthz returns the cheap pid once full identity verification echoes it", async () => { @@ -176,7 +176,7 @@ describe("findLiveProxy", () => { }); expect(verified).toEqual([1111]); - expect(live).toEqual({ pid: 1111, port: 58195, hostname: undefined }); + expect(live).toEqual({ pid: 1111, port: 58195, hostname: undefined, source: "runtime" }); }); test("a verifier answering with a DIFFERENT pid than the candidate is rejected (TOCTOU guard)", async () => { @@ -189,6 +189,6 @@ describe("findLiveProxy", () => { fetchFn: (async () => healthz(legacyBody)) as typeof fetch, }); - expect(live).toEqual({ pid: null, port: 58195, hostname: undefined }); + expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" }); }); });