Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ async function handleStop() {
if (detail) console.error(` ${detail}`);
if (err instanceof ProxyOwnershipRefusedError) {
ownershipBlocked = true;
console.error(" Skipping shared teardown (native Codex restore, Grok config): the foreign proxy is still running.");
console.error(" Skipping shared teardown (native Codex restore, Grok config): the refusing proxy is still running.");
}
}
} else {
Expand All @@ -979,7 +979,7 @@ async function handleStop() {
if (detail) console.error(` ${detail}`);
if (err instanceof ProxyOwnershipRefusedError) {
ownershipBlocked = true;
console.error(" Skipping shared teardown (native Codex restore, Grok config): the foreign proxy is still running.");
console.error(" Skipping shared teardown (native Codex restore, Grok config): the refusing proxy is still running.");
}
}
} else if (live) {
Expand Down
106 changes: 87 additions & 19 deletions src/lib/process-control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,49 @@ export type GracefulStopResult = boolean | "refused" | "teardown-unconfirmed";
*/
let lastRefusalMessage: string | null = null;

/**
* The server's machine-readable reason for the most recent 409, captured alongside the
* message so a refusal that arrives without a body still names the right cause. Without it
* the fallback has to guess, and guessing "ownership" sent operators to re-check
* CODEX_HOME for a refusal the scheduler wrapper had issued (#4169).
*/
let lastRefusalCode: 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;
}

/** The server's `code` for the most recent 409, or `null` when it sent none. */
export function lastStopRefusalCode(): string | null {
return lastRefusalCode;
}

/**
* Wording for a refusal whose body carried no message. Each branch mirrors a refusal the
* management API can return from `POST /api/stop`; the default stays cause-neutral because
* naming the wrong cause is worse than naming none — it costs the operator the time they
* spend acting on it.
*/
function refusalFallbackMessage(code: string | null): string {
switch (code) {
case "respawnable_service":
return "The running proxy refused to stop: a service manager that can respawn it owns "
+ "the process. Run `ocx stop`, which verifies the respawn window.";
case "self_unload_service":
return "The running proxy refused to stop: it is the installed service itself, so "
+ "stopping the manager from inside it would end the process before native Codex is "
+ "restored. Run `ocx stop`.";
case "service_state_unknown":
return "The running proxy refused to stop: the service manager state could not be read, "
+ "so it cannot tell whether a wrapper would respawn it. Run `ocx service status` to "
+ "see the query error.";
default:
return "The running proxy refused to stop and sent no reason. Run `ocx service status` "
+ "to inspect the service state.";
}
}

/**
* 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
Expand All @@ -104,9 +142,28 @@ export class ProxyOwnershipRefusedError extends Error {}
* attest the process exit code or completion of every drain/shutdown hook.
*/
export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): Promise<GracefulStopResult> {
return (await stopProxyGracefullyDetailed(pid, io)).result;
}

/**
* The refusal a single stop attempt received, carried back to that attempt's caller.
*
* Module-scoped state cannot do this job: two overlapping stops race, and the first would
* report the second's cause. The exported accessors stay as observational state for callers
* that only want the last refusal, but the error text is built from this per-call value.
*/
type StopRefusal = { message: string | null; code: string | null };

