From c7a74d096de0a6836140c7f62ea0b88dc826cd60 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 08:04:20 +0200 Subject: [PATCH 1/6] fix(service): bake WinSW admin env and retry stop-path probes Close the remaining #764 gaps: native WinSW now inherits install-time OPENCODEX_ADMIN_AUTH_TOKEN / OPENCODEX_ACL_TIMEOUT_MS, and service stop liveness retries short transport timeouts instead of treating a just-bound proxy as absent. --- src/lib/winsw.ts | 8 ++++++ src/server/proxy-liveness.ts | 47 +++++++++++++++++++++++++++--------- src/service.ts | 6 ++--- tests/proxy-liveness.test.ts | 31 ++++++++++++++++++++++++ tests/service.test.ts | 5 ++-- tests/winsw.test.ts | 13 ++++++++++ 6 files changed, 93 insertions(+), 17 deletions(-) diff --git a/src/lib/winsw.ts b/src/lib/winsw.ts index 42a5f9553..362f27636 100644 --- a/src/lib/winsw.ts +++ b/src/lib/winsw.ts @@ -87,12 +87,20 @@ export function buildWinswXml(entry: WinswEntry, env: NodeJS.ProcessEnv = proces })(); // Services never bake `--port 0` (parsePortOption rejects it); treat as default. const safeListenPort = listenPort > 0 && listenPort <= 65535 ? listenPort : 10100; + // SCM services do not inherit the interactive user environment. Bake install-time + // admin/ACL settings so `ocx service install --native` matches the scheduler case + // where a logon-time env block eventually appears (#764). The data-plane API token + // still uses a file pointer only — never embed OPENCODEX_API_AUTH_TOKEN. + const adminAuth = env.OPENCODEX_ADMIN_AUTH_TOKEN?.trim(); + const aclTimeout = env.OPENCODEX_ACL_TIMEOUT_MS?.trim(); const envLines = [ ` `, ` `, ` `, env.CODEX_HOME?.trim() ? ` ` : null, env.OPENCODEX_HOME?.trim() ? ` ` : null, + adminAuth ? ` ` : null, + aclTimeout ? ` ` : null, ].filter((line): line is string => Boolean(line)); return ` diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index 913afb057..5cda33fab 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -30,8 +30,21 @@ export interface LivenessIo { readRuntimeFn?: (pid?: number) => { pid?: number; port: number; hostname?: string } | null; configFn?: () => { port?: number; hostname?: string }; timeoutMs?: number; + /** + * How many times to retry a probe that failed with a transport error (timeout / + * connection refused). Definitive answers (non-OK HTTP, foreign /healthz body, pid + * mismatch) do not retry. Default 1 = no retry. Stop paths should pass 2–3 (#764). + */ + attempts?: number; + sleepFn?: (ms: number) => Promise; } +/** Default probe options for service stop / orphan cleanup — a just-bound proxy can miss a single 750ms probe. */ +export const SERVICE_STOP_LIVENESS: Pick = { + timeoutMs: 1500, + attempts: 3, +}; + export interface LiveProxy { pid: number | null; port: number; @@ -72,19 +85,29 @@ export async function proxyIdentityAt( io: LivenessIo = {}, ): Promise<{ pid: number | null } | null> { const fetchFn = io.fetchFn ?? fetch; - try { - const res = await fetchFn(`http://${probeHostname(opts.hostname)}:${port}/healthz`, { - signal: AbortSignal.timeout(io.timeoutMs ?? 750), - }); - if (!res.ok) return null; - const body = (await res.json().catch(() => null)) as HealthzIdentity | null; - if (!isOpencodexHealthz(body)) return null; - const pid = typeof body?.pid === "number" ? body.pid : null; - if (opts.expectedPid !== undefined && pid !== null && pid !== opts.expectedPid) return null; - return { pid }; - } catch { - return null; + const sleepFn = io.sleepFn ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); + const timeoutMs = io.timeoutMs ?? 750; + const attempts = Math.max(1, Math.min(Math.trunc(io.attempts ?? 1), 5)); + + for (let attempt = 1; attempt <= attempts; attempt++) { + try { + const res = await fetchFn(`http://${probeHostname(opts.hostname)}:${port}/healthz`, { + signal: AbortSignal.timeout(timeoutMs), + }); + if (!res.ok) return null; + const body = (await res.json().catch(() => null)) as HealthzIdentity | null; + if (!isOpencodexHealthz(body)) return null; + const pid = typeof body?.pid === "number" ? body.pid : null; + if (opts.expectedPid !== undefined && pid !== null && pid !== opts.expectedPid) return null; + return { pid }; + } catch { + // Transport failure (timeout / refused) — retry while budget remains; a proxy that + // has only just begun listening can miss a single short probe (#764). + if (attempt >= attempts) return null; + await sleepFn(100); + } } + return null; } /** diff --git a/src/service.ts b/src/service.ts index 4405018a6..36279badc 100644 --- a/src/service.ts +++ b/src/service.ts @@ -6,7 +6,7 @@ * restore it via the command. */ import { execFileSync, execSync } from "node:child_process"; -import { findLiveProxy } from "./server/proxy-liveness"; +import { findLiveProxy, SERVICE_STOP_LIVENESS } from "./server/proxy-liveness"; import { chmodSync, existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; @@ -1723,7 +1723,7 @@ export async function proxyStillLiveAfterStop(deps: { /** Whether the stopped supervisor can respawn its child; only then is polling worth the wait. */ canRespawn?: boolean; } = {}): Promise<{ port: number } | null> { - const findProxy = deps.findProxy ?? findLiveProxy; + const findProxy = deps.findProxy ?? (() => findLiveProxy(SERVICE_STOP_LIVENESS)); const sleep = deps.sleep ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); const now = deps.now ?? Date.now; const canRespawn = deps.canRespawn ?? process.platform === "win32"; @@ -1755,7 +1755,7 @@ async function stopTrackedProxyIfRunning(): Promise { } // Orphan recovery: the pid file can be missing/stale while the service wrapper keeps // a live proxy running — mirror `ocx stop`'s identity-checked findLiveProxy fallback. - const live = await findLiveProxy({ timeoutMs: 1500 }); + const live = await findLiveProxy(SERVICE_STOP_LIVENESS); const liveKillPid = verifiedKillTarget(live?.pid); if (liveKillPid !== null) { await stopProxy(liveKillPid); diff --git a/tests/proxy-liveness.test.ts b/tests/proxy-liveness.test.ts index 8a90c58c8..825c72b38 100644 --- a/tests/proxy-liveness.test.ts +++ b/tests/proxy-liveness.test.ts @@ -51,6 +51,37 @@ describe("proxyIdentityAt", () => { expect(await proxyIdentityAt(10100, { expectedPid: 1 }, { fetchFn: (async () => healthz(OURS)) as typeof fetch })).toBeNull(); expect(await proxyIdentityAt(10100, {}, { fetchFn: (async () => { throw new Error("refused"); }) as typeof fetch })).toBeNull(); }); + + test("retries transport failures and succeeds on a later attempt (#764)", async () => { + let calls = 0; + const sleeps: number[] = []; + const identity = await proxyIdentityAt(10100, {}, { + attempts: 3, + timeoutMs: 50, + sleepFn: async (ms) => { sleeps.push(ms); }, + fetchFn: (async () => { + calls += 1; + if (calls < 3) throw new Error("timeout"); + return healthz(OURS); + }) as typeof fetch, + }); + expect(identity).toEqual({ pid: 4242 }); + expect(calls).toBe(3); + expect(sleeps).toEqual([100, 100]); + }); + + test("does not retry a definitive foreign /healthz body", async () => { + let calls = 0; + const identity = await proxyIdentityAt(10100, {}, { + attempts: 3, + fetchFn: (async () => { + calls += 1; + return healthz({ ok: true }); + }) as typeof fetch, + }); + expect(identity).toBeNull(); + expect(calls).toBe(1); + }); }); describe("findLiveProxy", () => { diff --git a/tests/service.test.ts b/tests/service.test.ts index 382e9c23f..8e93a03c2 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -661,10 +661,11 @@ describe("service lifecycle cleanup ordering", () => { expect(service).toContain('verifyPidIdentity'); expect(service).toContain("removeRuntimePort(pid);"); expect(service).toContain('import { isProcessAlive, stopProxy } from "./lib/process-control";'); - expect(service).toContain('import { findLiveProxy } from "./server/proxy-liveness";'); + expect(service).toContain('import { findLiveProxy, SERVICE_STOP_LIVENESS } from "./server/proxy-liveness";'); expect(service).toContain('type TrackedProxyCleanupResult = "none" | "stale" | "stopped";'); expect(service).toContain("async function stopTrackedProxyIfRunning(): Promise"); - expect(service).toContain("await findLiveProxy({ timeoutMs: 1500 })"); + expect(service).toContain("await findLiveProxy(SERVICE_STOP_LIVENESS)"); + expect(service).toContain("SERVICE_STOP_LIVENESS"); expect(service).toContain("await stopProxy(trackedKillPid);"); expect(service).toContain("await stopProxy(liveKillPid);"); expect(service).toContain("removePid(pid);"); diff --git a/tests/winsw.test.ts b/tests/winsw.test.ts index 8c922d095..d02ebc58d 100644 --- a/tests/winsw.test.ts +++ b/tests/winsw.test.ts @@ -32,6 +32,19 @@ describe("winsw xml", () => { expect(xml).toContain(''); // The token VALUE never lands in the XML — only the file pointer. expect(xml).not.toContain("OPENCODEX_API_AUTH_TOKEN"); + expect(xml).not.toContain("OPENCODEX_ADMIN_AUTH_TOKEN"); + expect(xml).not.toContain("OPENCODEX_ACL_TIMEOUT_MS"); + }); + + test("bakes install-time admin auth and ACL timeout into the SCM env (#764)", () => { + const xml = buildWinswXml(entry, { + ...env, + OPENCODEX_ADMIN_AUTH_TOKEN: "admin-secret & more", + OPENCODEX_ACL_TIMEOUT_MS: "10000", + }); + + expect(xml).toContain(''); + expect(xml).toContain(''); }); test("escapes executable/arguments and configures restart + graceful stop", () => { From 29b961d391646bde47a7c89d629c803792a821b6 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 08:29:08 +0200 Subject: [PATCH 2/6] fix(winsw): never embed OPENCODEX_ADMIN_AUTH_TOKEN in service XML Codex P1: uninstall retains the WinSW XML, so baking the admin secret would leave it on disk. Keep ACL timeout + always-bake OPENCODEX_HOME so file-backed admin auth still resolves under SCM. --- src/lib/winsw.ts | 13 ++++++------- tests/winsw.test.ts | 10 ++++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/lib/winsw.ts b/src/lib/winsw.ts index 362f27636..5222e1228 100644 --- a/src/lib/winsw.ts +++ b/src/lib/winsw.ts @@ -87,19 +87,18 @@ export function buildWinswXml(entry: WinswEntry, env: NodeJS.ProcessEnv = proces })(); // Services never bake `--port 0` (parsePortOption rejects it); treat as default. const safeListenPort = listenPort > 0 && listenPort <= 65535 ? listenPort : 10100; - // SCM services do not inherit the interactive user environment. Bake install-time - // admin/ACL settings so `ocx service install --native` matches the scheduler case - // where a logon-time env block eventually appears (#764). The data-plane API token - // still uses a file pointer only — never embed OPENCODEX_API_AUTH_TOKEN. - const adminAuth = env.OPENCODEX_ADMIN_AUTH_TOKEN?.trim(); + // SCM services do not inherit the interactive user environment (#764). Bake: + // - OPENCODEX_HOME so file-backed admin auth (`admin-api-token`) resolves + // - OPENCODEX_ACL_TIMEOUT_MS when set (not a secret) + // Never embed OPENCODEX_ADMIN_AUTH_TOKEN or OPENCODEX_API_AUTH_TOKEN values in XML — + // those stay file-pointer / generated-file only (uninstall retains the XML). const aclTimeout = env.OPENCODEX_ACL_TIMEOUT_MS?.trim(); const envLines = [ ` `, ` `, ` `, env.CODEX_HOME?.trim() ? ` ` : null, - env.OPENCODEX_HOME?.trim() ? ` ` : null, - adminAuth ? ` ` : null, + ` `, aclTimeout ? ` ` : null, ].filter((line): line is string => Boolean(line)); return ` diff --git a/tests/winsw.test.ts b/tests/winsw.test.ts index d02ebc58d..e1157f121 100644 --- a/tests/winsw.test.ts +++ b/tests/winsw.test.ts @@ -30,21 +30,23 @@ describe("winsw xml", () => { expect(xml).toContain(''); expect(xml).toContain(''); - // The token VALUE never lands in the XML — only the file pointer. + expect(xml).toContain(' { + test("bakes install-time ACL timeout and never embeds the admin token (#764)", () => { const xml = buildWinswXml(entry, { ...env, OPENCODEX_ADMIN_AUTH_TOKEN: "admin-secret & more", OPENCODEX_ACL_TIMEOUT_MS: "10000", }); - expect(xml).toContain(''); expect(xml).toContain(''); + expect(xml).toContain(' { From 055812e862174c56ed9f19295b7b573b74eb12f7 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 08:36:52 +0200 Subject: [PATCH 3/6] test(service): assert stop-verification uses SERVICE_STOP_LIVENESS CodeRabbit: pin the proxyStillLiveAfterStop default findProxy closure, not only the orphan-cleanup call site. --- tests/service.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/service.test.ts b/tests/service.test.ts index 8e93a03c2..4c1a8f44c 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -665,6 +665,7 @@ describe("service lifecycle cleanup ordering", () => { expect(service).toContain('type TrackedProxyCleanupResult = "none" | "stale" | "stopped";'); expect(service).toContain("async function stopTrackedProxyIfRunning(): Promise"); expect(service).toContain("await findLiveProxy(SERVICE_STOP_LIVENESS)"); + expect(service).toContain("(() => findLiveProxy(SERVICE_STOP_LIVENESS))"); expect(service).toContain("SERVICE_STOP_LIVENESS"); expect(service).toContain("await stopProxy(trackedKillPid);"); expect(service).toContain("await stopProxy(liveKillPid);"); From 048703eb3d0d3ecbf7fe056bad41b265915a1989 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:37:03 +0200 Subject: [PATCH 4/6] test(sidebar): stop spawning real gh in star route tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows CI was timing out at Bun's 5s default while AUTH_TIMEOUT waited on a hung gh spawn — inject StarDeps so route tests stay hermetic. --- src/github/star-state.ts | 37 +++++++++++++++++++++++++----------- tests/sidebar-routes.test.ts | 20 +++++++++++++++---- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/github/star-state.ts b/src/github/star-state.ts index 06e15f139..9a0df408a 100644 --- a/src/github/star-state.ts +++ b/src/github/star-state.ts @@ -80,6 +80,18 @@ async function spawnGh(args: string[], timeoutMs: number): Promise<{ status: num } const defaultDeps: StarDeps = { runGh: spawnGh, nowMs: () => Date.now() }; +/** Test-only override so route tests never spawn a real `gh` (Windows CI hangs at AUTH_TIMEOUT). */ +let depsOverride: StarDeps | null = null; + +function activeDeps(deps?: StarDeps): StarDeps { + return deps ?? depsOverride ?? defaultDeps; +} + +/** Swap the default `gh` runner for tests; pass `null` to restore production deps. */ +export function setStarDepsForTests(deps: StarDeps | null): void { + depsOverride = deps; + invalidateStarStatusCache(); +} let cached: { timestamp: number; state: StarState } | null = null; /** Coalesces concurrent probes so parallel sidebar polls share one `gh` run. */ @@ -97,10 +109,11 @@ let generation = 0; * starred and 404 when not, so a non-zero exit is only meaningful once we know * the CLI is authenticated — hence the auth check first. */ -export async function probeStarState(deps: StarDeps = defaultDeps): Promise { - const auth = await deps.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS); +export async function probeStarState(deps?: StarDeps): Promise { + const d = activeDeps(deps); + const auth = await d.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS); if (!auth || auth.status !== 0) return "unauthenticated"; - const starred = await deps.runGh( + const starred = await d.runGh( ["api", "--hostname", GH_HOSTNAME, `/user/starred/${STAR_REPO}`], API_TIMEOUT_MS, ); @@ -109,8 +122,9 @@ export async function probeStarState(deps: StarDeps = defaultDeps): Promise { - const now = deps.nowMs(); +export async function getStarStatus(deps?: StarDeps): Promise { + const d = activeDeps(deps); + const now = d.nowMs(); if (cached && now - cached.timestamp < CACHE_TTL_MS) { return { state: cached.state, repo: STAR_REPO, url: STAR_REPO_URL }; } @@ -123,7 +137,7 @@ export async function getStarStatus(deps: StarDeps = defaultDeps): Promise { if (inflight === probe) inflight = null; // A write landed while this read was in flight — its result is authoritative. @@ -159,19 +173,20 @@ export function invalidateStarStatusCache(): void { * management API. */ export async function starRepository( - deps: StarDeps = defaultDeps, + deps?: StarDeps, ): Promise<{ ok: boolean; status: StarStatus; code?: StarErrorCode }> { - const auth = await deps.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS); + const d = activeDeps(deps); + const auth = await d.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS); if (!auth || auth.status !== 0) { generation += 1; - cached = { timestamp: deps.nowMs(), state: "unauthenticated" }; + cached = { timestamp: d.nowMs(), state: "unauthenticated" }; return { ok: false, status: { state: "unauthenticated", repo: STAR_REPO, url: STAR_REPO_URL }, code: "gh_unavailable", }; } - const result = await deps.runGh( + const result = await d.runGh( ["api", "--hostname", GH_HOSTNAME, "-X", "PUT", `/user/starred/${STAR_REPO}`], API_TIMEOUT_MS, ); @@ -186,6 +201,6 @@ export async function starRepository( // Authoritative: this call just starred the repo. Bumping the generation makes any // read that is still in flight discard its now-obsolete observation. generation += 1; - cached = { timestamp: deps.nowMs(), state: "starred" }; + cached = { timestamp: d.nowMs(), state: "starred" }; return { ok: true, status: { state: "starred", repo: STAR_REPO, url: STAR_REPO_URL } }; } diff --git a/tests/sidebar-routes.test.ts b/tests/sidebar-routes.test.ts index e7af90028..4feed9c0c 100644 --- a/tests/sidebar-routes.test.ts +++ b/tests/sidebar-routes.test.ts @@ -1,6 +1,6 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { handleManagementAPI } from "../src/server/management-api"; -import { invalidateStarStatusCache } from "../src/github/star-state"; +import { setStarDepsForTests } from "../src/github/star-state"; import type { OcxConfig } from "../src/types"; /** @@ -8,6 +8,9 @@ import type { OcxConfig } from "../src/types"; * machine; this file checks that the routes are actually reachable through the * management dispatcher and that the serialized bytes carry no `gh` output, token, * or account identifier. + * + * Star probes must not spawn a real `gh` here: on Windows the bare CLI can hang until + * AUTH_TIMEOUT (5s), which trips Bun's default test timeout under CI contention. */ const config = { port: 10100, @@ -31,6 +34,17 @@ async function call( return { status: res.status, body: raw ? JSON.parse(raw) : null, raw, routed: true }; } +beforeEach(() => { + setStarDepsForTests({ + runGh: async () => ({ status: 1 }), + nowMs: () => 1_000, + }); +}); + +afterEach(() => { + setStarDepsForTests(null); +}); + describe("GET /api/update/badge", () => { test("is routed and returns the badge shape", async () => { const { status, body } = await call("GET", "/api/update/badge"); @@ -52,7 +66,6 @@ describe("GET /api/update/badge", () => { describe("GET /api/github/star", () => { test("is routed and reports one of the three known states", async () => { - invalidateStarStatusCache(); const { status, body } = await call("GET", "/api/github/star"); expect(status).toBe(200); const star = body as Record; @@ -62,7 +75,6 @@ describe("GET /api/github/star", () => { }); test("never serializes gh output, tokens, or account identifiers", async () => { - invalidateStarStatusCache(); const { raw } = await call("GET", "/api/github/star"); // `gh auth status` prints "Logged in to github.com account " and the token // scopes; none of that may cross this boundary. From 7586a229be9c9e162248ca35e9ae6b9b2c872461 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:06:51 +0200 Subject: [PATCH 5/6] fix(liveness): bound stop probes and harden WinSW secret tests Stop-path discovery now shares a wall-clock deadline so multi-candidate SERVICE_STOP_LIVENESS retries cannot overrun the verification window, and tests assert resolved OPENCODEX_HOME plus absent API/admin token values. --- src/server/proxy-liveness.ts | 20 ++++++++++++++++++-- src/service.ts | 15 +++++++++++++-- tests/proxy-liveness.test.ts | 33 +++++++++++++++++++++++++++++++++ tests/service.test.ts | 4 ++-- tests/winsw.test.ts | 18 ++++++++++++++++-- 5 files changed, 82 insertions(+), 8 deletions(-) diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index 5cda33fab..367ec7b1f 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -37,6 +37,14 @@ export interface LivenessIo { */ attempts?: number; sleepFn?: (ms: number) => Promise; + /** + * Absolute wall-clock deadline for discovery. When set, each probe attempt aborts + * once the remaining budget cannot cover another fetch — so multi-candidate + * `findLiveProxy` under `SERVICE_STOP_LIVENESS` cannot overrun the stop-path + * verification window (#764 / CodeRabbit). + */ + deadlineAt?: number; + nowFn?: () => number; } /** Default probe options for service stop / orphan cleanup — a just-bound proxy can miss a single 750ms probe. */ @@ -86,10 +94,17 @@ export async function proxyIdentityAt( ): Promise<{ pid: number | null } | null> { const fetchFn = io.fetchFn ?? fetch; const sleepFn = io.sleepFn ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); - const timeoutMs = io.timeoutMs ?? 750; - const attempts = Math.max(1, Math.min(Math.trunc(io.attempts ?? 1), 5)); + const nowFn = io.nowFn ?? Date.now; + const baseTimeoutMs = io.timeoutMs ?? 750; + const requestedAttempts = Math.trunc(io.attempts ?? 1); + const attempts = Number.isNaN(requestedAttempts) + ? 1 + : Math.max(1, Math.min(requestedAttempts, 5)); for (let attempt = 1; attempt <= attempts; attempt++) { + const remainingMs = io.deadlineAt === undefined ? baseTimeoutMs : io.deadlineAt - nowFn(); + if (remainingMs <= 0) return null; + const timeoutMs = Math.min(baseTimeoutMs, remainingMs); try { const res = await fetchFn(`http://${probeHostname(opts.hostname)}:${port}/healthz`, { signal: AbortSignal.timeout(timeoutMs), @@ -104,6 +119,7 @@ export async function proxyIdentityAt( // Transport failure (timeout / refused) — retry while budget remains; a proxy that // has only just begun listening can miss a single short probe (#764). if (attempt >= attempts) return null; + if (io.deadlineAt !== undefined && io.deadlineAt - nowFn() <= 0) return null; await sleepFn(100); } } diff --git a/src/service.ts b/src/service.ts index 36279badc..bdf614079 100644 --- a/src/service.ts +++ b/src/service.ts @@ -1723,11 +1723,18 @@ export async function proxyStillLiveAfterStop(deps: { /** Whether the stopped supervisor can respawn its child; only then is polling worth the wait. */ canRespawn?: boolean; } = {}): Promise<{ port: number } | null> { - const findProxy = deps.findProxy ?? (() => findLiveProxy(SERVICE_STOP_LIVENESS)); const sleep = deps.sleep ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); const now = deps.now ?? Date.now; const canRespawn = deps.canRespawn ?? process.platform === "win32"; const deadline = now() + (canRespawn ? 7000 : 0); + // Single-shot (non-respawn) still needs one full SERVICE_STOP_LIVENESS budget; respawn + // polling shares the outer deadline so multi-candidate discovery cannot overrun it. + const findProxy = deps.findProxy ?? (() => { + const probeDeadline = canRespawn + ? deadline + : now() + (SERVICE_STOP_LIVENESS.timeoutMs! * SERVICE_STOP_LIVENESS.attempts! + 250); + return findLiveProxy({ ...SERVICE_STOP_LIVENESS, deadlineAt: probeDeadline, nowFn: now }); + }); for (;;) { try { const live = await findProxy(); @@ -1755,7 +1762,11 @@ async function stopTrackedProxyIfRunning(): Promise { } // Orphan recovery: the pid file can be missing/stale while the service wrapper keeps // a live proxy running — mirror `ocx stop`'s identity-checked findLiveProxy fallback. - const live = await findLiveProxy(SERVICE_STOP_LIVENESS); + // Cap multi-candidate discovery so stop cleanup cannot hang for three full retry budgets. + const live = await findLiveProxy({ + ...SERVICE_STOP_LIVENESS, + deadlineAt: Date.now() + 7000, + }); const liveKillPid = verifiedKillTarget(live?.pid); if (liveKillPid !== null) { await stopProxy(liveKillPid); diff --git a/tests/proxy-liveness.test.ts b/tests/proxy-liveness.test.ts index 825c72b38..9e8498eac 100644 --- a/tests/proxy-liveness.test.ts +++ b/tests/proxy-liveness.test.ts @@ -82,6 +82,39 @@ describe("proxyIdentityAt", () => { expect(identity).toBeNull(); expect(calls).toBe(1); }); + + test("NaN attempts fall back to a single probe", async () => { + let calls = 0; + const identity = await proxyIdentityAt(10100, {}, { + attempts: Number.NaN, + fetchFn: (async () => { + calls += 1; + return healthz(OURS); + }) as typeof fetch, + }); + expect(identity).toEqual({ pid: 4242 }); + expect(calls).toBe(1); + }); + + test("honors an aggregate deadline across transport retries", async () => { + let calls = 0; + let clock = 1_000; + const identity = await proxyIdentityAt(10100, {}, { + attempts: 3, + timeoutMs: 1_500, + deadlineAt: 1_000 + 1_200, + nowFn: () => clock, + sleepFn: async () => { clock += 100; }, + fetchFn: (async () => { + calls += 1; + clock += 1_500; + throw new Error("timeout"); + }) as typeof fetch, + }); + expect(identity).toBeNull(); + // First attempt spends the budget; remaining retries must not fire. + expect(calls).toBe(1); + }); }); describe("findLiveProxy", () => { diff --git a/tests/service.test.ts b/tests/service.test.ts index 4c1a8f44c..e2005372a 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -664,8 +664,8 @@ describe("service lifecycle cleanup ordering", () => { expect(service).toContain('import { findLiveProxy, SERVICE_STOP_LIVENESS } from "./server/proxy-liveness";'); expect(service).toContain('type TrackedProxyCleanupResult = "none" | "stale" | "stopped";'); expect(service).toContain("async function stopTrackedProxyIfRunning(): Promise"); - expect(service).toContain("await findLiveProxy(SERVICE_STOP_LIVENESS)"); - expect(service).toContain("(() => findLiveProxy(SERVICE_STOP_LIVENESS))"); + expect(service).toContain("...SERVICE_STOP_LIVENESS"); + expect(service).toContain("deadlineAt:"); expect(service).toContain("SERVICE_STOP_LIVENESS"); expect(service).toContain("await stopProxy(trackedKillPid);"); expect(service).toContain("await stopProxy(liveKillPid);"); diff --git a/tests/winsw.test.ts b/tests/winsw.test.ts index e1157f121..eb42df8ee 100644 --- a/tests/winsw.test.ts +++ b/tests/winsw.test.ts @@ -2,12 +2,23 @@ import { describe, expect, test } from "bun:test"; import { buildWinswXml, ensureWinswBinary, parseWinswStatus, probeScmRegistration, sha256Hex, installWinswService, statusWinswRaw, WINSW_SHA256, WINSW_SERVICE_ID } from "../src/lib/winsw"; import { parseServiceArgs, serviceReinstallArgs } from "../src/service"; import { loadServiceTokenFromFile } from "../src/lib/service-secrets"; +import { getConfigDir } from "../src/config"; import { mkdtempSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; const entry = { bun: "C:\\OpenCodex\\bun.exe", cli: "C:\\Open Codex\\cli & co\\index.ts" }; +function winswEnvValue(xml: string, name: string): string | null { + const match = xml.match(new RegExp(``)); + if (!match) return null; + return match[1]! + .replace(/&/g, "&") + .replace(/</g, "<") + .replace(/>/g, ">") + .replace(/"/g, '"'); +} + describe("winsw xml", () => { const env = { USERDOMAIN: "WORKGROUP", USERNAME: "jun", PATH: "C:\\bin;C:\\tools & more" } as NodeJS.ProcessEnv; @@ -30,7 +41,7 @@ describe("winsw xml", () => { expect(xml).toContain(''); expect(xml).toContain(''); - expect(xml).toContain(' { test("bakes install-time ACL timeout and never embeds the admin token (#764)", () => { const xml = buildWinswXml(entry, { ...env, + OPENCODEX_API_AUTH_TOKEN: "api-secret-value", OPENCODEX_ADMIN_AUTH_TOKEN: "admin-secret & more", OPENCODEX_ACL_TIMEOUT_MS: "10000", }); expect(xml).toContain(''); - expect(xml).toContain(' { From 3e95ba33f45936eec4c2b3ade71167d598b913f5 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:11:02 +0200 Subject: [PATCH 6/6] test(storage): raise cleanup suite timeout for Windows CI load The preview route test was failing at Bun's 5s default under runner contention while siblings already used 20s. --- tests/api-storage-cleanup.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/api-storage-cleanup.test.ts b/tests/api-storage-cleanup.test.ts index 8676f7115..0061b3db0 100644 --- a/tests/api-storage-cleanup.test.ts +++ b/tests/api-storage-cleanup.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 { managementFetch as fetch } from "./helpers/management-auth"; import { Database } from "bun:sqlite"; import { existsSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; @@ -9,6 +9,10 @@ import { startServer } from "../src/server"; import type { OcxConfig } from "../src/types"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; +// Windows CI under load can spend >5s just binding the proxy + previewing cleanup; +// Bun's default test budget then fails the suite before the assertion runs. +setDefaultTimeout(20_000); + let testDir = ""; let previousHome: string | undefined; let isolatedCodexHome: IsolatedCodexHome | null = null;