fix(codex): close app-server stdio gracefully

steipete · steipete · commit 6cfb08680e71 · 2026-05-08T02:58:27.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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.
diff --git a/extensions/codex/src/app-server/client.test.ts b/extensions/codex/src/app-server/client.test.ts
@@ -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,16 +289,44 @@ 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");
 
     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);
diff --git a/extensions/codex/src/app-server/shared-client.test.ts b/extensions/codex/src/app-server/shared-client.test.ts
@@ -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 () => {
diff --git a/extensions/codex/src/app-server/transport.ts b/extensions/codex/src/app-server/transport.ts
@@ -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);
   });
 }
