Skip to content

Commit 2f2c83c

Browse files
authored
fix(cli): carry status/doctor live proxy onto dig2-go (#642)
Carries #642. Go port deferred: #678.
1 parent f708d2b commit 2f2c83c

5 files changed

Lines changed: 94 additions & 36 deletions

File tree

src/cli/doctor.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { accessSync, constants, existsSync, readFileSync } from "node:fs";
1111
import { homedir } from "node:os";
1212
import { dirname, join } from "node:path";
1313
import { getConfigDir, getConfigPath, readConfigDiagnostics, readPid, readRuntimePort, resolveEnvValue } from "../config";
14+
import { findLiveProxy } from "../server/proxy-liveness";
1415
import { gracefulStopHost } from "../lib/process-control";
1516
import { maskAccountId } from "../lib/privacy";
1617
import { loadServiceTokenFromFile } from "../lib/service-secrets";
@@ -718,9 +719,21 @@ export async function runDoctor(args: string[] = []): Promise<void> {
718719
}
719720
}
720721

722+
// #618: identity-verified liveness first so pid-file absence does not hide a live service.
723+
// Reuse the diagnostics config already loaded above so doctor stays read-only on malformed JSON.
724+
const live = await findLiveProxy({
725+
configFn: () => ({ port: doctorConfig.port, hostname: doctorConfig.hostname }),
726+
});
727+
const livePid = live ? live.pid : readPid();
728+
const liveRuntime = live
729+
? { pid: live.pid ?? 0, port: live.port, hostname: live.hostname }
730+
: (livePid ? readRuntimePort(livePid) : null);
731+
721732
const currentProxyEnv = collectProxyEnv();
722733
const configuredProxy = collectConfiguredProxy();
723-
const runningProxyEnv = collectRunningProxyEnv();
734+
const runningProxyEnv = collectRunningProxyEnv({
735+
readPidFn: () => (live ? live.pid : readPid()),
736+
});
724737

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

