From 2aa5f0046676ccdbcdbba961aa36afe3b3f5535d Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sun, 9 Aug 2026 00:18:43 -0700 Subject: [PATCH 1/4] fix(claude): surface real error text on non-zero exit The claude adapter built its failure detail from stderr alone, so a CLI that reports its error on stdout (plain text or a structured JSON result event) degraded to a bare `claude exited with code 1: ` with nothing after the colon. Permanent-error classification read the same stderr-only string, so a low credit balance reported on stdout burned retries. Capture a bounded stdout tail, extract the CLI's own error text from it when the output is JSON, classify permanence against the combined detail, and say the CLI produced no output when both streams are empty. Closes #157 --- src/core/agents/claude.test.ts | 99 ++++++++++++++++++++++++++++++++++ src/core/agents/claude.ts | 76 ++++++++++++++++++++++++-- 2 files changed, 171 insertions(+), 4 deletions(-) diff --git a/src/core/agents/claude.test.ts b/src/core/agents/claude.test.ts index 85df836f..c14492a6 100644 --- a/src/core/agents/claude.test.ts +++ b/src/core/agents/claude.test.ts @@ -935,6 +935,105 @@ describe("ClaudeAgent", () => { ); }); + it("surfaces a structured stdout error when stderr is empty", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + emitLine(proc, { + type: "result", + subtype: "error_during_execution", + is_error: true, + result: "Invalid model name: claude-nonexistent-5", + }); + proc.emit("close", 1); + + await expect(promise).rejects.toThrow( + "claude exited with code 1: Invalid model name: claude-nonexistent-5", + ); + }); + + it("surfaces plain-text stdout output when stderr is empty", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + proc.stdout.emit("data", Buffer.from("Invalid API key - please run /login")); + proc.emit("close", 1); + + await expect(promise).rejects.toThrow( + "claude exited with code 1: Invalid API key - please run /login", + ); + }); + + it("reports both streams when stderr and stdout carry output", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + proc.stderr.emit("data", Buffer.from("something broke")); + emitLine(proc, { type: "error", error: { message: "rate limit exceeded" } }); + proc.emit("close", 1); + + await expect(promise).rejects.toThrow( + "claude exited with code 1: something broke\nrate limit exceeded", + ); + }); + + it("bounds the stdout tail included in the failure detail", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + proc.stdout.emit("data", Buffer.from("x".repeat(10_000) + "tail marker")); + proc.emit("close", 1); + + const message = await promise.then( + () => "", + (err: Error) => err.message, + ); + expect(message).toContain("tail marker"); + expect(message.length).toBeLessThan(5_000); + }); + + it("says so when a non-zero exit produced no output at all", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + proc.emit("close", 1); + + await expect(promise).rejects.toThrow( + "claude exited with code 1 and produced no output", + ); + }); + + it("marks a low credit balance reported on stdout as permanent", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + emitLine(proc, { + type: "result", + subtype: "error_during_execution", + is_error: true, + result: "Credit balance is too low to access Claude Code", + }); + proc.emit("close", 1); + + await expect(promise).rejects.toBeInstanceOf(PermanentAgentError); + await expect(promise).rejects.toMatchObject({ + detail: + "claude exited with code 1: Credit balance is too low to access Claude Code", + }); + }); + it("marks low credit balance exits as permanent", async () => { const proc = createMockProcess(); mockSpawn.mockReturnValue(proc); diff --git a/src/core/agents/claude.ts b/src/core/agents/claude.ts index 7c748977..b5112834 100644 --- a/src/core/agents/claude.ts +++ b/src/core/agents/claude.ts @@ -14,6 +14,8 @@ import { shutdownChildProcess } from "./managed-process.js"; import { 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; interface ClaudeAssistantEvent { type: "assistant"; @@ -192,8 +194,69 @@ function extendsUsage(next: TokenUsage, previous: TokenUsage): boolean { ); } -function isPermanentClaudeError(stderr: string): boolean { - return /credit balance\s+is\s+too\s+low/i.test(stderr); +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; + + const error = record.error; + if (typeof error === "string" && error.trim()) return error.trim(); + if (error && typeof error === "object") { + const message = (error as Record).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; +} + +/** + * 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 the + * raw tail so the reported detail is never empty when stdout had content. + */ +function extractStdoutError(stdoutTail: string): string { + 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. + } + } + return messages.length > 0 ? messages.join("\n") : stdoutTail.trim(); +} + +function formatExitFailure( + code: number | null, + stdoutTail: string, + stderr: string, +): string { + const segments = [stderr.trim(), extractStdoutError(stdoutTail)].filter( + Boolean, + ); + return segments.length > 0 + ? `claude exited with code ${code}: ${segments.join("\n")}` + : `claude exited with code ${code} and produced no output`; } export class ClaudeAgent implements Agent { @@ -252,6 +315,7 @@ export class ClaudeAgent implements Agent { let finalResultCleanupTimer: ReturnType | null = null; let closedAfterFinalCleanup = false; let stderr = ""; + let stdoutTail = ""; const cumulative: TokenUsage = { inputTokens: 0, outputTokens: 0, @@ -268,6 +332,10 @@ export class ClaudeAgent implements Agent { stderr += data.toString(); }); + child.stdout!.on("data", (data: Buffer) => { + stdoutTail = appendBoundedTail(stdoutTail, data.toString()); + }); + child.on("error", (err) => { reject(new Error(`Failed to spawn claude: ${err.message}`)); }); @@ -391,9 +459,9 @@ export class ClaudeAgent implements Agent { } logStream?.end(); if (code !== 0 && !closedAfterFinalCleanup) { - const detail = `claude exited with code ${code}: ${stderr}`; + const detail = formatExitFailure(code, stdoutTail, stderr); reject( - isPermanentClaudeError(stderr) + isPermanentClaudeError(detail) ? new PermanentAgentError( "claude credit balance too low - see gnhf.log", detail, From 050d4aa29b6e732fe245650a0d6e9a4064a009f8 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sun, 9 Aug 2026 02:27:28 -0700 Subject: [PATCH 2/4] no-mistakes(review): narrow claude permanent-error scope, bound raw tail, add e2e --- e2e/e2e.test.ts | 64 ++++++++++++++++++++++++++- e2e/fixtures/claude | 4 ++ e2e/fixtures/claude.cmd | 2 + e2e/fixtures/mock-claude-cli.mjs | 27 ++++++++++++ src/core/agents/claude.test.ts | 56 ++++++++++++++++++++++-- src/core/agents/claude.ts | 74 +++++++++++++++++++++++++------- 6 files changed, 206 insertions(+), 21 deletions(-) create mode 100755 e2e/fixtures/claude create mode 100644 e2e/fixtures/claude.cmd create mode 100755 e2e/fixtures/mock-claude-cli.mjs diff --git a/e2e/e2e.test.ts b/e2e/e2e.test.ts index c66ab743..a392d9d3 100644 --- a/e2e/e2e.test.ts +++ b/e2e/e2e.test.ts @@ -383,8 +383,7 @@ describe("gnhf e2e", () => { ); expect(agentRunErrorEntry).toBeDefined(); const agentError = agentRunErrorEntry?.error as - | { message?: string } - | undefined; + { message?: string } | undefined; expect(agentError?.message).toContain("OpenCode provider overloaded"); expect(agentError?.message).not.toContain( "Failed to parse opencode output", @@ -399,6 +398,67 @@ describe("gnhf e2e", () => { expect(iterationEnd?.success).toBe(false); }, 30_000); + it.each([ + { + label: "carried on stdout", + mode: "stdout-error", + expected: + "claude exited with code 1: Invalid model name: claude-nonexistent-5", + }, + { + label: "with both streams empty", + mode: "no-output", + expected: "claude exited with code 1 and produced no output", + }, + ])( + "surfaces the claude CLI's own failure text $label", + async ({ mode, expected }) => { + const cwd = createRepo(); + tempDirs.push(cwd); + const logDir = mkdtempSync(join(tmpdir(), "gnhf-e2e-logs-")); + tempDirs.push(logDir); + const mockLogPath = join(logDir, "mock-opencode.jsonl"); + + const result = await runCli( + cwd, + [ + "break the build", + "--agent", + "claude", + "--max-iterations", + "1", + "--prevent-sleep", + "off", + ], + { + env: { + ...createTestEnv(mockLogPath, tempDirs), + GNHF_MOCK_CLAUDE_MODE: mode, + }, + }, + ); + + expect(result.code).toBe(0); + + const debugLogPath = findRunLogPath(cwd); + const agentRunErrorEntry = readJsonLines(debugLogPath).find( + (entry) => entry.event === "agent:run:error", + ); + expect(agentRunErrorEntry).toBeDefined(); + const agentError = agentRunErrorEntry?.error as + { message?: string } | undefined; + expect(agentError?.message).toBe(expected); + + // The morning-after trace: notes.md is what the user actually reads. + const notes = readFileSync( + join(dirname(debugLogPath), "notes.md"), + "utf-8", + ); + expect(notes).toContain(`[ERROR] ${expected}`); + }, + 30_000, + ); + it("reads the objective from stdin", async () => { const cwd = createRepo(); tempDirs.push(cwd); diff --git a/e2e/fixtures/claude b/e2e/fixtures/claude new file mode 100755 index 00000000..0d06b7c2 --- /dev/null +++ b/e2e/fixtures/claude @@ -0,0 +1,4 @@ +#!/usr/bin/env sh + +SCRIPT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +exec node "$SCRIPT_DIR/mock-claude-cli.mjs" "$@" diff --git a/e2e/fixtures/claude.cmd b/e2e/fixtures/claude.cmd new file mode 100644 index 00000000..7e3f278d --- /dev/null +++ b/e2e/fixtures/claude.cmd @@ -0,0 +1,2 @@ +@echo off +node "%~dp0\mock-claude-cli.mjs" %* diff --git a/e2e/fixtures/mock-claude-cli.mjs b/e2e/fixtures/mock-claude-cli.mjs new file mode 100755 index 00000000..f4acfadb --- /dev/null +++ b/e2e/fixtures/mock-claude-cli.mjs @@ -0,0 +1,27 @@ +#!/usr/bin/env node + +// Stands in for the `claude` CLI on a failing run. The failure shape is picked +// with GNHF_MOCK_CLAUDE_MODE so one fixture covers every stream combination. + +import process from "node:process"; + +const mode = process.env.GNHF_MOCK_CLAUDE_MODE ?? "stdout-error"; + +if (mode === "no-output") { + process.exit(1); +} + +if (mode === "stderr-error") { + process.stderr.write("Invalid API key - please run /login\n"); + process.exit(1); +} + +process.stdout.write( + `${JSON.stringify({ + type: "result", + subtype: "error_during_execution", + is_error: true, + result: "Invalid model name: claude-nonexistent-5", + })}\n`, +); +process.exit(1); diff --git a/src/core/agents/claude.test.ts b/src/core/agents/claude.test.ts index c14492a6..556876cf 100644 --- a/src/core/agents/claude.test.ts +++ b/src/core/agents/claude.test.ts @@ -960,7 +960,10 @@ describe("ClaudeAgent", () => { const promise = agent.run("prompt", "/cwd"); - proc.stdout.emit("data", Buffer.from("Invalid API key - please run /login")); + proc.stdout.emit( + "data", + Buffer.from("Invalid API key - please run /login"), + ); proc.emit("close", 1); await expect(promise).rejects.toThrow( @@ -975,7 +978,10 @@ describe("ClaudeAgent", () => { const promise = agent.run("prompt", "/cwd"); proc.stderr.emit("data", Buffer.from("something broke")); - emitLine(proc, { type: "error", error: { message: "rate limit exceeded" } }); + emitLine(proc, { + type: "error", + error: { message: "rate limit exceeded" }, + }); proc.emit("close", 1); await expect(promise).rejects.toThrow( @@ -997,7 +1003,8 @@ describe("ClaudeAgent", () => { (err: Error) => err.message, ); expect(message).toContain("tail marker"); - expect(message.length).toBeLessThan(5_000); + expect(message).toContain("[...truncated"); + expect(message.length).toBeLessThan(600); }); it("says so when a non-zero exit produced no output at all", async () => { @@ -1034,6 +1041,49 @@ describe("ClaudeAgent", () => { }); }); + it("keeps a run retryable when only agent output quotes a permanent failure", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + emitLine(proc, { + type: "assistant", + message: { + id: "msg-1", + usage: { input_tokens: 1, output_tokens: 1 }, + content: [ + { + type: "text", + text: "The docs say 'credit balance is too low' aborts the run.", + }, + ], + }, + }); + proc.emit("close", 1); + + await expect(promise).rejects.not.toBeInstanceOf(PermanentAgentError); + await expect(promise).rejects.toThrow("claude exited with code 1:"); + }); + + it("keeps a run retryable when unparseable stdout quotes a permanent failure", async () => { + const proc = createMockProcess(); + mockSpawn.mockReturnValue(proc); + + const promise = agent.run("prompt", "/cwd"); + + proc.stdout.emit( + "data", + Buffer.from("grep: README.md: credit balance is too low"), + ); + proc.emit("close", 1); + + await expect(promise).rejects.not.toBeInstanceOf(PermanentAgentError); + await expect(promise).rejects.toThrow( + "claude exited with code 1: grep: README.md: credit balance is too low", + ); + }); + it("marks low credit balance exits as permanent", async () => { const proc = createMockProcess(); mockSpawn.mockReturnValue(proc); diff --git a/src/core/agents/claude.ts b/src/core/agents/claude.ts index b5112834..1d3efe86 100644 --- a/src/core/agents/claude.ts +++ b/src/core/agents/claude.ts @@ -16,6 +16,12 @@ import { 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"; @@ -227,12 +233,27 @@ function errorTextFromEvent(event: unknown): string | null { 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 the - * raw tail so the reported detail is never empty when stdout had content. + * 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): string { +function extractStdoutError(stdoutTail: string): StdoutFailure { const messages: string[] = []; for (const line of stdoutTail.split("\n")) { if (!line.trim()) continue; @@ -243,20 +264,41 @@ function extractStdoutError(stdoutTail: string): string { // Not JSON: covered by the raw-tail fallback below. } } - return messages.length > 0 ? messages.join("\n") : stdoutTail.trim(); + const structured = messages.join("\n"); + return { + structured, + reported: structured || elideRawTail(stdoutTail.trim()), + }; } -function formatExitFailure( +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, -): string { - const segments = [stderr.trim(), extractStdoutError(stdoutTail)].filter( - Boolean, - ); - return segments.length > 0 - ? `claude exited with code ${code}: ${segments.join("\n")}` - : `claude exited with code ${code} and produced no output`; +): 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 { @@ -459,14 +501,14 @@ export class ClaudeAgent implements Agent { } logStream?.end(); if (code !== 0 && !closedAfterFinalCleanup) { - const detail = formatExitFailure(code, stdoutTail, stderr); + const failure = describeExitFailure(code, stdoutTail, stderr); reject( - isPermanentClaudeError(detail) + failure.permanent ? new PermanentAgentError( "claude credit balance too low - see gnhf.log", - detail, + failure.detail, ) - : new Error(detail), + : new Error(failure.detail), ); return; } From c48faf7aa7390fc2e5ad872bd47c09fa1343177a Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sun, 9 Aug 2026 06:55:34 -0700 Subject: [PATCH 3/4] no-mistakes(document): Format Claude failure e2e coverage --- e2e/e2e.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e/e2e.test.ts b/e2e/e2e.test.ts index a392d9d3..23a4de76 100644 --- a/e2e/e2e.test.ts +++ b/e2e/e2e.test.ts @@ -383,7 +383,8 @@ describe("gnhf e2e", () => { ); expect(agentRunErrorEntry).toBeDefined(); const agentError = agentRunErrorEntry?.error as - { message?: string } | undefined; + | { message?: string } + | undefined; expect(agentError?.message).toContain("OpenCode provider overloaded"); expect(agentError?.message).not.toContain( "Failed to parse opencode output", @@ -446,7 +447,8 @@ describe("gnhf e2e", () => { ); expect(agentRunErrorEntry).toBeDefined(); const agentError = agentRunErrorEntry?.error as - { message?: string } | undefined; + | { message?: string } + | undefined; expect(agentError?.message).toBe(expected); // The morning-after trace: notes.md is what the user actually reads. From fd1edd2b8dd3507b64386ec4c900516360ff9790 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sun, 9 Aug 2026 07:17:34 -0700 Subject: [PATCH 4/4] no-mistakes: apply CI fixes --- e2e/e2e.test.ts | 6 +++++- e2e/fixtures/claude.cmd | 2 -- e2e/fixtures/windows/claude.cmd | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) delete mode 100644 e2e/fixtures/claude.cmd create mode 100644 e2e/fixtures/windows/claude.cmd diff --git a/e2e/e2e.test.ts b/e2e/e2e.test.ts index 23a4de76..434f5d22 100644 --- a/e2e/e2e.test.ts +++ b/e2e/e2e.test.ts @@ -16,6 +16,7 @@ import { afterEach, describe, expect, it } from "vitest"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const distCliPath = join(repoRoot, "dist", "cli.mjs"); const fixtureBinDir = join(repoRoot, "e2e", "fixtures"); +const windowsFixtureBinDir = join(fixtureBinDir, "windows"); // Empty gitconfig pointed at by GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM so the // developer's real ~/.gitconfig (which may enable commit.gpgsign, set a @@ -165,7 +166,10 @@ function createTestEnv( ...sanitizedGitEnv, HOME: home, USERPROFILE: home, - PATH: `${fixtureBinDir}${process.platform === "win32" ? ";" : ":"}${process.env.PATH ?? ""}`, + PATH: + process.platform === "win32" + ? `${windowsFixtureBinDir};${fixtureBinDir};${process.env.PATH ?? ""}` + : `${fixtureBinDir}:${process.env.PATH ?? ""}`, GNHF_MOCK_OPENCODE_LOG_PATH: mockLogPath, }; } diff --git a/e2e/fixtures/claude.cmd b/e2e/fixtures/claude.cmd deleted file mode 100644 index 7e3f278d..00000000 --- a/e2e/fixtures/claude.cmd +++ /dev/null @@ -1,2 +0,0 @@ -@echo off -node "%~dp0\mock-claude-cli.mjs" %* diff --git a/e2e/fixtures/windows/claude.cmd b/e2e/fixtures/windows/claude.cmd new file mode 100644 index 00000000..396a13fe --- /dev/null +++ b/e2e/fixtures/windows/claude.cmd @@ -0,0 +1,2 @@ +@echo off +node "%~dp0\..\mock-claude-cli.mjs" %*