mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-09 01:50:43 +00:00
fix(codex): close app-server stdio gracefully
This commit is contained in:
@@ -163,6 +163,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Gateway/watch: leave `OPENCLAW_TRACE_SYNC_IO` disabled by default in `pnpm gateway:watch:raw` so watch mode avoids noisy Node sync-I/O stack traces unless explicitly requested.
|
||||
- Codex app-server: close stdio stdin before force-killing the managed app-server, matching Codex single-client shutdown behavior and avoiding unsettled CLI exits after successful runs.
|
||||
- CLI/Codex: dispose registered agent harnesses during short-lived CLI shutdown so successful Codex-backed `agent --local` runs do not leave app-server child processes alive.
|
||||
- Agents/Codex: auto-enable the Codex harness plugin for one-shot OpenAI model overrides so `openclaw agent --local --model openai/...` does not fail with an unregistered `codex` harness.
|
||||
- Gateway/live tests: avoid full model-registry enumeration for explicit provider-qualified live model filters, preventing `.profile` OpenAI gateway profile runs from hanging before provider dispatch.
|
||||
|
||||
@@ -242,7 +242,7 @@ describe("CodexAppServerClient", () => {
|
||||
expect(harness.writes).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("force-stops app-server transports that ignore the graceful signal", async () => {
|
||||
it("waits for app-server transports to exit after closing stdin before force-stopping", async () => {
|
||||
vi.useFakeTimers();
|
||||
const process = Object.assign(new EventEmitter(), {
|
||||
stdin: {
|
||||
@@ -261,7 +261,8 @@ describe("CodexAppServerClient", () => {
|
||||
|
||||
__testing.closeCodexAppServerTransport(process, { forceKillDelayMs: 25 });
|
||||
|
||||
expect(process.kill).toHaveBeenCalledWith("SIGTERM");
|
||||
expect(process.stdin.end).toHaveBeenCalledTimes(1);
|
||||
expect(process.kill).not.toHaveBeenCalled();
|
||||
await vi.advanceTimersByTimeAsync(25);
|
||||
expect(process.kill).toHaveBeenCalledWith("SIGKILL");
|
||||
expect(process.unref).toHaveBeenCalledTimes(1);
|
||||
@@ -288,9 +289,10 @@ describe("CodexAppServerClient", () => {
|
||||
exitTimeoutMs: 100,
|
||||
forceKillDelayMs: 25,
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(25);
|
||||
|
||||
expect(process.kill).toHaveBeenCalledWith("SIGTERM");
|
||||
expect(process.stdin.end).toHaveBeenCalledTimes(1);
|
||||
expect(process.kill).not.toHaveBeenCalled();
|
||||
await vi.advanceTimersByTimeAsync(25);
|
||||
expect(process.kill).toHaveBeenCalledWith("SIGKILL");
|
||||
process.signalCode = "SIGKILL";
|
||||
process.emit("exit");
|
||||
@@ -298,6 +300,33 @@ describe("CodexAppServerClient", () => {
|
||||
await expect(closed).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it("keeps async shutdown alive until the exit timeout resolves", async () => {
|
||||
vi.useFakeTimers();
|
||||
const process = Object.assign(new EventEmitter(), {
|
||||
stdin: {
|
||||
write: vi.fn(),
|
||||
end: vi.fn(),
|
||||
destroy: vi.fn(),
|
||||
unref: vi.fn(),
|
||||
},
|
||||
stdout: Object.assign(new PassThrough(), { unref: vi.fn() }),
|
||||
stderr: Object.assign(new PassThrough(), { unref: vi.fn() }),
|
||||
exitCode: null as number | null,
|
||||
signalCode: null as string | null,
|
||||
kill: vi.fn(),
|
||||
unref: vi.fn(),
|
||||
});
|
||||
|
||||
const closed = __testing.closeCodexAppServerTransportAndWait(process, {
|
||||
exitTimeoutMs: 100,
|
||||
forceKillDelayMs: 25,
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
|
||||
await expect(closed).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it("handles stdin write errors without crashing the process", async () => {
|
||||
const harness = createClientHarness();
|
||||
clients.push(harness.client);
|
||||
|
||||
@@ -96,7 +96,7 @@ describe("shared Codex app-server client", () => {
|
||||
await expect(listPromise).rejects.toThrow(
|
||||
`Codex app-server ${MIN_CODEX_APP_SERVER_VERSION} or newer is required`,
|
||||
);
|
||||
expect(harness.process.kill).toHaveBeenCalledTimes(1);
|
||||
expect(harness.process.stdin.destroyed).toBe(true);
|
||||
startSpy.mockRestore();
|
||||
});
|
||||
|
||||
@@ -111,7 +111,7 @@ describe("shared Codex app-server client", () => {
|
||||
await expect(listCodexAppServerModels({ timeoutMs: 5 })).rejects.toThrow(
|
||||
"codex app-server initialize timed out",
|
||||
);
|
||||
expect(first.process.kill).toHaveBeenCalledTimes(1);
|
||||
expect(first.process.stdin.destroyed).toBe(true);
|
||||
|
||||
const secondList = listCodexAppServerModels({ timeoutMs: 1000 });
|
||||
await sendInitializeResult(second, "openclaw/0.125.0 (macOS; test)");
|
||||
@@ -128,7 +128,7 @@ describe("shared Codex app-server client", () => {
|
||||
await expect(createIsolatedCodexAppServerClient({ timeoutMs: 5 })).rejects.toThrow(
|
||||
"codex app-server initialize timed out",
|
||||
);
|
||||
expect(harness.process.kill).toHaveBeenCalledTimes(1);
|
||||
expect(harness.process.stdin.destroyed).toBe(true);
|
||||
});
|
||||
|
||||
it("passes the selected auth profile through the bridge helper", async () => {
|
||||
@@ -288,7 +288,7 @@ describe("shared Codex app-server client", () => {
|
||||
await expect(secondList).resolves.toEqual({ models: [] });
|
||||
|
||||
expect(startSpy).toHaveBeenCalledTimes(2);
|
||||
expect(first.process.kill).toHaveBeenCalledWith("SIGTERM");
|
||||
expect(first.process.stdin.destroyed).toBe(true);
|
||||
});
|
||||
|
||||
it("does not let a superseded shared-client failure tear down the newer client", async () => {
|
||||
@@ -347,7 +347,7 @@ describe("shared Codex app-server client", () => {
|
||||
await expect(firstList).resolves.toEqual({ models: [] });
|
||||
|
||||
expect(clearSharedCodexAppServerClientIfCurrent(first.client)).toBe(true);
|
||||
expect(first.process.kill).toHaveBeenCalledWith("SIGTERM");
|
||||
expect(first.process.stdin.destroyed).toBe(true);
|
||||
|
||||
const secondList = listCodexAppServerModels({ timeoutMs: 1000 });
|
||||
await sendInitializeResult(second, "openclaw/0.125.0 (macOS; test)");
|
||||
@@ -357,7 +357,7 @@ describe("shared Codex app-server client", () => {
|
||||
expect(clearSharedCodexAppServerClientIfCurrent(first.client)).toBe(false);
|
||||
expect(second.process.kill).not.toHaveBeenCalled();
|
||||
expect(clearSharedCodexAppServerClientIfCurrent(second.client)).toBe(true);
|
||||
expect(second.process.kill).toHaveBeenCalledWith("SIGTERM");
|
||||
expect(second.process.stdin.destroyed).toBe(true);
|
||||
});
|
||||
|
||||
it("uses a fresh websocket Authorization header after shared-client token rotation", async () => {
|
||||
|
||||
@@ -28,11 +28,8 @@ export function closeCodexAppServerTransport(
|
||||
child: CodexAppServerTransport,
|
||||
options: { forceKillDelayMs?: number } = {},
|
||||
): void {
|
||||
child.stdout.destroy?.();
|
||||
child.stderr.destroy?.();
|
||||
child.stdin.end?.();
|
||||
child.stdin.destroy?.();
|
||||
signalCodexAppServerTransport(child, "SIGTERM");
|
||||
const forceKillDelayMs = options.forceKillDelayMs ?? 1_000;
|
||||
const forceKill = setTimeout(
|
||||
() => {
|
||||
@@ -44,7 +41,11 @@ export function closeCodexAppServerTransport(
|
||||
Math.max(1, forceKillDelayMs),
|
||||
);
|
||||
forceKill.unref?.();
|
||||
child.once("exit", () => clearTimeout(forceKill));
|
||||
child.once("exit", () => {
|
||||
clearTimeout(forceKill);
|
||||
child.stdout.destroy?.();
|
||||
child.stderr.destroy?.();
|
||||
});
|
||||
child.unref?.();
|
||||
child.stdout.unref?.();
|
||||
child.stderr.unref?.();
|
||||
@@ -95,7 +96,6 @@ async function waitForCodexAppServerTransportExit(
|
||||
},
|
||||
Math.max(1, timeoutMs),
|
||||
);
|
||||
timeout.unref?.();
|
||||
child.once("exit", onExit);
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user