diff --git a/extensions/codex/src/app-server/shared-client.test.ts b/extensions/codex/src/app-server/shared-client.test.ts index 14639f3c2e9..2cc48fe433c 100644 --- a/extensions/codex/src/app-server/shared-client.test.ts +++ b/extensions/codex/src/app-server/shared-client.test.ts @@ -797,6 +797,32 @@ describe("shared Codex app-server client", () => { expect(second.process.stdin.destroyed).toBe(false); }); + it("suspect retirement closes a client that was already gracefully detached", async () => { + const first = createClientHarness(); + vi.spyOn(CodexAppServerClient, "start").mockReturnValueOnce(first.client); + + const lease = getLeasedSharedCodexAppServerClient({ timeoutMs: 1000 }); + await sendInitializeResult(first, "openclaw/0.142.0 (macOS; test)"); + await expect(lease).resolves.toBe(first.client); + + // Routine cleanup detaches gracefully; a later terminal-idle kill must + // still be able to fail the leaseholders off the poisoned process. + expect(retireSharedCodexAppServerClientIfCurrent(first.client)).toEqual({ + activeLeases: 1, + closed: false, + }); + expect(first.process.stdin.destroyed).toBe(false); + + expect( + retireSharedCodexAppServerClientIfCurrent(first.client, { failActiveLeases: true }), + ).toEqual({ + activeLeases: 1, + closed: true, + }); + expect(first.process.stdin.destroyed).toBe(true); + expect(releaseLeasedSharedCodexAppServerClient(first.client)).toBe(true); + }); + it("retires gracefully by default: leased clients close on release, not immediately", async () => { const first = createClientHarness(); vi.spyOn(CodexAppServerClient, "start").mockReturnValueOnce(first.client); diff --git a/extensions/codex/src/app-server/shared-client.ts b/extensions/codex/src/app-server/shared-client.ts index 29d12a304b3..9231292f976 100644 --- a/extensions/codex/src/app-server/shared-client.ts +++ b/extensions/codex/src/app-server/shared-client.ts @@ -34,6 +34,10 @@ type SharedCodexAppServerClientState = { leasedReleases: WeakMap void>>; }; +// Clients we already force-closed for suspect retirement; a repeat retire must +// report closed:false instead of pretending to close the corpse again. +const suspectClosedClients = new WeakSet(); + // Symbol.for shares one client table across duplicate module copies (dist + // src bundles in one process). Plugin updates restart the gateway, so every // copy writing this state runs the same code and the shape never migrates. @@ -436,6 +440,9 @@ export function retireSharedCodexAppServerClientIfCurrent( if (opts?.failActiveLeases) { entry.closeError = new Error("codex app-server client is closed"); const closed = closeRetiredSharedClientEntry(entry); + if (closed) { + suspectClosedClients.add(client); + } return { activeLeases: entry.activeLeases, closed }; } const closed = closeRetiredSharedClientEntryIfIdle(entry); @@ -444,6 +451,14 @@ export function retireSharedCodexAppServerClientIfCurrent( } const activeLeases = state.leasedReleases.get(client)?.length ?? 0; if (activeLeases > 0) { + // A gracefully detached client (e.g. one-shot cleanup) can still be leased + // when a later terminal-idle kill declares it suspect; the map miss must + // not let the poisoned process keep serving those co-leases. + if (opts?.failActiveLeases && !suspectClosedClients.has(client)) { + suspectClosedClients.add(client); + client.close(); + return { activeLeases, closed: true }; + } return { activeLeases, closed: false }; } return undefined;