diff --git a/AGENTS.md b/AGENTS.md index 61c2391..43c2fc0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,7 +38,7 @@ Built-in adapters split into two factories. NDJSON-style adapters (`claude-code` - Raw output capture (NDJSON lines for `lines` mode, raw chunks for `aggregate`) - Token estimator wiring via `StreamContext.feedAssistantText` - CLI resolution (direct command → `npx --yes ` fallback) -- Error precedence: `extracted.metadata.error` → `stderr` → `"Agent process exited with non-zero code"` +- Error precedence: `extracted.metadata.error` → spawn error → `stderr` → `"Agent process exited with non-zero code"` The NDJSON-style adapters (`claude-code`, `codex`) use `lines` mode for NDJSON parsing. Custom adapters can use either `lines` or `aggregate` mode (raw stdout capture). ACP-based adapters bypass `streamConfig` entirely - the ACP SDK handles framing. @@ -73,6 +73,7 @@ For built-in adapters, register the factory in `src/adapters/registry.ts`. Exter ### Error Handling - `AgentMetadata.error` is the canonical error field for failed runs +- A process that fails to start — `spawn()` throwing synchronously, or the child emitting `error` (e.g. `ENOENT`) — is a failed run with `metadata.error` set, never a thrown error; the runner and scoring treat it like any other failed run - Runner checks both `exitCode !== 0` and `metadata.error` for failure status - Friendly error classification in `src/ui/format.ts` via `friendlyError()` -maps common patterns (quota, rate limit, auth, timeout, network) to one-line messages - Error display: `↳ friendly message` below failed rows in tables, `Error:` line in detail views diff --git a/src/adapters/base/agent-adapter.ts b/src/adapters/base/agent-adapter.ts index 2b41ae5..b6de0c4 100644 --- a/src/adapters/base/agent-adapter.ts +++ b/src/adapters/base/agent-adapter.ts @@ -20,6 +20,22 @@ export const MAX_STDERR_BYTES = 100_000; /** Grace period between SIGTERM and SIGKILL for non-responsive processes. */ export const SIGTERM_TO_SIGKILL_MS = 5_000; +/** Failed `AgentOutput` for a process that never started (`spawn()` threw synchronously). */ +function failedToStart(startTime: Date, message: string): AgentOutput { + const endTime = new Date(); + return { + result: null, + transcript: [], + metadata: { + startTime: startTime.toISOString(), + endTime: endTime.toISOString(), + durationMs: endTime.getTime() - startTime.getTime(), + exitCode: 1, + error: message, + }, + }; +} + // --------------------------------------------------------------------------- // Context types passed to adapter callbacks // --------------------------------------------------------------------------- @@ -185,11 +201,6 @@ export type AgentAdapterSpec = { * SIGTERM → SIGKILL (with proper timer cleanup), exit promise ordering, raw * output capture, token estimator wiring, and the three outcome branches * (timed-out / non-zero exit with no result / success). - * - * Error precedence on failure: - * 1. `getResult(...).metadata.error` — wins if set - * 2. `stderr` — if non-empty - * 3. Generic `"Agent process exited with non-zero code"` */ export function createAgentAdapter(spec: AgentAdapterSpec): AgentAdapter { const timeoutMs = spec.timeoutMs ?? DEFAULT_TIMEOUT_MS; @@ -253,11 +264,18 @@ export function createAgentAdapter(spec: AgentAdapterSpec): AgentA }; // 5. Spawn - const child: ChildProcess = spawn(command, [...prefixArgs, ...args], { - cwd: input.workingDirectory, - stdio: ["pipe", "pipe", "pipe"], - env: input.env ?? { ...process.env }, - }); + let child: ChildProcess; + try { + child = spawn(command, [...prefixArgs, ...args], { + cwd: input.workingDirectory, + stdio: ["pipe", "pipe", "pipe"], + env: input.env ?? { ...process.env }, + }); + } catch (err) { + // spawn() throws synchronously for unusable arguments (e.g. a null + // byte in an arg) — fail the run instead of crashing the process. + return failedToStart(startTime, (err as Error).message); + } if (spec.promptVia === "stdin") { child.stdin?.on("error", () => {}); // Prevent an unhandled stream error if the child closes stdin early. @@ -272,8 +290,15 @@ export function createAgentAdapter(spec: AgentAdapterSpec): AgentA }); // 7. Register close listener BEFORE reading stdout (ordering matters) + // `error` fires when the child can't be started at all (e.g. ENOENT); + // a promise resolves once, so a later `close` is harmless. + let spawnError: Error | undefined; const exitPromise = new Promise((resolve) => { child.on("close", (code) => resolve(code ?? 1)); + child.on("error", (err) => { + spawnError = err; + resolve(1); + }); }); // 8. Buffer stderr with a size cap (and mirror to debug callback if any) @@ -420,7 +445,7 @@ export function createAgentAdapter(spec: AgentAdapterSpec): AgentA }); // 14. Error precedence - let error = extracted.metadata?.error; + let error = extracted.metadata?.error ?? spawnError?.message; if (!error && exitCode !== 0 && extracted.result === null) { error = stderr || "Agent process exited with non-zero code"; } diff --git a/test/unit/adapters/base/agent-adapter.test.ts b/test/unit/adapters/base/agent-adapter.test.ts index 00383a9..1f1bb34 100644 --- a/test/unit/adapters/base/agent-adapter.test.ts +++ b/test/unit/adapters/base/agent-adapter.test.ts @@ -24,8 +24,10 @@ function createMockProcess(opts: { exitCode?: number; delayMs?: number; hang?: boolean; + /** When set, emit an "error" event instead of "close" after the streams end (e.g. ENOENT). */ + error?: Error; }) { - const { stdout: stdoutLines = [], stderr: stderrLines = [], exitCode = 0, delayMs = 0, hang = false } = opts; + const { stdout: stdoutLines = [], stderr: stderrLines = [], exitCode = 0, delayMs = 0, hang = false, error } = opts; const stdout = new Readable({ read() {} }); const stderr = new Readable({ read() {} }); const stdin = { end: vi.fn(), on: vi.fn() }; @@ -36,7 +38,11 @@ function createMockProcess(opts: { for (const line of stderrLines) stderr.push(line); stdout.push(null); stderr.push(null); - if (!hang) proc.emit("close", exitCode); + if (error) { + proc.emit("error", error); + } else if (!hang) { + proc.emit("close", exitCode); + } }, delayMs); return proc; @@ -432,6 +438,31 @@ describe("createAgentAdapter", () => { expect(out.metadata.error?.length).toBeLessThan(200_000); }); + it("a synchronous spawn throw fails the run instead of crashing it", async () => { + mockSpawn.mockImplementation(() => { + throw new TypeError("The argument 'args[1]' must be a string without null bytes"); + }); + const adapter = createLinesTestAdapter(); + + const out = await adapter.run(makeInput()); + + expect(out.result).toBeNull(); + expect(out.transcript).toEqual([]); + expect(out.metadata.exitCode).not.toBe(0); + expect(out.metadata.error).toContain("null bytes"); + }); + + it("a child error event (e.g. ENOENT) fails the run instead of hanging", async () => { + mockSpawn.mockImplementation((() => + createMockProcess({ stdout: [], error: new Error("spawn test-bin ENOENT") })) as any); + + const adapter = createLinesTestAdapter(); + const out = await adapter.run(makeInput()); + + expect(out.metadata.error).toContain("ENOENT"); + expect(out.metadata.exitCode).not.toBe(0); + }); + it("promptVia: stdin writes the prompt to child.stdin and resolves without throwing", async () => { const fakeChild = createMockProcess({ stdout: ["ok\n"] }); mockSpawn.mockImplementation((() => fakeChild) as any);