From 1aceb714d66f5a33563729490e417e9824ff77af Mon Sep 17 00:00:00 2001 From: t Date: Wed, 9 Sep 2026 06:15:39 +0900 Subject: [PATCH 1/2] fix(cli): report the reason a proxy refused to stop /api/stop answers 409 for two different reasons. The original one is a scheduler wrapper under another CODEX_HOME that would respawn the proxy anyway; #4023 added a second, the proxy being the installed launchd or systemd job itself. stopProxy reported the first unconditionally, so an operator whose proxy is simply the service was told "a service installed under a different CODEX_HOME/OPENCODEX_HOME owns it. Run the stop from that home" and sent to a home that does not exist. The server already sends a precise message for each case. It is now carried through to the thrown error, with the previous text kept as the fallback when the body is unreadable. Found by the main-to-dev regression review for the 2.49.0 promotion. It needs a receipt-write failure plus a surviving managed proxy to reach, which is why neither the #4023 review nor its CI caught it. --- src/lib/process-control.ts | 35 +++++++++++++++++--- tests/lib/process-control-graceful.test.ts | 37 +++++++++++++++++++++- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/lib/process-control.ts b/src/lib/process-control.ts index 13ab3a0c95..e2262e9e96 100644 --- a/src/lib/process-control.ts +++ b/src/lib/process-control.ts @@ -71,7 +71,25 @@ export function gracefulStopHost(hostname: string | undefined): string { */ export type GracefulStopResult = boolean | "refused"; -/** A proxy declined shutdown because a service under another home owns it (HTTP 409). */ +/** + * The server's own explanation for the most recent 409, captured so `stopProxy` can report + * the real reason. There is more than one: a scheduler wrapper under another home, or the + * proxy being the installed service itself (#4023). Module-scoped because + * `GracefulStopResult` is a public contract with several callers, and widening it to carry + * the text would change every one of them for a message only this file reports. + */ +let lastRefusalMessage: string | null = null; + +/** The server's explanation for the most recent 409, or `null` when it sent none. */ +export function lastStopRefusalMessage(): string | null { + return lastRefusalMessage; +} + +/** + * A proxy declined shutdown (HTTP 409). There is more than one reason it can say no — a + * scheduler wrapper under another home, or the proxy being the installed service itself + * (#4023) — so the server's own message is carried through rather than guessed at. + */ export class ProxyOwnershipRefusedError extends Error {} /** @@ -111,7 +129,15 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): // would respawn it anyway). That is a policy answer, not a dead endpoint — escalating to // SIGTERM here would run the daemon's cleanup and strip shared config out from under the // still-running service. Report the refusal instead of forcing. - if (res.status === 409) return "refused"; + if (res.status === 409) { + lastRefusalMessage = await res.json() + .then(body => { + const message = (body as { message?: unknown } | null)?.message; + return typeof message === "string" && message.trim() ? message.trim() : null; + }) + .catch(() => null); + return "refused"; + } if (!res.ok) return false; } catch { return false; @@ -140,8 +166,9 @@ export async function stopProxy(pid: number, io: GracefulStopIo = {}): Promise { expect(noExit).toBe(false); }); }); + +describe("409 refusal reporting", () => { + test("a refusal carries the server's own reason, not the ownership guess", async () => { + // /api/stop answers 409 for more than one reason: a scheduler wrapper under another + // home, and (since #4023) the proxy being the installed launchd/systemd job itself. + // stopProxy used to report the first of those unconditionally, sending an operator + // whose proxy is simply the service to a CODEX_HOME that does not exist. + const selfUnload = "This proxy is running as the installed service, so stopping the manager" + + " from inside it would end this process before native Codex is restored." + + " Run `ocx stop`, which stops the service from outside and completes the restore." + + " Nothing was changed."; + const result = await stopProxyGracefully(7, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response( + JSON.stringify({ success: false, code: "self_unload_service", message: selfUnload }), + { status: 409, headers: { "content-type": "application/json" } }, + )) as typeof fetch, + waitExit: () => true, + env: {}, + }); + expect(result).toBe("refused"); + expect(lastStopRefusalMessage()).toBe(selfUnload); + }); + + test("a 409 with no readable body falls back rather than reporting a stale reason", async () => { + const result = await stopProxyGracefully(7, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response("not json", { status: 409 })) as typeof fetch, + waitExit: () => true, + env: {}, + }); + expect(result).toBe("refused"); + expect(lastStopRefusalMessage()).toBeNull(); + }); +}); From db8996df7e4c39d6df00106860f7e52ff5d68334 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 9 Sep 2026 06:25:54 +0900 Subject: [PATCH 2/2] test(xai): assert the 409 invariant without pinning the exact line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The source oracle matched `if (res.status === 409) return "refused"` verbatim, so capturing the server's refusal reason first broke it even though the invariant it protects — a 409 returns "refused" and never falls through to the forced-kill path — still holds. It now locates the 409 branch inside stopProxyGracefully and asserts the ordering: "refused" is returned before the !res.ok fallthrough. Removing that return still turns the test red, so the oracle is no weaker than before. --- tests/providers/xai/grok-lifecycle.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/providers/xai/grok-lifecycle.test.ts b/tests/providers/xai/grok-lifecycle.test.ts index 554ab98d65..88212768af 100644 --- a/tests/providers/xai/grok-lifecycle.test.ts +++ b/tests/providers/xai/grok-lifecycle.test.ts @@ -509,7 +509,18 @@ describe("POST /api/stop teardown", () => { test("a 409 does not escalate to a forced kill", () => { // Escalating would run the daemon's cleanup and strip shared config while the foreign // service keeps the proxy alive — the exact hole the ownership gate exists to close. - expect(PROCESS_CONTROL_SOURCE).toContain('if (res.status === 409) return "refused"'); + // The 409 branch may capture the server's reason first (#4023 added a second refusal + // cause), but it must still return "refused" without falling through to !res.ok. + const stopGracefully = sliceFn( + PROCESS_CONTROL_SOURCE, + "export async function stopProxyGracefully(", + "export async function stopProxy(", + ); + const four09At = stopGracefully.indexOf("res.status === 409"); + expect(four09At).toBeGreaterThan(-1); + expect(stopGracefully.slice(four09At)).toContain('return "refused"'); + expect(stopGracefully.indexOf('return "refused"', four09At)) + .toBeLessThan(stopGracefully.indexOf("if (!res.ok) return false;", four09At)); const stopProxyFn = sliceFn(PROCESS_CONTROL_SOURCE, "export async function stopProxy(", "export function killProxy("); const refusedAt = stopProxyFn.indexOf('graceful === "refused"');