Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
99 changes: 99 additions & 0 deletions src/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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":[]}}';
Expand Down Expand Up @@ -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({
Expand Down
77 changes: 69 additions & 8 deletions src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1047,31 +1047,64 @@ 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":<reason>,...}}`
// 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<string, string> = {
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<string, number> = {
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<T>(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<string> } {
): { candidates: string[]; observedKinds: Set<string>; terminalStopReason: string | undefined } {
const toolCandidates: string[] = [];
const messageChunks: string[] = [];
const thoughtChunks: string[] = [];
const observedKinds = new Set<string>();
// 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) {
continue;
}
let env: {
method?: string;
id?: unknown;
result?: { stopReason?: unknown };
params?: {
update?: {
sessionUpdate?: string;
Expand All @@ -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;
}
Expand Down Expand Up @@ -1120,18 +1164,35 @@ function acpxJsonCandidates(
...toolCandidates.toReversed(),
...(thoughtChunks.length > 0 ? [thoughtChunks.join("")] : []),
];
return { candidates, observedKinds };
return { candidates, observedKinds, terminalStopReason };
}

function parseAcpxJsonCandidates<T>(
candidates: string[],
observedKinds: Set<string>,
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,
Expand Down