From ceec900dc45bce77b27214cb671abfd07d10c2f6 Mon Sep 17 00:00:00 2001 From: Cole Tebou Date: Sun, 17 May 2026 20:26:58 +0000 Subject: [PATCH] fix(acpx-provider): surface stopReason as typed error codes Read acpx's terminal JSON-RPC result envelope (`{id, result: {stopReason, ...}}`) inside `extractAcpxJson` and map non-`end_turn` reasons to typed `ClawpatchError` codes: cancelled -> agent-cancelled (exit 6) refusal -> agent-refused (exit 7) max_tokens -> agent-truncated (exit 8) max_turns_exceeded -> agent-truncated (exit 8) unknown reason -> agent-cancelled (exit 8) defensively Previously, prompts that ended in `cancelled` / `refusal` / `max_tokens` emitted no `agent_message_chunk`, so they fell into the same `code: "malformed-output"` bucket as a real envelope-shape regression (see 20260517T184420-d52a72: four parser errors of three different shapes, all opaque `malformed-output`). On-call had to read raw NDJSON to triage. Now `malformed-output` keeps its narrow meaning: clawpatch could not parse a payload acpx claims is `end_turn`. The `end_turn`-with-no-chunks path also gets a clearer diagnostic noting the parser bug vs envelope break. Verified live against acpx 0.8.0 + claude-agent-acp 0.31.4. Terminal envelope shape: {"jsonrpc":"2.0","id":2,"result":{"stopReason":"end_turn","usage":{...}}} `ACPX_TESTED_VERSIONS` stays at `^0.8.0`; the stopReason envelope is present in this version. --- CHANGELOG.md | 1 + src/provider.test.ts | 89 ++++++++++++++++++++++++++++++++++++++++++++ src/provider.ts | 64 ++++++++++++++++++++++++++++++- 3 files changed, 152 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 570182c..88257bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Fixed first-time `clawpatch open-pr` branch creation to start from the recorded patch base. - Fixed command execution so providers that exit before reading stdin do not surface benign `EPIPE` errors. - Fixed `clawpatch ci --since` empty-review output so it reports `reviewed: 0`. +- Fixed acpx provider error reporting by reading the terminal `result.stopReason` envelope and surfacing non-`end_turn` reasons as typed `ClawpatchError` codes (`agent-cancelled`, `agent-refused`, `agent-truncated`) instead of opaque `malformed-output`. - Improved OpenCode malformed JSON diagnostics with output length, event kinds, and a bounded preview, thanks @rohitjavvadi. - Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi. - Fixed Bun package-manager detection to recognize the text `bun.lock` lockfile, thanks @austinm911. diff --git a/src/provider.test.ts b/src/provider.test.ts index 239185e..b261879 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -60,6 +60,30 @@ function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"); } +function terminalEnvelope(stopReason: string, id = 2): string { + return JSON.stringify({ + jsonrpc: "2.0", + id, + result: { stopReason, usage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 } }, + }); +} + +function expectStopReasonError( + fn: () => unknown, + expected: { code: string; exitCode: number; stopReason: string }, +): void { + try { + fn(); + } catch (err) { + expect(err).toBeInstanceOf(ClawpatchError); + expect((err as ClawpatchError).code).toBe(expected.code); + expect((err as ClawpatchError).exitCode).toBe(expected.exitCode); + expect((err as Error).message).toContain(`stopReason="${expected.stopReason}"`); + return; + } + throw new Error(`expected ClawpatchError with code ${expected.code}`); +} + describe("extractJson", () => { it("parses strict JSON directly", () => { const input = '{"findings":[],"inspected":{"files":[],"symbols":[],"notes":[]}}'; @@ -398,6 +422,71 @@ describe("extractAcpxJson", () => { expect(extractAcpxJson(stdout)).toEqual({ ok: true }); }); + it("preserves end_turn happy path with message chunks", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"ok":'), + textChunk("agent_message_chunk", "true}"), + terminalEnvelope("end_turn"), + ].join("\n"); + + expect(extractAcpxJson(stdout)).toEqual({ ok: true }); + }); + + it("surfaces stopReason cancelled as agent-cancelled", () => { + const stdout = [ + updateEnvelope({ sessionUpdate: "usage_update", usage: { inputTokens: 1, outputTokens: 0 } }), + terminalEnvelope("cancelled"), + ].join("\n"); + + expectStopReasonError(() => extractAcpxJson(stdout), { + code: "agent-cancelled", + exitCode: 6, + stopReason: "cancelled", + }); + }); + + it("surfaces stopReason refusal as agent-refused", () => { + const stdout = terminalEnvelope("refusal"); + + expectStopReasonError(() => extractAcpxJson(stdout), { + code: "agent-refused", + exitCode: 7, + stopReason: "refusal", + }); + }); + + it("surfaces stopReason max_tokens as agent-truncated", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"partial":'), + terminalEnvelope("max_tokens"), + ].join("\n"); + + expectStopReasonError(() => extractAcpxJson(stdout), { + code: "agent-truncated", + exitCode: 8, + stopReason: "max_tokens", + }); + }); + + it("maps unknown stopReason defensively to agent-cancelled", () => { + const stdout = terminalEnvelope("future_reason_xyz"); + + expectStopReasonError(() => extractAcpxJson(stdout), { + code: "agent-cancelled", + exitCode: 8, + stopReason: "future_reason_xyz", + }); + }); + + it("falls back to current behavior with no terminal envelope", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"legacy":'), + textChunk("agent_message_chunk", "true}"), + ].join("\n"); + + expect(extractAcpxJson(stdout)).toEqual({ legacy: true }); + }); + it("survives a 256-line NDJSON fixture over 8KB", () => { const filler = Array.from({ length: 255 }, (_, idx) => updateEnvelope({ diff --git a/src/provider.ts b/src/provider.ts index 3bc387d..9561c6a 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -784,11 +784,40 @@ function buildAcpxPrompt(prompt: string, schema: object, permission: "read" | "a ); } +// Map acpx promptResult.stopReason -> ClawpatchError code/exit pair. +// `end_turn` is the only successful reason; everything else surfaces as a +// typed error so callers can distinguish cancellation / refusal / truncation +// from an actual envelope-shape regression. +// +// Source: acpx/src/runtime/engine/manager.ts emits the terminal JSON-RPC +// response `{"jsonrpc":"2.0","id":N,"result":{"stopReason":,...}}` +// for every `session/prompt`. Known reasons in acpx 0.8.0 / claude-agent-acp +// 0.31.4 are `end_turn | cancelled | refusal | max_tokens` (plus +// `max_turns_exceeded`, surfaced for the agent-driven turn loop). +const ACPX_STOP_REASON_CODES: Record = { + cancelled: "agent-cancelled", + refusal: "agent-refused", + max_tokens: "agent-truncated", + max_turns_exceeded: "agent-truncated", +}; +const ACPX_STOP_EXIT_CODES: Record = { + cancelled: 6, + refusal: 7, + max_tokens: 8, + max_turns_exceeded: 8, +}; + export function extractAcpxJson(stdout: string): unknown { const toolCandidates: string[] = []; const messageChunks: string[] = []; const thoughtChunks: string[] = []; const observedKinds = new Set(); + // Last-seen terminal JSON-RPC response envelope: `{id, result: {stopReason, ...}}`. + // acpx emits exactly one per `session/prompt` turn (see + // acpx/src/runtime/engine/manager.ts). If this is anything other than + // "end_turn" the agent is telling us the turn produced no answer, and we + // should surface a typed error instead of trying to parse chunks. + let terminalStopReason: string | undefined; for (const line of stdout.split("\n")) { const trimmed = line.trim(); if (trimmed.length === 0) { @@ -796,6 +825,8 @@ export function extractAcpxJson(stdout: string): unknown { } let env: { method?: string; + id?: unknown; + result?: { stopReason?: unknown }; params?: { update?: { sessionUpdate?: string; @@ -809,6 +840,17 @@ export function extractAcpxJson(stdout: string): unknown { } catch { continue; } + if ( + env !== null && + typeof env === "object" && + Object.prototype.hasOwnProperty.call(env, "id") && + env.result !== undefined && + env.result !== null && + typeof env.result === "object" && + typeof env.result.stopReason === "string" + ) { + terminalStopReason = env.result.stopReason; + } if (env.method !== "session/update") { continue; } @@ -833,15 +875,33 @@ export function extractAcpxJson(stdout: string): unknown { toolCandidates.push(update.output); } } + // Step 1: if acpx terminated the turn with anything other than end_turn, + // surface that directly. No chunk-parsing — the agent already told us + // there is no answer in this turn. + if (terminalStopReason !== undefined && terminalStopReason !== "end_turn") { + const code = ACPX_STOP_REASON_CODES[terminalStopReason] ?? "agent-cancelled"; + const exit = ACPX_STOP_EXIT_CODES[terminalStopReason] ?? 8; + throw new ClawpatchError( + `acpx prompt did not complete: stopReason="${terminalStopReason}". ` + + `Observed envelope kinds: [${[...observedKinds].join(", ")}].`, + exit, + code, + ); + } const candidates = [ ...(messageChunks.length > 0 ? [messageChunks.join("")] : []), ...toolCandidates.toReversed(), ...(thoughtChunks.length > 0 ? [thoughtChunks.join("")] : []), ]; if (candidates.length === 0) { + const stopReasonNote = + terminalStopReason === "end_turn" + ? `acpx reported stopReason=end_turn but emitted no message chunks. ` + + `This is a clawpatch parser bug or a prompt that produced only tool calls. ` + : ``; throw new ClawpatchError( - `acpx provider produced no extractable text. Observed envelope kinds: ` + - `[${[...observedKinds].join(", ")}]. ` + + `acpx provider produced no extractable text. ${stopReasonNote}` + + `Observed envelope kinds: [${[...observedKinds].join(", ")}]. ` + `acpx envelope shape may have changed since clawpatch was tested ` + `against ${ACPX_TESTED_VERSIONS}. Check the installed acpx version.`, 8,