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(); + }); +}); 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"');