fix(agents): surface stdout-only CLI errors

src/core/agents/claude.ts

@@ -11,17 +11,14 @@ import {
   PermanentAgentError,
 } from "./types.js";
 import { shutdownChildProcess } from "./managed-process.js";
-import { parseJSONLStream, setupAbortHandler } from "./stream-utils.js";
+import {
+  appendExitOutputTail,
+  describeChildProcessExit,
+  parseJSONLStream,
+  setupAbortHandler,
+} from "./stream-utils.js";
 
 const DEFAULT_FINAL_RESULT_EXIT_GRACE_MS = 15_000;
-/** Upper bound on the stdout tail kept for non-zero-exit error reporting. */
-const MAX_EXIT_OUTPUT_CHARS = 4_000;
-/**
- * Tighter bound on unstructured stdout quoted back in the failure detail: that
- * text lands in notes.md and is replayed in every later iteration prompt.
- */
-const MAX_RAW_TAIL_CHARS = 400;
-const RAW_TAIL_ELISION = "[...truncated, full output in the iteration log] ";
 
 interface ClaudeAssistantEvent {
   type: "assistant";
@@ -204,103 +201,6 @@ function isPermanentClaudeError(output: string): boolean {
   return /credit balance\s+is\s+too\s+low/i.test(output);
 }
 
-/** Keep only the last `MAX_EXIT_OUTPUT_CHARS` characters so long streams stay bounded. */
-function appendBoundedTail(existing: string, chunk: string): string {
-  const combined = existing + chunk;
-  return combined.length > MAX_EXIT_OUTPUT_CHARS
-    ? combined.slice(combined.length - MAX_EXIT_OUTPUT_CHARS)
-    : combined;
-}
-
-function errorTextFromEvent(event: unknown): string | null {
-  if (!event || typeof event !== "object") return null;
-  const record = event as Record<string, unknown>;
-
-  const error = record.error;
-  if (typeof error === "string" && error.trim()) return error.trim();
-  if (error && typeof error === "object") {
-    const message = (error as Record<string, unknown>).message;
-    if (typeof message === "string" && message.trim()) return message.trim();
-  }
-
-  if (record.is_error === true || record.type === "error") {
-    for (const key of ["result", "message", "subtype"]) {
-      const value = record[key];
-      if (typeof value === "string" && value.trim()) return value.trim();
-    }
-  }
-
-  return null;
-}
-
-/** Quote only the end of unstructured output, marking what was dropped. */
-function elideRawTail(raw: string): string {
-  return raw.length > MAX_RAW_TAIL_CHARS
-    ? `${RAW_TAIL_ELISION}${raw.slice(raw.length - MAX_RAW_TAIL_CHARS)}`
-    : raw;
-}
-
-interface StdoutFailure {
-  /** Error text the CLI itself authored in structured stdout events. */
-  structured: string;
-  /** Text worth reporting: the structured text, or a short raw tail. */
-  reported: string;
-}
-
-/**
- * Pull the CLI's own error text out of its stdout, which is JSONL when the run
- * got far enough to stream events and plain text otherwise. Falls back to a
- * bounded raw tail so the reported detail is never empty when stdout had
- * content.
- */
-function extractStdoutError(stdoutTail: string): StdoutFailure {
-  const messages: string[] = [];
-  for (const line of stdoutTail.split("\n")) {
-    if (!line.trim()) continue;
-    try {
-      const message = errorTextFromEvent(JSON.parse(line));
-      if (message) messages.push(message);
-    } catch {
-      // Not JSON: covered by the raw-tail fallback below.
-    }
-  }
-  const structured = messages.join("\n");
-  return {
-    structured,
-    reported: structured || elideRawTail(stdoutTail.trim()),
-  };
-}
-
-interface ExitFailure {
-  detail: string;
-  permanent: boolean;
-}
-
-/**
- * Describe a non-zero exit. `detail` reports everything both streams offered,
- * while `permanent` is decided only from text the CLI itself authored - stderr
- * and structured stdout error fields - so agent output that merely quotes a
- * permanent-failure phrase cannot abort an otherwise retryable run.
- */
-function describeExitFailure(
-  code: number | null,
-  stdoutTail: string,
-  stderr: string,
-): ExitFailure {
-  const trimmedStderr = stderr.trim();
-  const stdoutError = extractStdoutError(stdoutTail);
-  const segments = [trimmedStderr, stdoutError.reported].filter(Boolean);
-  return {
-    detail:
-      segments.length > 0
-        ? `claude exited with code ${code}: ${segments.join("\n")}`
-        : `claude exited with code ${code} and produced no output`,
-    permanent: isPermanentClaudeError(
-      [trimmedStderr, stdoutError.structured].filter(Boolean).join("\n"),
-    ),
-  };
-}
-
 export class ClaudeAgent implements Agent {
   name = "claude";
 
@@ -375,7 +275,7 @@ export class ClaudeAgent implements Agent {
       });
 
       child.stdout!.on("data", (data: Buffer) => {
-        stdoutTail = appendBoundedTail(stdoutTail, data.toString());
+        stdoutTail = appendExitOutputTail(stdoutTail, data.toString());
       });
 
       child.on("error", (err) => {
@@ -501,9 +401,14 @@ export class ClaudeAgent implements Agent {
         }
         logStream?.end();
         if (code !== 0 && !closedAfterFinalCleanup) {
-          const failure = describeExitFailure(code, stdoutTail, stderr);
+          const failure = describeChildProcessExit(
+            "claude",
+            code,
+            stdoutTail,
+            stderr,
+          );
           reject(
-            failure.permanent
+            isPermanentClaudeError(failure.errorOutput)
               ? new PermanentAgentError(
                   "claude credit balance too low - see gnhf.log",
                   failure.detail,

src/core/agents/codex.test.ts

@@ -203,4 +203,21 @@ describe("CodexAgent", () => {
     );
     expect(proc.kill).not.toHaveBeenCalled();
   });
+
+  it("surfaces a structured error emitted on stdout after a non-zero exit", async () => {
+    const proc = createMockProcess();
+    mockSpawn.mockReturnValue(proc);
+    const agent = new CodexAgent("/tmp/schema.json");
+
+    const promise = agent.run("test prompt", "/work/dir");
+    proc.stdout.emit(
+      "data",
+      Buffer.from('{"type":"error","error":{"message":"login required"}}\n'),
+    );
+    proc.emit("close", 1);
+
+    await expect(promise).rejects.toThrow(
+      "codex exited with code 1: login required",
+    );
+  });
 });

src/core/agents/copilot.test.ts

@@ -332,4 +332,18 @@ describe("CopilotAgent", () => {
 
     await expect(promise).rejects.toThrow("Failed to parse copilot output");
   });
+
+  it("surfaces a structured error emitted on stdout after a non-zero exit", async () => {
+    const proc = createMockProcess();
+    mockSpawn.mockReturnValue(proc);
+    const agent = new CopilotAgent();
+
+    const promise = agent.run("test prompt", "/work/dir");
+    emitJson(proc, { type: "error", error: { message: "login required" } });
+    proc.emit("close", 1);
+
+    await expect(promise).rejects.toThrow(
+      "copilot exited with code 1: login required",
+    );
+  });
 });

src/core/agents/pi.test.ts

@@ -509,4 +509,18 @@ describe("PiAgent", () => {
 
     await expect(promise).rejects.toThrow("pi exited with code 2: bad things");
   });
+
+  it("surfaces a structured error emitted on stdout after a non-zero exit", async () => {
+    const proc = createMockProcess();
+    mockSpawn.mockReturnValue(proc);
+    const agent = new PiAgent();
+
+    const promise = agent.run("test prompt", "/work/dir");
+    emitJson(proc, { type: "error", error: { message: "login required" } });
+    proc.emit("close", 1);
+
+    await expect(promise).rejects.toThrow(
+      "pi exited with code 1: login required",
+    );
+  });
 });

