-
Notifications
You must be signed in to change notification settings - Fork 457
fix(cli): carry status/doctor live proxy onto dig2-go (#642) #679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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"; | ||
|
|
@@ -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(); | ||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a proxy is running, this branch now bypasses AGENTS.md reference: src/AGENTS.md:L24-L25 Useful? React with 👍 / 👎. |
||
| } | ||
| : await checkProxyHealth(listen); | ||
| const bunRuntime = durableBunRuntime(); | ||
| const service = diagnoseService(); | ||
| const serviceSummary = service.summary; | ||
|
|
@@ -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, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
findLiveProxydiscovers an orphaned legacy proxy, it intentionally returns a live result withpid: null; passing that null intocollectRunningProxyEnvmakes the doctor printno 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 ofnot_running, with focused coverage for this live-null-PID path.AGENTS.md reference: src/AGENTS.md:L24-L25
Useful? React with 👍 / 👎.