async function stopProxyGracefullyDetailed(
pid: number,
io: GracefulStopIo = {},
): Promise<{ result: GracefulStopResult; refusal: StopRefusal }> {
const refusal: StopRefusal = { message: null, code: null };
const done = (result: GracefulStopResult): { result: GracefulStopResult; refusal: StopRefusal } =>
({ result, refusal });
const readRuntime = io.readRuntime ?? readRuntimePort;
const runtime = io.runtimeEndpoint ?? readRuntime(pid);
if (!runtime?.port) return false;
if (!runtime?.port) return done(false);
const env = io.env ?? process.env;
const headers: Record<string, string> = {};
const token = configuredAdminToken(env.OPENCODEX_HOME?.trim() || undefined, env as NodeJS.ProcessEnv);
Expand All @@ -128,20 +185,31 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}):
// longer than a health poll so we prefer drain over taskkill /F.
signal: AbortSignal.timeout(io.exitTimeoutMs ? Math.min(io.exitTimeoutMs, 10_000) : 10_000),
});
// 409 is the proxy REFUSING to stop (a service installed under another home owns it and
// 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
// 409 is the proxy REFUSING to stop. There is more than one reason it can say no — a
// respawning service manager, the proxy being the installed service itself, or an
// unreadable scheduler state — so both the message and the code are captured rather
// than assumed. 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) {
lastRefusalMessage = await res.json()
const parsed = await res.json()
.then(body => {
const message = (body as { message?: unknown } | null)?.message;
return typeof message === "string" && message.trim() ? message.trim() : null;
const record = body as { message?: unknown; code?: unknown } | null;
const message = record?.message;
const code = record?.code;
return {
message: typeof message === "string" && message.trim() ? message.trim() : null,
code: typeof code === "string" && code.trim() ? code.trim() : null,
};
})
.catch(() => null);
return "refused";
.catch(() => ({ message: null, code: null }));
refusal.message = parsed.message;
refusal.code = parsed.code;
lastRefusalMessage = parsed.message;
lastRefusalCode = parsed.code;
return done("refused");
}
if (!res.ok) return false;
if (!res.ok) return done(false);
const body: unknown = await res.json().catch(() => null);
const expectedTeardown = io.deferSharedTeardownNonce ? "deferred" : "performed";
sharedTeardownConfirmed = body !== null
Expand All @@ -150,14 +218,14 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}):
&& "success" in body && body.success === true
&& "sharedTeardown" in body && body.sharedTeardown === expectedTeardown;
} catch {
return false;
return done(false);
}
const waitExit = io.waitExit ?? waitForExit;
// Honor the server's own drain window: /api/stop answers 200 first, then drains for
// config.shutdownTimeoutMs. Waiting less than that hard-kills mid-drain.
const exitTimeoutMs = io.exitTimeoutMs ?? drainDeadlineMs();
if (!waitExit(pid, exitTimeoutMs)) return false;
return sharedTeardownConfirmed ? true : "teardown-unconfirmed";
if (!waitExit(pid, exitTimeoutMs)) return done(false);
return done(sharedTeardownConfirmed ? true : "teardown-unconfirmed");
}

