From fa3a131b0ac55cd5c6c9a95105b04efab497c084 Mon Sep 17 00:00:00 2001 From: codingbo Date: Sat, 26 Sep 2026 20:58:33 +0800 Subject: [PATCH] fix(service): restart Windows service wrapper on unexpected bun termination Closes #5913 --- .../content/docs/reference/cli/lifecycle.md | 6 ++ src/cli/dispatch.ts | 2 +- src/cli/index.ts | 16 ++--- src/service/windows-taskxml.ts | 18 +++--- src/service/windows-wrapper-exit.ts | 8 +++ structure/ops/docs-and-release.md | 8 +++ structure/runtime.md | 4 +- tests/cli/cli-dispatch.test.ts | 4 +- tests/cli/cli-ready.test.ts | 10 ++-- .../windows/windows-service-wrappers.test.ts | 60 +++++++++++++++++++ 10 files changed, 109 insertions(+), 27 deletions(-) create mode 100644 src/service/windows-wrapper-exit.ts diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index 24a63d5c7ad..21dbee3153a 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -400,6 +400,12 @@ previous catalog from memory. ### `ocx service [install|repair|restart|start|stop|status|uninstall|remove]` +On Windows Task Scheduler, the service wrapper restarts the proxy after five seconds +even when an external tool terminates it with exit code 0. If another opencodex proxy +already owns the port, the wrapper exits deliberately. Use `ocx stop` or +`ocx service stop` to stop the service and its restart loop. After upgrading an +existing installation, run `ocx service repair` to refresh the generated wrapper. + Run opencodex as a login-managed background service (macOS **launchd**, Linux **systemd user unit**, Windows **Task Scheduler**) that auto-starts on login and auto-restarts on crash. Service runs set `OCX_SERVICE=1` so a restart does not churn the Codex config. diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 9df115fa24b..d45030d0be4 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -1059,7 +1059,7 @@ export type BusyPreferredPortDecision = * * Service-wrapper context keeps the semantics `decideStartWithLiveOwner` gives it: a * healthy proxy on the port means the port is served, and the wrapper's - * `if %ERRORLEVEL% NEQ 0` loop must see a zero exit rather than respawn every 5 seconds. + * retry loop must receive the intentional stay-out signal rather than respawn every 5 seconds. */ export function decideBusyPreferredPort(input: { preferredPort: number; diff --git a/src/cli/index.ts b/src/cli/index.ts index 827dbd6701b..1f257e8631b 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -1,4 +1,5 @@ #!/usr/bin/env bun +import { serviceStayOutExitCode } from "../service/windows-wrapper-exit"; import { spawn } from "node:child_process"; import { homedir } from "node:os"; import { join } from "node:path"; @@ -330,10 +331,9 @@ async function chooseListenPort( ocxService: process.env.OCX_SERVICE, }); if (decision === "service-stay-out") { - // Same contract as the pre-bind owner check: the wrapper's retry loop terminates - // on a zero exit, and the port it was asked to serve is already served. + // Signal intentional stay-out to wrappers that support the exit protocol. console.log(`Proxy already running (PID ${holder?.pid ?? "unknown"}, port ${preferred}); service wrapper staying out of the way.`); - throw new StartCommandExit(0); + throw new StartCommandExit(serviceStayOutExitCode()); } if (decision === "refuse-live-proxy") { console.error(`⚠️ Proxy already running (PID ${holder?.pid ?? "unknown"}, port ${preferred}). Use 'ocx stop' first.`); @@ -444,13 +444,9 @@ async function handleStart(options: { block?: boolean } = {}) { ocxService: process.env.OCX_SERVICE, }); if (decision === "service-stay-out") { - // Service-wrapper context (opencodex-service.cmd `:loop`): a healthy proxy from - // ANY source means the requested port is already served. Exit 0 so the wrapper's - // `if %ERRORLEVEL% NEQ 0` retry loop terminates instead of respawning every 5s - // against a listener it can never claim (observed as an endless - // "Proxy already running" service.log loop). + // A live owner is an intentional stay-out, not an unexpected child exit. console.log(`Proxy already running (PID ${owner.live.pid ?? owner.pidSnapshot ?? "unknown"}, port ${owner.live.port}); service wrapper staying out of the way.`); - process.exit(0); + process.exit(serviceStayOutExitCode()); } if (decision === "refuse") { console.error(`⚠️ Proxy already running (PID ${owner.live.pid ?? owner.pidSnapshot ?? "unknown"}, port ${owner.live.port}). Use 'ocx stop' first.`); @@ -513,7 +509,7 @@ async function handleStart(options: { block?: boolean } = {}) { }); if (decision === "service-stay-out") { console.log(`Proxy already running (PID ${fencedLive.pid ?? "unknown"}, port ${fencedLive.port}); service wrapper staying out of the way.`); - throw new StartCommandExit(0); + throw new StartCommandExit(serviceStayOutExitCode()); } if (decision === "refuse") { console.error(`⚠️ Proxy appeared before bind (PID ${fencedLive.pid ?? "unknown"}, port ${fencedLive.port}). Use 'ocx stop' first.`); diff --git a/src/service/windows-taskxml.ts b/src/service/windows-taskxml.ts index 38d8286d5b9..b3f1baa1917 100644 --- a/src/service/windows-taskxml.ts +++ b/src/service/windows-taskxml.ts @@ -1,3 +1,4 @@ +import { WINDOWS_WRAPPER_PROTOCOL_ENV, WINDOWS_WRAPPER_STAY_OUT_EXIT_CODE } from "./windows-wrapper-exit"; import { readFileSync } from "node:fs"; import { TASK, windowsServiceScriptPath, windowsLauncherVbsPath, windowsTaskXmlPath } from "./state"; import { windowsWscript } from "./windows-scheduler"; @@ -64,11 +65,13 @@ export function buildWindowsServiceScript( const path = process.env.PATH ?? ""; const lines = [ "@echo off", - "setlocal", + "setlocal EnableExtensions DisableDelayedExpansion", + 'set "ERRORLEVEL="', // The wrapper console is hidden by the wscript launcher (window style 0), so switching // it to UTF-8 is safe (no leak into user shells) and lets cmd parse UTF-8 remnants. "chcp 65001 >nul", windowsBatchSet("OCX_SERVICE", "1"), + windowsBatchSet(WINDOWS_WRAPPER_PROTOCOL_ENV, "1"), windowsBatchSet(BUN_RUNTIME_SOURCE_ENV, bunRuntimeSource), windowsBatchSet(BUN_RUNTIME_PATH_ENV, bun, "path"), windowsBatchSet("PATH", path, "pathList"), @@ -110,15 +113,16 @@ export function buildWindowsServiceScript( cli ? " exit /b 3" : null, cli ? ")" : null, cli ? `"%OCX_BUN%" "%OCX_CLI%" start --port ${port} >>"%OCX_SERVICE_LOG%" 2>&1` : `"%OCX_BUN%" start --port ${port} >>"%OCX_SERVICE_LOG%" 2>&1`, - "if %ERRORLEVEL% NEQ 0 (", - ' >>"%OCX_SERVICE_LOG%" echo [%DATE% %TIME%] child exited with code %ERRORLEVEL%; restarting in 5s', + // Stop commands kill the wrapper; a zero child exit alone is not a stop request. + `if "%ERRORLEVEL%"=="${WINDOWS_WRAPPER_STAY_OUT_EXIT_CODE}" goto stopped`, + '>>"%OCX_SERVICE_LOG%" echo [%DATE% %TIME%] child exited with code %ERRORLEVEL%; restarting in 5s', // `timeout` needs console stdin and dies with "Input redirection is not supported" // under Task Scheduler, turning the 5s cooldown into a hot restart loop; ping doesn't. - " ping -n 6 127.0.0.1 >nul", - " goto loop", - ")", + "ping -n 6 127.0.0.1 >nul", + "goto loop", + ":stopped", "endlocal", - "goto :eof", + "exit /b 0", "", // #1942/#1849: a power loss mid-swap leaves the live package dir missing/broken and // a sibling .ocx-backup-* holding the previous version. This wrapper lives OUTSIDE diff --git a/src/service/windows-wrapper-exit.ts b/src/service/windows-wrapper-exit.ts new file mode 100644 index 00000000000..c4892e18fc4 --- /dev/null +++ b/src/service/windows-wrapper-exit.ts @@ -0,0 +1,8 @@ +// Only wrappers advertising this protocol understand a nonzero intentional exit. +export const WINDOWS_WRAPPER_PROTOCOL_ENV = "OCX_WINDOWS_WRAPPER_PROTOCOL"; +export const WINDOWS_WRAPPER_STAY_OUT_EXIT_CODE = 42; + +export function serviceStayOutExitCode(env: NodeJS.ProcessEnv = process.env): number { + return env.OCX_SERVICE === "1" && env[WINDOWS_WRAPPER_PROTOCOL_ENV] === "1" + ? WINDOWS_WRAPPER_STAY_OUT_EXIT_CODE : 0; +} diff --git a/structure/ops/docs-and-release.md b/structure/ops/docs-and-release.md index ac414690dff..b2f84c4a57c 100644 --- a/structure/ops/docs-and-release.md +++ b/structure/ops/docs-and-release.md @@ -180,6 +180,14 @@ Those controls still have no owner, so there is no image-publish workflow or off ## Windows service wrapper and incomplete updates +The scheduler wrapper retries child exits, including zero, after five seconds. Only the +opt-in CLI stay-out code ends it successfully; missing Bun/CLI paths still exit with +installation error 3. Explicit service stop terminates the wrapper itself. +`src/service/windows-wrapper-exit.ts` defines the opt-in contract: new wrappers set +`OCX_WINDOWS_WRAPPER_PROTOCOL=1`, and all three CLI live-owner exits return 42 in that +service context. The wrapper translates 42 into a successful exit; legacy service +contexts retain exit 0. + > Decision record: [ADR-0082](../decisions/ADR-0082-windows-service-wrapper-and-incomplete-updates.md) ## GitHub workflow map diff --git a/structure/runtime.md b/structure/runtime.md index 84c3c0d7095..6d4fb758ffa 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -198,8 +198,8 @@ lost probe deletes this home's pid record and then binds a second listener that records and re-points Codex at itself. `probePortOwner` in `src/server/proxy-liveness.ts` asks the busy port directly, on both loopback families, independent of the pid and runtime records; the outcome is the pure decision `decideBusyPreferredPort` in `src/cli/dispatch.ts`. An opencodex -holder is refused with the same message the owner check prints (exit 0 instead under -`OCX_SERVICE=1`, so the wrapper loop terminates), and a holder that does not identify as opencodex +holder is refused with the same message the owner check prints (intentional stay-out under +`OCX_SERVICE=1`, using the [Windows wrapper protocol](ops/docs-and-release.md#windows-service-wrapper-and-incomplete-updates)), and a holder that does not identify as opencodex is reported as such rather than called foreign, because an identity probe cannot distinguish a foreign server from an unreachable one. An explicit `--port` still never hops — it waits for the pin through `src/server/port-reclaim.ts` — and a configured `port: 0` still means "ask the OS". diff --git a/tests/cli/cli-dispatch.test.ts b/tests/cli/cli-dispatch.test.ts index 2f474e054c1..9be4069cab9 100644 --- a/tests/cli/cli-dispatch.test.ts +++ b/tests/cli/cli-dispatch.test.ts @@ -436,8 +436,8 @@ describe("a busy preferred port never becomes a second proxy (#5004)", () => { expect(fn).toMatch(/decision === "refuse-live-proxy"[\s\S]{0,400}?StartCommandExit\(1\)/); expect(fn).toContain("Use 'ocx stop' first."); expect(fn).toMatch(/decision === "refuse-unidentified-holder"[\s\S]{0,700}?StartCommandExit\(1\)/); - // The wrapper's `if %ERRORLEVEL% NEQ 0` loop still terminates on a served port. - expect(fn).toMatch(/decision === "service-stay-out"[\s\S]{0,500}?StartCommandExit\(0\)/); + // The wrapper receives an explicit stay-out signal for a served port. + expect(fn).toMatch(/decision === "service-stay-out"[\s\S]{0,500}?StartCommandExit\(serviceStayOutExitCode\(\)\)/); }); test("the pre-bind owner probe spends the same budget before it deletes state", () => { diff --git a/tests/cli/cli-ready.test.ts b/tests/cli/cli-ready.test.ts index c214ecde156..e8ddf5eb769 100644 --- a/tests/cli/cli-ready.test.ts +++ b/tests/cli/cli-ready.test.ts @@ -844,8 +844,8 @@ describe("runReady production findLiveProxy deadline wiring (source-level)", () // ── handleStart service-wrapper exit guard (source-level) ───────────────────── // #764 follow-up: in OCX_SERVICE context a healthy proxy from ANY source must -// end handleStart with exit 0, so the opencodex-service.cmd `:loop` wrapper -// (retry on non-zero) does not respawn every 5s against a listener it can never +// end handleStart with the intentional stay-out code, so the service wrapper +// does not respawn every 5s against a listener it can never // claim. Source-level pin so a future edit cannot drop the guard silently. describe("handleStart OCX_SERVICE exit guard (source-level)", () => { const cliSource = readFileSync(repoPath("src/cli/index.ts"), "utf8"); @@ -854,14 +854,14 @@ describe("handleStart OCX_SERVICE exit guard (source-level)", () => { // The `OCX_SERVICE === "1"` comparison moved into `decideStartWithLiveOwner` // (src/cli/dispatch.ts), where the sentinel semantics are asserted at runtime // across the whole matrix (tests/cli/cli-dispatch.test.ts). This oracle pins the - // typed exits that the decision routes to: stay-out returns 0, the conflict returns 1. + // typed exits: stay-out uses the wrapper protocol, the conflict returns 1. expect(cliSource).toMatch(/decideStartWithLiveOwner\(\{/); // Anchor after the lease transaction begins. The earlier preflight has the same decision // pair but does not need a typed exit because it owns no lease yet. const transaction = cliSource.slice(cliSource.indexOf("bindAndPublishStartOwnership({")); const ownerBranch = transaction.slice(transaction.indexOf("decideStartWithLiveOwner({")); - const stayOut = ownerBranch.match(/decision === "service-stay-out"[\s\S]{0,800}?StartCommandExit\(0\)/); - expect(stayOut, "the service stay-out decision must return 0 when the port is already served").not.toBeNull(); + const stayOut = ownerBranch.match(/decision === "service-stay-out"[\s\S]{0,800}?StartCommandExit\(serviceStayOutExitCode\(\)\)/); + expect(stayOut, "the service stay-out decision must signal the wrapper when the port is already served").not.toBeNull(); const nonService = ownerBranch.match(/decision === "refuse"[\s\S]{0,500}?StartCommandExit\(1\)/); expect(nonService, "non-service refusal keeps the exit 1 conflict error").not.toBeNull(); }); diff --git a/tests/windows/windows-service-wrappers.test.ts b/tests/windows/windows-service-wrappers.test.ts index 4453e845a66..a86471a0b1b 100644 --- a/tests/windows/windows-service-wrappers.test.ts +++ b/tests/windows/windows-service-wrappers.test.ts @@ -124,3 +124,63 @@ describe("both teardown paths use the shared killer", () => { } }); }); + + +describe("scheduler child exit contract", () => { + test("zero and failure exits reach the cooldown; only explicit stay-out terminates", async () => { + const { buildWindowsServiceScript } = await import("../../src/service/windows-taskxml"); + for (const cli of ["C:\\ocx\\cli.ts", null]) { + const batch = buildWindowsServiceScript({ bun: "C:\\ocx\\bun.exe", bunRuntimeSource: "bundled", cli }, 10100, []); + const tail = batch.slice(batch.indexOf(' start --port 10100')).split("\r\n").slice(1); + expect(tail.slice(0, 6)).toEqual([ + 'if "%ERRORLEVEL%"=="42" goto stopped', + '>>"%OCX_SERVICE_LOG%" echo [%DATE% %TIME%] child exited with code %ERRORLEVEL%; restarting in 5s', + 'ping -n 6 127.0.0.1 >nul', + 'goto loop', + ':stopped', + 'endlocal', + ]); + expect(batch).toContain('set "OCX_WINDOWS_WRAPPER_PROTOCOL=1"'); + expect(batch).toContain('set "ERRORLEVEL="'); + expect(batch).toContain('exit /b 0'); + } + }); +}); + + +test("stay-out exit code is opt-in for new wrappers, preserving legacy services", async () => { + const { serviceStayOutExitCode } = await import("../../src/service/windows-wrapper-exit"); + expect(serviceStayOutExitCode({})).toBe(0); + expect(serviceStayOutExitCode({ OCX_SERVICE: "1" })).toBe(0); + expect(serviceStayOutExitCode({ OCX_WINDOWS_WRAPPER_PROTOCOL: "1" })).toBe(0); + expect(serviceStayOutExitCode({ OCX_SERVICE: "1", OCX_WINDOWS_WRAPPER_PROTOCOL: "1" })).toBe(42); + expect(serviceStayOutExitCode({ OCX_SERVICE: "1", OCX_WINDOWS_WRAPPER_PROTOCOL: "2" })).toBe(0); + const cli = read("src/cli/index.ts"); + const branches = [...cli.matchAll(/if \(decision === "service-stay-out"\) \{([\s\S]*?)\n\s*\}/g)]; + expect(branches).toHaveLength(3); + for (const branch of branches) expect(branch[1]).toContain("serviceStayOutExitCode()"); +}); + +test.skipIf(process.platform !== "win32")("cmd restarts zero/crash exits and stops on explicit stay-out", async () => { + const { mkdtempSync, writeFileSync, rmSync } = await import("node:fs"); + const { tmpdir } = await import("node:os"); + const { spawnSync } = await import("node:child_process"); + const { buildWindowsServiceScript } = await import("../../src/service/windows-taskxml"); + const dir = mkdtempSync(join(tmpdir(), "ocx-wrapper-exit-")); + try { + const batch = buildWindowsServiceScript({ bun: "bun.exe", bunRuntimeSource: "bundled", cli: null }, 10100, []); + const tail = batch.slice(batch.indexOf(' start --port 10100')).split("\r\n").slice(1).join("\r\n").split(":restore_backup")[0]; + for (const code of [0, 1, 42, 43, -1073741510]) { + const file = join(dir, "exit.cmd"); + // Exercise the generated control flow; replace only the cooldown to keep this fast. + writeFileSync(file, '@echo off\r\nsetlocal EnableExtensions DisableDelayedExpansion\r\nset "ERRORLEVEL="\r\nset "OCX_SERVICE_LOG=NUL"\r\n' + + `cmd /d /c exit ${code}\r\n` + tail.replace("ping -n 6 127.0.0.1 >nul", "rem skip cooldown") + + '\r\n:loop\r\nexit /b 99\r\n'); + const result = spawnSync("cmd.exe", ["/d", "/c", file], { timeout: 5000, env: { ...process.env, ERRORLEVEL: "42" } }); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(code === 42 ? 0 : 99); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } +});