Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions src/cli/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -718,9 +719,21 @@ export async function runDoctor(args: string[] = []): Promise<void> {
}
}

// #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()),
Comment on lines +734 to +735

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Distinguish a live proxy whose PID is unavailable

When findLiveProxy discovers an orphaned legacy proxy, it intentionally returns a live result with pid: null; passing that null into collectRunningProxyEnv makes the doctor print no running ocx proxy process found, even though the following memory section and restart hints treat the proxy as running. In this scenario the process environment is merely unavailable because its PID is unknown, so pass the live-state information through and render an unavailable/unknown-PID diagnostic instead of not_running, with focused coverage for this live-null-PID path.

AGENTS.md reference: src/AGENTS.md:L24-L25

Useful? React with 👍 / 👎.

});

console.log("\nCurrent doctor process proxy env (presence only)");
for (const row of currentProxyEnv) {
Expand All @@ -742,17 +755,10 @@ export async function runDoctor(args: string[] = []): Promise<void> {
}
}

// #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 {
Expand Down Expand Up @@ -815,8 +821,8 @@ export async function runDoctor(args: string[] = []): Promise<void> {
// 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);
Expand Down
60 changes: 47 additions & 13 deletions src/cli/status.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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<HealthCheck> {
const url = target.healthUrl;
const controller = new AbortController();
Expand Down Expand Up @@ -132,9 +140,33 @@ async function checkProxyHealth(target: ListenTarget): Promise<HealthCheck> {
export async function collectStatus(): Promise<CliStatusView> {
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)`,
Comment on lines +166 to +167

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve version and uptime in live health reports

When a proxy is running, this branch now bypasses checkProxyHealth and replaces its ok v<version>, uptime <seconds> result with only ok (pid ...). Consequently both the human-readable ocx status output and proxy.health.message in status --json lose existing diagnostic information on every successful live check. Preserve the version and uptime returned by the first identity probe—such as by carrying them through proxyIdentityAt/findLiveProxy—and add focused live-status coverage.

AGENTS.md reference: src/AGENTS.md:L24-L25

Useful? React with 👍 / 👎.

}
: await checkProxyHealth(listen);
const bunRuntime = durableBunRuntime();
const service = diagnoseService();
const serviceSummary = service.summary;
Expand Down Expand Up @@ -228,22 +260,24 @@ export async function collectStatus(): Promise<CliStatusView> {
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,
healthLabel: health.label,
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,
Expand Down
17 changes: 14 additions & 3 deletions src/server/proxy-liveness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}

/**
Expand Down Expand Up @@ -118,7 +120,7 @@ export async function findLiveProxy(io: LivenessIo = {}): Promise<LiveProxy | nu
// healthz confirmed the pid itself → trusted; a pidless legacy body did not,
// so the cheap pid must pass full identity verification before it is returned.
const trusted = identity.pid === pid ? pid : killablePid(pid);
return { pid: trusted, port: runtime.port, hostname: runtime.hostname };
return { pid: trusted, port: runtime.port, hostname: runtime.hostname, source: "runtime" };
}
}
}
Expand All @@ -133,12 +135,21 @@ export async function findLiveProxy(io: LivenessIo = {}): Promise<LiveProxy | nu
// Only the healthz-reported pid is authoritative here. The record's pid may be stale
// (its process dead, the port reused by a pidless legacy proxy) — synthesizing it
// would hand destructive callers (stopProxy → kill fallback) a reusable pid.
if (identity) return { pid: identity.pid ?? null, port: record.port, hostname: record.hostname };
if (identity) {
return { pid: identity.pid ?? null, port: record.port, hostname: record.hostname, source: "runtime" };
}
}

const config = configFn();
const port = config.port ?? 10100;
const identity = await proxyIdentityAt(port, { hostname: config.hostname }, io);
if (identity) return { pid: identity.pid ?? killablePid(pid), port, hostname: config.hostname };
if (identity) {
return {
pid: identity.pid ?? killablePid(pid),
port,
hostname: config.hostname,
source: "config",
};
}
return null;
}
9 changes: 8 additions & 1 deletion tests/cli-status-json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync, mkdirSync
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { selectListenTarget } from "../src/cli/status";
import { 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");
Expand Down Expand Up @@ -285,6 +285,13 @@ describe("CLI status JSON", () => {
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" },
Expand Down
16 changes: 8 additions & 8 deletions tests/proxy-liveness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"]);
});

Expand All @@ -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 () => {
Expand All @@ -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"]);
});

Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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" });
});
});
Loading