diff --git a/bin/ocx.mjs b/bin/ocx.mjs index c69ee692e..fd1cc9194 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -250,12 +250,40 @@ function runNpmSelfUpdate() { try { const svcArgs = serviceReinstallArgs(); const svc = spawnSync(process.execPath, svcArgs, { stdio: "inherit", windowsHide: true }); - if (svc.status !== 0) { + let needDirectStart = svc.status !== 0; + if (!needDirectStart) { + // Exit 0 can still leave stale/missing assets that never bring the proxy + // back — match the GUI/CLI fallthrough so /healthz is not left dead. + try { + const st = spawnSync(process.execPath, [launcher, "status", "--json"], { + encoding: "utf8", + timeout: 20_000, + windowsHide: true, + }); + if (st.status === 0 && typeof st.stdout === "string" && st.stdout.trim()) { + const parsed = JSON.parse(st.stdout); + const proxyUp = parsed?.proxy?.running === true || parsed?.proxy?.health?.ok === true; + const viable = parsed?.startup?.serviceViable === true; + if (!proxyUp && !viable) needDirectStart = true; + } else { + // status failed or empty — fail closed to direct start (match CLI). + needDirectStart = true; + } + } catch { + needDirectStart = true; + } + } + if (needDirectStart) { // On Windows, schtasks /create requires elevation. The launcher inherits the // user's (non-admin) token, so the service reinstall can fail with access - // denied. Fall back to a direct detached proxy start so the update never - // leaves the user without a running proxy. - console.warn("opencodex: service refresh failed — starting the proxy directly instead."); + // denied — or exit 0 while leaving a non-viable manager. Fall back to a + // direct detached proxy start so the update never leaves the user without + // a running proxy. + console.warn( + svc.status === 0 + ? "opencodex: service refresh left a non-viable manager — starting the proxy directly instead." + : "opencodex: service refresh failed — starting the proxy directly instead.", + ); console.warn(" Run 'ocx service install' as administrator to refresh the background service."); const env = { ...process.env }; delete env.OCX_SERVICE; diff --git a/src/adapters/cursor/native-exec-shell.ts b/src/adapters/cursor/native-exec-shell.ts index 28aa6468f..2410fb8dd 100644 --- a/src/adapters/cursor/native-exec-shell.ts +++ b/src/adapters/cursor/native-exec-shell.ts @@ -335,17 +335,17 @@ function waitForBackgroundShellClose(entry: BackgroundShellEntry): Promise { let settled = false; - const timer = backgroundShellRuntime.setTimer(() => { - if (settled) return; - settled = true; - resolveWait(false); - }, CURSOR_BACKGROUND_SHELL_TERM_GRACE_MS); // Deliberately REF'D: this is the bounded kill-grace wait that shutdown // drain awaits. Bun on Windows can starve unref'd timers when a pending // promise is the only other work, which would leave drainAndShutdown // waiting on this resolution forever. The timer self-clears within the // 2-second grace window (or earlier on close), so a ref cannot keep the // process alive beyond that bound. + const timer = backgroundShellRuntime.setTimer(() => { + if (settled) return; + settled = true; + resolveWait(false); + }, CURSOR_BACKGROUND_SHELL_TERM_GRACE_MS); void entry.closePromise.then(() => { if (settled) return; settled = true; diff --git a/src/cli/index.ts b/src/cli/index.ts index 0cc8a45d2..d73a38be8 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -129,16 +129,21 @@ async function chooseListenPort(requestedPort?: number): Promise { if (hardPin && preferred > 0) { const { reclaimListenPort } = await import("../server/port-reclaim"); await reclaimListenPort(preferred, config.hostname ?? "127.0.0.1", { - timeoutMs: 30_000, + // Ghost LISTEN rows with a dead PID can outlive the process for a while. + // SetTcpEntry(DELETE_TCB) needs elevation (often returns 317), so the only + // reliable non-admin recovery is to wait for the OS to release the TCB. + timeoutMs: 60_000, intervalMs: 100, scanIntervalMs: 500, killOcxHolders: false, - dropTcpRows: false, + dropTcpRows: true, }); } try { const selected = await findAvailablePort(preferred, config.hostname ?? "127.0.0.1", { - preferRetryMs: hardPin ? 0 : 750, + // After reclaim, keep probing briefly — ghost rows sometimes clear between + // the reclaim deadline and the final listen. Still never hop off `--port`. + preferRetryMs: hardPin ? 5_000 : 750, preferRetryIntervalMs: 50, allowEphemeralFallback: !hardPin, }); diff --git a/src/config.ts b/src/config.ts index d3927d83b..88bf9564d 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2173,9 +2173,13 @@ export function parsePidFile(raw: string): number | null { export function isOcxStartCommandLine(commandLine: string): boolean { const normalized = commandLine.toLowerCase().replace(/\\/g, "/"); // "src/cli.ts" matches pre-restructure installs still running; "src/cli/index.ts" is current. + // `@bitkyc08/.opencodex-*` is npm's in-place rename of the global package during + // `npm install -g` — a Windows service wrapper can respawn from that temp tree + // mid-update, and must still count as ocx for port reclaim. const hasOcxEntrypoint = normalized.includes("src/cli.ts") || normalized.includes("src/cli/index.ts") || normalized.includes("@bitkyc08/opencodex") + || /@bitkyc08\/\.opencodex-/.test(normalized) || /(?:^|[\s/"'])(?:ocx|opencodex)(?:\.cmd)?(?:$|[\s"'])/.test(normalized); return hasOcxEntrypoint && /(?:^|[\s"'])start(?:$|[\s"'])/.test(normalized); } diff --git a/src/server/management/system-restart.ts b/src/server/management/system-restart.ts index d622a0bf1..d16e0b71a 100644 --- a/src/server/management/system-restart.ts +++ b/src/server/management/system-restart.ts @@ -6,11 +6,14 @@ * recycle to reclaim RSS, not a teardown. * * Respawn policy (matches real supervisor configs in src/service.ts): - * - Supervised child (`OCX_SERVICE=1` + service installed): exit(1) so + * - Supervised child (`OCX_SERVICE=1` + viable service): exit(1) so * failure-only supervisors (systemd Restart=on-failure, WinSW onfailure, * Task Scheduler ERRORLEVEL loop) bring the proxy back. * - Otherwise: detached `ocx start --port ` (bypasses ensure's * codexAutoStart gate), mark recycle so exit cleanup keeps injection, exit(0). + * Installed-but-stale/missing service assets are NOT treated as supervised — + * exit(1) would leave the proxy dead with `Service: installed, stale or missing + * service assets` and a /healthz timeout. * - If detached spawn fails (sync throw or pre-start `error`): exit(1) without * markRecycling — after drain the listen socket is already closed, so a latch * reset cannot recover serving. Clear inherited `OCX_SERVICE` so exit cleanup @@ -27,7 +30,7 @@ import { markRecyclingForExit, setDraining, } from "../lifecycle"; -import { isServiceInstalled } from "../../service"; +import { isServiceViable } from "../../service"; import { readRuntimePort } from "../../config"; /** Fixed v1 drain window for the memory-card action (not config-driven). */ @@ -35,7 +38,8 @@ export const MEMORY_DRAIN_RESTART_MS = 60_000; export interface SystemRestartIo { drainAndShutdown?: typeof drainAndShutdown; - isServiceInstalled?: () => boolean; + /** True when a background service can actually respawn this process after exit(1). */ + isServiceViable?: () => boolean; isSupervisedServiceChild?: () => boolean; /** Must resolve only after the replacement process has actually started. */ spawnStart?: (port?: number) => void | Promise; @@ -66,8 +70,11 @@ function resolveListenPort(): number | undefined { return undefined; } -function isSupervisedServiceChild(): boolean { - return process.env.OCX_SERVICE === "1" && isServiceInstalled(); +function isSupervisedServiceChild(io: SystemRestartIo = {}): boolean { + if (process.env.OCX_SERVICE !== "1") return false; + // Presence is not enough: stale/missing service assets report installed but will not + // respawn after exit(1). Dashboard status/recovery must fall through to detached start. + return (io.isServiceViable ?? isServiceViable)(); } /** Stable, path-free spawn failure label for logs (never interpolate err.message). */ @@ -137,7 +144,7 @@ export function acceptSystemRestart(io: SystemRestartIo = restartIo): { schedule(async () => { const drain = io.drainAndShutdown ?? drainAndShutdown; await drain(undefined, MEMORY_DRAIN_RESTART_MS); - const supervised = (io.isSupervisedServiceChild ?? isSupervisedServiceChild)(); + const supervised = (io.isSupervisedServiceChild ?? (() => isSupervisedServiceChild(io)))(); if (supervised) { // Failure-only supervisors ignore exit(0); intentional non-zero triggers respawn. (io.exitProcess ?? ((code: number) => { process.exit(code); }))(1); diff --git a/src/server/port-reclaim.ts b/src/server/port-reclaim.ts index 10a031a72..05970badb 100644 --- a/src/server/port-reclaim.ts +++ b/src/server/port-reclaim.ts @@ -2,8 +2,10 @@ * Reclaim a listen port after stop/update so restart can stay on the configured * port instead of hopping to an ephemeral one (Windows CLOSE_WAIT / leftover ocx). * - * Killing is never the default: a process may be killed only when the caller - * supplies a non-empty explicit PID allowlist for a process it just stopped. + * Killing is never the default. A process may be killed only when the caller + * sets `killOcxHolders` and either supplies a non-empty `onlyKillPids` allowlist + * or enables `killAllOcxOnPort` for revalidated ocx listeners. Foreign (non-ocx) + * processes are never killed. */ import { execFileSync } from "node:child_process"; import { verifyPidIdentity } from "../config"; @@ -19,13 +21,24 @@ export type ReclaimListenPortOptions = WaitForPortOptions & { /** * When true AND `onlyKillPids` is a non-empty allowlist, those PIDs may be * killed after revalidation. Default false — never kill without an allowlist. + * When {@link killAllOcxOnPort} is also true, any ocx listener on this port + * may be killed even if it is not in `onlyKillPids`. */ killOcxHolders?: boolean; /** * Explicit PIDs the caller just stopped / hard-killed. An omitted or empty - * list means no process may be killed. + * list means no process may be killed — unless {@link killAllOcxOnPort} is set. */ onlyKillPids?: number[]; + /** + * When true with `killOcxHolders`, every live ocx listener on this port may be + * killed (re-checked each scan). Used by post-update restart so a Windows + * service wrapper that respawns a *new* bun PID mid-reclaim cannot stay + * protected just because it was absent from the pre-wait allowlist snapshot. + * Never kills foreign (non-ocx) processes — only allowlisted teardown PIDs + * and revalidated ocx listeners. + */ + killAllOcxOnPort?: boolean; /** * On Windows, force-delete IPv4 TCP rows for this local port via SetTcpEntry. * Default true on win32. Never kills foreign processes, never runs while a @@ -157,9 +170,9 @@ export function listListenPids(port: number): number[] { /** * Wait until `port` can bind. - * Never kills a process unless `killOcxHolders === true` and `onlyKillPids` is a - * non-empty allowlist of PIDs the caller itself just stopped — then revalidates - * immediately before each kill. + * Never kills a process unless `killOcxHolders === true` and either + * `onlyKillPids` is a non-empty allowlist or `killAllOcxOnPort` is set — then + * revalidates immediately before each kill. * Never kills foreign processes. Never drops TCP rows while a live foreign or * protected ocx listener owns the port, or when the listener scan failed. */ @@ -174,7 +187,9 @@ export async function reclaimListenPort( const allowedKillPids = new Set( (opts.onlyKillPids ?? []).filter(pid => Number.isSafeInteger(pid) && pid > 0), ); - const mayKill = opts.killOcxHolders === true && allowedKillPids.size > 0; + const killAllOcx = opts.killAllOcxOnPort === true; + const mayKill = opts.killOcxHolders === true + && (allowedKillPids.size > 0 || killAllOcx); const dropTcpRows = opts.dropTcpRows ?? process.platform === "win32"; const listFn = opts.listListenPidsFn ?? scanListenPids; const isAliveFn = opts.isAliveFn ?? isProcessAlive; @@ -209,18 +224,37 @@ export async function reclaimListenPort( if (pid === process.pid) continue; if (!isAliveFn(pid)) continue; // Windows may still list a dead owner briefly const isOcx = verifyOcxFn(pid) === pid; + const allowlisted = allowedKillPids.has(pid); + // Pre-update PIDs can fail verify while still LISTENing (dead owner still + // listed, or cmdline probe raced). Allowlisted teardown PIDs may be killed; + // unknown foreign claimants must remain fail-closed. if (!isOcx) { + if (mayKill && allowlisted) { + if (!killed.has(pid)) { + try { + killFn(pid); + killed.add(pid); + } catch { + // Kill failed: never SetTcpEntry while the process may still own the port. + protectedOcxListener = true; + } + } + if (!isAliveFn(pid)) killed.delete(pid); + else protectedOcxListener = true; + continue; + } foreignLive = true; continue; } - if (!mayKill || !allowedKillPids.has(pid)) { + const mayKillThis = allowlisted || killAllOcx; + if (!mayKill || !mayKillThis) { // Healthy / intentional ocx proxy — never steal its port. protectedOcxListener = true; continue; } if (!killed.has(pid)) { // Revalidate immediately before termination. - if (isAliveFn(pid) && verifyOcxFn(pid) === pid && allowedKillPids.has(pid)) { + if (isAliveFn(pid) && verifyOcxFn(pid) === pid && mayKillThis) { try { killFn(pid); killed.add(pid); @@ -233,8 +267,14 @@ export async function reclaimListenPort( protectedOcxListener = true; } } - // Only reset TCP rows after confirmed process death. - if (isAliveFn(pid)) protectedOcxListener = true; + // Respawning supervisors (Windows service :loop) mint a new PID after each + // kill — clear the per-PID "already tried" bit once the process is gone so a + // later child with a reused slot is not skipped, and keep reclaiming while live. + if (!isAliveFn(pid)) { + killed.delete(pid); + } else { + protectedOcxListener = true; + } } if (foreignLive || protectedOcxListener) { diff --git a/src/server/ports.ts b/src/server/ports.ts index bd96590b4..88bf97d53 100644 --- a/src/server/ports.ts +++ b/src/server/ports.ts @@ -16,6 +16,8 @@ export function isAddrInUse(err: unknown): boolean { export async function isPortAvailable(port: number, hostname = "127.0.0.1"): Promise { return await new Promise(resolve => { const server = createServer(); + // Fail closed: EACCES / EADDRNOTAVAIL / EPERM / unknown listen errors mean the + // requested bind is not available. Only the listening event reports free. server.once("error", () => resolve(false)); server.once("listening", () => { server.close(() => resolve(true)); diff --git a/src/server/windows-tcp-drop.ts b/src/server/windows-tcp-drop.ts index 1dcfcbc5c..80ff9712c 100644 --- a/src/server/windows-tcp-drop.ts +++ b/src/server/windows-tcp-drop.ts @@ -79,6 +79,12 @@ export type WindowsTcpDropResult = { dropped: number; /** IPv6 (or unparseable) rows skipped — never coerced into IPv4 wildcards. */ skippedIpv6: number; + /** + * SetTcpEntry returned ERROR_ACCESS_DENIED (5) or the UAC-non-elevated code (317). + * On a normal (non-admin) update worker this is expected — ghost LISTEN rows must + * clear by waiting for the OS, not by SetTcpEntry. + */ + accessDenied: number; }; function htons(port: number): number { @@ -133,21 +139,22 @@ function readNetstatAno(): string { */ export function dropWindowsTcpRowsForLocalPort(port: number): WindowsTcpDropResult { if (process.platform !== "win32" || !Number.isFinite(port) || port <= 0) { - return { dropped: 0, skippedIpv6: 0 }; + return { dropped: 0, skippedIpv6: 0, accessDenied: 0 }; } const setTcpEntry = loadSetTcpEntry(); - if (!setTcpEntry) return { dropped: 0, skippedIpv6: 0 }; + if (!setTcpEntry) return { dropped: 0, skippedIpv6: 0, accessDenied: 0 }; let output = ""; try { output = readNetstatAno(); } catch { - return { dropped: 0, skippedIpv6: 0 }; + return { dropped: 0, skippedIpv6: 0, accessDenied: 0 }; } const rows = parseTcpQuadsForLocalPort(output, Math.trunc(port)); let dropped = 0; let skippedIpv6 = 0; + let accessDenied = 0; for (const row of rows) { const localDw = ipv4ToWinUint32(row.localAddr); const remoteDw = ipv4ToWinUint32(row.remoteAddr); @@ -165,10 +172,13 @@ export function dropWindowsTcpRowsForLocalPort(port: number): WindowsTcpDropResu view.setUint32(12, remoteDw, true); view.setUint32(16, htons(row.remotePort), true); try { - if (setTcpEntry(ptr(buf)) === 0) dropped += 1; + const rc = setTcpEntry(ptr(buf)); + if (rc === 0) dropped += 1; + // 5 = ERROR_ACCESS_DENIED, 317 = non-elevated SetTcpEntry (MSDN). + else if (rc === 5 || rc === 317) accessDenied += 1; } catch { /* keep going */ } } - return { dropped, skippedIpv6 }; + return { dropped, skippedIpv6, accessDenied }; } diff --git a/src/service.ts b/src/service.ts index bdf614079..a066ac75b 100644 --- a/src/service.ts +++ b/src/service.ts @@ -1862,6 +1862,15 @@ export function isServiceInstalled(): boolean { return diagnoseService().installed; } +/** + * True when an installed background service can actually supervise the proxy. + * Presence alone is not enough: stale/missing assets, conflicts, and disabled + * registrations report `installed` but will not bring the proxy back after exit. + */ +export function isServiceViable(): boolean { + return diagnoseService().viable; +} + export interface ServiceDiagnostic { supported: boolean; installed: boolean; diff --git a/src/update/index.ts b/src/update/index.ts index 41bc64b1a..4a7033d01 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -321,16 +321,35 @@ export async function runUpdate(): Promise { windowsHide: true, }); if (svcStdio === "pipe") logSpawnOutput("", svc); - if (svc.status !== 0) { + const serviceRefreshed = svc.status === 0; + let serviceViable = serviceRefreshed; + if (serviceRefreshed) { + try { + const { isServiceViable } = await import("../service"); + serviceViable = isServiceViable(); + } catch { + serviceViable = false; + } + } + if (!serviceRefreshed || !serviceViable) { // On Windows, schtasks /create requires elevation. The CLI inherits the // user's (non-admin) token, so the service reinstall can fail with access - // denied. Fall back to a direct detached proxy start so the update never - // leaves the user without a running proxy — but only when the port is free. + // denied — or exit 0 while leaving stale/missing assets that never start + // the proxy. Fall back to a direct detached proxy start so the update + // never leaves the user without a running proxy — but only when the port is free. if (!freed) { - console.warn("⚠️ Service refresh failed and the captured port is still busy; not starting on another port."); + console.warn( + serviceRefreshed + ? "⚠️ Service refresh left a non-viable manager and the captured port is still busy; not starting on another port." + : "⚠️ Service refresh failed and the captured port is still busy; not starting on another port.", + ); console.warn(` Run 'ocx service install' as administrator, then 'ocx start --port ${capturedListen.port}'.`); } else { - console.warn("⚠️ Service refresh failed — starting the proxy directly instead."); + console.warn( + serviceRefreshed + ? "⚠️ Service refresh left a non-viable manager (stale or missing assets) — starting the proxy directly instead." + : "⚠️ Service refresh failed — starting the proxy directly instead.", + ); console.warn(" Run 'ocx service install' as administrator to refresh the background service."); const env = { ...process.env }; delete env.OCX_SERVICE; diff --git a/src/update/job.ts b/src/update/job.ts index e16b80240..d0482af28 100644 --- a/src/update/job.ts +++ b/src/update/job.ts @@ -1,12 +1,27 @@ -import { spawn, spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, readFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { spawn, spawnSync, type ChildProcess } from "node:child_process"; +import { existsSync, mkdirSync, openSync, readFileSync, writeSync } from "node:fs"; +import { dirname, join, sep } from "node:path"; import { fileURLToPath } from "node:url"; -import { atomicWriteFile, getConfigDir, loadConfig, readPid, readRuntimePort } from "../config"; +import { + atomicWriteFile, + getConfigDir, + loadConfig, + readPid, + readRuntimePort, + removePid, + removeRuntimePort, + verifyPidIdentity, +} from "../config"; import { isProcessAlive, killProxy } from "../lib/process-control"; -import { reclaimListenPort } from "../server/port-reclaim"; +import { + buildWindowsElevatedArgumentList, + resolveTrustedWindowsPowerShellExe, +} from "../lib/windows-elevation"; +import { stopWinswService } from "../lib/winsw"; +import { listListenPids, reclaimListenPort, scanListenPids, type ListenPidScan } from "../server/port-reclaim"; +import { dropWindowsTcpRowsForLocalPort } from "../server/windows-tcp-drop"; import { isOpencodexHealthz, probeHostname, proxyIdentityAt, type HealthzIdentity } from "../server/proxy-liveness"; -import { isServiceInstalled } from "../service"; +import { isServiceInstalled, isServiceViable, readServiceBackend, stopWindows } from "../service"; import { type Channel, type Installer, @@ -102,9 +117,102 @@ function nodeBin(): string { return process.platform === "win32" ? "node.exe" : "node"; } +/** + * Strict bind script: exit 0 only after listen+close. Any listen error (including + * Windows ghost-TCB failures under Bun) is busy — matches published `ocx start` + * probes that treat every listen error as unavailable. + */ +function strictBindProbeScript(port: number, hostname: string): string { + return [ + "const net=require('net');", + "const s=net.createServer();", + "s.once('error',()=>process.exit(2));", + `s.listen(${Math.trunc(port)},${JSON.stringify(hostname)},()=>s.close(()=>process.exit(0)));`, + "setTimeout(()=>process.exit(3),2500);", + ].join(""); +} + +function spawnBindProbe(bin: string, script: string): boolean { + try { + const r = spawnSync(bin, ["-e", script], { + windowsHide: true, + timeout: 4000, + stdio: "ignore", + }); + return r.status === 0; + } catch { + return false; + } +} + +/** + * Live global package Bun — not the npm rename tree the update worker may still + * be executing from (`@bitkyc08/.opencodex-*`). + */ +function livePackageBunPath(): string | null { + const launcher = packageLauncherPath(); + const root = join(dirname(launcher), ".."); + for (const name of ["bun.exe", "bun"]) { + const candidate = join(root, "node_modules", "bun", "bin", name); + if (existsSync(candidate)) return candidate; + } + return null; +} + +/** + * Port is free for post-update `ocx start` only when the runtime that will + * actually execute the start can bind. Prefer live package Bun; fall back to the + * worker runtime. Do not require a separate `node` binary (Bun-only installs). + */ +async function strictRuntimePortAvailable(port: number, hostname = "127.0.0.1"): Promise { + const script = strictBindProbeScript(port, hostname); + const bun = livePackageBunPath(); + if (bun) return spawnBindProbe(bun, script); + return spawnBindProbe(process.execPath, script); +} + +/** + * Wait until netstat reports no LISTEN owners on `port` AND the start runtime + * can bind. Dead PIDs still appear as holders while the ghost TCB lives; + * SetTcpEntry is a no-op without elevation (rc 317), so wait them out. + */ +async function waitForGhostListenClear( + port: number, + hostname: string, + listPids: (port: number) => number[], + timeoutMs: number, + sleep: (ms: number) => Promise, + aliveFn: (pid: number) => boolean = isProcessAlive, +): Promise<{ ok: boolean; accessDenied: boolean }> { + const deadline = Date.now() + timeoutMs; + let accessDenied = false; + while (Date.now() < deadline) { + const holders = listPids(port).filter(pid => pid !== process.pid); + const liveHolders = holders.filter(pid => aliveFn(pid)); + // Never SetTcpEntry while a live process still owns the port (foreign or ocx). + if (process.platform === "win32" && liveHolders.length === 0) { + try { + const drop = dropWindowsTcpRowsForLocalPort(port); + if (drop.accessDenied > 0) accessDenied = true; + } catch { /* best-effort */ } + } + if (holders.length === 0 && await strictRuntimePortAvailable(port, hostname)) { + return { ok: true, accessDenied }; + } + await sleep(500); + } + return { ok: false, accessDenied }; +} + function packageLauncherPath(): string { // This module lives at src/update/job.ts — the launcher is /bin/ocx.mjs. - return join(dirname(fileURLToPath(import.meta.url)), "..", "..", "bin", "ocx.mjs"); + // After `npm install -g`, import.meta.url can still point at npm's renamed temp + // tree (`@bitkyc08/.opencodex-*`). Prefer the live package path when that happens. + const fromMeta = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "bin", "ocx.mjs"); + if (!/[\\/]\.opencodex-/i.test(fromMeta) && existsSync(fromMeta)) return fromMeta; + const live = fromMeta.replace(/[\\/]@bitkyc08[\\/]\.opencodex-[^\\/]+/i, `${sep}@bitkyc08${sep}opencodex`); + if (live !== fromMeta && existsSync(live)) return live; + return fromMeta; } function formatCommand(bin: string, args: string[]): string { @@ -267,18 +375,68 @@ export function staleActiveUpdateJobReason( return null; } -const defaultStartUpdateJobDeps: StartUpdateJobDeps = { - checkForUpdateFn: channel => checkForUpdate(channel), - spawnWorkerFn: (jobId, channel, restart) => spawn( - process.execPath, - [process.argv[1], "__gui-update-worker", jobId, channel, restart ? "restart" : "no-restart"], - { +/** + * Spawn the GUI update worker without inheriting the proxy's LISTEN socket. + * + * On Windows, `spawn(..., { detached: true, stdio: "ignore" })` still inherits + * inheritable handles — including Bun.serve's LISTEN socket. After stop-first + * update kills the proxy PID, netstat keeps showing that dead PID as LISTENING + * until every inheriting child exits. The update worker was that child, so the + * port stayed busy for the whole job. Launch via PowerShell Start-Process so + * the worker is a fresh process tree with no inherited LISTEN handle. + */ +export function spawnGuiUpdateWorker( + jobId: string, + channel: Channel, + restart: boolean, +): UpdateWorkerProcess { + const script = process.argv[1]; + const args = [ + script, + "__gui-update-worker", + jobId, + channel, + restart ? "restart" : "no-restart", + ]; + if (process.platform !== "win32") { + return spawn(process.execPath, args, { detached: true, stdio: "ignore", windowsHide: true, env: { ...process.env, OCX_SERVICE: "1" }, - }, - ), + }); + } + + // Single -ArgumentList string with CommandLineToArgvW quoting so paths with + // spaces survive Start-Process's space-join (array elements lose outer quotes). + const psQuote = (value: string): string => `'${value.replace(/'/g, "''")}'`; + const argumentList = buildWindowsElevatedArgumentList(args); + const ps = [ + `$env:OCX_SERVICE = '1'`, + `$p = Start-Process -FilePath ${psQuote(process.execPath)} -ArgumentList ${psQuote(argumentList)} -WindowStyle Hidden -PassThru`, + `if (-not $p) { exit 1 }`, + `Write-Output $p.Id`, + ].join("; "); + const launched = spawnSync( + resolveTrustedWindowsPowerShellExe(), + ["-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", "-Command", ps], + { encoding: "utf8", windowsHide: true, timeout: 15_000 }, + ); + const pid = Number(String(launched.stdout ?? "").trim().split(/\r?\n/).pop()); + if (launched.status !== 0 || !Number.isSafeInteger(pid) || pid <= 0) { + const detail = String(launched.stderr ?? launched.stdout ?? "").trim() || `status ${launched.status}`; + throw new Error(`Windows update worker Start-Process failed: ${detail}`); + } + return { + pid, + unref() { /* Start-Process already detached */ }, + once() { /* startup errors are not wired across Start-Process */ }, + }; +} + +const defaultStartUpdateJobDeps: StartUpdateJobDeps = { + checkForUpdateFn: channel => checkForUpdate(channel), + spawnWorkerFn: spawnGuiUpdateWorker, isProcessAliveFn: isProcessAlive, nowMs: Date.now, }; @@ -369,18 +527,90 @@ function runLoggedCommand(job: UpdateJobState, bin: string, args: string[], time return { status: result.status, signal: result.signal }; } -function spawnDetachedStart(job: UpdateJobState, installer: Installer, port?: number): void { +/** + * Tear down anything that would make `ocx start` exit 1 with "already running" + * (service wrapper respawn, stale pidfile + live /healthz) before a pinned spawn. + */ +function preparePortForPinnedStart( + job: UpdateJobState, + port: number, + listPids: (port: number) => number[], + aliveFn: (pid: number) => boolean, + verifyOcx: (pid: number) => number | null = verifyPidIdentity, +): void { + stopWindowsServiceWrappersBestEffort(); + const pid = readPid(); + if (pid) { + updateJob(job, {}, `Clearing pre-start proxy PID ${pid} before pinned start.`); + try { killProxy(pid); } catch { /* best-effort */ } + removePid(pid); + } else { + removePid(); + } + removeRuntimePort(); + for (const holder of listPids(port)) { + if (holder === process.pid || !aliveFn(holder)) continue; + if (verifyOcx(holder) !== holder) { + updateJob( + job, + {}, + `Leaving foreign listen holder PID ${holder} on port ${port}; refusing collateral kill.`, + ); + continue; + } + updateJob(job, {}, `Stopping live ocx listen holder PID ${holder} on port ${port} before pinned start.`); + try { killProxy(holder); } catch { /* best-effort */ } + } + // Match reclaimListenPort: never SetTcpEntry while a live holder remains. + const liveRemain = listPids(port).filter(pid => pid !== process.pid && aliveFn(pid)); + if (process.platform === "win32" && liveRemain.length === 0) { + try { dropWindowsTcpRowsForLocalPort(port); } catch { /* best-effort */ } + } +} + +function spawnDetachedStart( + job: UpdateJobState, + installer: Installer, + port?: number, +): ChildProcess { const cmd = restartCommand(false, installer, packageLauncherPath(), port); const env = { ...process.env }; delete env.OCX_SERVICE; updateJob(job, {}, `$ ${cmd.display}`); + let stdio: "ignore" | [ "ignore", number, number ] = "ignore"; + let logFd: number | undefined; + try { + const logPath = join(getConfigDir(), "update-pinned-start.log"); + mkdirSync(getConfigDir(), { recursive: true }); + logFd = openSync(logPath, "a"); + writeSync(logFd, `\n--- ${new Date().toISOString()} ---\n$ ${cmd.display}\n`); + stdio = ["ignore", logFd, logFd]; + } catch { /* fall back to ignored stdio */ } const child = spawn(cmd.bin, cmd.args, { detached: true, - stdio: "ignore", + stdio, windowsHide: true, env, }); + child.once("error", err => { + try { + updateJob(job, {}, `Pinned start spawn error: ${err instanceof Error ? err.message : String(err)}`); + } catch { /* best-effort */ } + }); + // Foreground `ocx start` keeps the listen process; EADDRINUSE/ghost races exit quickly + // with stdio ignored — surface that so the job log explains a silent miss. + child.once("exit", (code, signal) => { + if (code === 0 && !signal) return; + try { + updateJob( + job, + {}, + `Pinned start exited early (code=${code ?? "null"} signal=${signal ?? "null"}).`, + ); + } catch { /* best-effort */ } + }); child.unref(); + return child; } /** Identity snapshot used to prove an npm self-update actually replaced the pre-update process. */ @@ -394,9 +624,17 @@ export interface RestartIo { waitForPort?: typeof reclaimListenPort; spawnStart?: (job: UpdateJobState, installer: Installer, port?: number) => void; serviceInstalledFn?: () => boolean; + /** + * After a service reinstall exits 0, only trust the service path when this is true. + * Defaults to {@link isServiceViable} — installed-but-stale assets must fall through + * to a direct proxy start so dashboard updates never leave /healthz dead. + */ + serviceViableFn?: () => boolean; probeProxy?: (port: number, hostname?: string) => Promise; /** Richer /healthz read for update-correlated restart evidence (pid + version). */ probeProxyIdentity?: (port: number, hostname?: string) => Promise; + /** Override the /healthz appearance window (default {@link RESTART_HEALTH_TIMEOUT_MS}). */ + healthTimeoutMs?: number; sleepMs?: (ms: number) => Promise; now?: () => number; /** Service-mode install/reinstall command (defaults to spawnSync via runLoggedCommand). */ @@ -411,6 +649,21 @@ export interface RestartIo { captured?: { port: number; hostname: string; oldPid?: number }, io?: RestartIo, ) => Promise; + /** + * PIDs currently LISTENing on the captured port. Used to widen the post-update + * kill allowlist beyond the pre-update PID (Windows often leaves a respawned + * ocx child that would otherwise be treated as a protected listener). + */ + listListenPidsFn?: (port: number) => number[]; + /** + * Full listen-PID scan (ok/fail). When omitted, {@link scanListenPids} is used + * so a probe failure is not mistaken for "no listeners". + */ + scanListenPidsFn?: (port: number) => ListenPidScan; + /** Identity check for listeners discovered via {@link listListenPidsFn}. */ + verifyOcxFn?: (pid: number) => number | null; + /** Liveness check when deciding whether a reclaim timeout still has live holders. */ + isAliveFn?: (pid: number) => boolean; } async function restartAfterUpdate( @@ -437,60 +690,291 @@ async function restartAfterUpdate( } const cmd = restartCommand(serviceInstalled, job.installer, packageLauncherPath(), port, svcArgs); const waitFn = io.waitForPort ?? reclaimListenPort; - const reclaimOpts = { + const listPids = io.listListenPidsFn ?? listListenPids; + const verifyOcx = io.verifyOcxFn ?? verifyPidIdentity; + const aliveFn = io.isAliveFn ?? isProcessAlive; + // Pre-update PID plus any ocx still LISTENing on the captured port. After a + // stop-first npm self-update Windows often leaves a respawned bun/node child + // that is not the captured PID; treating it as protected blocks reclaim and + // the direct-start fallback never binds. + const reclaimKillAllowlist = (): number[] => { + const allow = new Set(); + if (oldPid != null) allow.add(oldPid); + for (const pid of listPids(port)) { + if (pid === process.pid) continue; + if (verifyOcx(pid) === pid) allow.add(pid); + } + return [...allow]; + }; + const reclaimOptsFor = (onlyKillPids: number[]) => ({ timeoutMs: RESTART_PORT_RECLAIM_MS, intervalMs: 100, scanIntervalMs: 500, - killOcxHolders: oldPid != null, - onlyKillPids: oldPid != null ? [oldPid] : [], - }; + killOcxHolders: true, + // Windows scheduler wrappers can mint a *new* bun PID during the wait; keep + // killing every ocx listener on this port, not only the pre-wait snapshot. + // npm rename trees under `@bitkyc08/.opencodex-*` are classified as ocx by + // isOcxStartCommandLine — never kill unknown foreign claimants on this port. + killAllOcxOnPort: true, + onlyKillPids, + }); if (serviceInstalled) { - // Stop-first update already unloaded the service; reclaim the socket (only the - // captured old PID when trusted), then reinstall wrappers that bake `--port`. - const freed = await waitFn(port, hostname, reclaimOpts); - if (!freed) { - updateJob(job, {}, `Port ${port} still busy after ${Math.trunc(RESTART_PORT_RECLAIM_MS / 1000)}s; refusing to hop — reinstall may fail until the port is free.`); + // schtasks /end often leaves the hidden cmd/wscript wrapper alive; its :loop + // respawns `ocx start` a few seconds later and races port reclaim. End the + // task again and best-effort kill those wrappers before we touch the socket. + stopWindowsServiceWrappersBestEffort(); + // Stop-first update already unloaded the service; reclaim the socket, then + // reinstall wrappers that bake `--port`. + const preServiceAllow = reclaimKillAllowlist(); + const freed = await waitFn(port, hostname, reclaimOptsFor(preServiceAllow)); + let skipServiceInstall = false; + // Windows GUI update worker sets OCX_SERVICE=1 and is never elevated. + // `schtasks /create` will UAC-fail and can race the subsequent direct start. + // Keep systemd/launchd reinstall on non-Windows supervisors. + if (process.platform === "win32" && process.env.OCX_SERVICE === "1") { + updateJob(job, {}, "Skipping service reinstall from the non-elevated update worker; falling back to a direct proxy start."); + skipServiceInstall = true; } - const prevBake = process.env.OCX_BAKE_PORT; - process.env.OCX_BAKE_PORT = String(Math.trunc(port)); - let serviceOk = false; - try { - const run = io.runService ?? ((j, bin, args) => runLoggedCommand(j, bin, args, RESTART_TIMEOUT_MS)); - const result = run(job, cmd.bin, cmd.args); - serviceOk = result.status === 0; - if (!serviceOk) { - // On Windows, `schtasks /create` requires an elevated token. The update worker - // inherits the (non-admin) proxy's privileges, so a service-managed install - // updated from the GUI or a normal terminal fails here with access denied. - // Falling back to a direct proxy start keeps the update from leaving the proxy - // stopped; the stale service manager can be refreshed later with an admin - // `ocx service install`. - updateJob(job, {}, `Service reinstall failed (exit ${result.status ?? "?"}); falling back to a direct proxy start. Run 'ocx service install' as administrator to refresh the background service manager.`); + if (!freed && !skipServiceInstall) { + updateJob( + job, + {}, + `Port ${port} still busy after ${Math.trunc(RESTART_PORT_RECLAIM_MS / 1000)}s; refusing to hop — reinstall may fail until the port is free.` + + ` ${formatPortHolders(port, listPids, verifyOcx, preServiceAllow)}`, + ); + const liveAfter = listPids(port).filter(pid => pid !== process.pid && aliveFn(pid)); + if (liveAfter.length === 0) { + // Non-elevated `service install` will UAC-fail anyway; skip straight to + // the direct-start fallthrough instead of burning another minute on it. + updateJob(job, {}, "Skipping service reinstall after reclaim timeout with no live holders; falling back to a direct proxy start."); + skipServiceInstall = true; + } + } + if (!skipServiceInstall) { + const prevBake = process.env.OCX_BAKE_PORT; + process.env.OCX_BAKE_PORT = String(Math.trunc(port)); + let serviceOk = false; + try { + const run = io.runService ?? ((j, bin, args) => runLoggedCommand(j, bin, args, RESTART_TIMEOUT_MS)); + const result = run(job, cmd.bin, cmd.args); + serviceOk = result.status === 0; + if (!serviceOk) { + // On Windows, `schtasks /create` requires an elevated token. The update worker + // inherits the (non-admin) proxy's privileges, so a service-managed install + // updated from the GUI or a normal terminal fails here with access denied. + // Falling back to a direct proxy start keeps the update from leaving the proxy + // stopped; the stale service manager can be refreshed later with an admin + // `ocx service install`. + updateJob(job, {}, `Service reinstall failed (exit ${result.status ?? "?"}); falling back to a direct proxy start. Run 'ocx service install' as administrator to refresh the background service manager.`); + } + } finally { + if (prevBake === undefined) delete process.env.OCX_BAKE_PORT; + else process.env.OCX_BAKE_PORT = prevBake; + } + if (serviceOk) { + // Exit 0 is not enough: a reinstall can leave stale/missing assets (or a + // disabled/conflicting manager) that never brings /healthz back. Fall through + // to a direct start so browser-dashboard updates do not require a viable + // Background Service for recovery. + const viable = (io.serviceViableFn ?? isServiceViable)(); + if (viable) return; + updateJob( + job, + {}, + "Service reinstall exited 0 but the background service is not viable (stale or missing assets, disabled, or conflicting); falling back to a direct proxy start.", + ); } - } finally { - if (prevBake === undefined) delete process.env.OCX_BAKE_PORT; - else process.env.OCX_BAKE_PORT = prevBake; } - if (serviceOk) return; // Fall through to the direct proxy start below so the update never leaves the - // proxy stopped when the service reinstall could not run. + // proxy stopped when the service reinstall could not run or did not leave a + // viable supervisor. } const pid = readPid(); if (pid) { updateJob(job, {}, `Stopping current proxy PID ${pid}.`); - killProxy(pid); + try { + killProxy(pid); + } catch { + // A PID that resists taskkill must not abort recovery: reclaim + pinned start + // below are the path that repairs stuck Windows listeners. + } } + if (serviceInstalled) stopWindowsServiceWrappersBestEffort(); // Reclaim the captured port before the pinned start. Spawning `--port` while the old // socket is still busy is how Windows updates used to fail health checks (or hop). - // Only the trusted pre-update PID may be killed; never an arbitrary ocx listener. - const freed = await waitFn(port, hostname, reclaimOpts); + // killAllOcxOnPort covers wrapper-respawned bun PIDs minted during the wait. + const directAllow = reclaimKillAllowlist(); + const freed = await waitFn(port, hostname, reclaimOptsFor(directAllow)); if (!freed) { - updateJob(job, {}, `Port ${port} still busy after ${Math.trunc(RESTART_PORT_RECLAIM_MS / 1000)}s (reclaim could not free the socket); not starting on another port. Retry 'ocx start --port ${port}'.`); + const liveHolders = listPids(port).filter(pid => pid !== process.pid && aliveFn(pid)); + updateJob( + job, + {}, + `Port ${port} still busy after ${Math.trunc(RESTART_PORT_RECLAIM_MS / 1000)}s (reclaim could not free the socket).` + + ` ${formatPortHolders(port, listPids, verifyOcx, directAllow)}`, + ); + if (liveHolders.length > 0) { + updateJob(job, {}, `Live holder(s) remain on port ${port}; not starting on another port. Retry 'ocx start --port ${port}'.`); + return; + } + // Dead PIDs can still own LISTEN rows. SetTcpEntry needs elevation (rc 317 on a + // normal update worker), so poll until netstat is empty and the start runtime can bind. + updateJob( + job, + {}, + `No live holders on port ${port}; waiting for ghost LISTEN rows to clear before pinned start.`, + ); + // Injected spawnStart is the unit-test seam — skip the long OS wait. + if (!io.spawnStart) { + const sleep = io.sleepMs ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); + const cleared = await waitForGhostListenClear(port, hostname, listPids, 90_000, sleep); + if (cleared.accessDenied) { + updateJob(job, {}, "SetTcpEntry is non-elevated (access denied); relying on OS ghost-LISTEN expiry."); + } + if (!cleared.ok) { + updateJob( + job, + {}, + `Ghost LISTEN rows on port ${port} did not clear in time. ` + + `${formatPortHolders(port, listPids, verifyOcx, directAllow)} ` + + `Retry 'ocx start --port ${port}'.`, + ); + return; + } + } + } + // Injected spawnStart keeps unit tests deterministic (one call). Production path + // retries on missing /healthz after prepare + ghost-LISTEN clear. + if (io.spawnStart) { + io.spawnStart(job, job.installer, port); return; } - (io.spawnStart ?? spawnDetachedStart)(job, job.installer, port); + const sleep = io.sleepMs ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); + const probe = io.probeProxy ?? (async (p: number, host?: string) => ( + !!(await proxyIdentityAt(p, { hostname: host })) + )); + const probeIdentity = io.probeProxyIdentity ?? defaultProbeProxyIdentity; + const expectedVersion = typeof job.latestVersion === "string" && job.latestVersion.length > 0 + ? job.latestVersion + : null; + // Service wrappers can respawn a listener during reclaim; if it already reports the + // update target version, do not spawn a second start that exits "already running". + { + const identity = await probeIdentity(port, hostname); + if (identity && expectedVersion && identity.version === expectedVersion) { + updateJob( + job, + {}, + `Proxy already healthy on ${hostname}:${port} at ${expectedVersion}; skipping pinned start.`, + ); + return; + } + } + const attempts = 3; + // Longer than published hard-pin reclaim (30s) so a slow start can still report healthy. + const perAttemptHealthMs = 70_000; + let lastChild: ChildProcess | null = null; + for (let attempt = 1; attempt <= attempts; attempt++) { + if (attempt > 1) { + updateJob( + job, + {}, + `Pinned start attempt ${attempt - 1} did not become healthy on port ${port}; ` + + `retrying (${attempt}/${attempts}).`, + ); + if (lastChild?.pid && aliveFn(lastChild.pid)) { + try { killProxy(lastChild.pid); } catch { /* best-effort */ } + } + lastChild = null; + } + preparePortForPinnedStart(job, port, listPids, aliveFn, verifyOcx); + const ready = await waitForGhostListenClear( + port, + hostname, + listPids, + attempt === 1 ? (freed ? 15_000 : 5_000) : 30_000, + sleep, + ); + if (!ready.ok) { + updateJob( + job, + {}, + `Port ${port} not bindable before pinned start attempt ${attempt}; ` + + `${formatPortHolders(port, listPids, verifyOcx, directAllow)}`, + ); + continue; + } + lastChild = spawnDetachedStart(job, job.installer, port); + const healthDeadline = Date.now() + perAttemptHealthMs; + while (Date.now() < healthDeadline) { + if (await probe(port, hostname)) return; + await sleep(500); + } + } + // Exhausted retries: do not leave a hung pinned-start child owning the port. + if (lastChild?.pid && aliveFn(lastChild.pid)) { + try { killProxy(lastChild.pid); } catch { /* best-effort */ } + } +} + +/** Compact listen-holder summary for update-job logs when reclaim fails. */ +function formatPortHolders( + port: number, + listPids: (port: number) => number[], + verifyOcx: (pid: number) => number | null, + allow: number[], +): string { + const allowSet = new Set(allow); + const holders = listPids(port).map(pid => { + const tags = [ + verifyOcx(pid) === pid ? "ocx" : "foreign", + allowSet.has(pid) ? "allow" : "deny", + isProcessAlive(pid) ? "live" : "dead", + ]; + return `${pid}(${tags.join(",")})`; + }); + return `holders=[${holders.join(", ") || "none"}] allow=[${allow.join(", ") || "none"}]`; +} + +/** Stop the installed Windows backend and best-effort kill surviving :loop wrappers. */ +function stopWindowsServiceWrappersBestEffort(): void { + if (process.platform !== "win32") return; + try { + if (readServiceBackend() === "native") { + stopWinswService(); + return; + } + stopWindows(); + } catch { /* already stopped */ } + killWindowsServiceWrapperProcesses(); +} + +/** + * Best-effort termination of surviving Windows scheduler launcher/wrapper processes. + * `schtasks /end` ends the task instance but often leaves wscript/cmd running the + * `:loop` batch, which brings the proxy back during post-update reclaim. + */ +function killWindowsServiceWrapperProcesses(): void { + if (process.platform !== "win32") return; + try { + const ps = [ + "$pats = @('opencodex-service.cmd','opencodex-service-launcher.vbs');", + "Get-CimInstance Win32_Process | Where-Object {", + " if ($_.ProcessId -eq $PID) { return $false };", + " $c = $_.CommandLine; if (-not $c) { return $false };", + " foreach ($p in $pats) { if ($c -like ('*' + $p + '*')) { return $true } };", + " $false", + "} | ForEach-Object { Stop-Process -Id $_.ProcessId -Force -ErrorAction SilentlyContinue }", + ].join(" "); + spawnSync(resolveTrustedWindowsPowerShellExe(), [ + "-NoProfile", "-NoLogo", "-NonInteractive", "-WindowStyle", "Hidden", + "-Command", ps, + ], { stdio: "ignore", timeout: 5000, windowsHide: true }); + } catch { + /* best-effort */ + } } /** Exposed for tests: drives the non-service restart path with injected io. */ @@ -522,8 +1006,10 @@ async function awaitRestartedProxyHealthy( captured: { port: number; hostname: string }, io: RestartIo = {}, ): Promise { + // Fresh post-update starts are busy with catalog sync / OAuth; a single 750ms + // /healthz miss must not fail the job. Use a longer probe and tolerate brief blips. const probe = io.probeProxy ?? (async (port: number, hostname?: string) => ( - !!(await proxyIdentityAt(port, { hostname })) + !!(await proxyIdentityAt(port, { hostname }, { timeoutMs: 2_000, attempts: 3 })) )); const sleep = io.sleepMs ?? (async (ms: number) => { await new Promise(resolve => setTimeout(resolve, ms)); @@ -531,7 +1017,9 @@ async function awaitRestartedProxyHealthy( const now = io.now ?? (() => Date.now()); const port = captured.port; const hostname = captured.hostname; - const startDeadline = now() + RESTART_HEALTH_TIMEOUT_MS; + const startDeadline = now() + (io.healthTimeoutMs ?? RESTART_HEALTH_TIMEOUT_MS); + /** Consecutive failed probes before the stability window counts as a flap. */ + const stabilityMissLimit = 3; while (true) { // Always make one identity-aware probe at or after the boundary. A replacement @@ -540,10 +1028,16 @@ async function awaitRestartedProxyHealthy( if (await probe(port, hostname)) { updateJob(job, {}, `Proxy reported healthy on ${hostname}:${port}; confirming it stays up...`); const stableUntil = now() + RESTART_STABILITY_WINDOW_MS; + let misses = 0; while (now() < stableUntil) { - if (!(await probe(port, hostname))) { - updateJob(job, {}, `Proxy became unhealthy on ${hostname}:${port} during the stability window.`); - return { ok: false, reason: "flapped" }; + if (await probe(port, hostname)) { + misses = 0; + } else { + misses += 1; + if (misses >= stabilityMissLimit) { + updateJob(job, {}, `Proxy became unhealthy on ${hostname}:${port} during the stability window.`); + return { ok: false, reason: "flapped" }; + } } await sleep(500); } @@ -679,6 +1173,10 @@ export function npmSelfUpdateRestartEvidence( * and/or target version) so a surviving pre-update process cannot look like success. * After an explicit npm restart the same evidence is required again — health alone is * not enough when a no-op restart or failed port reclaim leaves the old proxy up. + * + * Browser-dashboard update recovery must not require a viable Background Service: when + * no service is installed (or reinstall leaves a non-viable/stale manager), the explicit + * path always falls through to a direct `ocx start --port` so /healthz can recover. */ export async function finishGuiUpdateRestart( job: UpdateJobState, @@ -689,28 +1187,53 @@ export async function finishGuiUpdateRestart( if (installer === "npm") { const serviceInstalled = (io.serviceInstalledFn ?? isServiceInstalled)(); if (serviceInstalled) { - const already = await awaitRestartedProxyHealthy(job, captured, io); - if (already.ok) { - const identity = await (io.probeProxyIdentity ?? defaultProbeProxyIdentity)( - captured.port, - captured.hostname, - ); - const evidence = npmSelfUpdateRestartEvidence(job, captured, identity); - if (evidence.ok) { + // Stop-first npm update leaves a dead PID's LISTEN row. Polling /healthz for the + // full 30s against that zombie keeps ESTABLISHED TCBs alive and blocks bind. + // If nothing live owns the port, skip straight to explicit restart. A failed + // listener scan must not look like "no listeners" — fall back to /healthz. + const aliveFn = io.isAliveFn ?? isProcessAlive; + const scan: ListenPidScan = io.scanListenPidsFn + ? io.scanListenPidsFn(captured.port) + : io.listListenPidsFn + // Test seam: injected list is always a successful scan. + ? { ok: true, pids: io.listListenPidsFn(captured.port) } + : scanListenPids(captured.port); + const liveListeners = scan.ok + ? scan.pids.filter(pid => pid !== process.pid && aliveFn(pid)) + : null; + if (liveListeners !== null && liveListeners.length === 0) { + updateJob(job, {}, "npm self-update did not leave a live listener; performing explicit restart..."); + } else { + if (!scan.ok) { updateJob( job, {}, - `Proxy already healthy on ${captured.hostname}:${captured.port} after npm self-update (${evidence.detail}); skipping redundant restart.`, + "Listener scan inconclusive after npm self-update; probing /healthz before deciding on explicit restart...", ); - return true; } - updateJob( - job, - {}, - `npm self-update left a healthy proxy but ${evidence.reason}; performing explicit restart...`, - ); - } else { - updateJob(job, {}, "npm self-update did not leave a healthy proxy; performing explicit restart..."); + const already = await awaitRestartedProxyHealthy(job, captured, io); + if (already.ok) { + const identity = await (io.probeProxyIdentity ?? defaultProbeProxyIdentity)( + captured.port, + captured.hostname, + ); + const evidence = npmSelfUpdateRestartEvidence(job, captured, identity); + if (evidence.ok) { + updateJob( + job, + {}, + `Proxy already healthy on ${captured.hostname}:${captured.port} after npm self-update (${evidence.detail}); skipping redundant restart.`, + ); + return true; + } + updateJob( + job, + {}, + `npm self-update left a healthy proxy but ${evidence.reason}; performing explicit restart...`, + ); + } else { + updateJob(job, {}, "npm self-update did not leave a healthy proxy; performing explicit restart..."); + } } } } diff --git a/tests/claude-messages-endpoint.test.ts b/tests/claude-messages-endpoint.test.ts index 50155f22a..9d09e2989 100644 --- a/tests/claude-messages-endpoint.test.ts +++ b/tests/claude-messages-endpoint.test.ts @@ -17,6 +17,7 @@ import { } from "../src/server/claude-messages"; import type { OcxConfig } from "../src/types"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; +import { SERVER_BUDGET_MS } from "./helpers/test-budget"; import { createTestTranslatorBudget } from "./helpers/translator-budget"; let testDir = ""; @@ -142,7 +143,7 @@ test("POST /v1/messages?beta=true streams an Anthropic-shaped turn end to end", server.stop(true); upstream.stop(true); } -}); +}, { timeout: SERVER_BUDGET_MS }); test("non-streaming /v1/messages returns an Anthropic message JSON", async () => { const upstream = mockChatUpstream(); diff --git a/tests/config.test.ts b/tests/config.test.ts index f362fa26a..d8e585fc0 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -1413,6 +1413,10 @@ describe("opencodex config defaults", () => { expect(isOcxStartCommandLine('bun run src/cli.ts start')).toBe(true); expect(isOcxStartCommandLine('"C:/tools/bun/bin/bun.exe" "run" "src/cli/index.ts" "start"')).toBe(true); expect(isOcxStartCommandLine('bun C:/tools/bun/install/global/node_modules/@bitkyc08/opencodex/src/cli.ts start')).toBe(true); + // npm's in-place rename during `npm install -g` (Windows service wrapper respawn mid-update). + expect(isOcxStartCommandLine( + 'bun C:/nvm/node_modules/@bitkyc08/.opencodex-1JejBqbZ/src/cli/index.ts start --port 10100', + )).toBe(true); expect(isOcxStartCommandLine("opencodex start")).toBe(true); expect(isOcxStartCommandLine("bun run src/cli.ts status")).toBe(false); diff --git a/tests/oauth-refresh-generic-lock.test.ts b/tests/oauth-refresh-generic-lock.test.ts index 57570429b..12039fe39 100644 --- a/tests/oauth-refresh-generic-lock.test.ts +++ b/tests/oauth-refresh-generic-lock.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test"; import { mkdirSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -10,6 +10,10 @@ import { import type { OAuthCredentials } from "../src/oauth/types"; import { getAccountCredential, getAccountSet, saveCredential } from "../src/oauth/store"; +// Gate/CAS refresh races can exceed the 5s default under windows-latest contention +// (same flake class as kiro-oauth / oauth queue budgets). +setDefaultTimeout(30_000); + const origHome = process.env.HOME; const origOcxHome = process.env.OPENCODEX_HOME; const origKimiRefresh = OAUTH_PROVIDERS.kimi!.refresh; diff --git a/tests/port-reclaim.test.ts b/tests/port-reclaim.test.ts index a48da461f..5eb607c41 100644 --- a/tests/port-reclaim.test.ts +++ b/tests/port-reclaim.test.ts @@ -63,7 +63,7 @@ describe("parseTcpQuadsForLocalPort / IPv6", () => { // reports skippedIpv6 for parsed IPv6 quads when netstat is readable. Assert the // return shape never coerces IPv6 into a positive dropped count from IPv4 APIs alone. if (process.platform !== "win32") { - expect(dropWindowsTcpRowsForLocalPort(10100)).toEqual({ dropped: 0, skippedIpv6: 0 }); + expect(dropWindowsTcpRowsForLocalPort(10100)).toEqual({ dropped: 0, skippedIpv6: 0, accessDenied: 0 }); } }); }); @@ -195,7 +195,7 @@ describe("reclaimListenPort", () => { }, dropTcpFn: port => { dropped.push(port); - return { dropped: 1, skippedIpv6: 0 }; + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; }, sleepMs: async () => {}, })).resolves.toBe(false); @@ -222,7 +222,7 @@ describe("reclaimListenPort", () => { }, dropTcpFn: port => { dropped.push(port); - return { dropped: 1, skippedIpv6: 0 }; + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; }, sleepMs: async () => {}, })).resolves.toBe(false); @@ -268,7 +268,7 @@ describe("reclaimListenPort", () => { dropTcpFn: port => { dropped.push(port); available = true; - return { dropped: 3, skippedIpv6: 1 }; + return { dropped: 3, skippedIpv6: 1, accessDenied: 0 }; }, sleepMs: async () => {}, })).resolves.toBe(true); @@ -299,60 +299,94 @@ describe("reclaimListenPort", () => { expect(killed).toEqual([4242]); }); - test("skips kill when allowlisted pid fails revalidation", async () => { + test("allowlisted pid with failing ocx revalidation is still killed (trusted teardown PID)", async () => { const killed: number[] = []; + let available = false; let checks = 0; await expect(reclaimListenPort(10100, "127.0.0.1", { - timeoutMs: 80, + timeoutMs: 200, intervalMs: 20, scanIntervalMs: 20, dropTcpRows: false, killOcxHolders: true, onlyKillPids: [100], - isAvailableFn: async () => false, - listListenPidsFn: () => [100], - isAliveFn: () => true, + isAvailableFn: async () => available, + listListenPidsFn: () => (available ? [] : [100]), + isAliveFn: () => !available, verifyOcxFn: pid => { checks += 1; - // First pass (scan identity) succeeds; revalidation immediately before kill fails. + // First pass (scan identity) succeeds; later scans reclassify as non-ocx. + // Allowlisted teardown PIDs still take the best-effort kill path. return checks === 1 ? pid : null; }, killFn: pid => { killed.push(pid); + available = true; }, sleepMs: async () => {}, - })).resolves.toBe(false); - expect(killed).toEqual([]); + })).resolves.toBe(true); + expect(killed).toEqual([100]); }); - test("does not drop TCP rows when allowlisted revalidation fails", async () => { + test("allowlisted revalidation failure still permits TCP drop after kill", async () => { const killed: number[] = []; const dropped: number[] = []; + let alive = true; + let available = false; let checks = 0; await expect(reclaimListenPort(10100, "127.0.0.1", { - timeoutMs: 80, + timeoutMs: 200, intervalMs: 20, scanIntervalMs: 20, dropTcpRows: true, killOcxHolders: true, onlyKillPids: [100], - isAvailableFn: async () => false, - listListenPidsFn: () => [100], - isAliveFn: () => true, + isAvailableFn: async () => available, + listListenPidsFn: () => (alive ? [100] : []), + isAliveFn: () => alive, verifyOcxFn: pid => { checks += 1; return checks === 1 ? pid : null; }, killFn: pid => { killed.push(pid); + alive = false; }, dropTcpFn: port => { dropped.push(port); - return { dropped: 1, skippedIpv6: 0 }; + available = true; + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; + }, + sleepMs: async () => {}, + })).resolves.toBe(true); + expect(killed).toEqual([100]); + expect(dropped).toEqual([10100]); + }); + + test("does not drop TCP rows while allowlisted non-ocx survives kill", async () => { + const killed: number[] = []; + const dropped: number[] = []; + await expect(reclaimListenPort(10100, "127.0.0.1", { + timeoutMs: 80, + intervalMs: 20, + scanIntervalMs: 20, + dropTcpRows: true, + killOcxHolders: true, + onlyKillPids: [100], + isAvailableFn: async () => false, + listListenPidsFn: () => [100], + isAliveFn: () => true, + verifyOcxFn: () => null, + killFn: pid => { + killed.push(pid); + }, + dropTcpFn: port => { + dropped.push(port); + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; }, sleepMs: async () => {}, })).resolves.toBe(false); - expect(killed).toEqual([]); + expect(killed).toEqual([100]); expect(dropped).toEqual([]); }); @@ -374,7 +408,7 @@ describe("reclaimListenPort", () => { }, dropTcpFn: port => { dropped.push(port); - return { dropped: 1, skippedIpv6: 0 }; + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; }, sleepMs: async () => {}, })).resolves.toBe(false); @@ -400,7 +434,7 @@ describe("reclaimListenPort", () => { }, dropTcpFn: port => { dropped.push(port); - return { dropped: 1, skippedIpv6: 0 }; + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; }, sleepMs: async () => {}, })).resolves.toBe(false); @@ -429,10 +463,97 @@ describe("reclaimListenPort", () => { dropTcpFn: port => { dropped.push(port); available = true; - return { dropped: 2, skippedIpv6: 0 }; + return { dropped: 2, skippedIpv6: 0, accessDenied: 0 }; + }, + sleepMs: async () => {}, + })).resolves.toBe(true); + expect(dropped).toEqual([10100]); + }); + + test("killAllOcxOnPort kills ocx listeners absent from the allowlist snapshot", async () => { + const killed: number[] = []; + let holder = 9001; + let available = false; + await expect(reclaimListenPort(10100, "127.0.0.1", { + timeoutMs: 200, + intervalMs: 20, + scanIntervalMs: 20, + dropTcpRows: false, + killOcxHolders: true, + killAllOcxOnPort: true, + onlyKillPids: [100], // pre-update PID — respawned child is 9001 + isAvailableFn: async () => available, + listListenPidsFn: () => (holder > 0 ? [holder] : []), + isAliveFn: pid => pid === holder, + verifyOcxFn: pid => pid, + killFn: pid => { + killed.push(pid); + if (pid === holder) holder = 0; + }, + sleepMs: async () => { + if (holder === 0) available = true; + }, + })).resolves.toBe(true); + expect(killed).toEqual([9001]); + }); + + test("foreign non-ocx claimant survives reclaim without allowlist or ocx identity", async () => { + const killed: number[] = []; + const dropped: number[] = []; + await expect(reclaimListenPort(10100, "127.0.0.1", { + timeoutMs: 80, + intervalMs: 20, + scanIntervalMs: 20, + dropTcpRows: true, + killOcxHolders: true, + killAllOcxOnPort: true, + onlyKillPids: [4242], // trusted old PID — different from the foreign holder + isAvailableFn: async () => false, + listListenPidsFn: () => [777], + isAliveFn: () => true, + verifyOcxFn: () => null, + killFn: pid => { + killed.push(pid); + }, + dropTcpFn: port => { + dropped.push(port); + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; + }, + sleepMs: async () => {}, + })).resolves.toBe(false); + expect(killed).toEqual([]); + expect(dropped).toEqual([]); + }); + + test("allowlisted PID that fails ocx verify still gets killed and does not block TCP drop", async () => { + const killed: number[] = []; + const dropped: number[] = []; + let alive = true; + let available = false; + await expect(reclaimListenPort(10100, "127.0.0.1", { + timeoutMs: 200, + intervalMs: 20, + scanIntervalMs: 20, + dropTcpRows: true, + killOcxHolders: true, + onlyKillPids: [14772], + isAvailableFn: async () => available, + // Windows often keeps a dead pre-update owner listed; cmdline probe already failed. + listListenPidsFn: () => (alive ? [14772] : []), + isAliveFn: () => alive, + verifyOcxFn: () => null, + killFn: pid => { + killed.push(pid); + alive = false; + }, + dropTcpFn: port => { + dropped.push(port); + available = true; + return { dropped: 1, skippedIpv6: 0, accessDenied: 0 }; }, sleepMs: async () => {}, })).resolves.toBe(true); + expect(killed).toEqual([14772]); expect(dropped).toEqual([10100]); }); }); diff --git a/tests/ports.test.ts b/tests/ports.test.ts index f95090bd0..fdf0dae07 100644 --- a/tests/ports.test.ts +++ b/tests/ports.test.ts @@ -118,4 +118,9 @@ describe("port selection", () => { expect(isAddrInUse(null)).toBe(false); expect(isAddrInUse("EADDRINUSE")).toBe(false); }); + + test("isPortAvailable is false for non-EADDRINUSE listen errors (fail closed)", async () => { + // 192.0.2.1 is TEST-NET-1 — typically EADDRNOTAVAIL / not assignable on desktop stacks. + expect(await isPortAvailable(54321, "192.0.2.1")).toBe(false); + }); }); diff --git a/tests/server-management-auth.test.ts b/tests/server-management-auth.test.ts index d63cd6744..336c37c0c 100644 --- a/tests/server-management-auth.test.ts +++ b/tests/server-management-auth.test.ts @@ -485,7 +485,14 @@ describe("management and data-plane credential separation", () => { if (target.endsWith("admin-api-token")) { return { success: true, exitCode: 0, timedOut: false, stdout: "" }; } - return { success: false, exitCode: null, timedOut: true, stdout: "" }; + // Directory ACE carries (OI)(CI). Timeout only directories so management auth + // fails closed while secret file hardens (config.json.tmp) can still + // succeed — startServer must persist config on real Windows. + const grant = args.find(arg => typeof arg === "string" && arg.includes("(F)")) ?? ""; + if (grant.includes("(OI)(CI)")) { + return { success: false, exitCode: null, timedOut: true, stdout: "" }; + } + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; }); resetHardenedStateForTests(); const state = initializeManagementAuthState(remoteConfig()); @@ -549,7 +556,15 @@ describe("management and data-plane credential separation", () => { saveConfig(remoteConfig()); process.env.USERNAME ??= "tester"; setPlatformForTests("win32"); - setIcaclsRunnerForTests(() => ({ success: false, exitCode: null, timedOut: true, stdout: "" })); + // Stall directory ACL only. Env admin bypasses the file-backed token path; + // secret file hardens for config persistence must still succeed on win32. + setIcaclsRunnerForTests(args => { + const grant = args.find(arg => typeof arg === "string" && arg.includes("(F)")) ?? ""; + if (grant.includes("(OI)(CI)")) { + return { success: false, exitCode: null, timedOut: true, stdout: "" }; + } + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); resetHardenedStateForTests(); const state = initializeManagementAuthState(remoteConfig()); diff --git a/tests/service.test.ts b/tests/service.test.ts index e2005372a..bf0ca62d5 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -746,6 +746,15 @@ describe("service diagnostics", () => { expect(deriveWindowsServiceDiagnostic({ ...base, ...installedEnabled, nativeStatus: "started" })).toMatchObject({ viable: false, conflict: true }); expect(deriveWindowsServiceDiagnostic({ ...base, nativeStatus: "stopped" })).toMatchObject({ installed: true, viable: false, startable: false, stale: true, running: false }); expect(deriveWindowsServiceDiagnostic({ ...base, nativeRepairAssetsOnly: true })).toMatchObject({ installed: false, viable: false, stale: true }); + // Missing on-disk assets while the task remains registered — the post-update status line. + const missingAssets = deriveWindowsServiceDiagnostic({ + ...base, + ...installedEnabled, + recordedBackend: "scheduler", + schedulerAssetsPresent: false, + }); + expect(missingAssets).toMatchObject({ installed: true, viable: false, stale: true, startable: false }); + expect(missingAssets.summary).toContain("stale or missing service assets"); }); test("a stopped healthy WinSW service remains startable from the tray", () => { diff --git a/tests/system-restart.test.ts b/tests/system-restart.test.ts index 16c585ceb..709ef0813 100644 --- a/tests/system-restart.test.ts +++ b/tests/system-restart.test.ts @@ -84,6 +84,61 @@ describe("acceptSystemRestart", () => { expect(calls).toEqual(["drain", "exit:1"]); }); + test("OCX_SERVICE with non-viable Background Service uses detached start", async () => { + const calls: string[] = []; + let scheduled: (() => void | Promise) | null = null; + const prev = process.env.OCX_SERVICE; + process.env.OCX_SERVICE = "1"; + + try { + acceptSystemRestart({ + isDraining: () => false, + getActiveTurnCount: () => 0, + // Installed-but-stale assets must NOT count as supervised recovery. + isServiceViable: () => false, + listenPort: () => 10123, + schedule: (fn) => { scheduled = fn; }, + setDraining: () => {}, + drainAndShutdown: async () => { calls.push("drain"); }, + spawnStart: (port) => { calls.push(`start:${port}`); }, + markRecycling: () => { calls.push("recycle"); }, + exitProcess: (code) => { calls.push(`exit:${code}`); }, + }); + + await scheduled!(); + expect(calls).toEqual(["drain", "start:10123", "recycle", "exit:0"]); + } finally { + if (prev === undefined) delete process.env.OCX_SERVICE; + else process.env.OCX_SERVICE = prev; + } + }); + + test("OCX_SERVICE with viable Background Service exits 1 for supervisor respawn", async () => { + const calls: string[] = []; + let scheduled: (() => void | Promise) | null = null; + const prev = process.env.OCX_SERVICE; + process.env.OCX_SERVICE = "1"; + + try { + acceptSystemRestart({ + isDraining: () => false, + getActiveTurnCount: () => 0, + isServiceViable: () => true, + schedule: (fn) => { scheduled = fn; }, + drainAndShutdown: async () => { calls.push("drain"); }, + spawnStart: () => { calls.push("start"); }, + markRecycling: () => { calls.push("recycle"); }, + exitProcess: (code) => { calls.push(`exit:${code}`); }, + }); + + await scheduled!(); + expect(calls).toEqual(["drain", "exit:1"]); + } finally { + if (prev === undefined) delete process.env.OCX_SERVICE; + else process.env.OCX_SERVICE = prev; + } + }); + test("spawn sync throw exits 1 without marking recycle", async () => { const calls: string[] = []; let scheduled: (() => void | Promise) | null = null; diff --git a/tests/translator-budget.test.ts b/tests/translator-budget.test.ts index 6a902d8a4..e0072a664 100644 --- a/tests/translator-budget.test.ts +++ b/tests/translator-budget.test.ts @@ -253,4 +253,5 @@ test("production adapter contract rejects omitted translator budgets at typechec expect(invalid.stdout.toString() + invalid.stderr.toString()).toContain("TS2554"); const valid = Bun.spawnSync(["bun", ...base, "tests/fixtures/translator-budget-required.valid.ts"]); expect(valid.exitCode).toBe(0); -}); + // Two bun x tsc --noEmit spawns exceed Bun's 5s default under windows-latest contention. +}, 60_000); diff --git a/tests/update-job.test.ts b/tests/update-job.test.ts index 301da09c4..d7ac75c16 100644 --- a/tests/update-job.test.ts +++ b/tests/update-job.test.ts @@ -119,7 +119,7 @@ describe("GUI update execution decisions", () => { test("restart waits on the captured pre-update port unconditionally and pins the spawn to it", async () => { // The stop-first update flow clears pid/runtime state before restartAfterUpdate runs, // so the wait must fire even with no readable pid — driven here via the io seam. - const waited: Array<{ port: number; hostname: string; opts?: { killOcxHolders?: boolean; onlyKillPids?: number[] } }> = []; + const waited: Array<{ port: number; hostname: string; opts?: { killOcxHolders?: boolean; onlyKillPids?: number[]; killAllOcxOnPort?: boolean } }> = []; const spawned: Array<{ port?: number }> = []; const job: UpdateJobState = { id: "restart-io", @@ -137,6 +137,7 @@ describe("GUI update execution decisions", () => { writeFileSync(updateJobPath(job.id), JSON.stringify(job)); await restartAfterUpdateForTests(job, { port: 12345, hostname: "127.0.0.1" }, { serviceInstalledFn: () => false, // drive the proxy-mode branch regardless of host state + listListenPidsFn: () => [], waitForPort: async (port, hostname, opts) => { waited.push({ port, @@ -144,6 +145,7 @@ describe("GUI update execution decisions", () => { opts: { killOcxHolders: opts?.killOcxHolders, onlyKillPids: opts?.onlyKillPids, + killAllOcxOnPort: (opts as { killAllOcxOnPort?: boolean } | undefined)?.killAllOcxOnPort, }, }); return true; @@ -155,13 +157,13 @@ describe("GUI update execution decisions", () => { expect(waited).toEqual([{ port: 12345, hostname: "127.0.0.1", - opts: { killOcxHolders: false, onlyKillPids: [] }, + opts: { killOcxHolders: true, onlyKillPids: [], killAllOcxOnPort: true }, }]); expect(spawned).toEqual([{ port: 12345 }]); }); - test("restart reclaim allowlists only the trusted oldPid", async () => { - const optsSeen: Array<{ killOcxHolders?: boolean; onlyKillPids?: number[] }> = []; + test("restart reclaim allowlists the trusted oldPid and kills any ocx on the port", async () => { + const optsSeen: Array<{ killOcxHolders?: boolean; onlyKillPids?: number[]; killAllOcxOnPort?: boolean }> = []; const job: UpdateJobState = { id: "restart-oldpid", status: "restarting", @@ -178,19 +180,51 @@ describe("GUI update execution decisions", () => { writeFileSync(updateJobPath(job.id), JSON.stringify(job)); await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1", oldPid: 4242 }, { serviceInstalledFn: () => false, + listListenPidsFn: () => [], waitForPort: async (_port, _hostname, opts) => { optsSeen.push({ killOcxHolders: opts?.killOcxHolders, onlyKillPids: opts?.onlyKillPids, + killAllOcxOnPort: (opts as { killAllOcxOnPort?: boolean } | undefined)?.killAllOcxOnPort, }); return true; }, spawnStart: () => {}, }); - expect(optsSeen).toEqual([{ killOcxHolders: true, onlyKillPids: [4242] }]); + expect(optsSeen).toEqual([{ killOcxHolders: true, onlyKillPids: [4242], killAllOcxOnPort: true }]); }); - test("restart refuses to spawn when the captured port never becomes free", async () => { + test("restart reclaim also allowlists leftover ocx listeners on the captured port", async () => { + const optsSeen: number[][] = []; + const job: UpdateJobState = { + id: "restart-leftover-ocx", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.39", + latestVersion: "2.7.40", + channel: "latest", + installer: "npm", + restart: true, + command: "", + log: [], + }; + writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + await restartAfterUpdateForTests(job, { port: 19111, hostname: "127.0.0.1", oldPid: 100 }, { + serviceInstalledFn: () => false, + // Simulate a respawned bun child that is not the pre-update PID. + listListenPidsFn: () => [100, 200], + verifyOcxFn: (pid) => (pid === 100 || pid === 200 ? pid : null), + waitForPort: async (_port, _hostname, opts) => { + optsSeen.push([...(opts?.onlyKillPids ?? [])].sort((a, b) => a - b)); + return true; + }, + spawnStart: () => {}, + }); + expect(optsSeen).toEqual([[100, 200]]); + }); + + test("restart refuses to spawn when a live holder still owns the captured port", async () => { const spawned: Array<{ port?: number }> = []; const job: UpdateJobState = { id: "restart-busy", @@ -206,16 +240,47 @@ describe("GUI update execution decisions", () => { log: [], }; writeFileSync(updateJobPath(job.id), JSON.stringify(job)); - await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1" }, { + await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1", oldPid: 100 }, { serviceInstalledFn: () => false, waitForPort: async () => false, + listListenPidsFn: () => [555], + isAliveFn: pid => pid === 555, spawnStart: (_job, _installer, port) => { spawned.push({ port }); }, }); expect(spawned).toEqual([]); const saved = readUpdateJob(job.id); - expect(saved?.log.some(line => line.includes("still busy") && line.includes("not starting on another port"))).toBe(true); + expect(saved?.log.some(line => line.includes("Live holder(s) remain"))).toBe(true); + }); + + test("restart attempts pinned start when reclaim times out with only dead holders", async () => { + const spawned: Array<{ port?: number }> = []; + const job: UpdateJobState = { + id: "restart-busy-dead", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.39", + latestVersion: "2.7.40", + channel: "latest", + installer: "npm", + restart: true, + command: "", + log: [], + }; + writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1", oldPid: 100 }, { + serviceInstalledFn: () => false, + waitForPort: async () => false, + listListenPidsFn: () => [100], + isAliveFn: () => false, + spawnStart: (_job, _installer, port) => { + spawned.push({ port }); + }, + }); + expect(spawned).toEqual([{ port: 10100 }]); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("waiting for ghost LISTEN rows to clear before pinned start"))).toBe(true); }); test("service restart waits on the captured port and clears OCX_BAKE_PORT after install", async () => { @@ -240,6 +305,7 @@ describe("GUI update execution decisions", () => { try { await restartAfterUpdateForTests(job, { port: 18765, hostname: "127.0.0.1" }, { serviceInstalledFn: () => true, + serviceViableFn: () => true, waitForPort: async (port, hostname) => { waited.push({ port, hostname: hostname ?? "" }); expect(process.env.OCX_BAKE_PORT).toBeUndefined(); @@ -275,16 +341,104 @@ describe("GUI update execution decisions", () => { log: [], }; writeFileSync(updateJobPath(job.id), JSON.stringify(job)); - await restartAfterUpdateForTests(job, { port: 19999, hostname: "127.0.0.1" }, { - serviceInstalledFn: () => true, - waitForPort: async () => true, - runService: () => ({ status: 1 }), - spawnStart: (_job, _installer, port) => { - spawned.push({ port: port ?? 0 }); + const prevService = process.env.OCX_SERVICE; + delete process.env.OCX_SERVICE; + try { + await restartAfterUpdateForTests(job, { port: 19999, hostname: "127.0.0.1" }, { + serviceInstalledFn: () => true, + serviceViableFn: () => false, + waitForPort: async () => true, + runService: () => ({ status: 1 }), + spawnStart: (_job, _installer, port) => { + spawned.push({ port: port ?? 0 }); + }, + }); + // The fallback must fire: direct proxy start instead of throwing. + expect(spawned).toEqual([{ port: 19999 }]); + } finally { + if (prevService === undefined) delete process.env.OCX_SERVICE; + else process.env.OCX_SERVICE = prevService; + } + }); + + test("service reinstall exit 0 with non-viable assets falls back to direct start", async () => { + const spawned: Array<{ port: number }> = []; + const job: UpdateJobState = { + id: "svc-stale-fallback", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.42", + latestVersion: "2.7.43", + channel: "latest", + installer: "npm", + restart: true, + command: "", + log: [], + }; + writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + const prevService = process.env.OCX_SERVICE; + delete process.env.OCX_SERVICE; + try { + await restartAfterUpdateForTests(job, { port: 19100, hostname: "127.0.0.1" }, { + serviceInstalledFn: () => true, + // Installed but stale/missing assets — the status line users see after a dead update. + serviceViableFn: () => false, + waitForPort: async () => true, + runService: () => ({ status: 0 }), + spawnStart: (_job, _installer, port) => { + spawned.push({ port: port ?? 0 }); + }, + }); + expect(spawned).toEqual([{ port: 19100 }]); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("not viable") && line.includes("direct proxy start"), + )).toBe(true); + } finally { + if (prevService === undefined) delete process.env.OCX_SERVICE; + else process.env.OCX_SERVICE = prevService; + } + }); + + test("dashboard update recovery does not require a Background Service", async () => { + let now = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "no-bg-service-recovery", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.42", + latestVersion: "2.7.43", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + const ok = await finishGuiUpdateRestart( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 501 }, + "npm", + { + // No Background Service installed — interactive dashboard update must still recover. + serviceInstalledFn: () => false, + serviceViableFn: () => false, + waitForPort: async () => true, + spawnStart: () => { restartCalls += 1; }, + probeProxy: async () => restartCalls > 0, + probeProxyIdentity: async () => ( + restartCalls > 0 ? { pid: 777, version: "2.7.43" } : null + ), + now: () => now, + sleepMs: async (ms) => { now += ms; }, }, - }); - // The fallback must fire: direct proxy start instead of throwing. - expect(spawned).toEqual([{ port: 19999 }]); + ); + expect(ok).toBe(true); + expect(restartCalls).toBe(1); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("skipping redundant restart"))).toBe(false); }); test("restart confirmation fails when the proxy never becomes healthy", async () => { @@ -428,6 +582,9 @@ describe("GUI update execution decisions", () => { "npm", { serviceInstalledFn: () => true, + // Soft-probe path only runs when a live listen owner exists. + listListenPidsFn: () => [222], + isAliveFn: () => true, probeProxy: async () => now >= 15_250, probeProxyIdentity: async () => ( now >= 15_250 ? { pid: 222, version: "2.7.43" } : null @@ -473,6 +630,8 @@ describe("GUI update execution decisions", () => { "npm", { serviceInstalledFn: () => true, + listListenPidsFn: () => [222], + isAliveFn: () => true, probeProxy: async () => true, probeProxyIdentity: async () => ({ pid: 222, version: "2.7.41" }), now: () => now, @@ -511,6 +670,8 @@ describe("GUI update execution decisions", () => { "npm", { serviceInstalledFn: () => true, + listListenPidsFn: () => [111], + isAliveFn: pid => pid === 111, // Soft probe stays healthy (old process). Explicit restart is a no-op. probeProxy: async () => true, probeProxyIdentity: async () => ({ pid: 111, version: "2.7.40" }), @@ -560,6 +721,8 @@ describe("GUI update execution decisions", () => { "npm", { serviceInstalledFn: () => true, + listListenPidsFn: () => [livePid], + isAliveFn: () => true, probeProxy: async () => true, probeProxyIdentity: async () => ({ pid: livePid, version: liveVersion }), now: () => now, @@ -580,7 +743,7 @@ describe("GUI update execution decisions", () => { )).toBe(true); }); - test("npm finish fails when port reclaim leaves the pre-update proxy healthy", async () => { + test("npm finish fails when port reclaim leaves a live pre-update holder", async () => { let now = 0; const job: UpdateJobState = { id: "npm-reclaim-stale", @@ -602,11 +765,13 @@ describe("GUI update execution decisions", () => { { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, "npm", { - // Direct path: reclaim failure returns without spawning a replacement. + // Live holder after reclaim timeout — must not spawn a second listener. serviceInstalledFn: () => false, waitForPort: async () => false, + listListenPidsFn: () => [111], + isAliveFn: pid => pid === 111, spawnStart: () => { - throw new Error("must not spawn when reclaim failed"); + throw new Error("must not spawn when a live holder remains"); }, probeProxy: async () => true, probeProxyIdentity: async () => ({ pid: 111, version: "2.7.40" }), @@ -619,10 +784,7 @@ describe("GUI update execution decisions", () => { status: "failed", restarted: false, }); - expect(readUpdateJob(job.id)?.error).toContain("still the pre-update PID"); - expect(readUpdateJob(job.id)?.log.some(line => - line.includes("still busy") && line.includes("not starting on another port"), - )).toBe(true); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("Live holder(s) remain"))).toBe(true); }); test("npm finish skips the soft probe for direct installs and restarts immediately", async () => { @@ -689,6 +851,7 @@ describe("GUI update execution decisions", () => { writeFileSync(updateJobPath(job.id), JSON.stringify(job)); const ok = await finishGuiUpdateRestart(job, { port: 10100, hostname: "127.0.0.1" }, "npm", { serviceInstalledFn: () => true, + listListenPidsFn: () => [], // Soft probe times out (proxy down after npm update); confirm after explicit restart succeeds. probeProxy: async () => restartCalls > 0, probeProxyIdentity: async () => ( @@ -708,6 +871,39 @@ describe("GUI update execution decisions", () => { )).toBe(true); }); + test("npm finish probes /healthz when listener scan fails instead of assuming no listener", async () => { + let now = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "npm-scan-fail-probe", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + const ok = await finishGuiUpdateRestart(job, { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, "npm", { + serviceInstalledFn: () => true, + scanListenPidsFn: () => ({ ok: false, error: "lsof/netstat unavailable" }), + probeProxy: async () => true, + probeProxyIdentity: async () => ({ pid: 222, version: "2.7.41" }), + now: () => now, + sleepMs: async (ms) => { now += ms; }, + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }); + expect(ok).toBe(true); + expect(restartCalls).toBe(0); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("Listener scan inconclusive"))).toBe(true); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("skipping redundant restart"))).toBe(true); + }); + test("bun finish always runs explicit restart even if a proxy is already healthy", async () => { let now = 0; let restartCalls = 0; diff --git a/tests/windows-deploy-close-regressions.test.ts b/tests/windows-deploy-close-regressions.test.ts index 46742f3a5..b914562ef 100644 --- a/tests/windows-deploy-close-regressions.test.ts +++ b/tests/windows-deploy-close-regressions.test.ts @@ -29,10 +29,26 @@ describe("update-job restart avoids the shell-less .cmd EINVAL (Windows, bun/sou expect(src).toContain("refusing to hop"); expect(src).toContain("runtimeTrusted"); expect(read("src/cli/index.ts")).toContain("allowEphemeralFallback: !hardPin"); - expect(read("src/cli/index.ts")).toContain("preferRetryMs: hardPin ? 0 : 750"); + expect(read("src/cli/index.ts")).toContain("preferRetryMs: hardPin ? 5_000 : 750"); expect(read("src/cli/index.ts")).toContain("Not opening the GUI"); expect(read("src/server/ports.ts")).toContain("allowEphemeralFallback"); }); + test("Windows GUI update worker is launched without inheriting the proxy LISTEN socket", () => { + // Direct spawn() inherits Bun.serve's LISTEN handle → ghost LISTEN with dead parent PID. + expect(src).toContain("function spawnGuiUpdateWorker"); + expect(src).toContain("Start-Process"); + expect(src).toContain("buildWindowsElevatedArgumentList"); + expect(src).toContain("resolveTrustedWindowsPowerShellExe"); + expect(src).toContain("spawnWorkerFn: spawnGuiUpdateWorker"); + // Foreign listeners must stay fail-closed; npm rename is covered by ocx identity. + expect(src).not.toContain("killAnyListenPidOnPort"); + expect(src).toContain('process.platform === "win32" && process.env.OCX_SERVICE === "1"'); + // Native WinSW installs must stop via stopWinswService, not Task Scheduler /end only. + expect(src).toContain("readServiceBackend"); + expect(src).toContain("stopWinswService"); + expect(src).toContain("$_.ProcessId -eq $PID"); + expect(src).toContain("lastChild?.pid && aliveFn(lastChild.pid)"); + }); }); describe("systemd detection tolerates a no-DBUS SSH session (F9)", () => {