From 2e0933098066082317fa228f30676ed433d66d02 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Tue, 24 Feb 2026 22:37:13 +0530 Subject: [PATCH 01/14] feat(agent-codex): add token tracking, approval policies, reasoning flag, binary detection, conversation resume, and app-server client Implements six Codex-specific improvements: - getSessionInfo() parses JSONL session files for token usage and cost estimation - Configurable approval/sandbox policies (--approval-mode, --dangerously-bypass-approvals-and-sandbox) - Auto-detect o-series models for --reasoning flag - resolveCodexBinary() discovers codex via which + fallback paths - getRestoreCommand() extracts thread context for conversation resume via re-injection - CodexAppServerClient JSON-RPC client for Codex app-server mode (thread/turn management, approval handling, model discovery) Closes #176, #177, #178, #179, #183, #184 Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.test.ts | 915 ++++++++++++++++++ .../agent-codex/src/app-server-client.ts | 467 +++++++++ .../plugins/agent-codex/src/index.test.ts | 598 +++++++++++- packages/plugins/agent-codex/src/index.ts | 331 ++++++- 4 files changed, 2270 insertions(+), 41 deletions(-) create mode 100644 packages/plugins/agent-codex/src/app-server-client.test.ts create mode 100644 packages/plugins/agent-codex/src/app-server-client.ts diff --git a/packages/plugins/agent-codex/src/app-server-client.test.ts b/packages/plugins/agent-codex/src/app-server-client.test.ts new file mode 100644 index 000000000..9cebc4a63 --- /dev/null +++ b/packages/plugins/agent-codex/src/app-server-client.test.ts @@ -0,0 +1,915 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { EventEmitter, PassThrough, Writable } from "node:stream"; + +// --------------------------------------------------------------------------- +// Hoisted mocks +// --------------------------------------------------------------------------- +const { mockSpawn } = vi.hoisted(() => ({ + mockSpawn: vi.fn(), +})); + +vi.mock("node:child_process", () => ({ + spawn: mockSpawn, +})); + +import { CodexAppServerClient } from "./app-server-client.js"; +import type { ApprovalDecision } from "./app-server-client.js"; + +// --------------------------------------------------------------------------- +// Helpers: fake child process +// --------------------------------------------------------------------------- +class FakeProcess extends EventEmitter { + stdin = new Writable({ + write(_chunk, _encoding, callback) { + callback(); + }, + }); + stdout = new PassThrough(); + stderr = new PassThrough(); + exitCode: number | null = null; + pid = 12345; + + /** Lines written to stdin (captured for assertions) */ + stdinLines: string[] = []; + + constructor() { + super(); + // Capture stdin writes + const origWrite = this.stdin.write.bind(this.stdin); + this.stdin.write = ((chunk: string | Buffer, ...args: unknown[]) => { + this.stdinLines.push(chunk.toString()); + return origWrite(chunk, ...args); + }) as typeof this.stdin.write; + } + + /** Simulate the server sending a line on stdout */ + sendLine(data: string): void { + this.stdout.write(data + "\n"); + } + + /** Simulate process exit */ + simulateExit(code: number | null = 0, signal: string | null = null): void { + this.exitCode = code; + this.emit("exit", code, signal); + } + + kill(_signal?: string): boolean { + return true; + } +} + +function createFakeProcess(): FakeProcess { + const proc = new FakeProcess(); + mockSpawn.mockReturnValue(proc); + return proc; +} + +/** Parse JSON-RPC messages written to stdin */ +function parseStdinMessages(proc: FakeProcess): Array> { + return proc.stdinLines + .flatMap((line) => line.split("\n")) + .filter((line) => line.trim()) + .map((line) => JSON.parse(line) as Record); +} + +/** Find a request by method in stdin messages */ +function findRequest(proc: FakeProcess, method: string): Record | undefined { + return parseStdinMessages(proc).find((msg) => msg["method"] === method); +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +// --------------------------------------------------------------------------- +// Shared helper: connect a client to a fake process (handshake) +// --------------------------------------------------------------------------- +async function connectClient( + client: CodexAppServerClient, + proc: FakeProcess, +): Promise { + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + if (initReq) { + proc.sendLine(JSON.stringify({ id: initReq["id"], result: {} })); + } + await connectPromise; +} + +async function closeClient( + client: CodexAppServerClient, + proc: FakeProcess, +): Promise { + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; +} + +// ========================================================================= +// Construction & Connection +// ========================================================================= +describe("CodexAppServerClient", () => { + describe("constructor", () => { + it("creates client with default options", () => { + const client = new CodexAppServerClient(); + expect(client.isConnected).toBe(false); + }); + + it("accepts custom options", () => { + const client = new CodexAppServerClient({ + binaryPath: "/custom/codex", + cwd: "/my/project", + requestTimeout: 5000, + }); + expect(client.isConnected).toBe(false); + }); + }); + + describe("connect", () => { + it("spawns codex app-server and verifies spawn args", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + expect(client.isConnected).toBe(true); + expect(mockSpawn).toHaveBeenCalledWith("codex", ["app-server"], expect.any(Object)); + + // Should have sent initialize request and initialized notification + const initReq = findRequest(proc, "initialize"); + expect(initReq).toBeDefined(); + const initializedNotif = findRequest(proc, "initialized"); + expect(initializedNotif).toBeDefined(); + + await closeClient(client, proc); + }); + + it("uses custom binary path", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ binaryPath: "/opt/codex", requestTimeout: 500 }); + await connectClient(client, proc); + + expect(mockSpawn).toHaveBeenCalledWith("/opt/codex", ["app-server"], expect.any(Object)); + await closeClient(client, proc); + }); + + it("throws if already connected", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + await expect(client.connect()).rejects.toThrow("already connected"); + await closeClient(client, proc); + }); + + it("throws if client is closed", async () => { + const client = new CodexAppServerClient(); + await client.close(); + await expect(client.connect()).rejects.toThrow("closed"); + }); + + it("forwards cwd and env to spawn", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ + cwd: "/my/project", + env: { OPENAI_API_KEY: "sk-test" }, + requestTimeout: 500, + }); + await connectClient(client, proc); + + expect(mockSpawn).toHaveBeenCalledWith("codex", ["app-server"], expect.objectContaining({ + cwd: "/my/project", + stdio: ["pipe", "pipe", "pipe"], + })); + // env should merge with process.env + const spawnCall = mockSpawn.mock.calls[0]; + expect(spawnCall[2].env).toBeDefined(); + expect(spawnCall[2].env.OPENAI_API_KEY).toBe("sk-test"); + + await closeClient(client, proc); + }); + + it("emits 'connected' event after handshake", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + const events: unknown[] = []; + client.on("connected", (result: unknown) => events.push(result)); + + await connectClient(client, proc); + + expect(events).toHaveLength(1); + await closeClient(client, proc); + }); + + it("sends clientInfo in initialize request", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + const initReq = findRequest(proc, "initialize"); + expect(initReq).toBeDefined(); + const params = initReq!["params"] as Record; + const clientInfo = params["clientInfo"] as Record; + expect(clientInfo["name"]).toBe("ao-agent-codex"); + expect(clientInfo["version"]).toBe("0.1.0"); + + await closeClient(client, proc); + }); + }); + + describe("close", () => { + it("sends SIGTERM to the process", async () => { + const proc = createFakeProcess(); + const killSpy = vi.spyOn(proc, "kill"); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + await closeClient(client, proc); + + expect(killSpy).toHaveBeenCalledWith("SIGTERM"); + expect(client.isConnected).toBe(false); + }); + + it("is idempotent — calling close twice is safe", async () => { + const client = new CodexAppServerClient(); + await client.close(); + await client.close(); // Should not throw + }); + + it("rejects pending requests on close", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + // Send a request that won't be answered + const requestPromise = client.threadList(); + + await closeClient(client, proc); + + await expect(requestPromise).rejects.toThrow("closed"); + }); + }); + + // ========================================================================= + // Request / Response + // ========================================================================= + describe("sendRequest", () => { + let proc: FakeProcess; + let client: CodexAppServerClient; + + beforeEach(async () => { + proc = createFakeProcess(); + client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + }); + + afterEach(async () => { + await closeClient(client, proc); + }); + + it("correlates responses by id", async () => { + const promise = client.sendRequest("thread/list", {}); + await new Promise((r) => setTimeout(r, 10)); + + const msgs = parseStdinMessages(proc); + const req = msgs.find((m) => m["method"] === "thread/list"); + expect(req).toBeDefined(); + + proc.sendLine(JSON.stringify({ + id: req!["id"], + result: { threads: [{ id: "t-1" }] }, + })); + + const result = await promise; + expect(result).toEqual({ threads: [{ id: "t-1" }] }); + }); + + it("rejects on JSON-RPC error response", async () => { + const promise = client.sendRequest("thread/start", {}); + await new Promise((r) => setTimeout(r, 10)); + + const msgs = parseStdinMessages(proc); + const req = msgs.find((m) => m["method"] === "thread/start"); + + proc.sendLine(JSON.stringify({ + id: req!["id"], + error: { code: -32600, message: "Invalid params" }, + })); + + await expect(promise).rejects.toThrow("Invalid params"); + }); + + it("times out if no response received", async () => { + // requestTimeout is 500ms + const promise = client.sendRequest("thread/start", {}); + await expect(promise).rejects.toThrow("timed out"); + }, 2000); + + it("throws if client is not initialized", async () => { + const uninitClient = new CodexAppServerClient(); + await expect(uninitClient.sendRequest("thread/list", {})).rejects.toThrow("not initialized"); + }); + + it("throws after client is closed", async () => { + await closeClient(client, proc); + // After close, initialized=false so "not initialized" check fires first + await expect(client.sendRequest("thread/list", {})).rejects.toThrow(); + // Prevent afterEach from double-closing + proc = createFakeProcess(); + client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + }); + + it("resolves to empty object when response has no result field", async () => { + const promise = client.sendRequest("thread/list", {}); + await new Promise((r) => setTimeout(r, 10)); + + const msgs = parseStdinMessages(proc); + const req = msgs.find((m) => m["method"] === "thread/list"); + + // Response with id but no result field + proc.sendLine(JSON.stringify({ id: req!["id"] })); + + const result = await promise; + expect(result).toEqual({}); + }); + }); + + // ========================================================================= + // Thread Management + // ========================================================================= + describe("thread management", () => { + let proc: FakeProcess; + let client: CodexAppServerClient; + + beforeEach(async () => { + proc = createFakeProcess(); + client = new CodexAppServerClient({ requestTimeout: 500 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + }); + + afterEach(async () => { + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + + it("threadStart sends thread/start request with params", async () => { + const promise = client.threadStart({ model: "o3-mini", cwd: "/project" }); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "thread/start"); + expect(req).toBeDefined(); + expect(req!["params"]).toEqual({ model: "o3-mini", cwd: "/project" }); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: { id: "thread-abc" } })); + const result = await promise; + expect(result).toEqual({ id: "thread-abc" }); + }); + + it("threadResume sends thread/resume request with threadId", async () => { + const promise = client.threadResume("thread-123"); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "thread/resume"); + expect(req).toBeDefined(); + expect(req!["params"]).toEqual({ threadId: "thread-123" }); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: { id: "thread-123" } })); + const result = await promise; + expect(result).toEqual({ id: "thread-123" }); + }); + + it("threadList sends thread/list request", async () => { + const promise = client.threadList("cursor-1", 10); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "thread/list"); + expect(req).toBeDefined(); + expect(req!["params"]).toEqual({ cursor: "cursor-1", limit: 10 }); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: { threads: [] } })); + await promise; + }); + + it("threadStart sends thread/start with empty params by default", async () => { + const promise = client.threadStart(); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "thread/start"); + expect(req).toBeDefined(); + expect(req!["params"]).toEqual({}); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: { id: "thread-default" } })); + const result = await promise; + expect(result).toEqual({ id: "thread-default" }); + }); + + it("threadArchive sends thread/archive request", async () => { + const promise = client.threadArchive("thread-old"); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "thread/archive"); + expect(req!["params"]).toEqual({ threadId: "thread-old" }); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: {} })); + await promise; + }); + }); + + // ========================================================================= + // Turn Management + // ========================================================================= + describe("turn management", () => { + let proc: FakeProcess; + let client: CodexAppServerClient; + + beforeEach(async () => { + proc = createFakeProcess(); + client = new CodexAppServerClient({ requestTimeout: 500 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + }); + + afterEach(async () => { + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + + it("turnStart sends turn/start with text input", async () => { + const promise = client.turnStart({ + threadId: "t-1", + input: "Fix the bug in auth.ts", + }); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "turn/start"); + expect(req).toBeDefined(); + expect(req!["params"]).toEqual({ + threadId: "t-1", + input: [{ type: "text", text: "Fix the bug in auth.ts" }], + }); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: { turnId: "turn-1" } })); + const result = await promise; + expect(result).toEqual({ turnId: "turn-1" }); + }); + + it("turnStart includes optional cwd and model params", async () => { + const promise = client.turnStart({ + threadId: "t-1", + input: "Fix it", + cwd: "/custom/dir", + model: "o3-mini", + }); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "turn/start"); + expect(req).toBeDefined(); + expect(req!["params"]).toEqual({ + threadId: "t-1", + input: [{ type: "text", text: "Fix it" }], + cwd: "/custom/dir", + model: "o3-mini", + }); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: { turnId: "turn-2" } })); + await promise; + }); + + it("turnInterrupt sends turn/interrupt", async () => { + const promise = client.turnInterrupt("t-1", "turn-1"); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "turn/interrupt"); + expect(req!["params"]).toEqual({ threadId: "t-1", turnId: "turn-1" }); + + proc.sendLine(JSON.stringify({ id: req!["id"], result: {} })); + await promise; + }); + }); + + // ========================================================================= + // Notifications + // ========================================================================= + describe("notifications", () => { + it("routes server notifications to handler", async () => { + const proc = createFakeProcess(); + const notifications: Array<{ method: string; params: Record }> = []; + + const client = new CodexAppServerClient({ + requestTimeout: 500, + onNotification: (method, params) => { + notifications.push({ method, params }); + }, + }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + // Server sends a notification + proc.sendLine(JSON.stringify({ + method: "turn/completed", + params: { threadId: "t-1", turnId: "turn-1" }, + })); + await new Promise((r) => setTimeout(r, 10)); + + expect(notifications).toHaveLength(1); + expect(notifications[0]).toEqual({ + method: "turn/completed", + params: { threadId: "t-1", turnId: "turn-1" }, + }); + + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + + it("emits notification events on the EventEmitter", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + const events: Array<[string, Record]> = []; + client.on("notification", (method: string, params: Record) => { + events.push([method, params]); + }); + + proc.sendLine(JSON.stringify({ + method: "thread/tokenUsage/updated", + params: { inputTokens: 100 }, + })); + await new Promise((r) => setTimeout(r, 10)); + + expect(events).toHaveLength(1); + expect(events[0]![0]).toBe("thread/tokenUsage/updated"); + + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + }); + + // ========================================================================= + // Approval Requests + // ========================================================================= + describe("approval handling", () => { + it("auto-accepts approvals when no handler is provided", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + // Server sends approval request + proc.sendLine(JSON.stringify({ + id: 0, + method: "item/fileChange/requestApproval", + params: { path: "test.ts" }, + })); + 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(); + + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + + it("falls back to 'decline' when approval handler throws", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ + requestTimeout: 500, + onApproval: async () => { + throw new Error("handler crashed"); + }, + }); + + await connectClient(client, proc); + + proc.sendLine(JSON.stringify({ + id: 99, + method: "item/fileChange/requestApproval", + params: { path: "dangerous.ts" }, + })); + await new Promise((r) => setTimeout(r, 10)); + + // Should have responded with "decline" due to handler error + const declineResponse = proc.stdinLines.find((l) => l.includes('"decline"')); + expect(declineResponse).toBeDefined(); + + await closeClient(client, proc); + }); + + it("emits 'approval' event for approval requests", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + const events: Array<[string | number, string, Record]> = []; + client.on("approval", (id: string | number, method: string, params: Record) => { + events.push([id, method, params]); + }); + + await connectClient(client, proc); + + proc.sendLine(JSON.stringify({ + id: 7, + method: "item/commandExecution/requestApproval", + params: { command: ["ls"] }, + })); + await new Promise((r) => setTimeout(r, 10)); + + expect(events).toHaveLength(1); + expect(events[0]![1]).toBe("item/commandExecution/requestApproval"); + + await closeClient(client, proc); + }); + + it("delegates to custom approval handler", async () => { + const proc = createFakeProcess(); + const decisions: ApprovalDecision[] = []; + + const client = new CodexAppServerClient({ + requestTimeout: 500, + onApproval: async (_id, _method, _params) => { + decisions.push("decline"); + return "decline"; + }, + }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + proc.sendLine(JSON.stringify({ + id: 42, + method: "item/commandExecution/requestApproval", + params: { command: ["rm", "-rf", "/"] }, + })); + await new Promise((r) => setTimeout(r, 10)); + + expect(decisions).toEqual(["decline"]); + + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + }); + + // ========================================================================= + // Process lifecycle + // ========================================================================= + describe("process lifecycle", () => { + it("rejects pending requests when process exits", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 5000 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + // Start a request + const requestPromise = client.threadList(); + await new Promise((r) => setTimeout(r, 10)); + + // Process exits unexpectedly + proc.simulateExit(1, null); + + await expect(requestPromise).rejects.toThrow("exited"); + expect(client.isConnected).toBe(false); + }); + + it("emits exit event on process exit", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + const exits: Array<[number | null, string | null]> = []; + client.on("exit", (code: number | null, signal: string | null) => { + exits.push([code, signal]); + }); + + proc.simulateExit(0, null); + expect(exits).toEqual([[0, null]]); + }); + + it("emits error and rejects pending on process spawn error", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 5000 }); + + await connectClient(client, proc); + + const errors: Error[] = []; + client.on("error", (err: Error) => errors.push(err)); + + // Start a request + const requestPromise = client.threadList(); + await new Promise((r) => setTimeout(r, 10)); + + // Process emits error + proc.emit("error", new Error("spawn ENOENT")); + + await expect(requestPromise).rejects.toThrow("spawn ENOENT"); + expect(errors).toHaveLength(1); + expect(errors[0]!.message).toBe("spawn ENOENT"); + expect(client.isConnected).toBe(false); + }); + + it("handles malformed JSON lines gracefully", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + // Send garbage — should not crash + proc.sendLine("not valid json {{{"); + proc.sendLine(""); + proc.sendLine(" "); + + // Client should still be connected + expect(client.isConnected).toBe(true); + + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + }); + + // ========================================================================= + // sendNotification & sendApprovalResponse + // ========================================================================= + describe("sendNotification", () => { + it("sends a notification without id (no response expected)", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + client.sendNotification("custom/event", { key: "value" }); + await new Promise((r) => setTimeout(r, 10)); + + const msg = parseStdinMessages(proc).find((m) => m["method"] === "custom/event"); + expect(msg).toBeDefined(); + expect(msg!["params"]).toEqual({ key: "value" }); + expect(msg!["id"]).toBeUndefined(); + + await closeClient(client, proc); + }); + + it("is a no-op on closed client", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + await closeClient(client, proc); + + // Should not throw + client.sendNotification("custom/event", {}); + }); + }); + + describe("sendApprovalResponse", () => { + it("sends approval response with decision", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + client.sendApprovalResponse(42, "acceptForSession"); + await new Promise((r) => setTimeout(r, 10)); + + const response = proc.stdinLines.find((l) => l.includes('"acceptForSession"')); + expect(response).toBeDefined(); + const parsed = JSON.parse(response!) as Record; + expect(parsed["id"]).toBe(42); + expect(parsed["result"]).toEqual({ decision: "acceptForSession" }); + + await closeClient(client, proc); + }); + + it("is a no-op on closed client", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + await closeClient(client, proc); + + // Should not throw + client.sendApprovalResponse("req-1", "cancel"); + }); + }); + + // ========================================================================= + // handleLine edge cases + // ========================================================================= + describe("handleLine edge cases", () => { + it("ignores JSON arrays", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + // Send a JSON array — should be silently ignored + proc.sendLine(JSON.stringify([1, 2, 3])); + await new Promise((r) => setTimeout(r, 10)); + + expect(client.isConnected).toBe(true); + await closeClient(client, proc); + }); + + it("ignores JSON primitives (string, number)", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + proc.sendLine(JSON.stringify("hello")); + proc.sendLine(JSON.stringify(42)); + proc.sendLine(JSON.stringify(null)); + await new Promise((r) => setTimeout(r, 10)); + + expect(client.isConnected).toBe(true); + await closeClient(client, proc); + }); + + it("ignores messages with unknown id and no method", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + // Message with id that doesn't match any pending request and no method + proc.sendLine(JSON.stringify({ id: "unknown-id-999", result: { data: "orphan" } })); + await new Promise((r) => setTimeout(r, 10)); + + expect(client.isConnected).toBe(true); + await closeClient(client, proc); + }); + }); + + // ========================================================================= + // Model Discovery + // ========================================================================= + describe("model discovery", () => { + it("modelList sends model/list request", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + const promise = client.modelList(); + await new Promise((r) => setTimeout(r, 10)); + + const req = findRequest(proc, "model/list"); + expect(req).toBeDefined(); + + proc.sendLine(JSON.stringify({ + id: req!["id"], + result: { models: [{ id: "o3-mini" }, { id: "gpt-4o" }] }, + })); + + const result = await promise; + expect(result).toEqual({ models: [{ id: "o3-mini" }, { id: "gpt-4o" }] }); + + const closePromise = client.close(); + proc.simulateExit(0); + await closePromise; + }); + }); +}); diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts new file mode 100644 index 000000000..8687b384c --- /dev/null +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -0,0 +1,467 @@ +/** + * Codex App-Server JSON-RPC Client + * + * Manages a `codex app-server` subprocess and communicates via + * newline-delimited JSON over stdin/stdout. Provides typed methods + * for thread management, turn execution, and conversation resume. + * + * Protocol reference: Codex app-server developer guide + * Implementation reference: codex-autorunner integrations/app_server/client.py + */ +import { spawn, type ChildProcess } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { createInterface, type Interface as ReadlineInterface } from "node:readline"; +import { EventEmitter } from "node:events"; + +// ============================================================================= +// Types +// ============================================================================= + +/** JSON-RPC request sent from client to server */ +export interface JsonRpcRequest { + id: string; + method: string; + params: Record; +} + +/** JSON-RPC response from server */ +export interface JsonRpcResponse { + id: string; + result?: Record; + error?: JsonRpcError; +} + +/** JSON-RPC error object */ +export interface JsonRpcError { + code: number; + message: string; + data?: unknown; +} + +/** JSON-RPC notification from server (no id) */ +export interface JsonRpcNotification { + method: string; + params: Record; +} + +/** Approval request from server (has id + method + params) */ +export interface JsonRpcApprovalRequest { + id: string | number; + method: string; + params: Record; +} + +/** Parsed message from the server — could be response, notification, or approval request */ +type ServerMessage = JsonRpcResponse | JsonRpcNotification | JsonRpcApprovalRequest; + +/** Callback for server notifications */ +export type NotificationHandler = (method: string, params: Record) => void; + +/** Callback for approval requests */ +export type ApprovalHandler = ( + id: string | number, + method: string, + params: Record, +) => Promise; + +/** Approval decision values */ +export type ApprovalDecision = + | "accept" + | "acceptForSession" + | "decline" + | "cancel"; + +/** Options for creating a CodexAppServerClient */ +export interface AppServerClientOptions { + /** Path to codex binary (default: "codex") */ + binaryPath?: string; + /** Working directory for the app-server process */ + cwd?: string; + /** Environment variables for the process */ + env?: Record; + /** Timeout for requests in ms (default: 60000) */ + requestTimeout?: number; + /** Handler for server notifications */ + onNotification?: NotificationHandler; + /** Handler for approval requests (auto-accepts if not provided) */ + onApproval?: ApprovalHandler; +} + +/** Thread start parameters */ +export interface ThreadStartParams { + model?: string; + modelProvider?: string; + cwd?: string; + approvalPolicy?: "suggest" | "auto-edit"; + sandbox?: "full" | "network-only" | "off"; + instructions?: string; +} + +/** Turn start parameters */ +export interface TurnStartParams { + threadId: string; + input: string; + cwd?: string; + model?: string; +} + +/** Pending request waiting for a response */ +interface PendingRequest { + resolve: (result: Record) => void; + reject: (error: Error) => void; + timer: ReturnType; +} + +// ============================================================================= +// Client Implementation +// ============================================================================= + +/** + * JSON-RPC client for Codex's app-server mode. + * + * Usage: + * ```ts + * const client = new CodexAppServerClient({ cwd: "/my/project" }); + * await client.connect(); + * + * const thread = await client.threadStart({ model: "o3-mini" }); + * const turn = await client.turnStart({ threadId: thread.id, input: "Fix the bug" }); + * + * await client.close(); + * ``` + */ +export class CodexAppServerClient extends EventEmitter { + private process: ChildProcess | null = null; + private readline: ReadlineInterface | null = null; + private pending = new Map(); + private initialized = false; + private closed = false; + + private readonly binaryPath: string; + private readonly cwd: string | undefined; + private readonly env: Record | undefined; + private readonly requestTimeout: number; + private readonly onNotification: NotificationHandler | undefined; + private readonly onApproval: ApprovalHandler | undefined; + + constructor(options: AppServerClientOptions = {}) { + super(); + this.binaryPath = options.binaryPath ?? "codex"; + this.cwd = options.cwd; + this.env = options.env; + this.requestTimeout = options.requestTimeout ?? 60_000; + this.onNotification = options.onNotification; + this.onApproval = options.onApproval; + } + + /** Whether the client is connected and initialized */ + get isConnected(): boolean { + return this.initialized && !this.closed && this.process !== null; + } + + /** + * Spawn the app-server process and perform the initialization handshake. + * Must be called before any other method. + */ + async connect(): Promise { + if (this.closed) throw new Error("Client is closed"); + if (this.initialized) throw new Error("Client is already connected"); + + this.process = spawn(this.binaryPath, ["app-server"], { + cwd: this.cwd, + env: this.env ? { ...process.env, ...this.env } : undefined, + stdio: ["pipe", "pipe", "pipe"], + }); + + if (!this.process.stdout || !this.process.stdin) { + throw new Error("Failed to open stdio pipes for codex app-server"); + } + + // Set up line-based reading from stdout + this.readline = createInterface({ input: this.process.stdout }); + this.readline.on("line", (line) => this.handleLine(line)); + + // Handle process exit + this.process.once("exit", (code, signal) => { + this.handleProcessExit(code, signal); + }); + + this.process.once("error", (err) => { + this.handleProcessError(err); + }); + + // Perform initialization handshake + await this.initialize(); + } + + /** + * Gracefully close the app-server process. + * Sends SIGTERM first, then SIGKILL after timeout. + */ + async close(): Promise { + if (this.closed) return; + this.closed = true; + this.initialized = false; + + // Reject all pending requests + for (const [id, pending] of this.pending) { + clearTimeout(pending.timer); + pending.reject(new Error("Client closed")); + this.pending.delete(id); + } + + if (this.readline) { + this.readline.close(); + this.readline = null; + } + + if (this.process && this.process.exitCode === null) { + const proc = this.process; + + await new Promise((resolve) => { + const killTimer = setTimeout(() => { + try { + proc.kill("SIGKILL"); + } catch { + // Already dead + } + resolve(); + }, 5_000); + + proc.once("exit", () => { + clearTimeout(killTimer); + resolve(); + }); + + try { + proc.kill("SIGTERM"); + } catch { + clearTimeout(killTimer); + resolve(); + } + }); + } + + this.process = null; + } + + // --------------------------------------------------------------------------- + // Thread Management + // --------------------------------------------------------------------------- + + /** Create a new conversation thread */ + async threadStart(params: ThreadStartParams = {}): Promise> { + return this.sendRequest("thread/start", { ...params }); + } + + /** Resume an existing conversation thread by ID */ + async threadResume(threadId: string): Promise> { + return this.sendRequest("thread/resume", { threadId }); + } + + /** List threads (with optional cursor-based pagination) */ + async threadList(cursor?: string, limit?: number): Promise> { + const params: Record = {}; + if (cursor) params["cursor"] = cursor; + if (limit !== undefined) params["limit"] = limit; + return this.sendRequest("thread/list", params); + } + + /** Archive a thread */ + async threadArchive(threadId: string): Promise> { + return this.sendRequest("thread/archive", { threadId }); + } + + // --------------------------------------------------------------------------- + // Turn Management + // --------------------------------------------------------------------------- + + /** Start a new turn (send a message to the agent) */ + async turnStart(params: TurnStartParams): Promise> { + return this.sendRequest("turn/start", { + threadId: params.threadId, + input: [{ type: "text", text: params.input }], + ...(params.cwd ? { cwd: params.cwd } : {}), + ...(params.model ? { model: params.model } : {}), + }); + } + + /** Interrupt a running turn */ + async turnInterrupt(threadId: string, turnId: string): Promise> { + return this.sendRequest("turn/interrupt", { threadId, turnId }); + } + + // --------------------------------------------------------------------------- + // Model Discovery + // --------------------------------------------------------------------------- + + /** List available models */ + async modelList(cursor?: string, limit?: number): Promise> { + const params: Record = {}; + if (cursor) params["cursor"] = cursor; + if (limit !== undefined) params["limit"] = limit; + return this.sendRequest("model/list", params); + } + + // --------------------------------------------------------------------------- + // Low-level Protocol + // --------------------------------------------------------------------------- + + /** Send a JSON-RPC request and wait for the response */ + async sendRequest(method: string, params: Record = {}): Promise> { + if (!this.initialized && method !== "initialize") { + throw new Error("Client not initialized — call connect() first"); + } + if (this.closed) throw new Error("Client is closed"); + if (!this.process?.stdin?.writable) { + throw new Error("stdin not writable — process may have exited"); + } + + const id = randomUUID(); + const request: JsonRpcRequest = { id, method, params }; + + return new Promise>((resolve, reject) => { + const timer = setTimeout(() => { + this.pending.delete(id); + reject(new Error(`Request ${method} timed out after ${this.requestTimeout}ms`)); + }, this.requestTimeout); + + this.pending.set(id, { resolve, reject, timer }); + this.writeLine(JSON.stringify(request)); + }); + } + + /** Send a JSON-RPC notification (no response expected) */ + sendNotification(method: string, params: Record = {}): void { + if (this.closed) return; + if (!this.process?.stdin?.writable) return; + + this.writeLine(JSON.stringify({ method, params })); + } + + /** Respond to an approval request from the server */ + sendApprovalResponse(id: string | number, decision: ApprovalDecision): void { + if (this.closed) return; + if (!this.process?.stdin?.writable) return; + + this.writeLine(JSON.stringify({ id, result: { decision } })); + } + + // --------------------------------------------------------------------------- + // Internal + // --------------------------------------------------------------------------- + + private async initialize(): Promise { + const result = await this.sendRequest("initialize", { + clientInfo: { + name: "ao-agent-codex", + title: "Agent Orchestrator — Codex Plugin", + version: "0.1.0", + }, + }); + + // Send the initialized notification to complete the handshake + this.sendNotification("initialized", {}); + this.initialized = true; + this.emit("connected", result); + } + + private writeLine(line: string): void { + if (!this.process?.stdin?.writable) return; + this.process.stdin.write(line + "\n"); + } + + private handleLine(line: string): void { + const trimmed = line.trim(); + if (!trimmed) return; + + let msg: ServerMessage; + try { + const parsed: unknown = JSON.parse(trimmed); + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return; + msg = parsed as ServerMessage; + } catch { + // Skip malformed lines + return; + } + + // Classify the message + if ("id" in msg && msg.id !== undefined) { + const id = String(msg.id); + + // Check if this is a response to a pending request + const pending = this.pending.get(id); + if (pending) { + this.pending.delete(id); + clearTimeout(pending.timer); + + if ("error" in msg && msg.error) { + pending.reject( + new Error(`JSON-RPC error ${msg.error.code}: ${msg.error.message}`), + ); + } else { + pending.resolve((msg as JsonRpcResponse).result ?? {}); + } + return; + } + + // If not a pending response, it's a server-initiated request (approval) + if ("method" in msg && typeof msg.method === "string") { + this.handleApprovalRequest(msg as JsonRpcApprovalRequest); + return; + } + } + + // Notification (no id, has method) + if ("method" in msg && typeof msg.method === "string" && !("id" in msg)) { + const notification = msg as JsonRpcNotification; + this.emit("notification", notification.method, notification.params); + if (this.onNotification) { + this.onNotification(notification.method, notification.params); + } + } + } + + private async handleApprovalRequest(request: JsonRpcApprovalRequest): Promise { + this.emit("approval", request.id, request.method, request.params); + + if (this.onApproval) { + try { + const decision = await this.onApproval(request.id, request.method, request.params); + this.sendApprovalResponse(request.id, decision); + } catch { + // On handler error, decline the request + this.sendApprovalResponse(request.id, "decline"); + } + } else { + // Default: auto-accept all approvals + this.sendApprovalResponse(request.id, "accept"); + } + } + + private handleProcessExit(code: number | null, signal: string | null): void { + this.initialized = false; + + // Reject all pending requests + const exitMsg = `codex app-server exited (code=${code}, signal=${signal})`; + for (const [id, pending] of this.pending) { + clearTimeout(pending.timer); + pending.reject(new Error(exitMsg)); + this.pending.delete(id); + } + + this.emit("exit", code, signal); + } + + private handleProcessError(err: Error): void { + this.initialized = false; + this.emit("error", err); + + // Reject all pending requests + for (const [id, pending] of this.pending) { + clearTimeout(pending.timer); + pending.reject(err); + this.pending.delete(id); + } + } +} diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 00dbeb119..449e0e7d1 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -9,14 +9,18 @@ const { mockWriteFile, mockMkdir, mockReadFile, + mockReaddir, mockRename, + mockStat, mockHomedir, } = vi.hoisted(() => ({ mockExecFileAsync: vi.fn(), mockWriteFile: vi.fn().mockResolvedValue(undefined), mockMkdir: vi.fn().mockResolvedValue(undefined), mockReadFile: vi.fn(), + mockReaddir: vi.fn(), mockRename: vi.fn().mockResolvedValue(undefined), + mockStat: vi.fn(), mockHomedir: vi.fn(() => "/mock/home"), })); @@ -31,7 +35,9 @@ vi.mock("node:fs/promises", () => ({ writeFile: mockWriteFile, mkdir: mockMkdir, readFile: mockReadFile, + readdir: mockReaddir, rename: mockRename, + stat: mockStat, })); vi.mock("node:crypto", () => ({ @@ -46,7 +52,7 @@ vi.mock("node:os", () => ({ homedir: mockHomedir, })); -import { create, manifest, default as defaultExport } from "./index.js"; +import { create, manifest, default as defaultExport, resolveCodexBinary } from "./index.js"; // --------------------------------------------------------------------------- // Test helpers @@ -148,9 +154,32 @@ describe("getLaunchCommand", () => { expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("codex"); }); - it("includes --full-auto when permissions=skip", () => { + it("includes --dangerously-bypass-approvals-and-sandbox when permissions=skip", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "skip" })); - expect(cmd).toContain("--full-auto"); + expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); + expect(cmd).not.toContain("--full-auto"); + }); + + it("includes --approval-mode auto-edit when permissions=auto-edit", () => { + // Cast needed: "auto-edit" not yet in AgentLaunchConfig type union + const cmd = agent.getLaunchCommand( + makeLaunchConfig({ permissions: "auto-edit" as AgentLaunchConfig["permissions"] }), + ); + expect(cmd).toContain("--approval-mode auto-edit"); + }); + + it("includes --approval-mode suggest when permissions=suggest", () => { + const cmd = agent.getLaunchCommand( + makeLaunchConfig({ permissions: "suggest" as AgentLaunchConfig["permissions"] }), + ); + expect(cmd).toContain("--approval-mode suggest"); + }); + + it("omits approval flags when permissions=default", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "default" })); + expect(cmd).not.toContain("--dangerously-bypass-approvals-and-sandbox"); + expect(cmd).not.toContain("--approval-mode"); + expect(cmd).not.toContain("--full-auto"); }); it("includes --model with shell-escaped value", () => { @@ -167,7 +196,7 @@ describe("getLaunchCommand", () => { const cmd = agent.getLaunchCommand( makeLaunchConfig({ permissions: "skip", model: "o3", prompt: "Go" }), ); - expect(cmd).toBe("codex --full-auto --model 'o3' -- 'Go'"); + expect(cmd).toBe("codex --dangerously-bypass-approvals-and-sandbox --model 'o3' --reasoning -- 'Go'"); }); it("escapes single quotes in prompt (POSIX shell escaping)", () => { @@ -203,9 +232,54 @@ describe("getLaunchCommand", () => { it("omits optional flags when not provided", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig()); - expect(cmd).not.toContain("--full-auto"); + expect(cmd).not.toContain("--dangerously-bypass-approvals-and-sandbox"); + expect(cmd).not.toContain("--approval-mode"); expect(cmd).not.toContain("--model"); expect(cmd).not.toContain("-c"); + expect(cmd).not.toContain("--reasoning"); + }); + + // -- Reasoning flag tests -- + describe("reasoning flag", () => { + it("adds --reasoning for o3 model", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "o3" })); + expect(cmd).toContain("--reasoning"); + }); + + it("adds --reasoning for o3-mini model", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "o3-mini" })); + expect(cmd).toContain("--reasoning"); + }); + + it("adds --reasoning for o4-mini model", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "o4-mini" })); + expect(cmd).toContain("--reasoning"); + }); + + it("adds --reasoning for O3 (case-insensitive)", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "O3" })); + expect(cmd).toContain("--reasoning"); + }); + + it("adds --reasoning for O4-MINI (case-insensitive)", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "O4-MINI" })); + expect(cmd).toContain("--reasoning"); + }); + + it("does NOT add --reasoning for gpt-4o model", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "gpt-4o" })); + expect(cmd).not.toContain("--reasoning"); + }); + + it("does NOT add --reasoning for gpt-4.1 model", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "gpt-4.1" })); + expect(cmd).not.toContain("--reasoning"); + }); + + it("does NOT add --reasoning when no model specified", () => { + const cmd = agent.getLaunchCommand(makeLaunchConfig()); + expect(cmd).not.toContain("--reasoning"); + }); }); }); @@ -479,18 +553,498 @@ describe("getActivityState", () => { }); // ========================================================================= -// getSessionInfo +// getSessionInfo — Codex JSONL parsing // ========================================================================= describe("getSessionInfo", () => { const agent = create(); - it("always returns null (not implemented)", async () => { - expect(await agent.getSessionInfo(makeSession())).toBeNull(); - expect(await agent.getSessionInfo(makeSession({ workspacePath: "/some/path" }))).toBeNull(); + // Helper to build JSONL content from lines + function jsonl(...lines: Record[]): string { + return lines.map((l) => JSON.stringify(l)).join("\n") + "\n"; + } + + it("returns null when workspacePath is null", async () => { + expect(await agent.getSessionInfo(makeSession({ workspacePath: null }))).toBeNull(); }); - it("returns null even with null workspacePath", async () => { - expect(await agent.getSessionInfo(makeSession({ workspacePath: null }))).toBeNull(); + it("returns null when workspacePath is undefined", async () => { + expect(await agent.getSessionInfo(makeSession({ workspacePath: undefined }))).toBeNull(); + }); + + it("returns null when ~/.codex/sessions/ directory does not exist", async () => { + mockReaddir.mockRejectedValue(new Error("ENOENT")); + expect(await agent.getSessionInfo(makeSession())).toBeNull(); + }); + + it("returns null when sessions directory is empty", async () => { + mockReaddir.mockResolvedValue([]); + expect(await agent.getSessionInfo(makeSession())).toBeNull(); + }); + + it("returns null when no session files match the workspace cwd", async () => { + mockReaddir.mockResolvedValue(["session-abc.jsonl"]); + mockReadFile.mockResolvedValue( + jsonl({ type: "session_meta", cwd: "/other/workspace", model: "gpt-4o" }), + ); + expect(await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" }))).toBeNull(); + }); + + it("returns session info with cost and model when matching session found", async () => { + const sessionContent = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "o3-mini" }, + { type: "event_msg", msg: { type: "token_count", input_tokens: 1000, output_tokens: 500, cached_tokens: 200, reasoning_tokens: 100 } }, + { type: "event_msg", msg: { type: "token_count", input_tokens: 2000, output_tokens: 300, cached_tokens: 0, reasoning_tokens: 0 } }, + ); + + mockReaddir.mockResolvedValue(["session-123.jsonl"]); + mockReadFile.mockResolvedValue(sessionContent); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + + expect(result).not.toBeNull(); + expect(result!.agentSessionId).toBe("session-123"); + expect(result!.summary).toBe("Codex session (o3-mini)"); + expect(result!.summaryIsFallback).toBe(true); + expect(result!.cost).toBeDefined(); + // input: 1000 + 200 (cached) + 2000 + 0 = 3200 + // output: 500 + 100 (reasoning) + 300 + 0 = 900 + expect(result!.cost!.inputTokens).toBe(3200); + expect(result!.cost!.outputTokens).toBe(900); + expect(result!.cost!.estimatedCostUsd).toBeGreaterThan(0); + }); + + it("picks the most recently modified matching session file", async () => { + const oldContent = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + ); + const newContent = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "o3" }, + ); + + mockReaddir.mockResolvedValue(["old-session.jsonl", "new-session.jsonl"]); + mockReadFile.mockImplementation((path: string) => { + if (path.includes("old-session")) return Promise.resolve(oldContent); + if (path.includes("new-session")) return Promise.resolve(newContent); + return Promise.reject(new Error("ENOENT")); + }); + mockStat.mockImplementation((path: string) => { + if (path.includes("old-session")) return Promise.resolve({ mtimeMs: 1000 }); + if (path.includes("new-session")) return Promise.resolve({ mtimeMs: 2000 }); + return Promise.reject(new Error("ENOENT")); + }); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + + expect(result).not.toBeNull(); + expect(result!.agentSessionId).toBe("new-session"); + expect(result!.summary).toBe("Codex session (o3)"); + }); + + it("handles corrupt/malformed JSONL lines gracefully", async () => { + const content = '{"type":"session_meta","cwd":"/workspace/test","model":"gpt-4o"}\n' + + "not valid json\n" + + '{"type":"event_msg","msg":{"type":"token_count","input_tokens":500,"output_tokens":200}}\n'; + + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(content); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + + expect(result).not.toBeNull(); + expect(result!.cost!.inputTokens).toBe(500); + expect(result!.cost!.outputTokens).toBe(200); + }); + + it("returns null summary when no model in session_meta", async () => { + const content = jsonl( + { type: "session_meta", cwd: "/workspace/test" }, + { type: "event_msg", msg: { type: "token_count", input_tokens: 100, output_tokens: 50 } }, + ); + + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(content); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + + expect(result).not.toBeNull(); + expect(result!.summary).toBeNull(); + }); + + it("returns undefined cost when no token_count events", async () => { + const content = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { type: "event_msg", msg: { type: "other_event" } }, + ); + + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(content); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + + expect(result).not.toBeNull(); + expect(result!.cost).toBeUndefined(); + }); + + it("handles unreadable session files gracefully", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockRejectedValue(new Error("EACCES")); + + expect(await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" }))).toBeNull(); + }); + + it("skips session files when stat throws", async () => { + const content = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(content); + mockStat.mockRejectedValue(new Error("EACCES")); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + // stat failed so no bestMatch can be established + expect(result).toBeNull(); + }); + + it("returns null when session JSONL has only empty/malformed lines", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + // First read (cwd check) has session_meta matching + // Second read (full parse) returns only garbage + let callCount = 0; + mockReadFile.mockImplementation(() => { + callCount++; + if (callCount <= 1) { + return Promise.resolve(jsonl( + { type: "session_meta", cwd: "/workspace/test" }, + )); + } + return Promise.resolve("not json\n\n \n"); + }); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + expect(result).toBeNull(); + }); + + it("ignores non-JSONL files in sessions directory", async () => { + mockReaddir.mockResolvedValue(["notes.txt", "config.json", "sess.jsonl"]); + const content = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + ); + mockReadFile.mockResolvedValue(content); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + expect(result).not.toBeNull(); + // readFile should only be called for the .jsonl file (+ possibly stat) + // Non-.jsonl files should be filtered out + const readFileCalls = mockReadFile.mock.calls.filter( + (call: string[]) => typeof call[0] === "string" && call[0].includes("sessions/"), + ); + expect(readFileCalls.length).toBe(2); // once for cwd check, once for full parse + }); +}); + +// ========================================================================= +// getRestoreCommand — conversation resume +// ========================================================================= +describe("getRestoreCommand", () => { + const agent = create(); + + function jsonl(...lines: Record[]): string { + return lines.map((l) => JSON.stringify(l)).join("\n") + "\n"; + } + + function makeProjectConfig(overrides: Record = {}) { + return { + name: "test-project", + repo: "owner/repo", + path: "/workspace/repo", + defaultBranch: "main", + sessionPrefix: "test", + ...overrides, + }; + } + + it("returns null when workspacePath is null", async () => { + const session = makeSession({ workspacePath: null }); + expect(await agent.getRestoreCommand!(session, makeProjectConfig())).toBeNull(); + }); + + it("returns null when workspacePath is undefined", async () => { + const session = makeSession({ workspacePath: undefined }); + expect(await agent.getRestoreCommand!(session, makeProjectConfig())).toBeNull(); + }); + + it("returns null when no matching session file found", async () => { + mockReaddir.mockRejectedValue(new Error("ENOENT")); + const session = makeSession({ workspacePath: "/workspace/test" }); + expect(await agent.getRestoreCommand!(session, makeProjectConfig())).toBeNull(); + }); + + it("returns null when session file has no usable context", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + // No threadId and no user messages → no context to re-inject + expect(await agent.getRestoreCommand!(session, makeProjectConfig())).toBeNull(); + }); + + it("builds restore command with thread context", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { threadId: "thread-abc-123" }, + { role: "user", content: "Fix the authentication bug" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); + + expect(cmd).not.toBeNull(); + expect(cmd).toContain("codex"); + expect(cmd).toContain("thread-abc-123"); + expect(cmd).toContain("Fix the authentication bug"); + }); + + it("includes permissions flags from project config", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { threadId: "thread-1" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig({ + agentConfig: { permissions: "skip" }, + })); + + expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); + }); + + it("includes --approval-mode auto-edit from project config", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { threadId: "thread-1" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig({ + agentConfig: { permissions: "auto-edit" }, + })); + + expect(cmd).toContain("--approval-mode auto-edit"); + expect(cmd).not.toContain("--dangerously-bypass-approvals-and-sandbox"); + }); + + it("includes --approval-mode suggest from project config", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { threadId: "thread-1" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig({ + agentConfig: { permissions: "suggest" }, + })); + + expect(cmd).toContain("--approval-mode suggest"); + }); + + it("includes model from project config (overrides session model)", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { threadId: "thread-1" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig({ + agentConfig: { model: "o3-mini" }, + })); + + expect(cmd).toContain("--model 'o3-mini'"); + expect(cmd).toContain("--reasoning"); + }); + + it("falls back to session model when project config has no model", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "o4-mini" }, + { threadId: "thread-1" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); + + expect(cmd).toContain("--model 'o4-mini'"); + expect(cmd).toContain("--reasoning"); + }); + + it("uses last user prompt for context when no threadId", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { role: "user", content: "Implement the search feature" }, + { role: "assistant", content: "I'll help with that" }, + { role: "user", content: "Now add tests" }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); + + expect(cmd).not.toBeNull(); + // Should contain the LAST user prompt + expect(cmd).toContain("Now add tests"); + }); + + it("truncates very long prompts", async () => { + const longPrompt = "x".repeat(600); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + mockReadFile.mockResolvedValue(jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { role: "user", content: longPrompt }, + )); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); + + expect(cmd).not.toBeNull(); + // The prompt should be truncated + expect(cmd!.length).toBeLessThan(700); + }); + + it("handles unreadable session files gracefully", async () => { + mockReaddir.mockResolvedValue(["sess.jsonl"]); + // First call for cwd check succeeds, second call for full parse fails + let callCount = 0; + mockReadFile.mockImplementation(() => { + callCount++; + if (callCount <= 1) { + return Promise.resolve(jsonl( + { type: "session_meta", cwd: "/workspace/test" }, + )); + } + return Promise.reject(new Error("EACCES")); + }); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + expect(await agent.getRestoreCommand!(session, makeProjectConfig())).toBeNull(); + }); +}); + +// ========================================================================= +// resolveCodexBinary +// ========================================================================= +describe("resolveCodexBinary", () => { + it("returns path from `which` when codex is found", async () => { + mockExecFileAsync.mockResolvedValue({ stdout: "/usr/local/bin/codex\n", stderr: "" }); + const result = await resolveCodexBinary(); + expect(result).toBe("/usr/local/bin/codex"); + expect(mockExecFileAsync).toHaveBeenCalledWith("which", ["codex"], { timeout: 10_000 }); + }); + + it("falls back to common locations when `which` fails", async () => { + mockExecFileAsync.mockRejectedValue(new Error("not found")); + mockStat.mockImplementation((path: string) => { + if (path === "/usr/local/bin/codex") { + return Promise.resolve({ mtimeMs: 1000 }); + } + return Promise.reject(new Error("ENOENT")); + }); + + const result = await resolveCodexBinary(); + expect(result).toBe("/usr/local/bin/codex"); + }); + + it("checks ~/.npm/bin/codex as fallback", async () => { + mockExecFileAsync.mockRejectedValue(new Error("not found")); + mockStat.mockImplementation((path: string) => { + if (path === "/mock/home/.npm/bin/codex") { + return Promise.resolve({ mtimeMs: 1000 }); + } + return Promise.reject(new Error("ENOENT")); + }); + + const result = await resolveCodexBinary(); + expect(result).toBe("/mock/home/.npm/bin/codex"); + }); + + it("returns 'codex' when not found anywhere", async () => { + mockExecFileAsync.mockRejectedValue(new Error("not found")); + mockStat.mockRejectedValue(new Error("ENOENT")); + + const result = await resolveCodexBinary(); + expect(result).toBe("codex"); + }); + + it("returns 'codex' when `which` returns empty stdout", async () => { + mockExecFileAsync.mockResolvedValue({ stdout: "", stderr: "" }); + mockStat.mockRejectedValue(new Error("ENOENT")); + + const result = await resolveCodexBinary(); + expect(result).toBe("codex"); + }); +}); + +// ========================================================================= +// postLaunchSetup — binary resolution +// ========================================================================= +describe("postLaunchSetup", () => { + it("has postLaunchSetup method", () => { + const agent = create(); + expect(typeof agent.postLaunchSetup).toBe("function"); + }); + + it("runs setup when session has workspacePath", async () => { + const agent = create(); + // which fails, stat fails → resolves to "codex" + mockExecFileAsync.mockRejectedValue(new Error("not found")); + mockStat.mockRejectedValue(new Error("ENOENT")); + mockReadFile.mockRejectedValue(new Error("ENOENT")); + await agent.postLaunchSetup!(makeSession({ workspacePath: "/workspace/test" })); + expect(mockMkdir).toHaveBeenCalled(); + }); + + it("returns early when session has no workspacePath", async () => { + const agent = create(); + mockExecFileAsync.mockRejectedValue(new Error("not found")); + mockStat.mockRejectedValue(new Error("ENOENT")); + await agent.postLaunchSetup!(makeSession({ workspacePath: undefined })); + expect(mockMkdir).not.toHaveBeenCalled(); + }); + + it("resolves binary and uses it in getLaunchCommand after postLaunchSetup", async () => { + const agent = create(); + mockExecFileAsync.mockResolvedValue({ stdout: "/opt/bin/codex\n", stderr: "" }); + mockReadFile.mockRejectedValue(new Error("ENOENT")); + + // Before postLaunchSetup, binary is "codex" + expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("codex"); + + // After postLaunchSetup resolves the binary + await agent.postLaunchSetup!(makeSession({ workspacePath: "/workspace/test" })); + + // Now getLaunchCommand should use the resolved binary + expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("/opt/bin/codex"); }); }); @@ -747,28 +1301,6 @@ describe("setupWorkspaceHooks", () => { }); }); -// ========================================================================= -// postLaunchSetup -// ========================================================================= -describe("postLaunchSetup", () => { - const agent = create(); - - it("has postLaunchSetup method", () => { - expect(typeof agent.postLaunchSetup).toBe("function"); - }); - - it("runs setup when session has workspacePath", async () => { - mockReadFile.mockRejectedValue(new Error("ENOENT")); - await agent.postLaunchSetup!(makeSession({ workspacePath: "/workspace/test" })); - expect(mockMkdir).toHaveBeenCalled(); - }); - - it("returns early when session has no workspacePath", async () => { - await agent.postLaunchSetup!(makeSession({ workspacePath: undefined })); - expect(mockMkdir).not.toHaveBeenCalled(); - }); -}); - // ========================================================================= // Shell wrapper content verification // ========================================================================= diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index 916d0f293..229400c79 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -5,15 +5,17 @@ import { type AgentLaunchConfig, type ActivityState, type ActivityDetection, + type CostEstimate, type PluginModule, + type ProjectConfig, type RuntimeHandle, type Session, type WorkspaceHooksConfig, } from "@composio/ao-core"; import { execFile } from "node:child_process"; -import { writeFile, mkdir, readFile, rename } from "node:fs/promises"; +import { writeFile, mkdir, readFile, readdir, rename, stat } from "node:fs/promises"; import { homedir } from "node:os"; -import { join } from "node:path"; +import { basename, join } from "node:path"; import { promisify } from "node:util"; import { randomBytes } from "node:crypto"; @@ -270,24 +272,244 @@ async function setupCodexWorkspace(workspacePath: string): Promise { } } +// ============================================================================= +// Codex Session JSONL Parsing (for getSessionInfo) +// ============================================================================= + +/** Codex session directory: ~/.codex/sessions/ */ +const CODEX_SESSIONS_DIR = join(homedir(), ".codex", "sessions"); + +/** Typed representation of a line in a Codex JSONL session file */ +interface CodexJsonlLine { + type?: string; + cwd?: string; + model?: string; + // Thread ID from thread_started notifications + threadId?: string; + // User message content (from user input events) + content?: string; + role?: string; + // event_msg with token_count subtype + msg?: { + type?: string; + input_tokens?: number; + output_tokens?: number; + cached_tokens?: number; + reasoning_tokens?: number; + }; +} + +/** + * Find Codex session files whose `session_meta` cwd matches the given workspace path. + * Returns the path to the most recently modified matching file, or null. + */ +async function findCodexSessionFile(workspacePath: string): Promise { + let entries: string[]; + try { + entries = await readdir(CODEX_SESSIONS_DIR); + } catch { + return null; + } + + const jsonlFiles = entries.filter((f) => f.endsWith(".jsonl")); + if (jsonlFiles.length === 0) return null; + + // Check each file for a matching cwd in the session_meta line + let bestMatch: { path: string; mtime: number } | null = null; + + for (const file of jsonlFiles) { + const fullPath = join(CODEX_SESSIONS_DIR, file); + try { + const content = await readFile(fullPath, "utf-8"); + // Only need to check the first few lines for session_meta + const lines = content.split("\n").slice(0, 10); + let cwdMatch = false; + for (const line of lines) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + const parsed: unknown = JSON.parse(trimmed); + if ( + typeof parsed === "object" && + parsed !== null && + !Array.isArray(parsed) && + (parsed as CodexJsonlLine).type === "session_meta" && + (parsed as CodexJsonlLine).cwd === workspacePath + ) { + cwdMatch = true; + break; + } + } catch { + // Skip malformed lines + } + } + if (cwdMatch) { + const s = await stat(fullPath); + if (!bestMatch || s.mtimeMs > bestMatch.mtime) { + bestMatch = { path: fullPath, mtime: s.mtimeMs }; + } + } + } catch { + // Skip unreadable files + } + } + + return bestMatch?.path ?? null; +} + +/** Parse a Codex JSONL session file and extract all lines */ +function parseCodexJsonl(content: string): CodexJsonlLine[] { + const result: CodexJsonlLine[] = []; + for (const line of content.split("\n")) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + const parsed: unknown = JSON.parse(trimmed); + if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) { + result.push(parsed as CodexJsonlLine); + } + } catch { + // Skip malformed lines + } + } + return result; +} + +/** Extract model name from Codex session_meta entry */ +function extractCodexModel(lines: CodexJsonlLine[]): string | null { + for (const line of lines) { + if (line.type === "session_meta" && typeof line.model === "string") { + return line.model; + } + } + return null; +} + +/** Aggregate token usage from Codex token_count events */ +function extractCodexCost(lines: CodexJsonlLine[]): CostEstimate | undefined { + let inputTokens = 0; + let outputTokens = 0; + + for (const line of lines) { + if (line.type === "event_msg" && line.msg?.type === "token_count") { + inputTokens += line.msg.input_tokens ?? 0; + inputTokens += line.msg.cached_tokens ?? 0; + outputTokens += line.msg.output_tokens ?? 0; + outputTokens += line.msg.reasoning_tokens ?? 0; + } + } + + if (inputTokens === 0 && outputTokens === 0) return undefined; + + // Rough cost estimate using GPT-4o pricing as baseline + // ($2.50/1M input, $10/1M output) + const estimatedCostUsd = + (inputTokens / 1_000_000) * 2.5 + (outputTokens / 1_000_000) * 10.0; + + return { inputTokens, outputTokens, estimatedCostUsd }; +} + +/** Extract the Codex thread ID from JSONL (from thread_started or session_meta) */ +function extractCodexThreadId(lines: CodexJsonlLine[]): string | null { + for (const line of lines) { + if (typeof line.threadId === "string" && line.threadId) { + return line.threadId; + } + } + return null; +} + +/** Extract the last user prompt from JSONL for context re-injection on resume */ +function extractLastUserPrompt(lines: CodexJsonlLine[]): string | null { + // Walk backwards to find the last user message + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i]; + if ( + line?.role === "user" && + typeof line.content === "string" && + line.content.trim().length > 0 + ) { + const content = line.content.trim(); + // Truncate very long prompts to avoid shell issues + return content.length > 500 ? content.substring(0, 500) + "..." : content; + } + } + return null; +} + +// ============================================================================= +// Binary Resolution +// ============================================================================= + +/** + * Resolve the Codex CLI binary path. + * Checks (in order): which, common fallback locations. + * Returns "codex" as final fallback (let the shell resolve it at runtime). + */ +export async function resolveCodexBinary(): Promise { + // 1. Try `which codex` + try { + const { stdout } = await execFileAsync("which", ["codex"], { timeout: 10_000 }); + const resolved = stdout.trim(); + if (resolved) return resolved; + } catch { + // Not found via which + } + + // 2. Check common locations + const home = homedir(); + const candidates = [ + "/usr/local/bin/codex", + join(home, ".npm", "bin", "codex"), + ]; + + for (const candidate of candidates) { + try { + await stat(candidate); + return candidate; + } catch { + // Not found at this location + } + } + + // 3. Fallback: let the shell resolve it + return "codex"; +} + // ============================================================================= // Agent Implementation // ============================================================================= function createCodexAgent(): Agent { + /** Cached resolved binary path (populated by init or first getLaunchCommand) */ + let resolvedBinary: string | null = null; + return { name: "codex", processName: "codex", getLaunchCommand(config: AgentLaunchConfig): string { - const parts: string[] = ["codex"]; + const binary = resolvedBinary ?? "codex"; + const parts: string[] = [binary]; - if (config.permissions === "skip") { - parts.push("--full-auto"); + // Approval mode mapping — cast to string for forward-compat with + // extended permission values not yet in AgentLaunchConfig type. + const perms = config.permissions as string | undefined; + if (perms === "skip") { + parts.push("--dangerously-bypass-approvals-and-sandbox"); + } else if (perms === "auto-edit") { + parts.push("--approval-mode", "auto-edit"); + } else if (perms === "suggest") { + parts.push("--approval-mode", "suggest"); } if (config.model) { parts.push("--model", shellEscape(config.model)); + + // Auto-detect o-series models that support extended thinking + if (/^o[34]/i.test(config.model)) { + parts.push("--reasoning"); + } } if (config.systemPromptFile) { @@ -409,9 +631,88 @@ function createCodexAgent(): Agent { } }, - async getSessionInfo(_session: Session): Promise { - // Codex doesn't have JSONL session files for introspection yet - return null; + async getSessionInfo(session: Session): Promise { + if (!session.workspacePath) return null; + + const sessionFile = await findCodexSessionFile(session.workspacePath); + if (!sessionFile) return null; + + let content: string; + try { + content = await readFile(sessionFile, "utf-8"); + } catch { + return null; + } + + const lines = parseCodexJsonl(content); + if (lines.length === 0) return null; + + const agentSessionId = basename(sessionFile, ".jsonl"); + const model = extractCodexModel(lines); + + return { + summary: model ? `Codex session (${model})` : null, + summaryIsFallback: true, + agentSessionId, + cost: extractCodexCost(lines), + }; + }, + + async getRestoreCommand(session: Session, project: ProjectConfig): Promise { + if (!session.workspacePath) return null; + + // Find the Codex session file for this workspace + const sessionFile = await findCodexSessionFile(session.workspacePath); + if (!sessionFile) return null; + + let content: string; + try { + content = await readFile(sessionFile, "utf-8"); + } catch { + return null; + } + + const lines = parseCodexJsonl(content); + if (lines.length === 0) return null; + + // Extract context for re-injection + const threadId = extractCodexThreadId(lines); + const lastPrompt = extractLastUserPrompt(lines); + const model = extractCodexModel(lines); + + // Build context summary for the new session + const contextParts: string[] = []; + if (threadId) contextParts.push(`Previous thread: ${threadId}.`); + if (lastPrompt) contextParts.push(`Last task: ${lastPrompt}`); + + if (contextParts.length === 0) return null; + + const contextPrompt = `Continue previous session. ${contextParts.join(" ")}`; + + // Build the restore command with the same flags as getLaunchCommand + const binary = resolvedBinary ?? "codex"; + const parts: string[] = [binary]; + + const perms = project.agentConfig?.permissions as string | undefined; + if (perms === "skip") { + parts.push("--dangerously-bypass-approvals-and-sandbox"); + } else if (perms === "auto-edit") { + parts.push("--approval-mode", "auto-edit"); + } else if (perms === "suggest") { + parts.push("--approval-mode", "suggest"); + } + + const effectiveModel = project.agentConfig?.model ?? model; + if (effectiveModel) { + parts.push("--model", shellEscape(effectiveModel as string)); + + if (/^o[34]/i.test(effectiveModel as string)) { + parts.push("--reasoning"); + } + } + + parts.push("--", shellEscape(contextPrompt)); + return parts.join(" "); }, async setupWorkspaceHooks(workspacePath: string, _config: WorkspaceHooksConfig): Promise { @@ -419,6 +720,10 @@ function createCodexAgent(): Agent { }, async postLaunchSetup(session: Session): Promise { + // Resolve binary path on first launch (cached for subsequent calls) + if (!resolvedBinary) { + resolvedBinary = await resolveCodexBinary(); + } if (!session.workspacePath) return; await setupCodexWorkspace(session.workspacePath); }, @@ -433,4 +738,14 @@ export function create(): Agent { return createCodexAgent(); } +export { CodexAppServerClient } from "./app-server-client.js"; +export type { + AppServerClientOptions, + ThreadStartParams, + TurnStartParams, + NotificationHandler, + ApprovalHandler, + ApprovalDecision, +} from "./app-server-client.js"; + export default { manifest, create } satisfies PluginModule; From 82317cbdeb413b86dbefb3aa3d87ac3bc3126b55 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Tue, 24 Feb 2026 23:47:28 +0530 Subject: [PATCH 02/14] fix(agent-codex): use correct Codex CLI flags, native resume, and date-sharded sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified all CLI flags against actual Codex documentation and fixed 6 issues: - --approval-mode → --ask-for-approval (untrusted/on-request/never) - --reasoning → -c model_reasoning_effort=high (config override) - Fake text re-injection → codex resume (native subcommand) - Flat readdir → recursive collectJsonlFiles() for YYYY/MM/DD sharding - ThreadStartParams values → correct Codex enum values - Added /opt/homebrew/bin/codex and ~/.cargo/bin/codex fallback paths Closes #176, #177, #178, #179, #183, #184 Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.ts | 9 +- .../plugins/agent-codex/src/index.test.ts | 169 +++++++++------- packages/plugins/agent-codex/src/index.ts | 188 ++++++++++-------- 3 files changed, 203 insertions(+), 163 deletions(-) diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts index 8687b384c..869dbe6e3 100644 --- a/packages/plugins/agent-codex/src/app-server-client.ts +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -92,9 +92,12 @@ export interface ThreadStartParams { model?: string; modelProvider?: string; cwd?: string; - approvalPolicy?: "suggest" | "auto-edit"; - sandbox?: "full" | "network-only" | "off"; - instructions?: string; + /** Codex approval policy: untrusted (ask for all), on-request, or never */ + approvalPolicy?: "untrusted" | "on-request" | "never"; + /** Codex sandbox mode */ + sandbox?: "read-only" | "workspace-write" | "danger-full-access"; + /** Personality/instruction preset */ + personality?: string; } /** Turn start parameters */ diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 449e0e7d1..86dfccf65 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -160,25 +160,25 @@ describe("getLaunchCommand", () => { expect(cmd).not.toContain("--full-auto"); }); - it("includes --approval-mode auto-edit when permissions=auto-edit", () => { + it("includes --ask-for-approval never when permissions=auto-edit", () => { // Cast needed: "auto-edit" not yet in AgentLaunchConfig type union const cmd = agent.getLaunchCommand( makeLaunchConfig({ permissions: "auto-edit" as AgentLaunchConfig["permissions"] }), ); - expect(cmd).toContain("--approval-mode auto-edit"); + expect(cmd).toContain("--ask-for-approval never"); }); - it("includes --approval-mode suggest when permissions=suggest", () => { + it("includes --ask-for-approval untrusted when permissions=suggest", () => { const cmd = agent.getLaunchCommand( makeLaunchConfig({ permissions: "suggest" as AgentLaunchConfig["permissions"] }), ); - expect(cmd).toContain("--approval-mode suggest"); + expect(cmd).toContain("--ask-for-approval untrusted"); }); it("omits approval flags when permissions=default", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ permissions: "default" })); expect(cmd).not.toContain("--dangerously-bypass-approvals-and-sandbox"); - expect(cmd).not.toContain("--approval-mode"); + expect(cmd).not.toContain("--ask-for-approval"); expect(cmd).not.toContain("--full-auto"); }); @@ -196,7 +196,7 @@ describe("getLaunchCommand", () => { const cmd = agent.getLaunchCommand( makeLaunchConfig({ permissions: "skip", model: "o3", prompt: "Go" }), ); - expect(cmd).toBe("codex --dangerously-bypass-approvals-and-sandbox --model 'o3' --reasoning -- 'Go'"); + expect(cmd).toBe("codex --dangerously-bypass-approvals-and-sandbox --model 'o3' -c model_reasoning_effort=high -- 'Go'"); }); it("escapes single quotes in prompt (POSIX shell escaping)", () => { @@ -233,52 +233,52 @@ describe("getLaunchCommand", () => { it("omits optional flags when not provided", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig()); expect(cmd).not.toContain("--dangerously-bypass-approvals-and-sandbox"); - expect(cmd).not.toContain("--approval-mode"); + expect(cmd).not.toContain("--ask-for-approval"); expect(cmd).not.toContain("--model"); expect(cmd).not.toContain("-c"); - expect(cmd).not.toContain("--reasoning"); + expect(cmd).not.toContain("model_reasoning_effort"); }); - // -- Reasoning flag tests -- - describe("reasoning flag", () => { - it("adds --reasoning for o3 model", () => { + // -- Reasoning effort tests -- + describe("reasoning effort", () => { + it("adds model_reasoning_effort=high for o3 model", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "o3" })); - expect(cmd).toContain("--reasoning"); + expect(cmd).toContain("-c model_reasoning_effort=high"); }); - it("adds --reasoning for o3-mini model", () => { + it("adds model_reasoning_effort=high for o3-mini model", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "o3-mini" })); - expect(cmd).toContain("--reasoning"); + expect(cmd).toContain("-c model_reasoning_effort=high"); }); - it("adds --reasoning for o4-mini model", () => { + it("adds model_reasoning_effort=high for o4-mini model", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "o4-mini" })); - expect(cmd).toContain("--reasoning"); + expect(cmd).toContain("-c model_reasoning_effort=high"); }); - it("adds --reasoning for O3 (case-insensitive)", () => { + it("adds model_reasoning_effort=high for O3 (case-insensitive)", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "O3" })); - expect(cmd).toContain("--reasoning"); + expect(cmd).toContain("-c model_reasoning_effort=high"); }); - it("adds --reasoning for O4-MINI (case-insensitive)", () => { + it("adds model_reasoning_effort=high for O4-MINI (case-insensitive)", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "O4-MINI" })); - expect(cmd).toContain("--reasoning"); + expect(cmd).toContain("-c model_reasoning_effort=high"); }); - it("does NOT add --reasoning for gpt-4o model", () => { + it("does NOT add reasoning effort for gpt-4o model", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "gpt-4o" })); - expect(cmd).not.toContain("--reasoning"); + expect(cmd).not.toContain("model_reasoning_effort"); }); - it("does NOT add --reasoning for gpt-4.1 model", () => { + it("does NOT add reasoning effort for gpt-4.1 model", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig({ model: "gpt-4.1" })); - expect(cmd).not.toContain("--reasoning"); + expect(cmd).not.toContain("model_reasoning_effort"); }); - it("does NOT add --reasoning when no model specified", () => { + it("does NOT add reasoning effort when no model specified", () => { const cmd = agent.getLaunchCommand(makeLaunchConfig()); - expect(cmd).not.toContain("--reasoning"); + expect(cmd).not.toContain("model_reasoning_effort"); }); }); }); @@ -729,18 +729,43 @@ describe("getSessionInfo", () => { expect(result).toBeNull(); }); + it("finds session files in date-sharded subdirectories (YYYY/MM/DD)", async () => { + // Simulate ~/.codex/sessions/2026/02/24/rollout-abc.jsonl + mockReaddir.mockImplementation((dir: string) => { + if (dir.endsWith("sessions")) return Promise.resolve(["2026"]); + if (dir.endsWith("2026")) return Promise.resolve(["02"]); + if (dir.endsWith("02")) return Promise.resolve(["24"]); + if (dir.endsWith("24")) return Promise.resolve(["rollout-abc.jsonl"]); + return Promise.resolve([]); + }); + mockStat.mockImplementation((path: string) => { + if (path.endsWith(".jsonl")) return Promise.resolve({ mtimeMs: 2000 }); + // Directories: 2026, 02, 24 + return Promise.resolve({ isDirectory: () => true }); + }); + const content = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "o3-mini" }, + ); + mockReadFile.mockResolvedValue(content); + + const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); + expect(result).not.toBeNull(); + expect(result!.agentSessionId).toBe("rollout-abc"); + expect(result!.summary).toBe("Codex session (o3-mini)"); + }); + it("ignores non-JSONL files in sessions directory", async () => { mockReaddir.mockResolvedValue(["notes.txt", "config.json", "sess.jsonl"]); const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, ); mockReadFile.mockResolvedValue(content); - mockStat.mockResolvedValue({ mtimeMs: 1000 }); + // Non-JSONL entries trigger stat to check isDirectory() + mockStat.mockResolvedValue({ mtimeMs: 1000, isDirectory: () => false }); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); expect(result).not.toBeNull(); - // readFile should only be called for the .jsonl file (+ possibly stat) - // Non-.jsonl files should be filtered out + // readFile should only be called for the .jsonl file const readFileCalls = mockReadFile.mock.calls.filter( (call: string[]) => typeof call[0] === "string" && call[0].includes("sessions/"), ); @@ -785,24 +810,24 @@ describe("getRestoreCommand", () => { expect(await agent.getRestoreCommand!(session, makeProjectConfig())).toBeNull(); }); - it("returns null when session file has no usable context", async () => { + it("returns null when session has no threadId", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); mockReadFile.mockResolvedValue(jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, + { role: "user", content: "Some prompt" }, )); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); - // No threadId and no user messages → no context to re-inject + // Native resume requires a threadId expect(await agent.getRestoreCommand!(session, makeProjectConfig())).toBeNull(); }); - it("builds restore command with thread context", async () => { + it("builds native resume command with codex resume ", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); mockReadFile.mockResolvedValue(jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { threadId: "thread-abc-123" }, - { role: "user", content: "Fix the authentication bug" }, )); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -810,12 +835,11 @@ describe("getRestoreCommand", () => { const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); expect(cmd).not.toBeNull(); - expect(cmd).toContain("codex"); + expect(cmd).toContain("codex resume"); expect(cmd).toContain("thread-abc-123"); - expect(cmd).toContain("Fix the authentication bug"); }); - it("includes permissions flags from project config", async () => { + it("includes --dangerously-bypass-approvals-and-sandbox from project config", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); mockReadFile.mockResolvedValue(jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, @@ -831,7 +855,7 @@ describe("getRestoreCommand", () => { expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); }); - it("includes --approval-mode auto-edit from project config", async () => { + it("includes --ask-for-approval never from project config", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); mockReadFile.mockResolvedValue(jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, @@ -844,11 +868,11 @@ describe("getRestoreCommand", () => { agentConfig: { permissions: "auto-edit" }, })); - expect(cmd).toContain("--approval-mode auto-edit"); + expect(cmd).toContain("--ask-for-approval never"); expect(cmd).not.toContain("--dangerously-bypass-approvals-and-sandbox"); }); - it("includes --approval-mode suggest from project config", async () => { + it("includes --ask-for-approval untrusted from project config", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); mockReadFile.mockResolvedValue(jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, @@ -861,7 +885,7 @@ describe("getRestoreCommand", () => { agentConfig: { permissions: "suggest" }, })); - expect(cmd).toContain("--approval-mode suggest"); + expect(cmd).toContain("--ask-for-approval untrusted"); }); it("includes model from project config (overrides session model)", async () => { @@ -878,7 +902,7 @@ describe("getRestoreCommand", () => { })); expect(cmd).toContain("--model 'o3-mini'"); - expect(cmd).toContain("--reasoning"); + expect(cmd).toContain("-c model_reasoning_effort=high"); }); it("falls back to session model when project config has no model", async () => { @@ -893,42 +917,7 @@ describe("getRestoreCommand", () => { const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); expect(cmd).toContain("--model 'o4-mini'"); - expect(cmd).toContain("--reasoning"); - }); - - it("uses last user prompt for context when no threadId", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( - { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, - { role: "user", content: "Implement the search feature" }, - { role: "assistant", content: "I'll help with that" }, - { role: "user", content: "Now add tests" }, - )); - mockStat.mockResolvedValue({ mtimeMs: 1000 }); - - const session = makeSession({ workspacePath: "/workspace/test" }); - const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); - - expect(cmd).not.toBeNull(); - // Should contain the LAST user prompt - expect(cmd).toContain("Now add tests"); - }); - - it("truncates very long prompts", async () => { - const longPrompt = "x".repeat(600); - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( - { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, - { role: "user", content: longPrompt }, - )); - mockStat.mockResolvedValue({ mtimeMs: 1000 }); - - const session = makeSession({ workspacePath: "/workspace/test" }); - const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); - - expect(cmd).not.toBeNull(); - // The prompt should be truncated - expect(cmd!.length).toBeLessThan(700); + expect(cmd).toContain("-c model_reasoning_effort=high"); }); it("handles unreadable session files gracefully", async () => { @@ -975,6 +964,32 @@ describe("resolveCodexBinary", () => { expect(result).toBe("/usr/local/bin/codex"); }); + it("checks /opt/homebrew/bin/codex as fallback", async () => { + mockExecFileAsync.mockRejectedValue(new Error("not found")); + mockStat.mockImplementation((path: string) => { + if (path === "/opt/homebrew/bin/codex") { + return Promise.resolve({ mtimeMs: 1000 }); + } + return Promise.reject(new Error("ENOENT")); + }); + + const result = await resolveCodexBinary(); + expect(result).toBe("/opt/homebrew/bin/codex"); + }); + + it("checks ~/.cargo/bin/codex as fallback (Rust-based codex)", async () => { + mockExecFileAsync.mockRejectedValue(new Error("not found")); + mockStat.mockImplementation((path: string) => { + if (path === "/mock/home/.cargo/bin/codex") { + return Promise.resolve({ mtimeMs: 1000 }); + } + return Promise.reject(new Error("ENOENT")); + }); + + const result = await resolveCodexBinary(); + expect(result).toBe("/mock/home/.cargo/bin/codex"); + }); + it("checks ~/.npm/bin/codex as fallback", async () => { mockExecFileAsync.mockRejectedValue(new Error("not found")); mockStat.mockImplementation((path: string) => { diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index 229400c79..d91f2cc18 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -299,58 +299,99 @@ interface CodexJsonlLine { }; } +/** + * Collect all JSONL files under a directory, recursively. + * Codex stores sessions in date-sharded directories: + * ~/.codex/sessions/YYYY/MM/DD/rollout-*.jsonl + */ +async function collectJsonlFiles(dir: string): Promise { + let entries: string[]; + try { + entries = await readdir(dir); + } catch { + return []; + } + + const results: string[] = []; + for (const entry of entries) { + const fullPath = join(dir, entry); + if (entry.endsWith(".jsonl")) { + results.push(fullPath); + } else { + // Recurse into subdirectories (YYYY/MM/DD structure) + try { + const s = await stat(fullPath); + if (s.isDirectory()) { + const nested = await collectJsonlFiles(fullPath); + results.push(...nested); + } + } catch { + // Skip inaccessible entries + } + } + } + return results; +} + +/** + * Check if the first few lines of a JSONL file contain a session_meta + * entry matching the given workspace path. Reads only the first 4 KB + * to avoid loading large rollout files into memory. + */ +async function sessionFileMatchesCwd( + filePath: string, + workspacePath: string, +): Promise { + try { + const content = await readFile(filePath, "utf-8"); + // Only parse the first few lines for the metadata header + const lines = content.split("\n").slice(0, 10); + for (const line of lines) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + const parsed: unknown = JSON.parse(trimmed); + if ( + typeof parsed === "object" && + parsed !== null && + !Array.isArray(parsed) && + (parsed as CodexJsonlLine).type === "session_meta" && + (parsed as CodexJsonlLine).cwd === workspacePath + ) { + return true; + } + } catch { + // Skip malformed lines + } + } + } catch { + // Unreadable file + } + return false; +} + /** * Find Codex session files whose `session_meta` cwd matches the given workspace path. + * Recursively scans ~/.codex/sessions/ (date-sharded: YYYY/MM/DD/rollout-*.jsonl). * Returns the path to the most recently modified matching file, or null. */ async function findCodexSessionFile(workspacePath: string): Promise { - let entries: string[]; - try { - entries = await readdir(CODEX_SESSIONS_DIR); - } catch { - return null; - } - - const jsonlFiles = entries.filter((f) => f.endsWith(".jsonl")); + const jsonlFiles = await collectJsonlFiles(CODEX_SESSIONS_DIR); if (jsonlFiles.length === 0) return null; - // Check each file for a matching cwd in the session_meta line let bestMatch: { path: string; mtime: number } | null = null; - for (const file of jsonlFiles) { - const fullPath = join(CODEX_SESSIONS_DIR, file); - try { - const content = await readFile(fullPath, "utf-8"); - // Only need to check the first few lines for session_meta - const lines = content.split("\n").slice(0, 10); - let cwdMatch = false; - for (const line of lines) { - const trimmed = line.trim(); - if (!trimmed) continue; - try { - const parsed: unknown = JSON.parse(trimmed); - if ( - typeof parsed === "object" && - parsed !== null && - !Array.isArray(parsed) && - (parsed as CodexJsonlLine).type === "session_meta" && - (parsed as CodexJsonlLine).cwd === workspacePath - ) { - cwdMatch = true; - break; - } - } catch { - // Skip malformed lines - } - } - if (cwdMatch) { - const s = await stat(fullPath); + for (const filePath of jsonlFiles) { + const matches = await sessionFileMatchesCwd(filePath, workspacePath); + if (matches) { + try { + const s = await stat(filePath); if (!bestMatch || s.mtimeMs > bestMatch.mtime) { - bestMatch = { path: fullPath, mtime: s.mtimeMs }; + bestMatch = { path: filePath, mtime: s.mtimeMs }; } + } catch { + // Skip if stat fails } - } catch { - // Skip unreadable files } } @@ -419,24 +460,6 @@ function extractCodexThreadId(lines: CodexJsonlLine[]): string | null { return null; } -/** Extract the last user prompt from JSONL for context re-injection on resume */ -function extractLastUserPrompt(lines: CodexJsonlLine[]): string | null { - // Walk backwards to find the last user message - for (let i = lines.length - 1; i >= 0; i--) { - const line = lines[i]; - if ( - line?.role === "user" && - typeof line.content === "string" && - line.content.trim().length > 0 - ) { - const content = line.content.trim(); - // Truncate very long prompts to avoid shell issues - return content.length > 500 ? content.substring(0, 500) + "..." : content; - } - } - return null; -} - // ============================================================================= // Binary Resolution // ============================================================================= @@ -456,10 +479,12 @@ export async function resolveCodexBinary(): Promise { // Not found via which } - // 2. Check common locations + // 2. Check common locations (npm global, Homebrew, Cargo — Codex is now Rust-based) const home = homedir(); const candidates = [ "/usr/local/bin/codex", + "/opt/homebrew/bin/codex", + join(home, ".cargo", "bin", "codex"), join(home, ".npm", "bin", "codex"), ]; @@ -492,23 +517,29 @@ function createCodexAgent(): Agent { const binary = resolvedBinary ?? "codex"; const parts: string[] = [binary]; - // Approval mode mapping — cast to string for forward-compat with + // Approval policy mapping — cast to string for forward-compat with // extended permission values not yet in AgentLaunchConfig type. + // Codex CLI uses --ask-for-approval (-a) with values: + // untrusted — approve before any state-mutating command + // on-request — approve only for privilege escalation + // never — no approval prompts (sandbox still applies) const perms = config.permissions as string | undefined; if (perms === "skip") { parts.push("--dangerously-bypass-approvals-and-sandbox"); } else if (perms === "auto-edit") { - parts.push("--approval-mode", "auto-edit"); + parts.push("--ask-for-approval", "never"); } else if (perms === "suggest") { - parts.push("--approval-mode", "suggest"); + parts.push("--ask-for-approval", "untrusted"); } if (config.model) { parts.push("--model", shellEscape(config.model)); - // Auto-detect o-series models that support extended thinking + // Auto-detect o-series models and enable reasoning via config override. + // Codex does not have a --reasoning flag; reasoning is controlled via + // the model_reasoning_effort config key. if (/^o[34]/i.test(config.model)) { - parts.push("--reasoning"); + parts.push("-c", "model_reasoning_effort=high"); } } @@ -675,43 +706,34 @@ function createCodexAgent(): Agent { const lines = parseCodexJsonl(content); if (lines.length === 0) return null; - // Extract context for re-injection + // Extract the session/thread ID for native resume const threadId = extractCodexThreadId(lines); - const lastPrompt = extractLastUserPrompt(lines); - const model = extractCodexModel(lines); + if (!threadId) return null; - // Build context summary for the new session - const contextParts: string[] = []; - if (threadId) contextParts.push(`Previous thread: ${threadId}.`); - if (lastPrompt) contextParts.push(`Last task: ${lastPrompt}`); - - if (contextParts.length === 0) return null; - - const contextPrompt = `Continue previous session. ${contextParts.join(" ")}`; - - // Build the restore command with the same flags as getLaunchCommand + // Use Codex's native `resume` subcommand for proper conversation resume. + // This restores the full thread state, not just a text prompt re-injection. const binary = resolvedBinary ?? "codex"; - const parts: string[] = [binary]; + const parts: string[] = [binary, "resume", shellEscape(threadId)]; + // Add approval policy flags from project config const perms = project.agentConfig?.permissions as string | undefined; if (perms === "skip") { parts.push("--dangerously-bypass-approvals-and-sandbox"); } else if (perms === "auto-edit") { - parts.push("--approval-mode", "auto-edit"); + parts.push("--ask-for-approval", "never"); } else if (perms === "suggest") { - parts.push("--approval-mode", "suggest"); + parts.push("--ask-for-approval", "untrusted"); } - const effectiveModel = project.agentConfig?.model ?? model; + const effectiveModel = project.agentConfig?.model ?? extractCodexModel(lines); if (effectiveModel) { parts.push("--model", shellEscape(effectiveModel as string)); if (/^o[34]/i.test(effectiveModel as string)) { - parts.push("--reasoning"); + parts.push("-c", "model_reasoning_effort=high"); } } - parts.push("--", shellEscape(contextPrompt)); return parts.join(" "); }, From 4e7129d50d7d78e6caff0b583f8e25e43724621d Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 00:38:58 +0530 Subject: [PATCH 03/14] =?UTF-8?q?fix(agent-codex):=20address=20review=20co?= =?UTF-8?q?mments=20=E2=80=94=20resource=20cleanup,=20concurrency,=20and?= =?UTF-8?q?=20memory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes 6 issues flagged by Cursor Bugbot on PR #188: 1. sessionFileMatchesCwd reads only first 4 KB via open()+read() instead of loading entire rollout file into memory (Medium) 2. Binary resolution race condition: added promise guard to prevent concurrent resolveCodexBinary() calls (Low) 3. Concurrent connect() calls: added connecting guard to prevent orphaned child processes (High) 4. connect() failure cleanup: wrap initialize() in try/catch that calls close() on failure (Medium) 5. readline cleanup on process exit/error: close readline in both handleProcessExit and handleProcessError (Medium) 6. stderr pipe drain: call stderr.resume() to prevent child blocking when stderr buffer fills (High) All 177 tests pass, typecheck clean. Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.test.ts | 88 +++++++++++ .../agent-codex/src/app-server-client.ts | 35 ++++- .../plugins/agent-codex/src/index.test.ts | 142 ++++++++++++------ packages/plugins/agent-codex/src/index.ts | 26 +++- 4 files changed, 235 insertions(+), 56 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 9cebc4a63..1221f2570 100644 --- a/packages/plugins/agent-codex/src/app-server-client.test.ts +++ b/packages/plugins/agent-codex/src/app-server-client.test.ts @@ -912,4 +912,92 @@ describe("CodexAppServerClient", () => { await closePromise; }); }); + + // ========================================================================= + // Concurrency & resource cleanup (review fixes) + // ========================================================================= + describe("concurrency guards", () => { + it("throws if connect() is called while already connecting", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + + // Start connect but don't complete the handshake + const connectPromise = client.connect(); + + // Second connect() should throw immediately + await expect(client.connect()).rejects.toThrow("already connecting"); + + // Complete the handshake to clean up + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + await closeClient(client, proc); + }); + + it("cleans up process when initialize handshake fails", async () => { + const proc = createFakeProcess(); + // Make kill trigger exit so close() doesn't hang + vi.spyOn(proc, "kill").mockImplementation((_signal?: string) => { + proc.simulateExit(1, "SIGTERM"); + return true; + }); + const client = new CodexAppServerClient({ requestTimeout: 200 }); + + // Start connect — don't answer the initialize request so it times out + await expect(client.connect()).rejects.toThrow("timed out"); + + // Client should have cleaned up — not connected, not connecting + expect(client.isConnected).toBe(false); + }); + + it("drains stderr without blocking the child process", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + // Write a large amount of data to stderr — should not block + const largeChunk = "x".repeat(65536); + proc.stderr.write(largeChunk); + await new Promise((r) => setTimeout(r, 10)); + + expect(client.isConnected).toBe(true); + await closeClient(client, proc); + }); + }); + + describe("readline cleanup on process exit/error", () => { + it("closes readline on unexpected process exit", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + // Spy on readline close — the client stores it privately so we check + // indirectly: after exit, sending another line should not cause errors + proc.simulateExit(1, null); + await new Promise((r) => setTimeout(r, 10)); + + // Client should be disconnected + expect(client.isConnected).toBe(false); + + // Sending data on stdout should not throw (readline is closed) + expect(() => proc.sendLine('{"id":"x","result":{}}')).not.toThrow(); + }); + + it("closes readline on process error", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await connectClient(client, proc); + + // Catch the error event on the client to prevent unhandled error + const errors: Error[] = []; + client.on("error", (err: Error) => errors.push(err)); + + proc.emit("error", new Error("process crashed")); + await new Promise((r) => setTimeout(r, 10)); + + expect(client.isConnected).toBe(false); + expect(errors).toHaveLength(1); + }); + }); }); diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts index 869dbe6e3..3b533cc44 100644 --- a/packages/plugins/agent-codex/src/app-server-client.ts +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -139,6 +139,7 @@ export class CodexAppServerClient extends EventEmitter { private pending = new Map(); private initialized = false; private closed = false; + private connecting = false; private readonly binaryPath: string; private readonly cwd: string | undefined; @@ -169,6 +170,9 @@ export class CodexAppServerClient extends EventEmitter { async connect(): Promise { if (this.closed) throw new Error("Client is closed"); if (this.initialized) throw new Error("Client is already connected"); + if (this.connecting) throw new Error("Client is already connecting"); + + this.connecting = true; this.process = spawn(this.binaryPath, ["app-server"], { cwd: this.cwd, @@ -177,9 +181,14 @@ export class CodexAppServerClient extends EventEmitter { }); if (!this.process.stdout || !this.process.stdin) { + this.connecting = false; throw new Error("Failed to open stdio pipes for codex app-server"); } + // Drain stderr to prevent the child process from blocking when + // the pipe buffer fills up. + this.process.stderr?.resume(); + // Set up line-based reading from stdout this.readline = createInterface({ input: this.process.stdout }); this.readline.on("line", (line) => this.handleLine(line)); @@ -193,8 +202,17 @@ export class CodexAppServerClient extends EventEmitter { this.handleProcessError(err); }); - // Perform initialization handshake - await this.initialize(); + // Perform initialization handshake. On failure, clean up the spawned + // process so the caller doesn't need to call close() explicitly. + try { + await this.initialize(); + } catch (err) { + this.connecting = false; + await this.close(); + throw err; + } + + this.connecting = false; } /** @@ -445,6 +463,12 @@ export class CodexAppServerClient extends EventEmitter { private handleProcessExit(code: number | null, signal: string | null): void { this.initialized = false; + // Close readline to release the event listener on the closed stdout stream + if (this.readline) { + this.readline.close(); + this.readline = null; + } + // Reject all pending requests const exitMsg = `codex app-server exited (code=${code}, signal=${signal})`; for (const [id, pending] of this.pending) { @@ -458,6 +482,13 @@ export class CodexAppServerClient extends EventEmitter { private handleProcessError(err: Error): void { this.initialized = false; + + // Close readline to release the event listener on the closed stdout stream + if (this.readline) { + this.readline.close(); + this.readline = null; + } + this.emit("error", err); // Reject all pending requests diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 86dfccf65..de5eea31f 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -12,6 +12,7 @@ const { mockReaddir, mockRename, mockStat, + mockOpen, mockHomedir, } = vi.hoisted(() => ({ mockExecFileAsync: vi.fn(), @@ -21,6 +22,7 @@ const { mockReaddir: vi.fn(), mockRename: vi.fn().mockResolvedValue(undefined), mockStat: vi.fn(), + mockOpen: vi.fn(), mockHomedir: vi.fn(() => "/mock/home"), })); @@ -38,6 +40,7 @@ vi.mock("node:fs/promises", () => ({ readdir: mockReaddir, rename: mockRename, stat: mockStat, + open: mockOpen, })); vi.mock("node:crypto", () => ({ @@ -114,9 +117,36 @@ function mockTmuxWithProcess(processName: string, found = true) { }); } +/** + * Create a mock file handle for `open()` that returns `content` from `read()`. + * Used by sessionFileMatchesCwd which reads only the first 4 KB. + */ +function makeFakeFileHandle(content: string) { + const buf = Buffer.from(content, "utf-8"); + return { + read: vi.fn().mockImplementation((buffer: Buffer, offset: number, length: number, _position: number) => { + const bytesToCopy = Math.min(length, buf.length); + buf.copy(buffer, offset, 0, bytesToCopy); + return Promise.resolve({ bytesRead: bytesToCopy, buffer }); + }), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +/** + * Set up mockOpen so that any `open(path, "r")` call returns a fake handle + * reading `content`. This is used by sessionFileMatchesCwd. + */ +function setupMockOpen(content: string) { + mockOpen.mockResolvedValue(makeFakeFileHandle(content)); +} + beforeEach(() => { vi.clearAllMocks(); mockHomedir.mockReturnValue("/mock/home"); + // Default: open() returns a handle with empty content (no session_meta match). + // Session tests call setupMockOpen(content) to override. + mockOpen.mockResolvedValue(makeFakeFileHandle("")); }); // ========================================================================= @@ -583,9 +613,9 @@ describe("getSessionInfo", () => { it("returns null when no session files match the workspace cwd", async () => { mockReaddir.mockResolvedValue(["session-abc.jsonl"]); - mockReadFile.mockResolvedValue( - jsonl({ type: "session_meta", cwd: "/other/workspace", model: "gpt-4o" }), - ); + const content = jsonl({ type: "session_meta", cwd: "/other/workspace", model: "gpt-4o" }); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); expect(await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" }))).toBeNull(); }); @@ -597,6 +627,7 @@ describe("getSessionInfo", () => { ); mockReaddir.mockResolvedValue(["session-123.jsonl"]); + setupMockOpen(sessionContent); mockReadFile.mockResolvedValue(sessionContent); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -623,6 +654,11 @@ describe("getSessionInfo", () => { ); mockReaddir.mockResolvedValue(["old-session.jsonl", "new-session.jsonl"]); + mockOpen.mockImplementation(async (path: string) => { + if (path.includes("old-session")) return makeFakeFileHandle(oldContent); + if (path.includes("new-session")) return makeFakeFileHandle(newContent); + throw new Error("ENOENT"); + }); mockReadFile.mockImplementation((path: string) => { if (path.includes("old-session")) return Promise.resolve(oldContent); if (path.includes("new-session")) return Promise.resolve(newContent); @@ -647,6 +683,7 @@ describe("getSessionInfo", () => { '{"type":"event_msg","msg":{"type":"token_count","input_tokens":500,"output_tokens":200}}\n'; mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -664,6 +701,7 @@ describe("getSessionInfo", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -680,6 +718,7 @@ describe("getSessionInfo", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -691,6 +730,9 @@ describe("getSessionInfo", () => { it("handles unreadable session files gracefully", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); + // open() finds matching session_meta, but readFile fails for full parse + setupMockOpen(jsonl({ type: "session_meta", cwd: "/workspace/test" })); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); mockReadFile.mockRejectedValue(new Error("EACCES")); expect(await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" }))).toBeNull(); @@ -701,6 +743,7 @@ describe("getSessionInfo", () => { { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, ); mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); mockReadFile.mockResolvedValue(content); mockStat.mockRejectedValue(new Error("EACCES")); @@ -711,18 +754,10 @@ describe("getSessionInfo", () => { it("returns null when session JSONL has only empty/malformed lines", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); - // First read (cwd check) has session_meta matching - // Second read (full parse) returns only garbage - let callCount = 0; - mockReadFile.mockImplementation(() => { - callCount++; - if (callCount <= 1) { - return Promise.resolve(jsonl( - { type: "session_meta", cwd: "/workspace/test" }, - )); - } - return Promise.resolve("not json\n\n \n"); - }); + // open() finds matching session_meta for cwd check + setupMockOpen(jsonl({ type: "session_meta", cwd: "/workspace/test" })); + // readFile (full parse) returns only garbage + mockReadFile.mockResolvedValue("not json\n\n \n"); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); @@ -746,6 +781,7 @@ describe("getSessionInfo", () => { const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "o3-mini" }, ); + setupMockOpen(content); mockReadFile.mockResolvedValue(content); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); @@ -759,17 +795,18 @@ describe("getSessionInfo", () => { const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, ); + setupMockOpen(content); mockReadFile.mockResolvedValue(content); // Non-JSONL entries trigger stat to check isDirectory() mockStat.mockResolvedValue({ mtimeMs: 1000, isDirectory: () => false }); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); expect(result).not.toBeNull(); - // readFile should only be called for the .jsonl file + // readFile should only be called once (full parse); cwd check uses open() const readFileCalls = mockReadFile.mock.calls.filter( (call: string[]) => typeof call[0] === "string" && call[0].includes("sessions/"), ); - expect(readFileCalls.length).toBe(2); // once for cwd check, once for full parse + expect(readFileCalls.length).toBe(1); // once for full parse (cwd check uses open()) }); }); @@ -811,11 +848,13 @@ describe("getRestoreCommand", () => { }); it("returns null when session has no threadId", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( + const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { role: "user", content: "Some prompt" }, - )); + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -824,11 +863,13 @@ describe("getRestoreCommand", () => { }); it("builds native resume command with codex resume ", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( + const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { threadId: "thread-abc-123" }, - )); + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -840,11 +881,13 @@ describe("getRestoreCommand", () => { }); it("includes --dangerously-bypass-approvals-and-sandbox from project config", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( + const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { threadId: "thread-1" }, - )); + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -856,11 +899,13 @@ describe("getRestoreCommand", () => { }); it("includes --ask-for-approval never from project config", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( + const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { threadId: "thread-1" }, - )); + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -873,11 +918,13 @@ describe("getRestoreCommand", () => { }); it("includes --ask-for-approval untrusted from project config", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( + const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { threadId: "thread-1" }, - )); + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -889,11 +936,13 @@ describe("getRestoreCommand", () => { }); it("includes model from project config (overrides session model)", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( + const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, { threadId: "thread-1" }, - )); + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -906,11 +955,13 @@ describe("getRestoreCommand", () => { }); it("falls back to session model when project config has no model", async () => { - mockReaddir.mockResolvedValue(["sess.jsonl"]); - mockReadFile.mockResolvedValue(jsonl( + const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "o4-mini" }, { threadId: "thread-1" }, - )); + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -922,17 +973,10 @@ describe("getRestoreCommand", () => { it("handles unreadable session files gracefully", async () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); - // First call for cwd check succeeds, second call for full parse fails - let callCount = 0; - mockReadFile.mockImplementation(() => { - callCount++; - if (callCount <= 1) { - return Promise.resolve(jsonl( - { type: "session_meta", cwd: "/workspace/test" }, - )); - } - return Promise.reject(new Error("EACCES")); - }); + // open() finds matching session_meta for cwd check + setupMockOpen(jsonl({ type: "session_meta", cwd: "/workspace/test" })); + // readFile (full parse) fails + mockReadFile.mockRejectedValue(new Error("EACCES")); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index d91f2cc18..bd4030102 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -13,7 +13,7 @@ import { type WorkspaceHooksConfig, } from "@composio/ao-core"; import { execFile } from "node:child_process"; -import { writeFile, mkdir, readFile, readdir, rename, stat } from "node:fs/promises"; +import { writeFile, mkdir, readFile, readdir, rename, stat, open } from "node:fs/promises"; import { homedir } from "node:os"; import { basename, join } from "node:path"; import { promisify } from "node:util"; @@ -343,8 +343,17 @@ async function sessionFileMatchesCwd( workspacePath: string, ): Promise { try { - const content = await readFile(filePath, "utf-8"); - // Only parse the first few lines for the metadata header + // Read only the first 4 KB — session_meta is always in the first few lines. + // Avoids loading large rollout files (100 MB+) into memory. + const handle = await open(filePath, "r"); + let content: string; + try { + const buffer = Buffer.allocUnsafe(4096); + const { bytesRead } = await handle.read(buffer, 0, 4096, 0); + content = buffer.subarray(0, bytesRead).toString("utf-8"); + } finally { + await handle.close(); + } const lines = content.split("\n").slice(0, 10); for (const line of lines) { const trimmed = line.trim(); @@ -508,6 +517,8 @@ export async function resolveCodexBinary(): Promise { function createCodexAgent(): Agent { /** Cached resolved binary path (populated by init or first getLaunchCommand) */ let resolvedBinary: string | null = null; + /** Guard against concurrent resolveCodexBinary() calls */ + let resolvingBinary: Promise | null = null; return { name: "codex", @@ -742,9 +753,14 @@ function createCodexAgent(): Agent { }, async postLaunchSetup(session: Session): Promise { - // Resolve binary path on first launch (cached for subsequent calls) + // Resolve binary path on first launch (cached for subsequent calls). + // Uses a promise guard to prevent concurrent calls from racing. if (!resolvedBinary) { - resolvedBinary = await resolveCodexBinary(); + if (!resolvingBinary) { + resolvingBinary = resolveCodexBinary(); + } + resolvedBinary = await resolvingBinary; + resolvingBinary = null; } if (!session.workspacePath) return; await setupCodexWorkspace(session.workspacePath); From c0105476dae9568d2eb023e99830fad113f6e4a2 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 01:00:44 +0530 Subject: [PATCH 04/14] =?UTF-8?q?fix(agent-codex):=20address=20follow-up?= =?UTF-8?q?=20review=20=E2=80=94=20retry=20after=20failed=20connect,=20bin?= =?UTF-8?q?ary=20guard=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 --- .../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); From 2dfbf8a77839d94fb5b897839629992194be65b8 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 01:19:36 +0530 Subject: [PATCH 05/14] fix(agent-codex): fix token double-counting and spawn error handling - Remove additive cached_tokens/reasoning_tokens from cost calculation (they are subsets of input_tokens/output_tokens per OpenAI API convention) - Wrap entire connect() body in try/catch so connecting flag resets if spawn() throws synchronously (EMFILE, ENOMEM) - Add test for spawn-throws-synchronously recovery - Update cost assertions to reflect correct non-additive token counting Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.test.ts | 21 +++++++ .../agent-codex/src/app-server-client.ts | 57 +++++++++---------- .../plugins/agent-codex/src/index.test.ts | 9 +-- packages/plugins/agent-codex/src/index.ts | 4 +- 4 files changed, 55 insertions(+), 36 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 70768b63e..1e58489c0 100644 --- a/packages/plugins/agent-codex/src/app-server-client.test.ts +++ b/packages/plugins/agent-codex/src/app-server-client.test.ts @@ -983,6 +983,27 @@ describe("CodexAppServerClient", () => { expect(client.isConnected).toBe(true); await closeClient(client, proc); }); + + it("resets connecting flag if spawn throws synchronously", async () => { + // Make spawn throw (e.g. EMFILE, ENOMEM) + mockSpawn.mockImplementationOnce(() => { + throw new Error("spawn EMFILE"); + }); + + const client = new CodexAppServerClient({ requestTimeout: 500 }); + await expect(client.connect()).rejects.toThrow("spawn EMFILE"); + + // connecting flag should be reset — a retry should not throw "already connecting" + const proc = createFakeProcess(); + const connectPromise = client.connect(); + await new Promise((r) => setTimeout(r, 10)); + const initReq = findRequest(proc, "initialize"); + proc.sendLine(JSON.stringify({ id: initReq!["id"], result: {} })); + await connectPromise; + + expect(client.isConnected).toBe(true); + await closeClient(client, proc); + }); }); describe("readline cleanup on process exit/error", () => { diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts index 01151988e..74bd5f4ce 100644 --- a/packages/plugins/agent-codex/src/app-server-client.ts +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -174,37 +174,34 @@ export class CodexAppServerClient extends EventEmitter { this.connecting = true; - this.process = spawn(this.binaryPath, ["app-server"], { - cwd: this.cwd, - env: this.env ? { ...process.env, ...this.env } : undefined, - stdio: ["pipe", "pipe", "pipe"], - }); - - if (!this.process.stdout || !this.process.stdin) { - this.connecting = false; - throw new Error("Failed to open stdio pipes for codex app-server"); - } - - // Drain stderr to prevent the child process from blocking when - // the pipe buffer fills up. - this.process.stderr?.resume(); - - // Set up line-based reading from stdout - this.readline = createInterface({ input: this.process.stdout }); - this.readline.on("line", (line) => this.handleLine(line)); - - // Handle process exit - this.process.once("exit", (code, signal) => { - this.handleProcessExit(code, signal); - }); - - this.process.once("error", (err) => { - this.handleProcessError(err); - }); - - // Perform initialization handshake. On failure, clean up the spawned - // process so the caller doesn't need to call close() explicitly. try { + this.process = spawn(this.binaryPath, ["app-server"], { + cwd: this.cwd, + env: this.env ? { ...process.env, ...this.env } : undefined, + stdio: ["pipe", "pipe", "pipe"], + }); + + if (!this.process.stdout || !this.process.stdin) { + throw new Error("Failed to open stdio pipes for codex app-server"); + } + + // Drain stderr to prevent the child process from blocking when + // the pipe buffer fills up. + this.process.stderr?.resume(); + + // Set up line-based reading from stdout + this.readline = createInterface({ input: this.process.stdout }); + this.readline.on("line", (line) => this.handleLine(line)); + + // Handle process exit + this.process.once("exit", (code, signal) => { + this.handleProcessExit(code, signal); + }); + + this.process.once("error", (err) => { + this.handleProcessError(err); + }); + await this.initialize(); } catch (err) { this.connecting = false; diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index de5eea31f..d47fb7612 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -638,10 +638,11 @@ describe("getSessionInfo", () => { expect(result!.summary).toBe("Codex session (o3-mini)"); expect(result!.summaryIsFallback).toBe(true); expect(result!.cost).toBeDefined(); - // input: 1000 + 200 (cached) + 2000 + 0 = 3200 - // output: 500 + 100 (reasoning) + 300 + 0 = 900 - expect(result!.cost!.inputTokens).toBe(3200); - expect(result!.cost!.outputTokens).toBe(900); + // cached_tokens/reasoning_tokens are subsets, not additive + // input: 1000 + 2000 = 3000 + // output: 500 + 300 = 800 + expect(result!.cost!.inputTokens).toBe(3000); + expect(result!.cost!.outputTokens).toBe(800); expect(result!.cost!.estimatedCostUsd).toBeGreaterThan(0); }); diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index 028c663b0..a6be6697f 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -443,9 +443,9 @@ function extractCodexCost(lines: CodexJsonlLine[]): CostEstimate | undefined { for (const line of lines) { if (line.type === "event_msg" && line.msg?.type === "token_count") { inputTokens += line.msg.input_tokens ?? 0; - inputTokens += line.msg.cached_tokens ?? 0; + // cached_tokens is a subset of input_tokens (not additive) outputTokens += line.msg.output_tokens ?? 0; - outputTokens += line.msg.reasoning_tokens ?? 0; + // reasoning_tokens is a subset of output_tokens (not additive) } } From e7033048f25ffd085893a5f60dc934e5f19bfe81 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 01:29:41 +0530 Subject: [PATCH 06/14] fix(agent-codex): prevent symlink cycle in recursive session scan - Use lstat instead of stat in collectJsonlFiles so symlinks to directories are never followed (isDirectory returns false for symlinks) - Add MAX_SESSION_SCAN_DEPTH=4 cap as defense-in-depth (Codex uses YYYY/MM/DD structure, max 3 levels) - Update tests to mock lstat for directory checks separately from stat Co-Authored-By: Claude Opus 4.6 --- .../plugins/agent-codex/src/index.test.ts | 20 ++++++++++++------- packages/plugins/agent-codex/src/index.ts | 19 +++++++++++++----- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index d47fb7612..9bf4d11fd 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -12,6 +12,7 @@ const { mockReaddir, mockRename, mockStat, + mockLstat, mockOpen, mockHomedir, } = vi.hoisted(() => ({ @@ -22,6 +23,7 @@ const { mockReaddir: vi.fn(), mockRename: vi.fn().mockResolvedValue(undefined), mockStat: vi.fn(), + mockLstat: vi.fn(), mockOpen: vi.fn(), mockHomedir: vi.fn(() => "/mock/home"), })); @@ -40,6 +42,7 @@ vi.mock("node:fs/promises", () => ({ readdir: mockReaddir, rename: mockRename, stat: mockStat, + lstat: mockLstat, open: mockOpen, })); @@ -147,6 +150,8 @@ beforeEach(() => { // Default: open() returns a handle with empty content (no session_meta match). // Session tests call setupMockOpen(content) to override. mockOpen.mockResolvedValue(makeFakeFileHandle("")); + // Default: lstat rejects (no subdirectories). Session tests override as needed. + mockLstat.mockRejectedValue(new Error("ENOENT")); }); // ========================================================================= @@ -774,11 +779,10 @@ describe("getSessionInfo", () => { if (dir.endsWith("24")) return Promise.resolve(["rollout-abc.jsonl"]); return Promise.resolve([]); }); - mockStat.mockImplementation((path: string) => { - if (path.endsWith(".jsonl")) return Promise.resolve({ mtimeMs: 2000 }); - // Directories: 2026, 02, 24 - return Promise.resolve({ isDirectory: () => true }); - }); + // lstat is used by collectJsonlFiles to check subdirectories (avoids symlink cycles) + mockLstat.mockResolvedValue({ isDirectory: () => true }); + // stat is used by findCodexSessionFile to get mtimeMs of matching JSONL files + mockStat.mockResolvedValue({ mtimeMs: 2000 }); const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "o3-mini" }, ); @@ -798,8 +802,10 @@ describe("getSessionInfo", () => { ); setupMockOpen(content); mockReadFile.mockResolvedValue(content); - // Non-JSONL entries trigger stat to check isDirectory() - mockStat.mockResolvedValue({ mtimeMs: 1000, isDirectory: () => false }); + // Non-JSONL entries trigger lstat to check isDirectory() + mockLstat.mockResolvedValue({ isDirectory: () => false }); + // stat is used to get mtimeMs for matching JSONL files + mockStat.mockResolvedValue({ mtimeMs: 1000 }); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); expect(result).not.toBeNull(); diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index a6be6697f..ced06398b 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -13,7 +13,7 @@ import { type WorkspaceHooksConfig, } from "@composio/ao-core"; import { execFile } from "node:child_process"; -import { writeFile, mkdir, readFile, readdir, rename, stat, open } from "node:fs/promises"; +import { writeFile, mkdir, readFile, readdir, rename, stat, lstat, open } from "node:fs/promises"; import { homedir } from "node:os"; import { basename, join } from "node:path"; import { promisify } from "node:util"; @@ -303,8 +303,16 @@ interface CodexJsonlLine { * Collect all JSONL files under a directory, recursively. * Codex stores sessions in date-sharded directories: * ~/.codex/sessions/YYYY/MM/DD/rollout-*.jsonl + * + * Uses lstat (not stat) so symlinks to directories are never followed, + * preventing infinite loops from symlink cycles. Max depth is capped at 4 + * (YYYY/MM/DD + 1 buffer) as an additional safety guard. */ -async function collectJsonlFiles(dir: string): Promise { +const MAX_SESSION_SCAN_DEPTH = 4; + +async function collectJsonlFiles(dir: string, depth = 0): Promise { + if (depth > MAX_SESSION_SCAN_DEPTH) return []; + let entries: string[]; try { entries = await readdir(dir); @@ -318,11 +326,12 @@ async function collectJsonlFiles(dir: string): Promise { if (entry.endsWith(".jsonl")) { results.push(fullPath); } else { - // Recurse into subdirectories (YYYY/MM/DD structure) + // Recurse into subdirectories (YYYY/MM/DD structure). + // Use lstat to avoid following symlinks that could create cycles. try { - const s = await stat(fullPath); + const s = await lstat(fullPath); if (s.isDirectory()) { - const nested = await collectJsonlFiles(fullPath); + const nested = await collectJsonlFiles(fullPath, depth + 1); results.push(...nested); } } catch { From 3f76338bb4415897f75c0fa60e831ae437512e37 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 01:41:47 +0530 Subject: [PATCH 07/14] fix(agent-codex): add jsonrpc 2.0 field and fix resume flag ordering - Add "jsonrpc": "2.0" to all JSON-RPC requests, notifications, and approval responses for spec compliance - Place flags before positional threadId in resume command for CLI parser compatibility (codex resume --flags ) - Add test assertions for jsonrpc field and flag ordering Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.test.ts | 2 ++ .../agent-codex/src/app-server-client.ts | 7 +++--- .../plugins/agent-codex/src/index.test.ts | 24 +++++++++++++++++++ packages/plugins/agent-codex/src/index.ts | 6 ++++- 4 files changed, 35 insertions(+), 4 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 1e58489c0..47eb7e991 100644 --- a/packages/plugins/agent-codex/src/app-server-client.test.ts +++ b/packages/plugins/agent-codex/src/app-server-client.test.ts @@ -137,8 +137,10 @@ describe("CodexAppServerClient", () => { // Should have sent initialize request and initialized notification const initReq = findRequest(proc, "initialize"); expect(initReq).toBeDefined(); + expect(initReq!["jsonrpc"]).toBe("2.0"); const initializedNotif = findRequest(proc, "initialized"); expect(initializedNotif).toBeDefined(); + expect(initializedNotif!["jsonrpc"]).toBe("2.0"); await closeClient(client, proc); }); diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts index 74bd5f4ce..5d5a753c2 100644 --- a/packages/plugins/agent-codex/src/app-server-client.ts +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -19,6 +19,7 @@ import { EventEmitter } from "node:events"; /** JSON-RPC request sent from client to server */ export interface JsonRpcRequest { + jsonrpc: "2.0"; id: string; method: string; params: Record; @@ -339,7 +340,7 @@ export class CodexAppServerClient extends EventEmitter { } const id = randomUUID(); - const request: JsonRpcRequest = { id, method, params }; + const request: JsonRpcRequest = { jsonrpc: "2.0", id, method, params }; return new Promise>((resolve, reject) => { const timer = setTimeout(() => { @@ -357,7 +358,7 @@ export class CodexAppServerClient extends EventEmitter { if (this.closed) return; if (!this.process?.stdin?.writable) return; - this.writeLine(JSON.stringify({ method, params })); + this.writeLine(JSON.stringify({ jsonrpc: "2.0", method, params })); } /** Respond to an approval request from the server */ @@ -365,7 +366,7 @@ export class CodexAppServerClient extends EventEmitter { if (this.closed) return; if (!this.process?.stdin?.writable) return; - this.writeLine(JSON.stringify({ id, result: { decision } })); + this.writeLine(JSON.stringify({ jsonrpc: "2.0", id, result: { decision } })); } // --------------------------------------------------------------------------- diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 9bf4d11fd..5ddbc5057 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -942,6 +942,30 @@ describe("getRestoreCommand", () => { expect(cmd).toContain("--ask-for-approval untrusted"); }); + it("places flags before positional threadId in resume command", async () => { + const content = jsonl( + { type: "session_meta", cwd: "/workspace/test", model: "o3-mini" }, + { threadId: "thread-order-test" }, + ); + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + mockReadFile.mockResolvedValue(content); + mockStat.mockResolvedValue({ mtimeMs: 1000 }); + + const session = makeSession({ workspacePath: "/workspace/test" }); + const cmd = await agent.getRestoreCommand!(session, makeProjectConfig({ + agentConfig: { permissions: "auto-edit", model: "o3-mini" }, + })); + + expect(cmd).not.toBeNull(); + // threadId should come after all flags + const threadIdIdx = cmd!.indexOf("thread-order-test"); + const flagIdx = cmd!.indexOf("--ask-for-approval"); + const modelIdx = cmd!.indexOf("--model"); + expect(flagIdx).toBeLessThan(threadIdIdx); + expect(modelIdx).toBeLessThan(threadIdIdx); + }); + it("includes model from project config (overrides session model)", async () => { const content = jsonl( { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index ced06398b..25a9c681a 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -732,8 +732,9 @@ function createCodexAgent(): Agent { // Use Codex's native `resume` subcommand for proper conversation resume. // This restores the full thread state, not just a text prompt re-injection. + // Flags are placed before the positional threadId for CLI parser compatibility. const binary = resolvedBinary ?? "codex"; - const parts: string[] = [binary, "resume", shellEscape(threadId)]; + const parts: string[] = [binary, "resume"]; // Add approval policy flags from project config const perms = project.agentConfig?.permissions as string | undefined; @@ -754,6 +755,9 @@ function createCodexAgent(): Agent { } } + // Positional threadId goes last, after all flags + parts.push(shellEscape(threadId)); + return parts.join(" "); }, From da7d3ec80ecc7110009bb2d40971dbab77650abd Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 09:23:15 +0530 Subject: [PATCH 08/14] fix(agent-codex): stream JSONL, shell-escape binary, guard close() - Replace full-file readFile with streaming createReadStream + readline for potentially huge Codex session files (100MB+) - Shell-escape resolved binary path in launch/restore commands - Prevent connect() failure from resetting an explicit close() call - Remove dead helper functions replaced by streamCodexSessionData Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.ts | 8 +- .../plugins/agent-codex/src/index.test.ts | 63 ++++++- packages/plugins/agent-codex/src/index.ts | 157 ++++++++---------- 3 files changed, 132 insertions(+), 96 deletions(-) diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts index 5d5a753c2..76eea7626 100644 --- a/packages/plugins/agent-codex/src/app-server-client.ts +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -174,6 +174,7 @@ export class CodexAppServerClient extends EventEmitter { if (this.connecting) throw new Error("Client is already connecting"); this.connecting = true; + const wasClosedBefore = this.closed; try { this.process = spawn(this.binaryPath, ["app-server"], { @@ -208,8 +209,11 @@ export class CodexAppServerClient extends EventEmitter { 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; + // transient handshake failure — but only if close() wasn't already + // called externally before this connect() attempt. + if (!wasClosedBefore) { + this.closed = false; + } throw err; } diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 5ddbc5057..1cee65eb3 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -14,6 +14,7 @@ const { mockStat, mockLstat, mockOpen, + mockCreateReadStream, mockHomedir, } = vi.hoisted(() => ({ mockExecFileAsync: vi.fn(), @@ -25,6 +26,7 @@ const { mockStat: vi.fn(), mockLstat: vi.fn(), mockOpen: vi.fn(), + mockCreateReadStream: vi.fn(), mockHomedir: vi.fn(() => "/mock/home"), })); @@ -52,12 +54,14 @@ vi.mock("node:crypto", () => ({ vi.mock("node:fs", () => ({ existsSync: vi.fn(() => false), + createReadStream: mockCreateReadStream, })); vi.mock("node:os", () => ({ homedir: mockHomedir, })); +import { Readable } from "node:stream"; import { create, manifest, default as defaultExport, resolveCodexBinary } from "./index.js"; // --------------------------------------------------------------------------- @@ -144,6 +148,22 @@ function setupMockOpen(content: string) { mockOpen.mockResolvedValue(makeFakeFileHandle(content)); } +/** + * Create a Readable stream from a string. Used to mock createReadStream + * for the streaming JSONL parser (streamCodexSessionData). + */ +function makeContentStream(content: string): Readable { + return Readable.from(Buffer.from(content, "utf-8")); +} + +/** + * Set up mockCreateReadStream to return a readable stream with the given content. + * Used by getSessionInfo/getRestoreCommand which now stream files line-by-line. + */ +function setupMockStream(content: string) { + mockCreateReadStream.mockReturnValue(makeContentStream(content)); +} + beforeEach(() => { vi.clearAllMocks(); mockHomedir.mockReturnValue("/mock/home"); @@ -152,6 +172,9 @@ beforeEach(() => { mockOpen.mockResolvedValue(makeFakeFileHandle("")); // Default: lstat rejects (no subdirectories). Session tests override as needed. mockLstat.mockRejectedValue(new Error("ENOENT")); + // Default: createReadStream returns an empty stream. Session tests call + // setupMockStream(content) to override. + mockCreateReadStream.mockReturnValue(makeContentStream("")); }); // ========================================================================= @@ -186,7 +209,7 @@ describe("getLaunchCommand", () => { const agent = create(); it("generates base command", () => { - expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("codex"); + expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("'codex'"); }); it("includes --dangerously-bypass-approvals-and-sandbox when permissions=skip", () => { @@ -231,7 +254,7 @@ describe("getLaunchCommand", () => { const cmd = agent.getLaunchCommand( makeLaunchConfig({ permissions: "skip", model: "o3", prompt: "Go" }), ); - expect(cmd).toBe("codex --dangerously-bypass-approvals-and-sandbox --model 'o3' -c model_reasoning_effort=high -- 'Go'"); + expect(cmd).toBe("'codex' --dangerously-bypass-approvals-and-sandbox --model 'o3' -c model_reasoning_effort=high -- 'Go'"); }); it("escapes single quotes in prompt (POSIX shell escaping)", () => { @@ -633,6 +656,7 @@ describe("getSessionInfo", () => { mockReaddir.mockResolvedValue(["session-123.jsonl"]); setupMockOpen(sessionContent); + setupMockStream(sessionContent); mockReadFile.mockResolvedValue(sessionContent); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -670,6 +694,11 @@ describe("getSessionInfo", () => { if (path.includes("new-session")) return Promise.resolve(newContent); return Promise.reject(new Error("ENOENT")); }); + mockCreateReadStream.mockImplementation((path: string) => { + if (path.includes("old-session")) return makeContentStream(oldContent); + if (path.includes("new-session")) return makeContentStream(newContent); + return makeContentStream(""); + }); mockStat.mockImplementation((path: string) => { if (path.includes("old-session")) return Promise.resolve({ mtimeMs: 1000 }); if (path.includes("new-session")) return Promise.resolve({ mtimeMs: 2000 }); @@ -690,6 +719,7 @@ describe("getSessionInfo", () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -740,6 +770,7 @@ describe("getSessionInfo", () => { setupMockOpen(jsonl({ type: "session_meta", cwd: "/workspace/test" })); mockStat.mockResolvedValue({ mtimeMs: 1000 }); mockReadFile.mockRejectedValue(new Error("EACCES")); + mockCreateReadStream.mockImplementation(() => { throw new Error("EACCES"); }); expect(await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" }))).toBeNull(); }); @@ -764,10 +795,15 @@ describe("getSessionInfo", () => { setupMockOpen(jsonl({ type: "session_meta", cwd: "/workspace/test" })); // readFile (full parse) returns only garbage mockReadFile.mockResolvedValue("not json\n\n \n"); + setupMockStream("not json\n\n \n"); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); - expect(result).toBeNull(); + // With streaming parser, garbage lines are skipped gracefully and a result + // is returned with null summary and undefined cost (no valid data extracted). + expect(result).not.toBeNull(); + expect(result!.summary).toBeNull(); + expect(result!.cost).toBeUndefined(); }); it("finds session files in date-sharded subdirectories (YYYY/MM/DD)", async () => { @@ -787,6 +823,7 @@ describe("getSessionInfo", () => { { type: "session_meta", cwd: "/workspace/test", model: "o3-mini" }, ); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); @@ -801,6 +838,7 @@ describe("getSessionInfo", () => { { type: "session_meta", cwd: "/workspace/test", model: "gpt-4o" }, ); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); // Non-JSONL entries trigger lstat to check isDirectory() mockLstat.mockResolvedValue({ isDirectory: () => false }); @@ -809,11 +847,12 @@ describe("getSessionInfo", () => { const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); expect(result).not.toBeNull(); - // readFile should only be called once (full parse); cwd check uses open() + // With streaming parser, readFile is no longer used for full parse; + // cwd check uses open(), data extraction uses createReadStream. const readFileCalls = mockReadFile.mock.calls.filter( (call: string[]) => typeof call[0] === "string" && call[0].includes("sessions/"), ); - expect(readFileCalls.length).toBe(1); // once for full parse (cwd check uses open()) + expect(readFileCalls.length).toBe(0); // streaming replaces readFile for full parse }); }); @@ -876,6 +915,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -883,7 +923,7 @@ describe("getRestoreCommand", () => { const cmd = await agent.getRestoreCommand!(session, makeProjectConfig()); expect(cmd).not.toBeNull(); - expect(cmd).toContain("codex resume"); + expect(cmd).toContain("'codex' resume"); expect(cmd).toContain("thread-abc-123"); }); @@ -894,6 +934,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -912,6 +953,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -931,6 +973,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -949,6 +992,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -973,6 +1017,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -992,6 +1037,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); + setupMockStream(content); mockReadFile.mockResolvedValue(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); @@ -1008,6 +1054,7 @@ describe("getRestoreCommand", () => { setupMockOpen(jsonl({ type: "session_meta", cwd: "/workspace/test" })); // readFile (full parse) fails mockReadFile.mockRejectedValue(new Error("EACCES")); + mockCreateReadStream.mockImplementation(() => { throw new Error("EACCES"); }); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); @@ -1128,13 +1175,13 @@ describe("postLaunchSetup", () => { mockReadFile.mockRejectedValue(new Error("ENOENT")); // Before postLaunchSetup, binary is "codex" - expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("codex"); + expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("'codex'"); // After postLaunchSetup resolves the binary await agent.postLaunchSetup!(makeSession({ workspacePath: "/workspace/test" })); // Now getLaunchCommand should use the resolved binary - expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("/opt/bin/codex"); + expect(agent.getLaunchCommand(makeLaunchConfig())).toBe("'/opt/bin/codex'"); }); }); diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index 25a9c681a..92bd95a0c 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -13,9 +13,11 @@ import { type WorkspaceHooksConfig, } from "@composio/ao-core"; import { execFile } from "node:child_process"; +import { createReadStream } from "node:fs"; import { writeFile, mkdir, readFile, readdir, rename, stat, lstat, open } from "node:fs/promises"; import { homedir } from "node:os"; import { basename, join } from "node:path"; +import { createInterface } from "node:readline"; import { promisify } from "node:util"; import { randomBytes } from "node:crypto"; @@ -416,66 +418,54 @@ async function findCodexSessionFile(workspacePath: string): Promise { + try { + const data: CodexSessionData = { model: null, threadId: null, inputTokens: 0, outputTokens: 0 }; + const rl = createInterface({ + input: createReadStream(filePath, { encoding: "utf-8" }), + crlfDelay: Infinity, + }); + + for await (const line of rl) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + const parsed: unknown = JSON.parse(trimmed); + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) continue; + const entry = parsed as CodexJsonlLine; + + if (entry.type === "session_meta" && typeof entry.model === "string") { + data.model = entry.model; + } + if (typeof entry.threadId === "string" && entry.threadId) { + data.threadId = entry.threadId; + } + if (entry.type === "event_msg" && entry.msg?.type === "token_count") { + data.inputTokens += entry.msg.input_tokens ?? 0; + data.outputTokens += entry.msg.output_tokens ?? 0; + } + } catch { + // Skip malformed lines } - } catch { - // Skip malformed lines } + + return data; + } catch { + return null; } - return result; -} - -/** Extract model name from Codex session_meta entry */ -function extractCodexModel(lines: CodexJsonlLine[]): string | null { - for (const line of lines) { - if (line.type === "session_meta" && typeof line.model === "string") { - return line.model; - } - } - return null; -} - -/** Aggregate token usage from Codex token_count events */ -function extractCodexCost(lines: CodexJsonlLine[]): CostEstimate | undefined { - let inputTokens = 0; - let outputTokens = 0; - - for (const line of lines) { - if (line.type === "event_msg" && line.msg?.type === "token_count") { - inputTokens += line.msg.input_tokens ?? 0; - // cached_tokens is a subset of input_tokens (not additive) - outputTokens += line.msg.output_tokens ?? 0; - // reasoning_tokens is a subset of output_tokens (not additive) - } - } - - if (inputTokens === 0 && outputTokens === 0) return undefined; - - // Rough cost estimate using GPT-4o pricing as baseline - // ($2.50/1M input, $10/1M output) - const estimatedCostUsd = - (inputTokens / 1_000_000) * 2.5 + (outputTokens / 1_000_000) * 10.0; - - return { inputTokens, outputTokens, estimatedCostUsd }; -} - -/** Extract the Codex thread ID from JSONL (from thread_started or session_meta) */ -function extractCodexThreadId(lines: CodexJsonlLine[]): string | null { - for (const line of lines) { - if (typeof line.threadId === "string" && line.threadId) { - return line.threadId; - } - } - return null; } // ============================================================================= @@ -535,7 +525,7 @@ function createCodexAgent(): Agent { getLaunchCommand(config: AgentLaunchConfig): string { const binary = resolvedBinary ?? "codex"; - const parts: string[] = [binary]; + const parts: string[] = [shellEscape(binary)]; // Approval policy mapping — cast to string for forward-compat with // extended permission values not yet in AgentLaunchConfig type. @@ -688,24 +678,28 @@ function createCodexAgent(): Agent { const sessionFile = await findCodexSessionFile(session.workspacePath); if (!sessionFile) return null; - let content: string; - try { - content = await readFile(sessionFile, "utf-8"); - } catch { - return null; - } - - const lines = parseCodexJsonl(content); - if (lines.length === 0) return null; + // Stream the file line-by-line to avoid loading potentially huge + // rollout files (100 MB+) entirely into memory. + const data = await streamCodexSessionData(sessionFile); + if (!data) return null; const agentSessionId = basename(sessionFile, ".jsonl"); - const model = extractCodexModel(lines); + + const cost: CostEstimate | undefined = + data.inputTokens === 0 && data.outputTokens === 0 + ? undefined + : { + inputTokens: data.inputTokens, + outputTokens: data.outputTokens, + estimatedCostUsd: + (data.inputTokens / 1_000_000) * 2.5 + (data.outputTokens / 1_000_000) * 10.0, + }; return { - summary: model ? `Codex session (${model})` : null, + summary: data.model ? `Codex session (${data.model})` : null, summaryIsFallback: true, agentSessionId, - cost: extractCodexCost(lines), + cost, }; }, @@ -716,25 +710,16 @@ function createCodexAgent(): Agent { const sessionFile = await findCodexSessionFile(session.workspacePath); if (!sessionFile) return null; - let content: string; - try { - content = await readFile(sessionFile, "utf-8"); - } catch { - return null; - } - - const lines = parseCodexJsonl(content); - if (lines.length === 0) return null; - - // Extract the session/thread ID for native resume - const threadId = extractCodexThreadId(lines); - if (!threadId) return null; + // Stream the file line-by-line to avoid loading potentially huge + // rollout files (100 MB+) entirely into memory. + const data = await streamCodexSessionData(sessionFile); + if (!data?.threadId) return null; // Use Codex's native `resume` subcommand for proper conversation resume. // This restores the full thread state, not just a text prompt re-injection. // Flags are placed before the positional threadId for CLI parser compatibility. const binary = resolvedBinary ?? "codex"; - const parts: string[] = [binary, "resume"]; + const parts: string[] = [shellEscape(binary), "resume"]; // Add approval policy flags from project config const perms = project.agentConfig?.permissions as string | undefined; @@ -746,7 +731,7 @@ function createCodexAgent(): Agent { parts.push("--ask-for-approval", "untrusted"); } - const effectiveModel = project.agentConfig?.model ?? extractCodexModel(lines); + const effectiveModel = project.agentConfig?.model ?? data.model; if (effectiveModel) { parts.push("--model", shellEscape(effectiveModel as string)); @@ -756,7 +741,7 @@ function createCodexAgent(): Agent { } // Positional threadId goes last, after all flags - parts.push(shellEscape(threadId)); + parts.push(shellEscape(data.threadId)); return parts.join(" "); }, From 89d2b5ead3d588f0eb1c626d3c1a0de92eadeddf Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 11:51:09 +0530 Subject: [PATCH 09/14] test(agent-codex): add missing setupMockStream calls to edge-case tests Three tests relied on the empty default stream coincidentally matching expected values instead of actually exercising the streaming parser. Added setupMockStream(content) and stronger assertions to verify the parser correctly handles missing model, missing token events, and missing threadId. Co-Authored-By: Claude Opus 4.6 --- packages/plugins/agent-codex/src/index.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 1cee65eb3..f272775bd 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -738,13 +738,17 @@ describe("getSessionInfo", () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); - mockReadFile.mockResolvedValue(content); + setupMockStream(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); expect(result).not.toBeNull(); expect(result!.summary).toBeNull(); + // Verify cost was actually parsed from the stream (not just defaulting to undefined) + expect(result!.cost).toBeDefined(); + expect(result!.cost!.inputTokens).toBe(100); + expect(result!.cost!.outputTokens).toBe(50); }); it("returns undefined cost when no token_count events", async () => { @@ -755,13 +759,15 @@ describe("getSessionInfo", () => { mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); - mockReadFile.mockResolvedValue(content); + setupMockStream(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const result = await agent.getSessionInfo(makeSession({ workspacePath: "/workspace/test" })); expect(result).not.toBeNull(); expect(result!.cost).toBeUndefined(); + // Verify model was actually parsed from the stream (not just defaulting to null) + expect(result!.summary).toContain("gpt-4o"); }); it("handles unreadable session files gracefully", async () => { @@ -900,7 +906,7 @@ describe("getRestoreCommand", () => { ); mockReaddir.mockResolvedValue(["sess.jsonl"]); setupMockOpen(content); - mockReadFile.mockResolvedValue(content); + setupMockStream(content); mockStat.mockResolvedValue({ mtimeMs: 1000 }); const session = makeSession({ workspacePath: "/workspace/test" }); From 3f81936467fbc4175e7f2a87da4b19cda67ff040 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 12:08:00 +0530 Subject: [PATCH 10/14] fix(agent-codex): reject pending requests before emitting error event Move this.emit("error", err) after the pending-rejection loop in handleProcessError, matching handleProcessExit's order. Without this, emit("error") with no listeners throws synchronously, skipping cleanup of pending requests. Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.test.ts | 22 +++++++++++++++++++ .../agent-codex/src/app-server-client.ts | 7 +++--- 2 files changed, 26 insertions(+), 3 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 47eb7e991..c2b5db7ea 100644 --- a/packages/plugins/agent-codex/src/app-server-client.test.ts +++ b/packages/plugins/agent-codex/src/app-server-client.test.ts @@ -747,6 +747,28 @@ describe("CodexAppServerClient", () => { expect(client.isConnected).toBe(false); }); + it("rejects pending requests before emitting error (no listener crash safe)", async () => { + const proc = createFakeProcess(); + const client = new CodexAppServerClient({ requestTimeout: 5000 }); + + await connectClient(client, proc); + + // Do NOT attach an error listener — emit("error") will throw + // Start a request that will be pending + const requestPromise = client.threadList(); + await new Promise((r) => setTimeout(r, 10)); + + // Process emits error — without a listener, emit("error") throws. + // Pending requests must still be rejected before that throw. + try { + proc.emit("error", new Error("spawn ENOENT")); + } catch { + // Expected: uncaught error event from EventEmitter + } + + await expect(requestPromise).rejects.toThrow("spawn ENOENT"); + }); + it("handles malformed JSON lines gracefully", 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 76eea7626..d8b165052 100644 --- a/packages/plugins/agent-codex/src/app-server-client.ts +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -494,13 +494,14 @@ export class CodexAppServerClient extends EventEmitter { this.readline = null; } - this.emit("error", err); - - // Reject all pending requests + // Reject all pending requests before emitting "error" — emit("error") + // with no listeners throws synchronously, which would skip cleanup. for (const [id, pending] of this.pending) { clearTimeout(pending.timer); pending.reject(err); this.pending.delete(id); } + + this.emit("error", err); } } From 9f1614dd0dc2f809f9d4aab70aad368d8b21fbf5 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 12:28:50 +0530 Subject: [PATCH 11/14] refactor(agent-codex): extract shared CLI flag helpers Extract duplicated approval-policy and model/reasoning flag logic from getLaunchCommand and getRestoreCommand into shared appendApprovalFlags and appendModelFlags helpers. Single source of truth for flag mapping. Co-Authored-By: Claude Opus 4.6 --- packages/plugins/agent-codex/src/index.ts | 72 +++++++++-------------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index 92bd95a0c..b3d700c92 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -513,6 +513,30 @@ export async function resolveCodexBinary(): Promise { // Agent Implementation // ============================================================================= +/** Append approval-policy flags to a command parts array */ +function appendApprovalFlags(parts: string[], permissions: string | undefined): void { + if (permissions === "skip") { + parts.push("--dangerously-bypass-approvals-and-sandbox"); + } else if (permissions === "auto-edit") { + parts.push("--ask-for-approval", "never"); + } else if (permissions === "suggest") { + parts.push("--ask-for-approval", "untrusted"); + } +} + +/** Append model and reasoning flags to a command parts array */ +function appendModelFlags(parts: string[], model: string | undefined): void { + if (!model) return; + parts.push("--model", shellEscape(model)); + + // Auto-detect o-series models and enable reasoning via config override. + // Codex does not have a --reasoning flag; reasoning is controlled via + // the model_reasoning_effort config key. + if (/^o[34]/i.test(model)) { + parts.push("-c", "model_reasoning_effort=high"); + } +} + function createCodexAgent(): Agent { /** Cached resolved binary path (populated by init or first getLaunchCommand) */ let resolvedBinary: string | null = null; @@ -527,31 +551,8 @@ function createCodexAgent(): Agent { const binary = resolvedBinary ?? "codex"; const parts: string[] = [shellEscape(binary)]; - // Approval policy mapping — cast to string for forward-compat with - // extended permission values not yet in AgentLaunchConfig type. - // Codex CLI uses --ask-for-approval (-a) with values: - // untrusted — approve before any state-mutating command - // on-request — approve only for privilege escalation - // never — no approval prompts (sandbox still applies) - const perms = config.permissions as string | undefined; - if (perms === "skip") { - parts.push("--dangerously-bypass-approvals-and-sandbox"); - } else if (perms === "auto-edit") { - parts.push("--ask-for-approval", "never"); - } else if (perms === "suggest") { - parts.push("--ask-for-approval", "untrusted"); - } - - if (config.model) { - parts.push("--model", shellEscape(config.model)); - - // Auto-detect o-series models and enable reasoning via config override. - // Codex does not have a --reasoning flag; reasoning is controlled via - // the model_reasoning_effort config key. - if (/^o[34]/i.test(config.model)) { - parts.push("-c", "model_reasoning_effort=high"); - } - } + appendApprovalFlags(parts, config.permissions as string | undefined); + appendModelFlags(parts, config.model); if (config.systemPromptFile) { // Codex reads developer instructions from a file via config override @@ -721,24 +722,9 @@ function createCodexAgent(): Agent { const binary = resolvedBinary ?? "codex"; const parts: string[] = [shellEscape(binary), "resume"]; - // Add approval policy flags from project config - const perms = project.agentConfig?.permissions as string | undefined; - if (perms === "skip") { - parts.push("--dangerously-bypass-approvals-and-sandbox"); - } else if (perms === "auto-edit") { - parts.push("--ask-for-approval", "never"); - } else if (perms === "suggest") { - parts.push("--ask-for-approval", "untrusted"); - } - - const effectiveModel = project.agentConfig?.model ?? data.model; - if (effectiveModel) { - parts.push("--model", shellEscape(effectiveModel as string)); - - if (/^o[34]/i.test(effectiveModel as string)) { - parts.push("-c", "model_reasoning_effort=high"); - } - } + appendApprovalFlags(parts, project.agentConfig?.permissions as string | undefined); + const effectiveModel = (project.agentConfig?.model ?? data.model) as string | undefined; + appendModelFlags(parts, effectiveModel ?? undefined); // Positional threadId goes last, after all flags parts.push(shellEscape(data.threadId)); From b6eb4d8888196b16328f684a86d77ed65e280c38 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 13:52:52 +0530 Subject: [PATCH 12/14] fix(agent-codex): catch approval emit throw, remove dead wasClosedBefore guard - Wrap entire handleApprovalRequest body in try/catch so a throwing "approval" listener doesn't cause an unhandled promise rejection - Remove wasClosedBefore variable that was always false (the closed check on entry guarantees it), simplifying the connect catch block Co-Authored-By: Claude Opus 4.6 --- .../agent-codex/src/app-server-client.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/plugins/agent-codex/src/app-server-client.ts b/packages/plugins/agent-codex/src/app-server-client.ts index d8b165052..a550000d9 100644 --- a/packages/plugins/agent-codex/src/app-server-client.ts +++ b/packages/plugins/agent-codex/src/app-server-client.ts @@ -174,7 +174,6 @@ export class CodexAppServerClient extends EventEmitter { if (this.connecting) throw new Error("Client is already connecting"); this.connecting = true; - const wasClosedBefore = this.closed; try { this.process = spawn(this.binaryPath, ["app-server"], { @@ -209,11 +208,9 @@ export class CodexAppServerClient extends EventEmitter { this.connecting = false; await this.close(); // Reset closed flag so the client can retry connect() after a - // transient handshake failure — but only if close() wasn't already - // called externally before this connect() attempt. - if (!wasClosedBefore) { - this.closed = false; - } + // transient handshake failure. The guard on line 172 ensures + // this.closed is always false when we reach this point. + this.closed = false; throw err; } @@ -449,19 +446,19 @@ export class CodexAppServerClient extends EventEmitter { } private async handleApprovalRequest(request: JsonRpcApprovalRequest): Promise { - this.emit("approval", request.id, request.method, request.params); + try { + this.emit("approval", request.id, request.method, request.params); - if (this.onApproval) { - try { + if (this.onApproval) { const decision = await this.onApproval(request.id, request.method, request.params); this.sendApprovalResponse(request.id, decision); - } catch { - // On handler error, decline the request - this.sendApprovalResponse(request.id, "decline"); + } else { + // Default: auto-accept all approvals + this.sendApprovalResponse(request.id, "accept"); } - } else { - // Default: auto-accept all approvals - this.sendApprovalResponse(request.id, "accept"); + } catch { + // On any error (listener throw or handler rejection), decline the request + this.sendApprovalResponse(request.id, "decline"); } } From c307ae30e000ae06610e4ccf476232bc317917ba Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 15:58:25 +0530 Subject: [PATCH 13/14] feat(agent-codex): implement mtime-based activity state detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the Codex session file's mtime as a proxy for agent activity. Codex continuously appends to its rollout JSONL while working, so: - recently modified file → active - stale file (past threshold) → idle - process not running → exited This replaces the previous null return with real activity detection, using the same threshold constant (DEFAULT_READY_THRESHOLD_MS = 5min) as the Claude Code plugin. Co-Authored-By: Claude Opus 4.6 --- .../plugins/agent-codex/src/index.test.ts | 57 ++++++++++++++----- packages/plugins/agent-codex/src/index.ts | 45 ++++++++++----- 2 files changed, 76 insertions(+), 26 deletions(-) diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index f272775bd..1a7d383e4 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -575,39 +575,70 @@ describe("getActivityState", () => { it("returns exited when no runtimeHandle", async () => { const session = makeSession({ runtimeHandle: null }); const result = await agent.getActivityState(session); - expect(result).toEqual({ state: "exited" }); + expect(result?.state).toBe("exited"); + expect(result?.timestamp).toBeInstanceOf(Date); }); it("returns exited when process is not running", async () => { mockExecFileAsync.mockRejectedValue(new Error("tmux not running")); const session = makeSession({ runtimeHandle: makeTmuxHandle() }); const result = await agent.getActivityState(session); - expect(result).toEqual({ state: "exited" }); + expect(result?.state).toBe("exited"); + expect(result?.timestamp).toBeInstanceOf(Date); }); - it("returns null (unknown) when process is running", async () => { + it("returns null when process is running but no workspacePath", async () => { mockTmuxWithProcess("codex"); - const session = makeSession({ runtimeHandle: makeTmuxHandle() }); + const session = makeSession({ runtimeHandle: makeTmuxHandle(), workspacePath: undefined }); expect(await agent.getActivityState(session)).toBeNull(); }); + it("returns null when process is running but no session file found", async () => { + mockTmuxWithProcess("codex"); + mockReaddir.mockRejectedValue(new Error("ENOENT")); + const session = makeSession({ runtimeHandle: makeTmuxHandle(), workspacePath: "/workspace/test" }); + expect(await agent.getActivityState(session)).toBeNull(); + }); + + it("returns active when session file was recently modified", async () => { + mockTmuxWithProcess("codex"); + const content = '{"type":"session_meta","cwd":"/workspace/test"}\n'; + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + // mtime = now (just modified) + mockStat.mockResolvedValue({ mtimeMs: Date.now(), mtime: new Date() }); + + const session = makeSession({ runtimeHandle: makeTmuxHandle(), workspacePath: "/workspace/test" }); + const result = await agent.getActivityState(session); + expect(result?.state).toBe("active"); + expect(result?.timestamp).toBeInstanceOf(Date); + }); + + it("returns idle when session file is stale", async () => { + mockTmuxWithProcess("codex"); + const content = '{"type":"session_meta","cwd":"/workspace/test"}\n'; + mockReaddir.mockResolvedValue(["sess.jsonl"]); + setupMockOpen(content); + // mtime = 10 minutes ago (past the 5-minute threshold) + const staleTime = Date.now() - 600_000; + mockStat.mockResolvedValue({ mtimeMs: staleTime, mtime: new Date(staleTime) }); + + const session = makeSession({ runtimeHandle: makeTmuxHandle(), workspacePath: "/workspace/test" }); + const result = await agent.getActivityState(session); + expect(result?.state).toBe("idle"); + expect(result?.timestamp).toBeInstanceOf(Date); + }); + it("returns exited when process handle has dead PID", async () => { const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { throw new Error("ESRCH"); }); const session = makeSession({ runtimeHandle: makeProcessHandle(999) }); const result = await agent.getActivityState(session); - expect(result).toEqual({ state: "exited" }); + expect(result?.state).toBe("exited"); + expect(result?.timestamp).toBeInstanceOf(Date); killSpy.mockRestore(); }); - - it("does not include timestamp in exited state", async () => { - const session = makeSession({ runtimeHandle: null }); - const result = await agent.getActivityState(session); - // The Codex implementation returns { state: "exited" } without timestamp - expect(result).toEqual({ state: "exited" }); - expect(result?.timestamp).toBeUndefined(); - }); }); // ========================================================================= diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index b3d700c92..c1dd03d75 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -1,4 +1,5 @@ import { + DEFAULT_READY_THRESHOLD_MS, shellEscape, type Agent, type AgentSessionInfo, @@ -606,20 +607,38 @@ function createCodexAgent(): Agent { return "active"; }, - async getActivityState(session: Session, _readyThresholdMs?: number): Promise { - // Check if process is running first - if (!session.runtimeHandle) return { state: "exited" }; - const running = await this.isProcessRunning(session.runtimeHandle); - if (!running) return { state: "exited" }; + async getActivityState(session: Session, readyThresholdMs?: number): Promise { + const threshold = readyThresholdMs ?? DEFAULT_READY_THRESHOLD_MS; - // NOTE: Codex stores rollout files in a global ~/.codex/sessions/ directory - // without workspace-specific scoping. When multiple Codex sessions run in - // parallel, we cannot reliably determine which rollout file belongs to which - // session. Until Codex provides per-workspace session tracking, we return - // null (unknown) rather than guessing. See issue #13 for details. - // - // TODO: Implement proper per-session activity detection when Codex supports it. - return null; + // Check if process is running first + const exitedAt = new Date(); + if (!session.runtimeHandle) return { state: "exited", timestamp: exitedAt }; + const running = await this.isProcessRunning(session.runtimeHandle); + if (!running) return { state: "exited", timestamp: exitedAt }; + + // Use session file mtime as a proxy for activity. Codex continuously + // appends to its rollout JSONL file while working, so a recently + // modified file means the agent is active. + if (!session.workspacePath) return null; + + const sessionFile = await findCodexSessionFile(session.workspacePath); + if (!sessionFile) return null; + + try { + const s = await stat(sessionFile); + const timestamp = s.mtime; + const ageMs = Date.now() - s.mtimeMs; + + if (ageMs <= threshold) { + // File was recently modified — agent is actively working + return { state: "active", timestamp }; + } + + // File is stale — agent finished or is idle + return { state: "idle", timestamp }; + } catch { + return null; + } }, async isProcessRunning(handle: RuntimeHandle): Promise { From 433189a79b71b19cc1427d2b6b0a69a4e8a078d9 Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Wed, 25 Feb 2026 16:25:24 +0530 Subject: [PATCH 14/14] perf(agent-codex): cache findCodexSessionFile to avoid double scan Add a 30-second TTL cache for session file lookups so that getActivityState, getSessionInfo, and getRestoreCommand called in the same refresh cycle reuse the result instead of re-scanning ~/.codex/sessions/ independently. Co-Authored-By: Claude Opus 4.6 --- .../plugins/agent-codex/src/index.test.ts | 3 +- packages/plugins/agent-codex/src/index.ts | 30 +++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 1a7d383e4..363825244 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -62,7 +62,7 @@ vi.mock("node:os", () => ({ })); import { Readable } from "node:stream"; -import { create, manifest, default as defaultExport, resolveCodexBinary } from "./index.js"; +import { create, manifest, default as defaultExport, resolveCodexBinary, _resetSessionFileCache } from "./index.js"; // --------------------------------------------------------------------------- // Test helpers @@ -166,6 +166,7 @@ function setupMockStream(content: string) { beforeEach(() => { vi.clearAllMocks(); + _resetSessionFileCache(); mockHomedir.mockReturnValue("/mock/home"); // Default: open() returns a handle with empty content (no session_meta match). // Session tests call setupMockOpen(content) to override. diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index c1dd03d75..0ebb69039 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -538,6 +538,25 @@ function appendModelFlags(parts: string[], model: string | undefined): void { } } +/** TTL for session file path cache (ms). Prevents redundant filesystem scans + * when getActivityState and getSessionInfo are called in the same refresh cycle. */ +const SESSION_FILE_CACHE_TTL_MS = 30_000; + +/** Module-level session file cache shared across the agent instance lifetime. + * Keyed by workspace path, stores the resolved file path and an expiry timestamp. */ +const sessionFileCache = new Map(); + +/** Find session file with caching to avoid double scans per refresh cycle */ +async function findCodexSessionFileCached(workspacePath: string): Promise { + const cached = sessionFileCache.get(workspacePath); + if (cached && Date.now() < cached.expiry) { + return cached.path; + } + const result = await findCodexSessionFile(workspacePath); + sessionFileCache.set(workspacePath, { path: result, expiry: Date.now() + SESSION_FILE_CACHE_TTL_MS }); + return result; +} + function createCodexAgent(): Agent { /** Cached resolved binary path (populated by init or first getLaunchCommand) */ let resolvedBinary: string | null = null; @@ -621,7 +640,7 @@ function createCodexAgent(): Agent { // modified file means the agent is active. if (!session.workspacePath) return null; - const sessionFile = await findCodexSessionFile(session.workspacePath); + const sessionFile = await findCodexSessionFileCached(session.workspacePath); if (!sessionFile) return null; try { @@ -695,7 +714,7 @@ function createCodexAgent(): Agent { async getSessionInfo(session: Session): Promise { if (!session.workspacePath) return null; - const sessionFile = await findCodexSessionFile(session.workspacePath); + const sessionFile = await findCodexSessionFileCached(session.workspacePath); if (!sessionFile) return null; // Stream the file line-by-line to avoid loading potentially huge @@ -727,7 +746,7 @@ function createCodexAgent(): Agent { if (!session.workspacePath) return null; // Find the Codex session file for this workspace - const sessionFile = await findCodexSessionFile(session.workspacePath); + const sessionFile = await findCodexSessionFileCached(session.workspacePath); if (!sessionFile) return null; // Stream the file line-by-line to avoid loading potentially huge @@ -782,6 +801,11 @@ export function create(): Agent { return createCodexAgent(); } +/** @internal Clear the session file cache. Exported for testing only. */ +export function _resetSessionFileCache(): void { + sessionFileCache.clear(); +} + export { CodexAppServerClient } from "./app-server-client.js"; export type { AppServerClientOptions,