src/core/agents/stream-utils.test.ts

@@ -8,9 +8,11 @@ import {
 
 function createMockChild() {
   const child = new EventEmitter() as EventEmitter & {
+    stdout: EventEmitter;
     stderr: EventEmitter;
     kill: ReturnType<typeof vi.fn>;
   };
+  child.stdout = new EventEmitter();
   child.stderr = new EventEmitter();
   child.kill = vi.fn();
   return child;
@@ -93,6 +95,35 @@ describe("setupChildProcessHandlers", () => {
     );
   });
 
+  it("rejects with a structured stdout error when stderr is empty", () => {
+    const child = createMockChild();
+    const reject = vi.fn();
+
+    setupChildProcessHandlers(child as never, "codex", null, reject, vi.fn());
+
+    child.stdout.emit(
+      "data",
+      Buffer.from('{"type":"error","error":{"message":"login required"}}\n'),
+    );
+    child.emit("close", 1);
+
+    expect(reject).toHaveBeenCalledWith(
+      new Error("codex exited with code 1: login required"),
+    );
+  });
+
+  it("says so when a non-zero exit produced no output", () => {
+    const child = createMockChild();
+    const reject = vi.fn();
+
+    setupChildProcessHandlers(child as never, "pi", null, reject, vi.fn());
+    child.emit("close", 1);
+
+    expect(reject).toHaveBeenCalledWith(
+      new Error("pi exited with code 1 and produced no output"),
+    );
+  });
+
   it("wraps spawn errors and resolves successful exits through the success callback", () => {
     const child = createMockChild();
     const reject = vi.fn();

src/core/agents/stream-utils.ts

@@ -2,8 +2,105 @@ import type { ChildProcess } from "node:child_process";
 import type { Readable } from "node:stream";
 import type { WriteStream } from "node:fs";
 
+/** Upper bound on the stdout tail kept for non-zero-exit error reporting. */
+const MAX_EXIT_OUTPUT_CHARS = 4_000;
 /**
- * Wire stderr collection, spawn-error handling, and the common close-handler
+ * Tighter bound on unstructured stdout quoted back in the failure detail: that
+ * text lands in notes.md and is replayed in every later iteration prompt.
+ */
+const MAX_RAW_TAIL_CHARS = 400;
+const RAW_TAIL_ELISION = "[...truncated, full output in the iteration log] ";
+
+/** Keep only the end of a stream so long-running processes stay bounded. */
+export function appendExitOutputTail(existing: string, chunk: string): string {
+  const combined = existing + chunk;
+  return combined.length > MAX_EXIT_OUTPUT_CHARS
+    ? combined.slice(combined.length - MAX_EXIT_OUTPUT_CHARS)
+    : combined;
+}
+
+function errorTextFromEvent(event: unknown): string | null {
+  if (!event || typeof event !== "object") return null;
+  const record = event as Record<string, unknown>;
+
+  const error = record.error;
+  if (typeof error === "string" && error.trim()) return error.trim();
+  if (error && typeof error === "object") {
+    const message = (error as Record<string, unknown>).message;
+    if (typeof message === "string" && message.trim()) return message.trim();
+  }
+
+  if (record.is_error === true || record.type === "error") {
+    for (const key of ["result", "message", "subtype"]) {
+      const value = record[key];
+      if (typeof value === "string" && value.trim()) return value.trim();
+    }
+  }
+
+  return null;
+}
+
+function elideRawTail(raw: string): string {
+  return raw.length > MAX_RAW_TAIL_CHARS
+    ? `${RAW_TAIL_ELISION}${raw.slice(raw.length - MAX_RAW_TAIL_CHARS)}`
+    : raw;
+}
+
+interface StdoutFailure {
+  structured: string;
+  reported: string;
+}
+
+function extractStdoutError(stdoutTail: string): StdoutFailure {
+  const messages: string[] = [];
+  for (const line of stdoutTail.split("\n")) {
+    if (!line.trim()) continue;
+    try {
+      const message = errorTextFromEvent(JSON.parse(line));
+      if (message) messages.push(message);
+    } catch {
+      // Not JSON: covered by the raw-tail fallback below.
+    }
+  }
+  const structured = messages.join("\n");
+  return {
+    structured,
+    reported: structured || elideRawTail(stdoutTail.trim()),
+  };
+}
+
+export interface ChildProcessExitFailure {
+  detail: string;
+  /** CLI-authored error text suitable for permanent-error classification. */
+  errorOutput: string;
+}
+
+/**
+ * Describe a non-zero exit. The detail reports both streams, while
+ * `errorOutput` excludes unstructured stdout that may merely quote an error.
+ */
+export function describeChildProcessExit(
+  agentName: string,
+  code: number | null,
+  stdoutTail: string,
+  stderr: string,
+): ChildProcessExitFailure {
+  const trimmedStderr = stderr.trim();
+  const stdoutError = extractStdoutError(stdoutTail);
+  const segments = [trimmedStderr, stdoutError.reported].filter(Boolean);
+  return {
+    detail:
+      segments.length > 0
+        ? `${agentName} exited with code ${code}: ${segments.join("\n")}`
+        : `${agentName} exited with code ${code} and produced no output`,
+    errorOutput: [trimmedStderr, stdoutError.structured]
+      .filter(Boolean)
+      .join("\n"),
+  };
+}
+
+/**
+ * Wire output collection, spawn-error handling, and the common close-handler
  * prefix (logStream.end + non-zero exit code rejection) for a child process.
  * Calls `onSuccess` only when the process exits with code 0.
  */
@@ -15,6 +112,11 @@ export function setupChildProcessHandlers(
   onSuccess: () => void,
 ): void {
   let stderr = "";
+  let stdoutTail = "";
+
+  child.stdout!.on("data", (data: Buffer) => {
+    stdoutTail = appendExitOutputTail(stdoutTail, data.toString());
+  });
 
   child.stderr!.on("data", (data: Buffer) => {
     stderr += data.toString();
@@ -27,7 +129,13 @@ export function setupChildProcessHandlers(
   child.on("close", (code) => {
     logStream?.end();
     if (code !== 0) {
-      reject(new Error(`${agentName} exited with code ${code}: ${stderr}`));
+      const failure = describeChildProcessExit(
+        agentName,
+        code,
+        stdoutTail,
+        stderr,
+      );
+      reject(new Error(failure.detail));
       return;
     }
     onSuccess();