From ae405931015b58ca4784feafcfb188fa4b4c5b1e Mon Sep 17 00:00:00 2001 From: Cole Tebou Date: Sun, 17 May 2026 20:30:58 +0000 Subject: [PATCH 1/4] feat(review): retry transient acpx failures + pass --prompt-retries Two-layer retry for transient acpx review failures. Layer 1 (acpx): runAcpxJson now passes --prompt-retries (default 1) so the underlying agent can recover stream cuts and partial JSON internally without the whole feature failing. Override via CLAWPATCH_ACPX_PROMPT_RETRIES; 0 disables. Layer 2 (clawpatch): reviewFeature wraps provider.review() in a single retry that fires only on ClawpatchError with code === "malformed-output". Override via CLAWPATCH_REVIEW_RETRIES; 0 disables. Deterministic failures (provider-auth, unsupported-provider, agent-refused, agent-cancelled) and provider-failure (already covered by layer 1) are not retried at this layer. The retry is inside reviewFeature so the work-stealing loop in runReview does not double-claim the feature lock. Emits a "feature-retry" progress event between attempts. --- CHANGELOG.md | 1 + src/app.test.ts | 267 +++++++++++++++++++++++++++++++++++++++++++ src/app.ts | 71 +++++++++++- src/provider.test.ts | 111 +++++++++++++++++- src/provider.ts | 32 +++++- 5 files changed, 471 insertions(+), 11 deletions(-) create mode 100644 src/app.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 570182c..6ce5a6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Added `clawpatch ci` to initialize, map, review, write a report, and append a GitHub Actions step summary in one CI-friendly command. - Added `clawpatch open-pr --patch ` to turn an applied patch attempt into an explicit GitHub pull request. - Added review prompt provenance and budget accounting for included files, omitted files, prompt bytes, and approximate tokens. +- Added two-layer retry for transient acpx review failures: `runAcpxJson` now passes `--prompt-retries 1` (override via `CLAWPATCH_ACPX_PROMPT_RETRIES`) so the agent can recover stream cuts internally, and `reviewFeature` retries the whole acpx invocation once on `malformed-output` errors (override via `CLAWPATCH_REVIEW_RETRIES`). Deterministic failures (`provider-auth`, `unsupported-provider`, `agent-refused`, `agent-cancelled`) still fail fast. - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. - Fixed `clawpatch open-pr` so repositories without default-branch metadata use a dedicated patch branch and let GitHub choose the PR base. - Fixed `clawpatch open-pr` retries to push the recorded patch commit instead of any later local branch tip. diff --git a/src/app.test.ts b/src/app.test.ts new file mode 100644 index 0000000..7829cf7 --- /dev/null +++ b/src/app.test.ts @@ -0,0 +1,267 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { __testing as appTesting, AppContext } from "./app.js"; +import { ClawpatchError } from "./errors.js"; +import type { ReviewOutput } from "./types.js"; + +// eslint-disable-next-line no-underscore-dangle +const { isRetryableReviewError, reviewRetries, runProviderReviewWithRetry } = appTesting; + +const QUIET_CONTEXT: AppContext = { + root: "/tmp/test-root", + options: { + root: "/tmp/test-root", + json: false, + plain: false, + quiet: true, + verbose: false, + debug: false, + noColor: true, + noInput: true, + }, +}; + +function emptyReview(): ReviewOutput { + return { findings: [], inspected: { files: [], symbols: [], notes: ["ok"] } }; +} + +function withEnv(name: string, value: string | undefined, fn: () => void): void { + const previous = process.env[name]; + if (value === undefined) { + delete process.env[name]; + } else { + process.env[name] = value; + } + try { + fn(); + } finally { + if (previous === undefined) { + delete process.env[name]; + } else { + process.env[name] = previous; + } + } +} + +describe("isRetryableReviewError", () => { + it("returns true for malformed-output ClawpatchError", () => { + expect(isRetryableReviewError(new ClawpatchError("bad", 8, "malformed-output"))).toBe(true); + }); + + it("returns false for provider-auth", () => { + expect(isRetryableReviewError(new ClawpatchError("nope", 4, "provider-auth"))).toBe(false); + }); + + it("returns false for unsupported-provider", () => { + expect(isRetryableReviewError(new ClawpatchError("nope", 2, "unsupported-provider"))).toBe( + false, + ); + }); + + it("returns false for agent-refused", () => { + expect(isRetryableReviewError(new ClawpatchError("nope", 1, "agent-refused"))).toBe(false); + }); + + it("returns false for agent-cancelled", () => { + expect(isRetryableReviewError(new ClawpatchError("nope", 1, "agent-cancelled"))).toBe(false); + }); + + it("returns false for provider-failure (acpx-layer handles those)", () => { + expect(isRetryableReviewError(new ClawpatchError("nope", 1, "provider-failure"))).toBe(false); + }); + + it("returns false for plain Error", () => { + expect(isRetryableReviewError(new Error("oops"))).toBe(false); + }); +}); + +describe("reviewRetries", () => { + afterEach(() => { + delete process.env["CLAWPATCH_REVIEW_RETRIES"]; + }); + + it("defaults to 1", () => { + delete process.env["CLAWPATCH_REVIEW_RETRIES"]; + expect(reviewRetries()).toBe(1); + }); + + it("respects 0 (disables retry)", () => { + withEnv("CLAWPATCH_REVIEW_RETRIES", "0", () => { + expect(reviewRetries()).toBe(0); + }); + }); + + it("respects positive integer override", () => { + withEnv("CLAWPATCH_REVIEW_RETRIES", "2", () => { + expect(reviewRetries()).toBe(2); + }); + }); + + it("falls back to 1 on garbage input", () => { + withEnv("CLAWPATCH_REVIEW_RETRIES", "abc", () => { + expect(reviewRetries()).toBe(1); + }); + }); +}); + +function fakeProvider(reviewImpl: (...args: unknown[]) => Promise): { + name: string; + check: () => Promise; + map: () => Promise; + review: (...args: unknown[]) => Promise; + fix: () => Promise; + revalidate: () => Promise; +} { + return { + name: "fake", + async check(): Promise { + return "fake"; + }, + async map(): Promise { + throw new Error("not used"); + }, + review: reviewImpl, + async fix(): Promise { + throw new Error("not used"); + }, + async revalidate(): Promise { + throw new Error("not used"); + }, + }; +} + +describe("runProviderReviewWithRetry", () => { + afterEach(() => { + delete process.env["CLAWPATCH_REVIEW_RETRIES"]; + }); + + it("returns output on first try without retry", async () => { + delete process.env["CLAWPATCH_REVIEW_RETRIES"]; + const review = vi.fn().mockResolvedValue(emptyReview()); + const provider = fakeProvider(review); + const result = await runProviderReviewWithRetry({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + provider: provider as any, + root: "/tmp", + prompt: "hi", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: {} as any, + context: QUIET_CONTEXT, + featureId: "feat_x", + index: 0, + total: 1, + }); + expect(result.findings).toEqual([]); + expect(review).toHaveBeenCalledTimes(1); + }); + + it("retries once on malformed-output then succeeds", async () => { + delete process.env["CLAWPATCH_REVIEW_RETRIES"]; + const review = vi + .fn() + .mockRejectedValueOnce(new ClawpatchError("garbled", 8, "malformed-output")) + .mockResolvedValueOnce(emptyReview()); + const provider = fakeProvider(review); + const result = await runProviderReviewWithRetry({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + provider: provider as any, + root: "/tmp", + prompt: "hi", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: {} as any, + context: QUIET_CONTEXT, + featureId: "feat_x", + index: 0, + total: 1, + }); + expect(result.findings).toEqual([]); + expect(review).toHaveBeenCalledTimes(2); + }); + + it("does NOT retry on provider-auth", async () => { + delete process.env["CLAWPATCH_REVIEW_RETRIES"]; + const err = new ClawpatchError("auth", 4, "provider-auth"); + const review = vi.fn().mockRejectedValue(err); + const provider = fakeProvider(review); + await expect( + runProviderReviewWithRetry({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + provider: provider as any, + root: "/tmp", + prompt: "hi", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: {} as any, + context: QUIET_CONTEXT, + featureId: "feat_x", + index: 0, + total: 1, + }), + ).rejects.toBe(err); + expect(review).toHaveBeenCalledTimes(1); + }); + + it("does NOT retry on agent-cancelled", async () => { + delete process.env["CLAWPATCH_REVIEW_RETRIES"]; + const err = new ClawpatchError("cancel", 1, "agent-cancelled"); + const review = vi.fn().mockRejectedValue(err); + const provider = fakeProvider(review); + await expect( + runProviderReviewWithRetry({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + provider: provider as any, + root: "/tmp", + prompt: "hi", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: {} as any, + context: QUIET_CONTEXT, + featureId: "feat_x", + index: 0, + total: 1, + }), + ).rejects.toBe(err); + expect(review).toHaveBeenCalledTimes(1); + }); + + it("respects CLAWPATCH_REVIEW_RETRIES=0 (no retry, malformed-output fails on first attempt)", async () => { + process.env["CLAWPATCH_REVIEW_RETRIES"] = "0"; + const err = new ClawpatchError("garbled", 8, "malformed-output"); + const review = vi.fn().mockRejectedValue(err); + const provider = fakeProvider(review); + await expect( + runProviderReviewWithRetry({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + provider: provider as any, + root: "/tmp", + prompt: "hi", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: {} as any, + context: QUIET_CONTEXT, + featureId: "feat_x", + index: 0, + total: 1, + }), + ).rejects.toBe(err); + expect(review).toHaveBeenCalledTimes(1); + }); + + it("re-throws after maxAttempts when malformed-output persists", async () => { + process.env["CLAWPATCH_REVIEW_RETRIES"] = "1"; + const err = new ClawpatchError("garbled", 8, "malformed-output"); + const review = vi.fn().mockRejectedValue(err); + const provider = fakeProvider(review); + await expect( + runProviderReviewWithRetry({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + provider: provider as any, + root: "/tmp", + prompt: "hi", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: {} as any, + context: QUIET_CONTEXT, + featureId: "feat_x", + index: 0, + total: 1, + }), + ).rejects.toBe(err); + expect(review).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/app.ts b/src/app.ts index 05be074..7a48666 100644 --- a/src/app.ts +++ b/src/app.ts @@ -675,11 +675,16 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId mode, customPrompt, ); - const providerOutput = await provider.review( - loaded.root, - reviewPrompt.prompt, - providerOptions(config), - ); + const providerOutput = await runProviderReviewWithRetry({ + provider, + root: loaded.root, + prompt: reviewPrompt.prompt, + options: providerOptions(config), + context, + featureId: feature.featureId, + index, + total, + }); const reviewOutput = { ...providerOutput, findings: reviewFindingsForMode(providerOutput.findings, mode).slice( @@ -773,6 +778,55 @@ async function reviewFeature(options: ReviewFeatureOptions): Promise<{ findingId } } +type ReviewProvider = ReturnType; +type ProviderReviewOutput = Awaited>; + +async function runProviderReviewWithRetry(args: { + provider: ReviewProvider; + root: string; + prompt: string; + options: Parameters[2]; + context: AppContext; + featureId: string; + index: number; + total: number; +}): Promise { + const { provider, root, prompt, options, context, featureId, index, total } = args; + const maxAttempts = 1 + reviewRetries(); + let lastError: unknown; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + return await provider.review(root, prompt, options); + } catch (error: unknown) { + lastError = error; + if (!isRetryableReviewError(error) || attempt === maxAttempts) { + throw error; + } + emitProgress(context, "review", "feature-retry", { + index: index + 1, + total, + feature: featureId, + attempt, + reason: error instanceof ClawpatchError ? error.code : "unknown", + }); + } + } + throw lastError ?? new ClawpatchError("review retry exhausted", 1, "review-retry-exhausted"); +} + +function reviewRetries(): number { + const raw = process.env["CLAWPATCH_REVIEW_RETRIES"]; + if (raw === undefined) { + return 1; + } + const parsed = Number(raw); + return Number.isFinite(parsed) && parsed >= 0 ? Math.floor(parsed) : 1; +} + +function isRetryableReviewError(error: unknown): boolean { + return error instanceof ClawpatchError && error.code === "malformed-output"; +} + export async function revalidateCommand( context: AppContext, flags: Record, @@ -2031,3 +2085,10 @@ function stringFlag(flags: Record, name: string): stri const value = flags[name]; return typeof value === "string" ? value : undefined; } + +// eslint-disable-next-line no-underscore-dangle +export const __testing = { + isRetryableReviewError, + reviewRetries, + runProviderReviewWithRetry, +}; diff --git a/src/provider.test.ts b/src/provider.test.ts index 239185e..36b9429 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -6,9 +6,11 @@ import { revalidateOutputSchema, reviewOutputSchema } from "./types.js"; // eslint-disable-next-line no-underscore-dangle const { - addCodexSandboxArgs, - addCodexModelArgs, acpxFailureMessage, + acpxPromptRetries, + addCodexModelArgs, + addCodexSandboxArgs, + buildAcpxJsonArgs, codexFailureMessage, extractAcpxJson, extractOpencodeJson, @@ -18,6 +20,24 @@ const { providerJsonSchema, } = __testing; +function withEnv(name: string, value: string | undefined, fn: () => void): void { + const previous = process.env[name]; + if (value === undefined) { + delete process.env[name]; + } else { + process.env[name] = value; + } + try { + fn(); + } finally { + if (previous === undefined) { + delete process.env[name]; + } else { + process.env[name] = previous; + } + } +} + function updateEnvelope(update: object): string { return JSON.stringify({ jsonrpc: "2.0", @@ -569,3 +589,90 @@ describe("providerByName", () => { expect(providerByName("mock-fail").name).toBe("mock-fail"); }); }); + +describe("acpxPromptRetries", () => { + afterEach(() => { + delete process.env["CLAWPATCH_ACPX_PROMPT_RETRIES"]; + }); + + it("defaults to 1 when env var is unset", () => { + delete process.env["CLAWPATCH_ACPX_PROMPT_RETRIES"]; + expect(acpxPromptRetries()).toBe(1); + }); + + it("respects a numeric env override", () => { + withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "3", () => { + expect(acpxPromptRetries()).toBe(3); + }); + }); + + it("treats 0 as a valid override (disables retries)", () => { + withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "0", () => { + expect(acpxPromptRetries()).toBe(0); + }); + }); + + it("falls back to 1 on invalid input", () => { + withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "not-a-number", () => { + expect(acpxPromptRetries()).toBe(1); + }); + }); + + it("falls back to 1 on negative input", () => { + withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "-2", () => { + expect(acpxPromptRetries()).toBe(1); + }); + }); +}); + +describe("buildAcpxJsonArgs", () => { + afterEach(() => { + delete process.env["CLAWPATCH_ACPX_PROMPT_RETRIES"]; + }); + + it("includes --prompt-retries 1 by default", () => { + delete process.env["CLAWPATCH_ACPX_PROMPT_RETRIES"]; + const args = buildAcpxJsonArgs("/tmp/repo", null, "read"); + expect(args).toEqual([ + "--cwd", + "/tmp/repo", + "--approve-reads", + "--format", + "json", + "--json-strict", + "--suppress-reads", + "--prompt-retries", + "1", + "codex", + "exec", + "--file", + "-", + ]); + }); + + it("honors CLAWPATCH_ACPX_PROMPT_RETRIES env override", () => { + withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "4", () => { + const args = buildAcpxJsonArgs("/tmp/repo", null, "approve"); + const idx = args.indexOf("--prompt-retries"); + expect(idx).toBeGreaterThanOrEqual(0); + expect(args[idx + 1]).toBe("4"); + expect(args).toContain("--approve-all"); + }); + }); + + it("omits --prompt-retries when CLAWPATCH_ACPX_PROMPT_RETRIES=0", () => { + withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "0", () => { + const args = buildAcpxJsonArgs("/tmp/repo", null, "read"); + expect(args).not.toContain("--prompt-retries"); + }); + }); + + it("passes through agent and model from parseAcpxAgent", () => { + delete process.env["CLAWPATCH_ACPX_PROMPT_RETRIES"]; + const args = buildAcpxJsonArgs("/tmp/repo", "gamma:opus", "read"); + const modelIdx = args.indexOf("--model"); + expect(modelIdx).toBeGreaterThanOrEqual(0); + expect(args[modelIdx + 1]).toBe("opus"); + expect(args).toContain("gamma"); + }); +}); diff --git a/src/provider.ts b/src/provider.ts index 3bc387d..52015f6 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -735,20 +735,33 @@ export function parseAcpxAgent(model: string | null): { return { agent: model.slice(0, idx), agentModel: model.slice(idx + 1) }; } -async function runAcpxJson( +function buildAcpxJsonArgs( root: string, - prompt: string, model: string | null, - schema: object, permission: "read" | "approve", -): Promise { +): string[] { const { agent, agentModel } = parseAcpxAgent(model); const permFlag = permission === "read" ? "--approve-reads" : "--approve-all"; const args = ["--cwd", root, permFlag, "--format", "json", "--json-strict", "--suppress-reads"]; + const promptRetries = acpxPromptRetries(); + if (promptRetries > 0) { + args.push("--prompt-retries", String(promptRetries)); + } if (agentModel !== null) { args.push("--model", agentModel); } args.push(agent, "exec", "--file", "-"); + return args; +} + +async function runAcpxJson( + root: string, + prompt: string, + model: string | null, + schema: object, + permission: "read" | "approve", +): Promise { + const args = buildAcpxJsonArgs(root, model, permission); const result = await runCommandArgs( "acpx", args, @@ -1062,11 +1075,22 @@ function acpxTimeoutMs(): number { return Number.isFinite(parsed) && parsed > 0 ? parsed : ACPX_DEFAULT_TIMEOUT_MS; } +function acpxPromptRetries(): number { + const raw = process.env["CLAWPATCH_ACPX_PROMPT_RETRIES"]; + if (raw === undefined) { + return 1; + } + const parsed = Number(raw); + return Number.isFinite(parsed) && parsed >= 0 ? Math.floor(parsed) : 1; +} + // eslint-disable-next-line no-underscore-dangle export const __testing = { acpxFailureMessage, + acpxPromptRetries, addCodexModelArgs, addCodexSandboxArgs, + buildAcpxJsonArgs, codexFailureMessage, extractAcpxJson, extractOpencodeJson, From dd6ef061f327115066200e256855f6074834a3c8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 20 May 2026 03:12:15 +0100 Subject: [PATCH 2/4] docs(changelog): note acpx retry support --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71cbf4d..f0afffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ - Added `clawpatch ci` to initialize, map, review, write a report, and append a GitHub Actions step summary in one CI-friendly command. - Added `clawpatch open-pr --patch ` to turn an applied patch attempt into an explicit GitHub pull request. - Added review prompt provenance and budget accounting for included files, omitted files, prompt bytes, and approximate tokens. -- Added two-layer retry for transient acpx review failures: `runAcpxJson` now passes `--prompt-retries 1` (override via `CLAWPATCH_ACPX_PROMPT_RETRIES`) so the agent can recover stream cuts internally, and `reviewFeature` retries the whole acpx invocation once on `malformed-output` errors (override via `CLAWPATCH_REVIEW_RETRIES`). Deterministic failures (`provider-auth`, `unsupported-provider`, `agent-refused`, `agent-cancelled`) still fail fast. +- Added retries for transient acpx JSON review failures via `--prompt-retries` and `CLAWPATCH_REVIEW_RETRIES`, thanks @coletebou. - Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes. - Fixed `clawpatch open-pr` so repositories without default-branch metadata use a dedicated patch branch and let GitHub choose the PR base. - Fixed `clawpatch open-pr` retries to push the recorded patch commit instead of any later local branch tip. From 9014831bc31f8e59b066121affe1459345d7fea4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 20 May 2026 03:15:49 +0100 Subject: [PATCH 3/4] fix(provider): keep acpx retries read-only --- src/provider.test.ts | 10 +++++++++- src/provider.ts | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/provider.test.ts b/src/provider.test.ts index 36b9429..9d3d10e 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -652,11 +652,19 @@ describe("buildAcpxJsonArgs", () => { it("honors CLAWPATCH_ACPX_PROMPT_RETRIES env override", () => { withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "4", () => { - const args = buildAcpxJsonArgs("/tmp/repo", null, "approve"); + const args = buildAcpxJsonArgs("/tmp/repo", null, "read"); const idx = args.indexOf("--prompt-retries"); expect(idx).toBeGreaterThanOrEqual(0); expect(args[idx + 1]).toBe("4"); + expect(args).toContain("--approve-reads"); + }); + }); + + it("omits --prompt-retries for approve mode", () => { + withEnv("CLAWPATCH_ACPX_PROMPT_RETRIES", "4", () => { + const args = buildAcpxJsonArgs("/tmp/repo", null, "approve"); expect(args).toContain("--approve-all"); + expect(args).not.toContain("--prompt-retries"); }); }); diff --git a/src/provider.ts b/src/provider.ts index 52015f6..433e766 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -744,7 +744,7 @@ function buildAcpxJsonArgs( const permFlag = permission === "read" ? "--approve-reads" : "--approve-all"; const args = ["--cwd", root, permFlag, "--format", "json", "--json-strict", "--suppress-reads"]; const promptRetries = acpxPromptRetries(); - if (promptRetries > 0) { + if (permission === "read" && promptRetries > 0) { args.push("--prompt-retries", String(promptRetries)); } if (agentModel !== null) { From 9737667909b415b49b2efebf07c6b18251cebcec Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 20 May 2026 03:22:17 +0100 Subject: [PATCH 4/4] fix(provider): isolate acpx retry output --- src/provider.test.ts | 42 +++++++++++++++++++++++ src/provider.ts | 82 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 107 insertions(+), 17 deletions(-) diff --git a/src/provider.test.ts b/src/provider.test.ts index 9d3d10e..a50afc3 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -14,6 +14,7 @@ const { codexFailureMessage, extractAcpxJson, extractOpencodeJson, + parseAcpxJsonOutput, parseAcpxAgent, parseCodexJson, piThinkingLevel, @@ -389,6 +390,47 @@ describe("extractAcpxJson", () => { expect(extractAcpxJson(stdout)).toEqual({ ok: true }); }); + it("prefers a later complete message after a stale retry attempt", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"ok":false}'), + textChunk("agent_message_chunk", '{"ok":true}'), + ].join("\n"); + + expect( + parseAcpxJsonOutput(stdout, (output) => { + if ( + typeof output === "object" && + output !== null && + (output as { ok?: unknown }).ok === true + ) { + return output; + } + throw new Error("wrong attempt"); + }), + ).toEqual({ ok: true }); + }); + + it("recovers from a partial message before a retry attempt", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"ok":'), + textChunk("agent_message_chunk", '{"ok":true}'), + ].join("\n"); + + expect(parseAcpxJsonOutput(stdout, (output) => output)).toEqual({ ok: true }); + }); + + it("keeps scanning when a retry-safe suffix is only a nested object", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"findings":[],"inspected":'), + textChunk("agent_message_chunk", '{"files":[],"symbols":[],"notes":[]}}'), + ].join("\n"); + + expect(parseAcpxJsonOutput(stdout, (output) => reviewOutputSchema.parse(output))).toEqual({ + findings: [], + inspected: { files: [], symbols: [], notes: [] }, + }); + }); + it("throws malformed-output with observed envelope kinds when nothing is extractable", () => { const stdout = updateEnvelope({ sessionUpdate: "usage_update", diff --git a/src/provider.ts b/src/provider.ts index 433e766..2a58108 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -144,24 +144,28 @@ const acpxProvider: Provider = { return `${version} (tested against ${ACPX_TESTED_VERSIONS})`; }, async map(root: string, prompt: string, options: ProviderOptions): Promise { - const output = await runAcpxJson(root, prompt, options.model, agentMapJsonSchema, "read"); - return agentMapOutputSchema.parse(output); + return runAcpxJson(root, prompt, options.model, agentMapJsonSchema, "read", (output) => + agentMapOutputSchema.parse(output), + ); }, async review(root: string, prompt: string, options: ProviderOptions): Promise { - const output = await runAcpxJson(root, prompt, options.model, reviewJsonSchema, "read"); - return reviewOutputSchema.parse(output); + return runAcpxJson(root, prompt, options.model, reviewJsonSchema, "read", (output) => + reviewOutputSchema.parse(output), + ); }, async fix(root: string, prompt: string, options: ProviderOptions): Promise { - const output = await runAcpxJson(root, prompt, options.model, fixPlanJsonSchema, "approve"); - return fixPlanOutputSchema.parse(output); + return runAcpxJson(root, prompt, options.model, fixPlanJsonSchema, "approve", (output) => + fixPlanOutputSchema.parse(output), + ); }, async revalidate( root: string, prompt: string, options: ProviderOptions, ): Promise { - const output = await runAcpxJson(root, prompt, options.model, revalidateJsonSchema, "read"); - return revalidateOutputSchema.parse(output); + return runAcpxJson(root, prompt, options.model, revalidateJsonSchema, "read", (output) => + revalidateOutputSchema.parse(output), + ); }, }; @@ -754,13 +758,14 @@ function buildAcpxJsonArgs( return args; } -async function runAcpxJson( +async function runAcpxJson( root: string, prompt: string, model: string | null, schema: object, permission: "read" | "approve", -): Promise { + parseOutput: (output: unknown) => T, +): Promise { const args = buildAcpxJsonArgs(root, model, permission); const result = await runCommandArgs( "acpx", @@ -776,7 +781,7 @@ async function runAcpxJson( "provider-failure", ); } - return extractAcpxJson(result.stdout); + return parseAcpxJsonOutput(result.stdout, parseOutput); } function buildAcpxPrompt(prompt: string, schema: object, permission: "read" | "approve"): string { @@ -798,6 +803,19 @@ function buildAcpxPrompt(prompt: string, schema: object, permission: "read" | "a } export function extractAcpxJson(stdout: string): unknown { + const { candidates, observedKinds } = acpxJsonCandidates(stdout, false); + return parseAcpxJsonCandidates(candidates, observedKinds, (output) => output); +} + +function parseAcpxJsonOutput(stdout: string, parseOutput: (output: unknown) => T): T { + const { candidates, observedKinds } = acpxJsonCandidates(stdout, true); + return parseAcpxJsonCandidates(candidates, observedKinds, parseOutput); +} + +function acpxJsonCandidates( + stdout: string, + retrySafe: boolean, +): { candidates: string[]; observedKinds: Set } { const toolCandidates: string[] = []; const messageChunks: string[] = []; const thoughtChunks: string[] = []; @@ -846,11 +864,25 @@ export function extractAcpxJson(stdout: string): unknown { toolCandidates.push(update.output); } } - const candidates = [ - ...(messageChunks.length > 0 ? [messageChunks.join("")] : []), - ...toolCandidates.toReversed(), - ...(thoughtChunks.length > 0 ? [thoughtChunks.join("")] : []), - ]; + const candidates = retrySafe + ? [ + ...chunkSuffixCandidates(messageChunks), + ...toolCandidates.toReversed(), + ...chunkSuffixCandidates(thoughtChunks), + ] + : [ + ...(messageChunks.length > 0 ? [messageChunks.join("")] : []), + ...toolCandidates.toReversed(), + ...(thoughtChunks.length > 0 ? [thoughtChunks.join("")] : []), + ]; + return { candidates, observedKinds }; +} + +function parseAcpxJsonCandidates( + candidates: string[], + observedKinds: Set, + parseOutput: (output: unknown) => T, +): T { if (candidates.length === 0) { throw new ClawpatchError( `acpx provider produced no extractable text. Observed envelope kinds: ` + @@ -868,7 +900,7 @@ export function extractAcpxJson(stdout: string): unknown { try { const parsed = extractJson(text); if (parsed !== null) { - return parsed; + return parseOutput(parsed); } throw new Error("no JSON object found"); } catch (err) { @@ -885,6 +917,21 @@ export function extractAcpxJson(stdout: string): unknown { ); } +function chunkSuffixCandidates(chunks: string[]): string[] { + const candidates: string[] = []; + const seen = new Set(); + let suffix = ""; + for (let index = chunks.length - 1; index >= 0; index -= 1) { + suffix = `${chunks[index] ?? ""}${suffix}`; + const candidate = suffix.trim(); + if (candidate.length > 0 && !seen.has(candidate)) { + candidates.push(candidate); + seen.add(candidate); + } + } + return candidates; +} + async function runGrokJson( root: string, prompt: string, @@ -1093,6 +1140,7 @@ export const __testing = { buildAcpxJsonArgs, codexFailureMessage, extractAcpxJson, + parseAcpxJsonOutput, extractOpencodeJson, parseAcpxAgent, parseCodexJson,