From c0105476dae9568d2eb023e99830fad113f6e4a2 Mon Sep 17 00:00:00 2001
From: suraj-markup <sk9261712674@gmail.com>
Date: Wed, 25 Feb 2026 01:00:44 +0530
Subject: [PATCH] =?UTF-8?q?fix(agent-codex):=20address=20follow-up=20revie?=
 =?UTF-8?q?w=20=E2=80=94=20retry=20after=20failed=20connect,=20binary=20gu?=
 =?UTF-8?q?ard=20recovery,=20lint=20fixes?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Reset closed flag after failed connect() so client can retry
- Wrap binary resolve guard in try/finally to clear on rejection
- Fix duplicate import and unused variable lint errors in tests
- Add test: client retries connect after transient handshake failure

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../agent-codex/src/app-server-client.test.ts | 31 +++++++++++++++----
 .../agent-codex/src/app-server-client.ts      |  3 ++
 packages/plugins/agent-codex/src/index.ts     |  7 +++--
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/packages/plugins/agent-codex/src/app-server-client.test.ts b/packages/plugins/agent-codex/src/app-server-client.test.ts
index 1221f2570..70768b63e 100644
--- a/packages/plugins/agent-codex/src/app-server-client.test.ts
+++ b/packages/plugins/agent-codex/src/app-server-client.test.ts
@@ -12,8 +12,7 @@ vi.mock("node:child_process", () => ({
   spawn: mockSpawn,
 }));
 
-import { CodexAppServerClient } from "./app-server-client.js";
-import type { ApprovalDecision } from "./app-server-client.js";
+import { CodexAppServerClient, type ApprovalDecision } from "./app-server-client.js";
 
 // ---------------------------------------------------------------------------
 // Helpers: fake child process
@@ -591,10 +590,6 @@ describe("CodexAppServerClient", () => {
       await new Promise((r) => setTimeout(r, 10));
 
       // Should have auto-responded with accept
-      const responses = parseStdinMessages(proc).filter(
-        (m) => m["id"] === "0" || m["id"] === 0,
-      );
-      // Response should contain decision: "accept"
       const approvalResponse = proc.stdinLines.find((l) => l.includes('"accept"'));
       expect(approvalResponse).toBeDefined();
 
@@ -951,6 +946,30 @@ describe("CodexAppServerClient", () => {
       expect(client.isConnected).toBe(false);
     });
 
+    it("allows retry after a failed connect (does not permanently close)", async () => {
+      // First attempt: spawn a process that times out during handshake
+      const proc1 = createFakeProcess();
+      vi.spyOn(proc1, "kill").mockImplementation((_signal?: string) => {
+        proc1.simulateExit(1, "SIGTERM");
+        return true;
+      });
+      const client = new CodexAppServerClient({ requestTimeout: 200 });
+
+      await expect(client.connect()).rejects.toThrow("timed out");
+      expect(client.isConnected).toBe(false);
+
+      // Second attempt should NOT throw "Client is closed" — it should be retryable
+      const proc2 = createFakeProcess();
+      const connectPromise = client.connect();
+      await new Promise((r) => setTimeout(r, 10));
+      const initReq = findRequest(proc2, "initialize");
+      proc2.sendLine(JSON.stringify({ id: initReq!["id"], result: {} }));
+      await connectPromise;
+
+      expect(client.isConnected).toBe(true);
+      await closeClient(client, proc2);
+    });
+
     it("drains stderr without blocking the child process", async () => {
       const proc = createFakeProcess();
       const client = new CodexAppServerClient({ requestTimeout: 500 });
diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts
index 3b533cc44..01151988e 100644
--- a/packages/plugins/agent-codex/src/app-server-client.ts
+++ b/packages/plugins/agent-codex/src/app-server-client.ts
@@ -209,6 +209,9 @@ export class CodexAppServerClient extends EventEmitter {
     } catch (err) {
       this.connecting = false;
       await this.close();
+      // Reset closed flag so the client can retry connect() after a
+      // transient handshake failure instead of being permanently unusable.
+      this.closed = false;
       throw err;
     }
 
diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts
index bd4030102..028c663b0 100644
--- a/packages/plugins/agent-codex/src/index.ts
+++ b/packages/plugins/agent-codex/src/index.ts
@@ -759,8 +759,11 @@ function createCodexAgent(): Agent {
         if (!resolvingBinary) {
           resolvingBinary = resolveCodexBinary();
         }
-        resolvedBinary = await resolvingBinary;
-        resolvingBinary = null;
+        try {
+          resolvedBinary = await resolvingBinary;
+        } finally {
+          resolvingBinary = null;
+        }
       }
       if (!session.workspacePath) return;
       await setupCodexWorkspace(session.workspacePath);