diff --git a/e2e/e2e.test.ts b/e2e/e2e.test.ts index c66ab743..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, }; } @@ -399,6 +403,68 @@ 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/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/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" %* diff --git a/src/core/agents/claude.test.ts b/src/core/agents/claude.test.ts index 85df836f..556876cf 100644 --- a/src/core/agents/claude.test.ts +++ b/src/core/agents/claude.test.ts @@ -935,6 +935,155 @@ 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).toContain("[...truncated"); + expect(message.length).toBeLessThan(600); + }); + + 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("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 7c748977..1d3efe86 100644 --- a/src/core/agents/claude.ts +++ b/src/core/agents/claude.ts @@ -14,6 +14,14 @@ 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; +/** + * 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"; @@ -192,8 +200,105 @@ 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; +} + +/** 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 { @@ -252,6 +357,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 +374,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,14 +501,14 @@ export class ClaudeAgent implements Agent { } logStream?.end(); if (code !== 0 && !closedAfterFinalCleanup) { - const detail = `claude exited with code ${code}: ${stderr}`; + const failure = describeExitFailure(code, stdoutTail, stderr); reject( - isPermanentClaudeError(stderr) + failure.permanent ? new PermanentAgentError( "claude credit balance too low - see gnhf.log", - detail, + failure.detail, ) - : new Error(detail), + : new Error(failure.detail), ); return; }