diff --git a/docs/adr/0001-gui-update-worker.md b/docs/adr/0001-gui-update-worker.md index 4df96bc62..056db8e8a 100644 --- a/docs/adr/0001-gui-update-worker.md +++ b/docs/adr/0001-gui-update-worker.md @@ -27,6 +27,19 @@ return and remain healthy for a short stability window before marking the job su keeps `update-job.json` honest on Windows cases where npm leaves the bundled Bun runtime in a bad state and the restarted proxy dies a few seconds later. +For npm installs specifically, `node ocx.mjs update` already stops the proxy and reinstalls / +starts the managed service (or falls back to a direct start). When a background service was +installed, the worker therefore confirms that self-update restart first — but only skips the +redundant second `service install` when /healthz shows update-correlated evidence (a new PID +vs the pre-update capture, and/or the job's target version). A bare healthy identity is not +enough: a surviving pre-update process would otherwise look like success. Direct (non-service) +npm installs skip the probe-first path entirely, because the launcher only prints `ocx start` +and never brings the proxy back on its own — waiting would always burn the full health timeout +before the worker's explicit restart. A second install would call `stopWindows()` on the healthy +listener and often fail elevation from the non-interactive worker, leaving the captured port +(default 10100) dead until a manual restart. Bun global installs still always take the explicit +restart path because `bun add -g` does not restart the proxy. + ## Consequences - The GUI request handler stays responsive and does not overwrite its own running module graph. diff --git a/gui/src/client-resource.ts b/gui/src/client-resource.ts index da5b2e667..b99cf6753 100644 --- a/gui/src/client-resource.ts +++ b/gui/src/client-resource.ts @@ -299,3 +299,14 @@ export function setClientResourceData(key: string, data: T) { store.snapshot = { data, error: undefined, loading: false }; emit(store); } + +/** Test-only: drop every module cache entry so suite order cannot skip cold-start fetches. */ +export function clearClientResourceStoresForTests(): void { + for (const store of stores.values()) { + clearPollTimer(store); + store.inflight?.abort(); + store.inflight = null; + store.inflightOwner = null; + } + stores.clear(); +} diff --git a/gui/tests/debug-put-install-order.test.tsx b/gui/tests/debug-put-install-order.test.tsx index 9d72665cd..4e3aeb822 100644 --- a/gui/tests/debug-put-install-order.test.tsx +++ b/gui/tests/debug-put-install-order.test.tsx @@ -2,11 +2,14 @@ import { afterEach, beforeEach, expect, test } from "bun:test"; import { Window } from "happy-dom"; import { act } from "react"; import type { Root } from "react-dom/client"; +import { clearClientResourceStoresForTests } from "../src/client-resource"; import { LanguageProvider } from "../src/i18n/provider"; import Debug from "../src/pages/Debug"; import type { DebugSettings } from "../src/pages/debug-shared"; const globals = ["document", "window", "navigator", "localStorage", "IS_REACT_ACT_ENVIRONMENT", "ResizeObserver"] as const; +/** Survives afterEach restore so a late React 19 dispatchSetState can read window.event. */ +const WINDOW_EVENT_STUB: { event: undefined } = { event: undefined }; let previousGlobals: Record<(typeof globals)[number], unknown>; let testWindow: Window; const originalFetch = globalThis.fetch; @@ -63,8 +66,13 @@ function installLayoutStubs(win: Window): void { } beforeEach(() => { + // Prior Debug tests share `debug-settings:http://localhost`; a warm module cache skips + // the cold-start GET and leaves pollCount at 0 (seen on Windows CI after mutation-busy). + clearClientResourceStoresForTests(); previousGlobals = Object.fromEntries(globals.map(key => [key, Reflect.get(globalThis, key)])) as typeof previousGlobals; testWindow = new Window({ url: "http://localhost/#debug" }); + // React 19 resolveUpdatePriority reads window.event; happy-dom omits the IE legacy field. + Object.defineProperty(testWindow, "event", { configurable: true, writable: true, value: undefined }); Object.defineProperties(globalThis, { document: { configurable: true, value: testWindow.document }, window: { configurable: true, value: testWindow }, @@ -75,11 +83,35 @@ beforeEach(() => { installLayoutStubs(testWindow); }); -afterEach(() => { +afterEach(async () => { globalThis.fetch = originalFetch; + // Drain deferred React 19 work before restoring globals (same pattern as rail-hover). + await act(async () => { + for (let i = 0; i < 5; i++) { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.resolve(); + } + }); testWindow.close(); + clearClientResourceStoresForTests(); for (const key of globals) { - Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + let value = previousGlobals[key]; + if (key === "window") { + if (value == null || typeof value !== "object") { + value = WINDOW_EVENT_STUB; + } else if (!Object.prototype.hasOwnProperty.call(value, "event")) { + try { + Object.defineProperty(value, "event", { + configurable: true, + writable: true, + value: undefined, + }); + } catch { + value = WINDOW_EVENT_STUB; + } + } + } + Object.defineProperty(globalThis, key, { configurable: true, value }); } }); diff --git a/src/update/job.ts b/src/update/job.ts index e43f85e73..62a01716e 100644 --- a/src/update/job.ts +++ b/src/update/job.ts @@ -5,7 +5,7 @@ import { fileURLToPath } from "node:url"; import { atomicWriteFile, getConfigDir, loadConfig, readPid, readRuntimePort } from "../config"; import { killProxy } from "../lib/process-control"; import { reclaimListenPort } from "../server/port-reclaim"; -import { proxyIdentityAt } from "../server/proxy-liveness"; +import { isOpencodexHealthz, probeHostname, proxyIdentityAt, type HealthzIdentity } from "../server/proxy-liveness"; import { isServiceInstalled } from "../service"; import { type Channel, @@ -292,12 +292,20 @@ function spawnDetachedStart(job: UpdateJobState, installer: Installer, port?: nu child.unref(); } +/** Identity snapshot used to prove an npm self-update actually replaced the pre-update process. */ +export interface RestartProxyIdentity { + pid: number | null; + version?: string; +} + /** Test seam: the wait/spawn pair is injectable so the restart path is verifiable. */ export interface RestartIo { waitForPort?: typeof reclaimListenPort; spawnStart?: (job: UpdateJobState, installer: Installer, port?: number) => void; serviceInstalledFn?: () => boolean; probeProxy?: (port: number, hostname?: string) => Promise; + /** Richer /healthz read for update-correlated restart evidence (pid + version). */ + probeProxyIdentity?: (port: number, hostname?: string) => Promise; sleepMs?: (ms: number) => Promise; now?: () => number; /** Service-mode install/reinstall command (defaults to spawnSync via runLoggedCommand). */ @@ -306,6 +314,12 @@ export interface RestartIo { bin: string, args: string[], ) => { status: number | null; signal?: NodeJS.Signals | null }; + /** Override the explicit restart path (used by finishGuiUpdateRestart tests). */ + restartAfterUpdateFn?: ( + job: UpdateJobState, + captured?: { port: number; hostname: string; oldPid?: number }, + io?: RestartIo, + ) => Promise; } async function restartAfterUpdate( @@ -404,25 +418,19 @@ function restartFailureHint(port: number): string { + "reinstall with 'npm install -g --allow-scripts=bun @bitkyc08/opencodex'."; } +type AwaitHealthyResult = + | { ok: true } + | { ok: false; reason: "timeout" | "flapped" }; + /** - * Confirm that the detached/service restart really came back and stayed up. The GUI worker - * used to mark success immediately after spawning the new process, which hid Windows cases - * where npm left the bundled Bun runtime half-updated and the restarted proxy died seconds - * later. A healthy /healthz must appear, then remain healthy for one short stability window. + * Wait for an identity-checked /healthz on the captured listen target, then require a short + * stability window. Soft: never marks the job failed (callers decide whether to fail or retry). */ -async function confirmRestartedProxy( +async function awaitRestartedProxyHealthy( job: UpdateJobState, captured: { port: number; hostname: string }, io: RestartIo = {}, -): Promise { - /* [Decision Log] - - 목적과 의도: GUI update job이 detached restart 요청만 보고 성공 처리하지 않도록, 실제 프록시 복귀 여부를 확인한다. - - 기존 구현 및 제약 조건: update-job.json은 spawn/service reinstall 직후 `succeeded`로 끝났고, Windows npm/Bun 교체 실패처럼 몇 초 후 죽는 재시작을 잡지 못했다. - - 검토한 주요 대안: (1) 포트 점유만 확인 — 외부 프로세스/죽기 직전 프로세스를 성공으로 오인할 수 있다. (2) 무기한 /healthz 폴링 — UX가 느려지고 worker 종료 시점이 불명확하다. (3) 짧은 healthy 등장 + 안정성 창 확인 — 실제 복귀를 확인하면서도 대기 시간을 제한할 수 있다. - - 선택한 방식: identity-aware /healthz probe가 일정 시간 안에 나타나고, 추가 안정성 창 동안 유지되는지 확인한다. - - 다른 대안 대신 이 방식을 선택한 이유: GUI는 "업데이트가 설치됐지만 재시작은 실패"를 분리해 알려줘야 하며, 이 방식이 가장 적은 오탐으로 그 경계를 만든다. - - 장점, 단점 및 영향: 장점은 silent restart failure가 update-job 상태로 드러난다는 점이다. 단점은 성공 판정이 최대 30초 늦어질 수 있다는 점이며, 대신 실제 복귀를 더 정확히 반영한다. - */ +): Promise { const probe = io.probeProxy ?? (async (port: number, hostname?: string) => ( !!(await proxyIdentityAt(port, { hostname })) )); @@ -440,25 +448,50 @@ async function confirmRestartedProxy( const stableUntil = now() + RESTART_STABILITY_WINDOW_MS; while (now() < stableUntil) { if (!(await probe(port, hostname))) { - updateJob(job, { - status: "failed", - restarted: false, - error: `proxy restart became unhealthy on ${hostname}:${port}`, - }, restartFailureHint(port)); - return false; + updateJob(job, {}, `Proxy became unhealthy on ${hostname}:${port} during the stability window.`); + return { ok: false, reason: "flapped" }; } await sleep(500); } updateJob(job, {}, `Proxy stayed healthy for ${Math.trunc(RESTART_STABILITY_WINDOW_MS / 1000)}s after restart.`); - return true; + return { ok: true }; } await sleep(250); } + return { ok: false, reason: "timeout" }; +} + +/** + * Confirm that the detached/service restart really came back and stayed up. The GUI worker + * used to mark success immediately after spawning the new process, which hid Windows cases + * where npm left the bundled Bun runtime half-updated and the restarted proxy died seconds + * later. A healthy /healthz must appear, then remain healthy for one short stability window. + */ +async function confirmRestartedProxy( + job: UpdateJobState, + captured: { port: number; hostname: string }, + io: RestartIo = {}, +): Promise { + /* [Decision Log] + - 목적과 의도: GUI update job이 detached restart 요청만 보고 성공 처리하지 않도록, 실제 프록시 복귀 여부를 확인한다. + - 기존 구현 및 제약 조건: update-job.json은 spawn/service reinstall 직후 `succeeded`로 끝났고, Windows npm/Bun 교체 실패처럼 몇 초 후 죽는 재시작을 잡지 못했다. + - 검토한 주요 대안: (1) 포트 점유만 확인 — 외부 프로세스/죽기 직전 프로세스를 성공으로 오인할 수 있다. (2) 무기한 /healthz 폴링 — UX가 느려지고 worker 종료 시점이 불명확하다. (3) 짧은 healthy 등장 + 안정성 창 확인 — 실제 복귀를 확인하면서도 대기 시간을 제한할 수 있다. + - 선택한 방식: identity-aware /healthz probe가 일정 시간 안에 나타나고, 추가 안정성 창 동안 유지되는지 확인한다. + - 다른 대안 대신 이 방식을 선택한 이유: GUI는 "업데이트가 설치됐지만 재시작은 실패"를 분리해 알려줘야 하며, 이 방식이 가장 적은 오탐으로 그 경계를 만든다. + - 장점, 단점 및 영향: 장점은 silent restart failure가 update-job 상태로 드러난다는 점이다. 단점은 성공 판정이 최대 30초 늦어질 수 있다는 점이며, 대신 실제 복귀를 더 정확히 반영한다. + */ + const result = await awaitRestartedProxyHealthy(job, captured, io); + if (result.ok) return true; + const port = captured.port; + const hostname = captured.hostname; + const error = result.reason === "flapped" + ? `proxy restart became unhealthy on ${hostname}:${port}` + : `proxy restart never became healthy on ${hostname}:${port}`; updateJob(job, { status: "failed", restarted: false, - error: `proxy restart never became healthy on ${hostname}:${port}`, + error, }, restartFailureHint(port)); return false; } @@ -471,6 +504,178 @@ export function confirmRestartAfterUpdateForTests( return confirmRestartedProxy(job, captured, io); } +async function defaultProbeProxyIdentity( + port: number, + hostname?: string, +): Promise { + try { + const res = await fetch(`http://${probeHostname(hostname)}:${port}/healthz`, { + signal: AbortSignal.timeout(750), + }); + if (!res.ok) return null; + const body = (await res.json().catch(() => null)) as HealthzIdentity | null; + if (!isOpencodexHealthz(body)) return null; + return { + pid: typeof body?.pid === "number" ? body.pid : null, + ...(typeof body?.version === "string" ? { version: body.version } : {}), + }; + } catch { + return null; + } +} + +/** + * Health alone is not enough to skip the GUI worker restart: a surviving pre-update + * process is still identity-healthy. Require update-correlated evidence — a new PID + * when the pre-update PID was captured, and/or /healthz reporting the job's target + * version when PID evidence is unavailable. + */ +export function npmSelfUpdateRestartEvidence( + job: Pick, + captured: { oldPid?: number }, + identity: RestartProxyIdentity | null, +): { ok: true; detail: string } | { ok: false; reason: string } { + if (!identity) return { ok: false, reason: "could not read proxy identity" }; + + const oldPid = typeof captured.oldPid === "number" && captured.oldPid > 0 + ? captured.oldPid + : undefined; + const livePid = typeof identity.pid === "number" && identity.pid > 0 ? identity.pid : null; + const expected = typeof job.latestVersion === "string" && job.latestVersion.length > 0 + ? job.latestVersion + : null; + const versionMatches = expected !== null && identity.version === expected; + + if (oldPid !== undefined) { + if (livePid === oldPid) { + return { ok: false, reason: "still the pre-update PID" }; + } + if (livePid !== null) { + if (expected !== null && identity.version && identity.version !== expected) { + return { ok: false, reason: `new pid but version ${identity.version} !== expected ${expected}` }; + } + return { ok: true, detail: `pid changed ${oldPid}→${livePid}` }; + } + // Pre-update PID known but healthz omitted pid — only accept matching target version. + if (versionMatches) return { ok: true, detail: `version ${identity.version}` }; + return { ok: false, reason: "no PID in healthz and version did not match the update target" }; + } + + if (versionMatches) return { ok: true, detail: `version ${identity.version}` }; + if (expected !== null && identity.version && identity.version !== expected) { + return { ok: false, reason: `version ${identity.version} !== expected ${expected}` }; + } + return { ok: false, reason: "no pre-update PID capture and no expected-version match" }; +} + +/** + * Post-install restart for the GUI worker. + * + * npm installs run `node ocx.mjs update`, which already stops the proxy and reinstalls / + * starts the service (or falls back to a direct start). A second `service install` here + * calls `stopWindows()` on that healthy listener, then often fails elevation from the + * non-interactive worker — leaving the captured port (default 10100) dead until a manual + * restart. Prefer confirming the npm self-update's own restart first; only re-run restart + * when that probe fails. Bun/source installs still always take the explicit restart path. + * + * Probe-first applies only to service-managed npm installs: without a service, `ocx.mjs` + * only prints `ocx start` and never brings the proxy back, so waiting would always burn + * the full health timeout. Skipping also requires update-correlated evidence (PID change + * 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. + */ +export async function finishGuiUpdateRestart( + job: UpdateJobState, + captured: { port: number; hostname: string; oldPid?: number }, + installer: Installer, + io: RestartIo = {}, +): Promise { + 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) { + 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..."); + } + } + } + const restartFn = io.restartAfterUpdateFn ?? restartAfterUpdate; + await restartFn(job, captured, io); + if (installer !== "npm") { + // Bun/source: health alone remains enough unless a richer identity probe is supplied. + if (!io.probeProxyIdentity) return confirmRestartedProxy(job, captured, io); + } + return confirmNpmExplicitRestart(job, captured, io); +} + +/** + * After an explicit npm (or identity-aware) restart, require update-correlated + * evidence — not merely a healthy OpenCodex listener. A no-op restart or a + * failed port reclaim can leave the pre-update process on the captured port; + * `confirmRestartedProxy` alone would treat that as success. + */ +async function confirmNpmExplicitRestart( + job: UpdateJobState, + captured: { port: number; hostname: string; oldPid?: number }, + io: RestartIo = {}, +): Promise { + const healthy = await awaitRestartedProxyHealthy(job, captured, io); + if (!healthy.ok) { + const port = captured.port; + const hostname = captured.hostname; + const error = healthy.reason === "flapped" + ? `proxy restart became unhealthy on ${hostname}:${port}` + : `proxy restart never became healthy on ${hostname}:${port}`; + updateJob(job, { + status: "failed", + restarted: false, + error, + }, restartFailureHint(port)); + return false; + } + + const identity = await (io.probeProxyIdentity ?? defaultProbeProxyIdentity)( + captured.port, + captured.hostname, + ); + const evidence = npmSelfUpdateRestartEvidence(job, captured, identity); + if (!evidence.ok) { + updateJob(job, { + status: "failed", + restarted: false, + error: `proxy restart did not show update-correlated identity (${evidence.reason})`, + }, restartFailureHint(captured.port)); + return false; + } + + updateJob( + job, + {}, + `Proxy restart confirmed on ${captured.hostname}:${captured.port} (${evidence.detail}).`, + ); + return true; +} + export async function runGuiUpdateWorker(jobId: string, channel: Channel, restart: boolean): Promise { let job = readUpdateJob(jobId); const check = checkForUpdate(channel); @@ -590,8 +795,7 @@ export async function runGuiUpdateWorker(jobId: string, channel: Channel, restar if (restart) { job = updateJob(job, { status: "restarting" }, "Update installed. Restarting proxy..."); - await restartAfterUpdate(job, captured); - if (!(await confirmRestartedProxy(job, captured))) return; + if (!(await finishGuiUpdateRestart(job, captured, check.installer))) return; updateJob(job, { status: "succeeded", restarted: true }, "Restart requested and proxy is healthy."); return; } diff --git a/tests/update-job.test.ts b/tests/update-job.test.ts index dbf65cdb7..82c85b1b9 100644 --- a/tests/update-job.test.ts +++ b/tests/update-job.test.ts @@ -5,6 +5,8 @@ import { join } from "node:path"; import { checkForUpdate, confirmRestartAfterUpdateForTests, + finishGuiUpdateRestart, + npmSelfUpdateRestartEvidence, readUpdateJob, restartCommand, restartAfterUpdateForTests, @@ -354,6 +356,7 @@ describe("GUI update execution decisions", () => { installer: "npm", restart: true, command: "", + releaseNotesUrl: "", log: [], }; writeFileSync(updateJobPath(job.id), JSON.stringify(job)); @@ -366,6 +369,320 @@ describe("GUI update execution decisions", () => { expect(readUpdateJob(job.id)?.log.some(line => line.includes("stayed healthy for 15s after restart"))).toBe(true); }); + test("npm finish skips redundant restart when service self-update left a replaced healthy proxy", async () => { + let now = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "npm-skip-redundant", + 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, + 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("skipping redundant restart") && line.includes("10100") && line.includes("pid changed"), + )).toBe(true); + }); + + test("npm finish fails when stale PID survives a no-op explicit restart", async () => { + let now = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "npm-stale-healthy", + 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, + // Soft probe stays healthy (old process). Explicit restart is a no-op. + probeProxy: async () => true, + probeProxyIdentity: async () => ({ pid: 111, version: "2.7.40" }), + now: () => now, + sleepMs: async (ms) => { now += ms; }, + restartAfterUpdateFn: async () => { + restartCalls += 1; + now = 0; + }, + }, + ); + expect(ok).toBe(false); + expect(restartCalls).toBe(1); + expect(readUpdateJob(job.id)).toMatchObject({ + 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 the pre-update PID") && line.includes("performing explicit restart"), + )).toBe(true); + }); + + test("npm finish succeeds when explicit restart yields a new PID at the target version", async () => { + let now = 0; + let restartCalls = 0; + let livePid = 111; + let liveVersion = "2.7.40"; + const job: UpdateJobState = { + id: "npm-explicit-replaced", + 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, + probeProxy: async () => true, + probeProxyIdentity: async () => ({ pid: livePid, version: liveVersion }), + now: () => now, + sleepMs: async (ms) => { now += ms; }, + restartAfterUpdateFn: async () => { + restartCalls += 1; + livePid = 222; + liveVersion = "2.7.41"; + now = 0; + }, + }, + ); + expect(ok).toBe(true); + expect(restartCalls).toBe(1); + expect(readUpdateJob(job.id)?.status).not.toBe("failed"); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("Proxy restart confirmed") && line.includes("pid changed"), + )).toBe(true); + }); + + test("npm finish fails when port reclaim leaves the pre-update proxy healthy", async () => { + let now = 0; + const job: UpdateJobState = { + id: "npm-reclaim-stale", + 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", + { + // Direct path: reclaim failure returns without spawning a replacement. + serviceInstalledFn: () => false, + waitForPort: async () => false, + spawnStart: () => { + throw new Error("must not spawn when reclaim failed"); + }, + probeProxy: async () => true, + probeProxyIdentity: async () => ({ pid: 111, version: "2.7.40" }), + now: () => now, + sleepMs: async (ms) => { now += ms; }, + }, + ); + expect(ok).toBe(false); + expect(readUpdateJob(job.id)).toMatchObject({ + 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); + }); + + test("npm finish skips the soft probe for direct installs and restarts immediately", async () => { + let now = 0; + let restartCalls = 0; + let nowBeforeRestart = -1; + const job: UpdateJobState = { + id: "npm-direct-immediate", + 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" }, "npm", { + serviceInstalledFn: () => false, + probeProxy: async () => { + // Only becomes healthy after the explicit restart (launcher printed `ocx start` only). + return restartCalls > 0; + }, + probeProxyIdentity: async () => ( + restartCalls > 0 ? { pid: 333, version: "2.7.41" } : null + ), + now: () => now, + sleepMs: async (ms) => { now += ms; }, + restartAfterUpdateFn: async () => { + nowBeforeRestart = now; + restartCalls += 1; + now = 0; + }, + }); + expect(ok).toBe(true); + expect(restartCalls).toBe(1); + // Soft probe-first must not run — otherwise the clock would advance before restart. + expect(nowBeforeRestart).toBe(0); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("skipping redundant restart"))).toBe(false); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("npm self-update did not leave"))).toBe(false); + }); + + test("npm finish falls back to explicit restart when self-update left the proxy down", async () => { + let now = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "npm-fallback-restart", + 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" }, "npm", { + serviceInstalledFn: () => true, + // Soft probe times out (proxy down after npm update); confirm after explicit restart succeeds. + probeProxy: async () => restartCalls > 0, + probeProxyIdentity: async () => ( + restartCalls > 0 ? { pid: 444, version: "2.7.41" } : null + ), + now: () => now, + sleepMs: async (ms) => { now += ms; }, + restartAfterUpdateFn: async () => { + restartCalls += 1; + now = 0; // reset clock so post-restart health wait has a fresh window + }, + }); + expect(ok).toBe(true); + expect(restartCalls).toBe(1); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("performing explicit restart"), + )).toBe(true); + }); + + test("bun finish always runs explicit restart even if a proxy is already healthy", async () => { + let now = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "bun-always-restart", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "bun", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + const ok = await finishGuiUpdateRestart(job, { port: 10100, hostname: "127.0.0.1" }, "bun", { + probeProxy: async () => true, + now: () => now, + sleepMs: async (ms) => { now += ms; }, + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }); + expect(ok).toBe(true); + expect(restartCalls).toBe(1); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("skipping redundant restart"))).toBe(false); + }); + + test("npmSelfUpdateRestartEvidence requires a PID change or target version", () => { + expect(npmSelfUpdateRestartEvidence( + { latestVersion: "2.7.41" }, + { oldPid: 111 }, + { pid: 111, version: "2.7.41" }, + )).toMatchObject({ ok: false, reason: "still the pre-update PID" }); + + expect(npmSelfUpdateRestartEvidence( + { latestVersion: "2.7.41" }, + { oldPid: 111 }, + { pid: 222, version: "2.7.41" }, + )).toMatchObject({ ok: true }); + + expect(npmSelfUpdateRestartEvidence( + { latestVersion: "2.7.41" }, + {}, + { pid: null, version: "2.7.41" }, + )).toMatchObject({ ok: true }); + + expect(npmSelfUpdateRestartEvidence( + { latestVersion: "2.7.41" }, + {}, + { pid: 222, version: "2.7.40" }, + )).toMatchObject({ ok: false }); + }); + test("a running job prevents a second update job", () => { const now = new Date().toISOString(); const job: UpdateJobState = {