fix(codex): fail detached-but-leased clients on suspect retirement

This commit is contained in:
Ayaan Zaidi 2026-07-08 17:35:35 +05:30
parent 80b2b709ca
commit bb1da73b14
2 changed files with 41 additions and 0 deletions

View file

@ -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);

View file

@ -34,6 +34,10 @@ type SharedCodexAppServerClientState = {
leasedReleases: WeakMap<CodexAppServerClient, Array<() => 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<CodexAppServerClient>();
// 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;