diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c07264..b5075e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - Added Maven project mapping for root, nested, and multi-module Java/Kotlin projects with Spring role slices, Maven validation defaults, and `pom.xml` detection, thanks @julianshess. - Added a release-prep checklist for auditing changelog, package metadata, and dry-run package contents without publishing. - Improved bounded source grouping so large flat directories split repeated filename families like command, plugin, doctor, and runtime files into more coherent review slices. +- 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`, thanks @coletebou. - Improved OpenCode malformed JSON diagnostics with output length, event kinds, and a bounded preview, thanks @rohitjavvadi. - Fixed finding signatures so equivalent evidence remains stable across re-reviews, thanks @rohitjavvadi. - Fixed provider exit-code classification for stdout-only authentication and quota failures, thanks @rohitjavvadi. diff --git a/src/provider.test.ts b/src/provider.test.ts index 3c8db0c..c659693 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -103,6 +103,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":[]}}'; @@ -571,6 +595,81 @@ 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: 1, + stopReason: "cancelled", + }); + }); + + it("surfaces stopReason refusal as agent-refused", () => { + const stdout = terminalEnvelope("refusal"); + + expectStopReasonError(() => extractAcpxJson(stdout), { + code: "agent-refused", + exitCode: 1, + 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("surfaces stopReason max_turn_requests as agent-truncated", () => { + const stdout = terminalEnvelope("max_turn_requests"); + + expectStopReasonError(() => extractAcpxJson(stdout), { + code: "agent-truncated", + exitCode: 8, + stopReason: "max_turn_requests", + }); + }); + + 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 33c62de..6c3f6b5 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -1047,24 +1047,55 @@ 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 | max_turn_requests` +// (plus the older `max_turns_exceeded` spelling seen in agent-driven turn loops). +const ACPX_STOP_REASON_CODES: Record = { + cancelled: "agent-cancelled", + refusal: "agent-refused", + max_tokens: "agent-truncated", + max_turn_requests: "agent-truncated", + max_turns_exceeded: "agent-truncated", +}; +const ACPX_STOP_EXIT_CODES: Record = { + cancelled: 1, + refusal: 1, + max_tokens: 8, + max_turn_requests: 8, + max_turns_exceeded: 8, +}; + export function extractAcpxJson(stdout: string): unknown { - const { candidates, observedKinds } = acpxJsonCandidates(stdout, false); - return parseAcpxJsonCandidates(candidates, observedKinds, (output) => output); + const { candidates, observedKinds, terminalStopReason } = acpxJsonCandidates(stdout, false); + return parseAcpxJsonCandidates(candidates, observedKinds, terminalStopReason, (output) => output); } function parseAcpxJsonOutput(stdout: string, parseOutput: (output: unknown) => T): T { - const { candidates, observedKinds } = acpxJsonCandidates(stdout, true); - return parseAcpxJsonCandidates(candidates, observedKinds, parseOutput); + const { candidates, observedKinds, terminalStopReason } = acpxJsonCandidates(stdout, true); + return parseAcpxJsonCandidates(candidates, observedKinds, terminalStopReason, parseOutput); } function acpxJsonCandidates( stdout: string, retrySafe: boolean, -): { candidates: string[]; observedKinds: Set } { +): { candidates: string[]; observedKinds: Set; terminalStopReason: string | undefined } { 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) { @@ -1072,6 +1103,8 @@ function acpxJsonCandidates( } let env: { method?: string; + id?: unknown; + result?: { stopReason?: unknown }; params?: { update?: { sessionUpdate?: string; @@ -1085,6 +1118,17 @@ function acpxJsonCandidates( } 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; } @@ -1120,18 +1164,35 @@ function acpxJsonCandidates( ...toolCandidates.toReversed(), ...(thoughtChunks.length > 0 ? [thoughtChunks.join("")] : []), ]; - return { candidates, observedKinds }; + return { candidates, observedKinds, terminalStopReason }; } function parseAcpxJsonCandidates( candidates: string[], observedKinds: Set, + terminalStopReason: string | undefined, parseOutput: (output: unknown) => T, ): T { + 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, + ); + } + 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,