745-
// #314: service-process memory/runtime identity via the authed management
746-
// endpoint. readPid() FIRST (liveness), then the pid-scoped runtime record —
747-
// readRuntimePort alone can serve a stale file pointing at a foreign port.
748-
// Hoisted out of the block below: the Hints section reuses the same liveness pair
749-
// for the proxy-down restart hint.
750-
const livePid = readPid();
751-
const liveRuntime = livePid ? readRuntimePort(livePid) : null;
752758
console.log("\nMemory / runtime");
753759
{
754760
const runtime = liveRuntime;
755-
if (!runtime) {
761+
if (!runtime || !live) {
756762
console.log(` -- doctor process Bun ${Bun.version} (this is NOT the service process)`);
757763
console.log(" -- no running ocx proxy found (no live pid/runtime record)");
758764
} else {
@@ -815,8 +821,8 @@ export async function runDoctor(args: string[] = []): Promise<void> {
815821
// Hints, not fixes.
816822
const hints: string[] = [];
817823
const proxyDown = proxyDownRestartHint({
818-
proxyRunning: Boolean(livePid && liveRuntime),
819-
port: doctorConfig.port ?? 10100,
824+
proxyRunning: Boolean(live),
825+
port: live?.port ?? doctorConfig.port ?? 10100,
820826
serviceViable: startup.serviceViable,
821827
});
822828
if (proxyDown) hints.push(proxyDown);

src/cli/status.ts

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { durableBunRuntime } from "../lib/bun-runtime";
22
import { codexAutoStartEnabled, getConfigPath, getPidPath, readConfigDiagnostics, readPid, readRuntimePort, type RuntimePortState } from "../config";
33
import { diagnoseCodexBundledPlugins, type CodexPluginsDiagnostic } from "../codex/plugins-doctor";
4-
import { isOpencodexHealthz, probeHostname } from "../server/proxy-liveness";
4+
import { findLiveProxy, isOpencodexHealthz, probeHostname } from "../server/proxy-liveness";
55
import type { OcxConfig } from "../types";
66
import { diagnoseService } from "../service";
77
import { collectStartupHealth, type StartupHealth } from "../codex/autostart-health";
@@ -102,6 +102,14 @@ export function selectListenTarget(
102102
};
103103
}
104104

105+
/** Prefer live result (including authoritative null pid) over the on-disk pid file. */
106+
export function resolveStatusPid(
107+
live: { pid: number | null } | null,
108+
pidFile: number | null,
109+
): number | null {
110+
return live ? live.pid : pidFile;
111+
}
112+
105113
async function checkProxyHealth(target: ListenTarget): Promise<HealthCheck> {
106114
const url = target.healthUrl;
107115
const controller = new AbortController();
@@ -132,9 +140,33 @@ async function checkProxyHealth(target: ListenTarget): Promise<HealthCheck> {
132140
export async function collectStatus(): Promise<CliStatusView> {
133141
const configDiagnostics = readConfigDiagnostics();
134142
const config = configDiagnostics.config;
135-
const pid = readPid();
136-
const listen = selectListenTarget(config, pid, pid ? readRuntimePort(pid) : null);
137-
const health = await checkProxyHealth(listen);
143+
// Prefer identity-verified liveness (runtime-port + /healthz) over ocx.pid alone (#618).
144+
// Pass the already-resolved diagnostics config so findLiveProxy does not re-load and
145+
// warn on malformed config.json (status --json must stay stderr-clean).
146+
const live = await findLiveProxy({
147+
configFn: () => ({ port: config.port, hostname: config.hostname }),
148+
});
149+
const pidFile = readPid();
150+
// Preserve an authoritative null from orphan/legacy liveness — do not restore pidFile.
151+
const pid = resolveStatusPid(live, pidFile);
152+
const listen = live
153+
? {
154+
port: live.port,
155+
hostname: live.hostname,
156+
source: live.source,
157+
healthUrl: `http://${probeHostname(live.hostname)}:${live.port}/healthz`,
158+
dashboardUrl: `http://localhost:${live.port}/`,
159+
}
160+
: selectListenTarget(config, pidFile, pidFile ? readRuntimePort(pidFile) : null);
161+
// findLiveProxy already identity-probed /healthz; avoid a second fetch that can race.
162+
const health = live
163+
? {
164+
ok: true,
165+
url: listen.healthUrl,
166+
message: `ok (pid ${live.pid ?? "unknown"})`,
167+
label: `${listen.healthUrl} ok (live)`,
168+
}
169+
: await checkProxyHealth(listen);
138170
const bunRuntime = durableBunRuntime();
139171
const service = diagnoseService();
140172
const serviceSummary = service.summary;
@@ -228,22 +260,24 @@ export async function collectStatus(): Promise<CliStatusView> {
228260
runtimeVersion: clampActive ? (lastClamp?.runtimeVersion ?? null) : null,
229261
},
230262
};
231-
const proxyLabel = pid && health.ok
232-
? `running (PID ${pid})`
233-
: pid
234-
? `PID file points to PID ${pid}, but health check failed`
235-
: health.ok
236-
? "reachable, but PID file is missing or stale"
237-
: "not running";
263+
const proxyLabel = live
264+
? `running (PID ${live.pid ?? pid ?? "unknown"})`
265+
: pid && health.ok
266+
? `running (PID ${pid})`
267+
: pid
268+
? `PID file points to PID ${pid}, but health check failed`
269+
: health.ok
270+
? "reachable, but PID file is missing or stale"
271+
: "not running";
238272

239273
return {
240274
proxyLabel,
241275
healthLabel: health.label,
242276
json: {
243277
schemaVersion: 1,
244278
proxy: {
245-
running: Boolean(pid && health.ok),
246-
pid,
279+
running: Boolean(live) || Boolean(pid && health.ok),
280+
pid: live?.pid ?? pid,
247281
health: {
248282
ok: health.ok,
249283
url: health.url,

src/server/proxy-liveness.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export interface LiveProxy {
3737
port: number;
3838
/** Raw bind hostname the probe succeeded against; compose URLs via `probeHostname`. */
3939
hostname?: string;
40+
/** Whether the successful probe used runtime-port metadata or the configured listen port. */
41+
source: "runtime" | "config";
4042
}
4143

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

139143
const config = configFn();
140144
const port = config.port ?? 10100;
141145
const identity = await proxyIdentityAt(port, { hostname: config.hostname }, io);
142-
if (identity) return { pid: identity.pid ?? killablePid(pid), port, hostname: config.hostname };
146+
if (identity) {
147+
return {
148+
pid: identity.pid ?? killablePid(pid),
149+
port,
150+
hostname: config.hostname,
151+
source: "config",
152+
};
153+
}
143154
return null;
144155
}

tests/cli-status-json.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync, mkdirSync
44
import { tmpdir } from "node:os";
55
import { dirname, join } from "node:path";
66
import { fileURLToPath } from "node:url";
7-
import { selectListenTarget } from "../src/cli/status";
7+
import { resolveStatusPid, selectListenTarget } from "../src/cli/status";
88

99
const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url)));
1010
const cliPath = join(repoRoot, "src", "cli", "index.ts");
@@ -285,6 +285,13 @@ describe("CLI status JSON", () => {
285285
expect(target.dashboardUrl).toBe("http://localhost:58195/");
286286
});
287287

288+
test("resolveStatusPid preserves an authoritative null from live orphan checks", () => {
289+
expect(resolveStatusPid({ pid: null }, 4242)).toBeNull();
290+
expect(resolveStatusPid({ pid: 1111 }, 4242)).toBe(1111);
291+
expect(resolveStatusPid(null, 4242)).toBe(4242);
292+
expect(resolveStatusPid(null, null)).toBeNull();
293+
});
294+
288295
test("listen target brackets raw IPv6 hostnames in the health URL", () => {
289296
const target = selectListenTarget(
290297
{ port: 10100, hostname: "::1" },

tests/proxy-liveness.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ describe("findLiveProxy", () => {
6666
}) as typeof fetch,
6767
});
6868

69-
expect(live).toEqual({ pid: 4242, port: 58195 });
69+
expect(live).toEqual({ pid: 4242, port: 58195, source: "runtime" });
7070
expect(urls).toEqual(["http://127.0.0.1:58195/healthz"]);
7171
});
7272

@@ -78,7 +78,7 @@ describe("findLiveProxy", () => {
7878
fetchFn: (async () => healthz(OURS)) as typeof fetch,
7979
});
8080

81-
expect(live).toEqual({ pid: 4242, port: 10100 });
81+
expect(live).toEqual({ pid: 4242, port: 10100, source: "config" });
8282
});
8383

8484
test("a foreign listener on the configured port is not treated as our proxy", async () => {
@@ -104,7 +104,7 @@ describe("findLiveProxy", () => {
104104
}) as typeof fetch,
105105
});
106106

107-
expect(live).toEqual({ pid: 4242, port: 58195, hostname: "::1" });
107+
expect(live).toEqual({ pid: 4242, port: 58195, hostname: "::1", source: "runtime" });
108108
expect(urls).toEqual(["http://[::1]:58195/healthz"]);
109109
});
110110

