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

obviyus · obviyus · commit c66a2e58956f · 2026-07-08T17:41:08.000+05:30
diff --git 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
@@ -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;
