diff --git a/src/update/index.ts b/src/update/index.ts index 05e2d9aa73..9763f3580f 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -27,6 +27,26 @@ export function historyRestoreIncomplete(configDir = getConfigDir()): boolean { } } +/** + * Whether the update may replace the package once `ocx stop` has returned, and + * whether its exit status is worth mentioning. + * + * The only precondition the update has is that nothing is still serving — + * package replacement cannot run under a live proxy. But `ocx stop` does more + * than stop the proxy: it also restores Codex resume-history and reverts + * environment ownership, and it reports a non-zero status when any of that + * fails. A busy history DB is ordinary on Windows, so reading the status as the + * gate aborted updates whose proxy had already stopped cleanly, leaving nothing + * on the port and the old version installed (#3008). + */ +export function classifyStopForUpdate(outcome: { + status: number | null; + proxyStillUp: boolean; +}): { proceed: false } | { proceed: true; stopStatus: number | null } { + if (outcome.proxyStillUp) return { proceed: false }; + return { proceed: true, stopStatus: outcome.status === 0 ? null : outcome.status }; +} + export const PKG = "@bitkyc08/opencodex"; const HERE = dirname(fileURLToPath(import.meta.url)); // .../opencodex/src/update @@ -256,7 +276,11 @@ export async function runUpdate(): Promise { windowsHide: true, }); if (stopStdio === "pipe") logSpawnOutput("", stop); - if (stop.status !== 0 || readPid() || readRuntimePort()) { + const stopOutcome = classifyStopForUpdate({ + status: stop.status, + proxyStillUp: Boolean(readPid() || readRuntimePort()), + }); + if (!stopOutcome.proceed) { if (trayWasRunning) { try { const { startWindowsTray } = await import("../tray/windows"); @@ -266,6 +290,11 @@ export async function runUpdate(): Promise { console.error("⚠️ Could not stop the running proxy; aborting the update. Run 'ocx stop' and retry."); process.exit(1); } + if (stopOutcome.stopStatus !== null) { + console.warn( + `⚠️ 'ocx stop' exited ${stopOutcome.stopStatus}, but the proxy is stopped; continuing the update.`, + ); + } if (historyRestoreIncomplete()) { console.warn( "⚠️ Codex resume-history metadata restore is incomplete (a backup manifest remains).\n" + diff --git a/tests/update-stop-classification.test.ts b/tests/update-stop-classification.test.ts new file mode 100644 index 0000000000..24843f5d2c --- /dev/null +++ b/tests/update-stop-classification.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, test } from "bun:test"; +import { classifyStopForUpdate } from "../src/update/index"; + +/** + * #3008: a dashboard update aborted after `ocx stop` had already stopped the + * proxy, because resume-history restoration exited non-zero and the update read + * the stop command's status as its gate. The port was left with no listener and + * the old package still installed. + */ +describe("classifyStopForUpdate", () => { + test("proceeds when the proxy is down, even if stop reported failure", () => { + expect(classifyStopForUpdate({ status: 1, proxyStillUp: false })).toEqual({ + proceed: true, + stopStatus: 1, + }); + }); + + test("reports no status to warn about when stop succeeded", () => { + expect(classifyStopForUpdate({ status: 0, proxyStillUp: false })).toEqual({ + proceed: true, + stopStatus: null, + }); + }); + + test("refuses while a proxy is still up, whatever stop reported", () => { + // Replacing package files under a live proxy leaves it executing mixed + // old/new code, so this is the one condition that must still abort. + expect(classifyStopForUpdate({ status: 0, proxyStillUp: true })).toEqual({ proceed: false }); + expect(classifyStopForUpdate({ status: 1, proxyStillUp: true })).toEqual({ proceed: false }); + }); + + test("treats a signal-killed stop as non-fatal once the proxy is down", () => { + // spawnSync reports status null when the child died from a signal. + expect(classifyStopForUpdate({ status: null, proxyStillUp: false })).toEqual({ + proceed: true, + stopStatus: null, + }); + }); +});