diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index e85de611..1516c84a 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -145,6 +145,8 @@ For `codex_local`, Paperclip also manages a per-company Codex home under the ins - `~/.paperclip/instances/default/companies//codex-home` +If the `codex` CLI is not installed or not on `PATH`, `codex_local` agent runs fail at execution time with a clear adapter error. Quota polling uses a short-lived `codex app-server` subprocess: when `codex` cannot be spawned, that provider reports `ok: false` in aggregated quota results and the API server keeps running (it must not exit on a missing binary). + ## Worktree-local Instances When developing from multiple git worktrees, do not point two Paperclip servers at the same embedded PostgreSQL data directory. diff --git a/packages/adapters/codex-local/src/server/quota-spawn-error.test.ts b/packages/adapters/codex-local/src/server/quota-spawn-error.test.ts new file mode 100644 index 00000000..85d1e44c --- /dev/null +++ b/packages/adapters/codex-local/src/server/quota-spawn-error.test.ts @@ -0,0 +1,85 @@ +import { EventEmitter } from "node:events"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { ChildProcess } from "node:child_process"; +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; + +const { mockSpawn } = vi.hoisted(() => ({ + mockSpawn: vi.fn(), +})); + +vi.mock("node:child_process", async (importOriginal) => { + const cp = await importOriginal(); + return { + ...cp, + spawn: (...args: Parameters) => mockSpawn(...args) as ReturnType, + }; +}); + +import { getQuotaWindows } from "./quota.js"; + +function createChildThatErrorsOnMicrotask(err: Error): ChildProcess { + const child = new EventEmitter() as ChildProcess; + const stream = Object.assign(new EventEmitter(), { + setEncoding: () => {}, + }); + Object.assign(child, { + stdout: stream, + stderr: Object.assign(new EventEmitter(), { setEncoding: () => {} }), + stdin: { write: vi.fn(), end: vi.fn() }, + kill: vi.fn(), + }); + queueMicrotask(() => { + child.emit("error", err); + }); + return child; +} + +describe("CodexRpcClient spawn failures", () => { + let previousCodexHome: string | undefined; + let isolatedCodexHome: string | undefined; + + beforeEach(() => { + mockSpawn.mockReset(); + // After the RPC path fails, getQuotaWindows() calls readCodexToken() which + // reads $CODEX_HOME/auth.json (default ~/.codex). Point CODEX_HOME at an + // empty temp directory so we never hit real host auth or the WHAM network. + previousCodexHome = process.env.CODEX_HOME; + isolatedCodexHome = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-codex-spawn-test-")); + process.env.CODEX_HOME = isolatedCodexHome; + }); + + afterEach(() => { + if (isolatedCodexHome) { + try { + fs.rmSync(isolatedCodexHome, { recursive: true, force: true }); + } catch { + /* ignore */ + } + isolatedCodexHome = undefined; + } + if (previousCodexHome === undefined) { + delete process.env.CODEX_HOME; + } else { + process.env.CODEX_HOME = previousCodexHome; + } + }); + + it("does not crash the process when codex is missing; getQuotaWindows returns ok: false", async () => { + const enoent = Object.assign(new Error("spawn codex ENOENT"), { + code: "ENOENT", + errno: -2, + syscall: "spawn codex", + path: "codex", + }); + mockSpawn.mockImplementation(() => createChildThatErrorsOnMicrotask(enoent)); + + const result = await getQuotaWindows(); + + expect(result.ok).toBe(false); + expect(result.windows).toEqual([]); + expect(result.error).toContain("Codex app-server"); + expect(result.error).toContain("spawn codex ENOENT"); + }); +}); diff --git a/packages/adapters/codex-local/src/server/quota.ts b/packages/adapters/codex-local/src/server/quota.ts index 7bc771e4..3c0ac3bf 100644 --- a/packages/adapters/codex-local/src/server/quota.ts +++ b/packages/adapters/codex-local/src/server/quota.ts @@ -432,6 +432,13 @@ class CodexRpcClient { } this.pending.clear(); }); + this.proc.on("error", (err: Error) => { + for (const request of this.pending.values()) { + clearTimeout(request.timer); + request.reject(err); + } + this.pending.clear(); + }); } private onStdout(chunk: string) { diff --git a/packages/adapters/codex-local/vitest.config.ts b/packages/adapters/codex-local/vitest.config.ts new file mode 100644 index 00000000..f624398e --- /dev/null +++ b/packages/adapters/codex-local/vitest.config.ts @@ -0,0 +1,7 @@ +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + environment: "node", + }, +}); diff --git a/vitest.config.ts b/vitest.config.ts index 9bf83928..3ec05081 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,6 +2,13 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { - projects: ["packages/db", "packages/adapters/opencode-local", "server", "ui", "cli"], + projects: [ + "packages/db", + "packages/adapters/codex-local", + "packages/adapters/opencode-local", + "server", + "ui", + "cli", + ], }, });