@@ -120,7 +120,7 @@ describe("findLiveProxy", () => {
120120

121121
// The record's pid 1111 may be dead/reused — synthesizing it would let `ocx stop`
122122
// kill an unrelated process via the taskkill/kill fallback.
123-
expect(live).toEqual({ pid: null, port: 58195, hostname: undefined });
123+
expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" });
124124
});
125125

126126
test("an orphaned record whose healthz pid mismatches is rejected (config fallback still runs)", async () => {
@@ -145,7 +145,7 @@ describe("findLiveProxy", () => {
145145

146146
// The runtime probe fails the pid check; the config fallback probes the same port
147147
// without a pid expectation and adopts the reported live pid instead.
148-
expect(live).toEqual({ pid: 9999, port: 58195 });
148+
expect(live).toEqual({ pid: 9999, port: 58195, source: "config" });
149149
});
150150

151151
test("a pidless legacy healthz never promotes an unverified cheap pid to a kill target", async () => {
@@ -158,7 +158,7 @@ describe("findLiveProxy", () => {
158158
fetchFn: (async () => healthz(legacyBody)) as typeof fetch,
159159
});
160160

161-
expect(live).toEqual({ pid: null, port: 58195, hostname: undefined });
161+
expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" });
162162
});
163163

164164
test("a pidless legacy healthz returns the cheap pid once full identity verification echoes it", async () => {
@@ -176,7 +176,7 @@ describe("findLiveProxy", () => {
176176
});
177177

178178
expect(verified).toEqual([1111]);
179-
expect(live).toEqual({ pid: 1111, port: 58195, hostname: undefined });
179+
expect(live).toEqual({ pid: 1111, port: 58195, hostname: undefined, source: "runtime" });
180180
});
181181

182182
test("a verifier answering with a DIFFERENT pid than the candidate is rejected (TOCTOU guard)", async () => {
@@ -189,6 +189,6 @@ describe("findLiveProxy", () => {
189189
fetchFn: (async () => healthz(legacyBody)) as typeof fetch,
190190
});
191191

192-
expect(live).toEqual({ pid: null, port: 58195, hostname: undefined });
192+
expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime" });
193193
});
194194
});

0 commit comments

Comments
 (0)