diff --git a/CHANGELOG.md b/CHANGELOG.md index c4bc3e2aeae..54eea8e7fe4 100644 --- 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 index d1572550f1b..67ee0f0d31f 100644 --- 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,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); diff --git a/extensions/codex/src/app-server/shared-client.test.ts b/extensions/codex/src/app-server/shared-client.test.ts index 7e028411e00..a9705716a16 100644 --- 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 index 2634c665dae..a20a0c6e973 100644 --- 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); }); }