function drainDeadlineMs(): number {
Expand All @@ -172,14 +240,14 @@ function drainDeadlineMs(): number {
export async function stopProxy(pid: number, io: GracefulStopIo = {}): Promise<boolean> {
if (!isProcessAlive(pid)) return false;
const runtime = io.runtimeEndpoint ?? readRuntimePort(pid);
const graceful = await stopProxyGracefully(pid, io);
const { result: graceful, refusal } = await stopProxyGracefullyDetailed(pid, io);
if (graceful === "refused") {
// The proxy refused on purpose (foreign service owns it). Forcing would strip shared
// config while that service keeps the proxy alive.
// The proxy refused on purpose. Forcing would strip shared config while whatever owns
// the process keeps it alive. The server's own message is preferred; the fallback is
// selected from its code so an empty body still names the right cause. Both come from
// THIS attempt, so an overlapping stop cannot lend it the wrong reason.
throw new ProxyOwnershipRefusedError(
lastRefusalMessage
?? "The running proxy refused to stop: a service installed under a different "
+ "CODEX_HOME/OPENCODEX_HOME owns it. Run the stop from that home.",
refusal.message ?? refusalFallbackMessage(refusal.code),
);
}
if (graceful === "teardown-unconfirmed") {
Expand Down
105 changes: 104 additions & 1 deletion tests/lib/process-control-graceful.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, test } from "bun:test";
import { gracefulStopHost, lastStopRefusalMessage, stopProxyGracefully } from "../../src/lib/process-control";
import { gracefulStopHost, lastStopRefusalCode, lastStopRefusalMessage, ProxyOwnershipRefusedError, stopProxy, stopProxyGracefully } from "../../src/lib/process-control";

function okResponse(): Response {
return new Response(JSON.stringify({ success: true, sharedTeardown: "performed" }), { status: 200 });
Expand Down Expand Up @@ -198,4 +198,107 @@ describe("409 refusal reporting", () => {
expect(result).toBe("refused");
expect(lastStopRefusalMessage()).toBeNull();
});

test("the refusal code is captured alongside the message", async () => {
// The message alone cannot drive the fallback: a refusal that arrives with an empty or
// unparseable body still has to name a cause, and #4169 showed what happens when the
// fallback guesses one — the operator re-checks CODEX_HOME for a refusal the scheduler
// wrapper issued.
await stopProxyGracefully(7, {
readRuntime: () => ({ port: 10100 }),
fetchFn: (async () => new Response(
JSON.stringify({ success: false, code: "respawnable_service", message: "wrapper owns it" }),
{ status: 409, headers: { "content-type": "application/json" } },
)) as typeof fetch,
waitExit: () => true,
env: {},
});
expect(lastStopRefusalCode()).toBe("respawnable_service");

await stopProxyGracefully(7, {
readRuntime: () => ({ port: 10100 }),
fetchFn: (async () => new Response("not json", { status: 409 })) as typeof fetch,
waitExit: () => true,
env: {},
});
expect(lastStopRefusalCode()).toBeNull();
});

test("a refusal without a message falls back by code, never to an ownership claim", async () => {
const refusalFor = async (code: string | null): Promise<string> => {
const body = code === null ? "not json" : JSON.stringify({ success: false, code });
try {
await stopProxy(process.pid, {
readRuntime: () => ({ port: 10100 }),
fetchFn: (async () => new Response(body, {
status: 409,
headers: { "content-type": "application/json" },
})) as typeof fetch,
waitExit: () => { throw new Error("must not wait for a refused stop"); },
env: {},
});
} catch (err) {
if (err instanceof ProxyOwnershipRefusedError) return err.message;
throw err;
}
throw new Error("stopProxy must throw on a refusal");
};

const respawnable = await refusalFor("respawnable_service");
expect(respawnable).toContain("respawn");
expect(respawnable).toContain("ocx stop");

const selfUnload = await refusalFor("self_unload_service");
expect(selfUnload).toContain("installed service itself");

const unknownState = await refusalFor("service_state_unknown");
expect(unknownState).toContain("ocx service status");

const noBody = await refusalFor(null);
expect(noBody).toContain("sent no reason");

// None of them may assert the cause that #4169 was filed for.
for (const message of [respawnable, selfUnload, unknownState, noBody]) {
expect(message).not.toContain("CODEX_HOME");
expect(message).not.toContain("OPENCODEX_HOME");
}
});

test("concurrent refusals each keep their own cause", async () => {
// Reading the reason from module state lets one stop publish its refusal and a second
// overwrite it before the first continuation consumes it. Starting both together is
// what actually reproduces that: verified against the pre-fix global handoff, where
// this schedule fails with the first call throwing the second's cause
// ("...it is the installed service itself..." for the respawnable_service stop).
// A schedule that lets one call finish entirely before resuming the other does NOT
// discriminate — the parked call republishes its own globals last and passes either way.
const refusalOf = (code: string) => async (): Promise<string> => {
try {
await stopProxy(process.pid, {
readRuntime: () => ({ port: 10100 }),
fetchFn: (async () => new Response(JSON.stringify({ success: false, code }), {
status: 409,
headers: { "content-type": "application/json" },
})) as typeof fetch,
waitExit: () => { throw new Error("must not wait for a refused stop"); },
env: {},
});
} catch (err) {
if (err instanceof ProxyOwnershipRefusedError) return err.message;
throw err;
}
throw new Error("stopProxy must throw on a refusal");
};

// Repeated because the interleaving is scheduler-dependent; the pre-fix code fails on
// the first iteration, but a single run would be a weak guard against reintroduction.
for (let i = 0; i < 20; i++) {
const [respawnable, selfUnload] = await Promise.all([
refusalOf("respawnable_service")(),
refusalOf("self_unload_service")(),
]);
expect(respawnable).toContain("respawn");
expect(selfUnload).toContain("installed service itself");
}
});
});
19 changes: 12 additions & 7 deletions tests/providers/xai/grok-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe("Grok fence lifecycle wiring", () => {
// shared teardown must be skipped at both call sites, exactly like the service-manager path.
const ownershipRefusals = stopFn.match(/err instanceof ProxyOwnershipRefusedError[\s\S]{0,200}?ownershipBlocked = true;/g);
expect(ownershipRefusals).toHaveLength(2);
expect(stopFn.match(/Skipping shared teardown \(native Codex restore, Grok config\): the foreign proxy is still running\./g)).toHaveLength(2);
expect(stopFn.match(/Skipping shared teardown \(native Codex restore, Grok config\): the refusing proxy is still running\./g)).toHaveLength(2);
expect(PROCESS_CONTROL_SOURCE).toContain("throw new ProxyOwnershipRefusedError(");
});

Expand Down Expand Up @@ -507,20 +507,25 @@ 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.
// Escalating would run the daemon's cleanup and strip shared config while the refusing
// service keeps the proxy alive — the exact hole the refusal gate exists to close.
// 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.
// cause, #4169 the code that names it), but it must still yield "refused" without
// falling through to !res.ok. Matched loosely so a wrapped return (`done("refused")`)
// still satisfies the invariant this guards, which is ordering, not spelling.
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 refusedReturn = /return (?:done\()?"refused"/;
const okFallthrough = /if \(!res\.ok\) return (?:done\()?false/;
const afterFour09 = stopGracefully.slice(four09At);
expect(afterFour09).toMatch(refusedReturn);
expect(afterFour09.search(refusedReturn))
.toBeLessThan(afterFour09.search(okFallthrough));

const stopProxyFn = sliceFn(PROCESS_CONTROL_SOURCE, "export async function stopProxy(", "export function killProxy(");
const refusedAt = stopProxyFn.indexOf('graceful === "refused"');
Expand Down
Loading