From 17c8dd4e0cc26aab53664e63ff8e483d5d38aef6 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 16 May 2026 08:29:18 -0700 Subject: [PATCH 1/3] feat(provider): add acpx provider for any ACP-compatible coding agent Routes review/fix/revalidate through openclaw/acpx so clawpatch can use Codex, Claude, Pi, Gemini, or any other ACP-compatible coding agent behind one provider name. Defaults to codex as the underlying agent so existing users keep working unchanged; select a different agent via `--model claude`, `--model pi`, or `--model claude:sonnet-4-5`. The new `runCommandRaw` exec helper preserves full stdout for stream-protocol consumers; the existing `runCommand` still truncates output > 8KB, so existing callers are unaffected. The NDJSON parser reads agent_message_chunk, agent_thought_chunk, and tool_call_result envelopes so non-codex agents that route final output differently still work. The malformed-output diagnostic names the observed envelope kinds so acpx-version-shape drift is diagnosable rather than silent. --- CHANGELOG.md | 9 ++- README.md | 3 +- docs/providers.md | 40 ++++++++- src/exec.ts | 17 +++- src/provider.test.ts | 170 +++++++++++++++++++++++++++++++++++++++ src/provider.ts | 187 ++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 416 insertions(+), 10 deletions(-) create mode 100644 src/provider.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c6c86a..e83c619 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,19 @@ ## Unreleased +### Added + +- `acpx` provider for routing review / fix / revalidate through any ACP-compatible coding agent via openclaw/acpx. Defaults to codex as the underlying agent; pick a different one with `model: claude`, `model: pi`, `model: claude:sonnet-4-5`, etc. +- `runCommandRaw` exec helper that preserves full stdout for stream-protocol consumers. The existing `runCommand` continues to truncate stdout > 8KB. - Added Next.js route mapping for `src/app` and `src/pages` layouts, thanks @obatried. - Added first-pass Python mapping for project metadata, console scripts, source groups, pytest suites, and conservative validation defaults, thanks @xiamx. - Added progress output for `clawpatch revalidate`, thanks @twidtwid. -- Improved Node/TypeScript mapping for large workspaces by splitting package source trees into bounded review groups with package-local tests. - Added generic nested SwiftPM, Apple/Xcode, and Gradle/Android app mapping. +### Changed + +- Improved Node/TypeScript mapping for large workspaces by splitting package source trees into bounded review groups with package-local tests. + ## 0.1.0 - 2026-05-15 - Added the initial strict TypeScript `clawpatch` CLI scaffold with `init`, `map`, `status`, `review`, `report`, `fix`, `revalidate`, `doctor`, and `clean-locks`. diff --git a/README.md b/README.md index 24afcab..4ab6f2f 100644 --- a/README.md +++ b/README.md @@ -73,11 +73,10 @@ working tree during the explicit fix command. Supported provider names today: - `codex`: local Codex CLI +- `acpx`: any ACP-compatible coding agent (Codex / Claude / Pi / Gemini / ...) via openclaw/acpx - `mock`: deterministic test provider - `mock-fail`: failure test provider -Direct OpenAI, Claude, Gemini, and provider panels are not implemented yet. - ## Commands - `clawpatch init`: create `.clawpatch/`, detect project basics, write config diff --git a/docs/providers.md b/docs/providers.md index f4e08cc..47d1438 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -14,10 +14,13 @@ clawpatch doctor Provider names today: - `codex`: shells out to `codex exec` +- `acpx`: routes review/fix/revalidate through any ACP-compatible coding agent (Codex / Claude / Pi / Gemini / OpenClaw ACP / ...) via openclaw/acpx - `mock`: deterministic provider for tests and fixtures - `mock-fail`: failure provider for tests -Codex invocation: +## codex provider + +Invocation: - review: read-only sandbox - revalidate: read-only sandbox @@ -25,6 +28,38 @@ Codex invocation: - output: strict JSON schema via `--output-schema` - final message capture: `--output-last-message` +## acpx provider + +Routes through `acpx exec`, where `` is any ACP-compatible coding agent. + +- review / revalidate: `--deny-all` (auto-deny all permission prompts; effectively read-only) +- fix: `--approve-all` (auto-approve all permission prompts) + +**Permission model caveat.** `acpx --approve-all` is not the same as `codex --sandbox workspace-write`. Codex's workspace-write is an enforced sandbox: the runtime confines filesystem writes to the workspace and blocks network. acpx's `--approve-all` is a permission-prompt auto-approver. The underlying agent still has whatever filesystem and network access its own runtime grants. When running `clawpatch fix --provider acpx` on code you do not control, run inside a dedicated git worktree so the agent's blast radius is bounded by the filesystem you exposed. + +### Agent selection + +Pick the underlying ACP agent via `model`. Last-colon split, so the substring after the final `:` is the model ID: + +| `model` value | Agent | Model | +| ------------------- | --------------- | ------------- | +| (null / unset) | `codex` | agent default | +| `codex` | `codex` | agent default | +| `claude` | `claude` | agent default | +| `claude:sonnet-4-5` | `claude` | `sonnet-4-5` | +| `pi` | `pi` | agent default | +| `ollama:llama3:70b` | `ollama:llama3` | `70b` | + +### Migrating from `--provider codex` + +`--provider codex --model gpt-5-codex` is not equivalent to `--provider acpx --model gpt-5-codex`. The latter selects an agent named `gpt-5-codex`. The correct migration is `--provider acpx --model codex:gpt-5-codex`. + +### Tested versions + +clawpatch was tested against `acpx@^0.8.0`. acpx is pre-1.0 and its NDJSON envelope shape may evolve. The provider's `check()` method reports the installed version against the tested range. If `extractAcpxJson` cannot find a known chunk kind, the malformed-output error names the envelope kinds it observed so version mismatches are diagnosable. + +## Selection + Model selection: ```bash @@ -38,6 +73,3 @@ Provider selection: clawpatch review --provider codex CLAWPATCH_PROVIDER=codex clawpatch review ``` - -Direct OpenAI, Claude, Gemini, local-model, and multi-model panel providers are -not implemented yet. diff --git a/src/exec.ts b/src/exec.ts index e288e1f..e74333d 100644 --- a/src/exec.ts +++ b/src/exec.ts @@ -5,6 +5,19 @@ export async function runCommand( command: string, cwd: string, input?: string, +): Promise { + const result = await runCommandRaw(command, cwd, input); + return { + ...result, + stdout: trimOutput(result.stdout), + stderr: trimOutput(result.stderr), + }; +} + +export async function runCommandRaw( + command: string, + cwd: string, + input?: string, ): Promise { const started = Date.now(); const child = spawn(command, { @@ -35,8 +48,8 @@ export async function runCommand( cwd, exitCode, durationMs: Date.now() - started, - stdout: trimOutput(stdout), - stderr: trimOutput(stderr), + stdout, + stderr, }; } diff --git a/src/provider.test.ts b/src/provider.test.ts new file mode 100644 index 0000000..9ed1625 --- /dev/null +++ b/src/provider.test.ts @@ -0,0 +1,170 @@ +import { describe, expect, it } from "vitest"; +import { ClawpatchError } from "./errors.js"; +import { __testing } from "./provider.js"; + +// eslint-disable-next-line no-underscore-dangle +const { extractAcpxJson, parseAcpxAgent } = __testing; + +function updateEnvelope(update: object): string { + return JSON.stringify({ + jsonrpc: "2.0", + method: "session/update", + params: { sessionId: "session-1", update }, + }); +} + +function textChunk( + sessionUpdate: "agent_message_chunk" | "agent_thought_chunk", + text: string, +): string { + return updateEnvelope({ + sessionUpdate, + content: { type: "text", text }, + }); +} + +function toolResult(output: string): string { + return updateEnvelope({ + sessionUpdate: "tool_call_result", + output, + }); +} + +function expectMalformed(fn: () => unknown, message: RegExp): void { + try { + fn(); + } catch (err) { + expect(err).toBeInstanceOf(ClawpatchError); + expect((err as ClawpatchError).code).toBe("malformed-output"); + expect((err as ClawpatchError).exitCode).toBe(8); + expect((err as Error).message).toMatch(message); + return; + } + throw new Error("expected malformed-output"); +} + +describe("parseAcpxAgent", () => { + it("defaults null model to codex/null", () => { + expect(parseAcpxAgent(null)).toEqual({ agent: "codex", agentModel: null }); + }); + + it("maps a bare agent name to agent/null", () => { + expect(parseAcpxAgent("claude")).toEqual({ agent: "claude", agentModel: null }); + }); + + it("splits agent and model on a single colon", () => { + expect(parseAcpxAgent("claude:sonnet-4-5")).toEqual({ + agent: "claude", + agentModel: "sonnet-4-5", + }); + }); + + it("splits on the last colon for model ids that contain colons", () => { + expect(parseAcpxAgent("ollama:llama3:70b")).toEqual({ + agent: "ollama:llama3", + agentModel: "70b", + }); + }); +}); + +describe("extractAcpxJson", () => { + it("reconstructs JSON from agent_message_chunk stream", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"findings":'), + textChunk("agent_message_chunk", '[],"inspected":{"files":[],"symbols":[],"notes":[]}}'), + ].join("\n"); + + expect(extractAcpxJson(stdout)).toEqual({ + findings: [], + inspected: { files: [], symbols: [], notes: [] }, + }); + }); + + it("reconstructs JSON from agent_thought_chunk stream", () => { + const stdout = [ + textChunk("agent_thought_chunk", '{"outcome":"fixed",'), + textChunk("agent_thought_chunk", '"reasoning":"ok","commands":[]}'), + ].join("\n"); + + expect(extractAcpxJson(stdout)).toEqual({ + outcome: "fixed", + reasoning: "ok", + commands: [], + }); + }); + + it("reads tool_call_result output when chunks are absent", () => { + const stdout = toolResult( + '{"summary":"plan","findingIds":[],"plannedFiles":[],"risk":"low","steps":[],"validationCommands":[]}', + ); + + expect(extractAcpxJson(stdout)).toEqual({ + summary: "plan", + findingIds: [], + plannedFiles: [], + risk: "low", + steps: [], + validationCommands: [], + }); + }); + + it("strips json markdown fences", () => { + const stdout = textChunk("agent_message_chunk", '```json\n{"ok":true}\n```'); + + expect(extractAcpxJson(stdout)).toEqual({ ok: true }); + }); + + it("tolerates a prose preamble before the JSON object", () => { + const stdout = textChunk("agent_message_chunk", 'Here is the JSON:\n{"ok":true}'); + + expect(extractAcpxJson(stdout)).toEqual({ ok: true }); + }); + + it("throws malformed-output with observed envelope kinds when nothing is extractable", () => { + const stdout = updateEnvelope({ + sessionUpdate: "usage_update", + usage: { inputTokens: 1, outputTokens: 2 }, + }); + + expectMalformed(() => extractAcpxJson(stdout), /no extractable text.*usage_update.*\^0\.8\.0/u); + }); + + it("throws malformed-output on unparseable concatenation", () => { + const stdout = [ + textChunk("agent_message_chunk", '{"ok":'), + textChunk("agent_message_chunk", "not-json}"), + ].join("\n"); + + expectMalformed(() => extractAcpxJson(stdout), /unparseable JSON/u); + }); + + it("ignores initialize, session/new, and result envelopes", () => { + const stdout = [ + JSON.stringify({ jsonrpc: "2.0", method: "initialize", result: { output: '{"bad":true}' } }), + JSON.stringify({ jsonrpc: "2.0", method: "session/new", result: { output: '{"bad":true}' } }), + JSON.stringify({ jsonrpc: "2.0", id: 1, result: { output: '{"bad":true}' } }), + textChunk("agent_message_chunk", '{"ok":true}'), + ].join("\n"); + + expect(extractAcpxJson(stdout)).toEqual({ ok: true }); + }); + + it("survives a 256-line NDJSON fixture over 8KB", () => { + const filler = Array.from({ length: 255 }, (_, idx) => + updateEnvelope({ + sessionUpdate: "usage_update", + usage: { + inputTokens: idx, + outputTokens: idx + 1, + note: "x".repeat(80), + }, + }), + ); + const lines = [...filler, textChunk("agent_message_chunk", '{"large":true}')]; + const stdout = lines.join("\n"); + + expect(lines).toHaveLength(256); + expect(stdout.length).toBeGreaterThan(8_000); + expect(extractAcpxJson(stdout)).toEqual({ large: true }); + }); +}); diff --git a/src/provider.ts b/src/provider.ts index 350a444..5b4eb39 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -1,7 +1,7 @@ import { mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { runCommand } from "./exec.js"; +import { runCommand, runCommandRaw } from "./exec.js"; import { ClawpatchError } from "./errors.js"; import { FixPlanOutput, @@ -24,6 +24,9 @@ export function providerByName(name: string): Provider { if (name === "codex") { return codexProvider; } + if (name === "acpx") { + return acpxProvider; + } if (name === "mock") { return mockProvider; } @@ -56,6 +59,36 @@ const codexProvider: Provider = { }, }; +const ACPX_TESTED_VERSIONS = "^0.8.0"; + +const acpxProvider: Provider = { + name: "acpx", + async check(root: string): Promise { + const result = await runCommand("acpx --version", root); + if (result.exitCode !== 0) { + throw new ClawpatchError( + "acpx CLI not available. Install: npm install -g acpx@latest", + 4, + "provider-auth", + ); + } + const version = result.stdout.trim(); + return `${version} (tested against ${ACPX_TESTED_VERSIONS})`; + }, + async review(root: string, prompt: string, model: string | null): Promise { + const output = await runAcpxJson(root, prompt, model, reviewJsonSchema, "deny"); + return reviewOutputSchema.parse(output); + }, + async fix(root: string, prompt: string, model: string | null): Promise { + const output = await runAcpxJson(root, prompt, model, fixPlanJsonSchema, "approve"); + return fixPlanOutputSchema.parse(output); + }, + async revalidate(root: string, prompt: string, model: string | null): Promise { + const output = await runAcpxJson(root, prompt, model, revalidateJsonSchema, "deny"); + return revalidateOutputSchema.parse(output); + }, +}; + const mockProvider: Provider = { name: "mock", async check(): Promise { @@ -165,6 +198,142 @@ async function runCodexJson( return JSON.parse(raw) as unknown; } +export function parseAcpxAgent(model: string | null): { + agent: string; + agentModel: string | null; +} { + if (model === null) { + return { agent: "codex", agentModel: null }; + } + const idx = model.lastIndexOf(":"); + if (idx === -1) { + return { agent: model, agentModel: null }; + } + return { agent: model.slice(0, idx), agentModel: model.slice(idx + 1) }; +} + +async function runAcpxJson( + root: string, + prompt: string, + model: string | null, + schema: object, + permission: "deny" | "approve", +): Promise { + const { agent, agentModel } = parseAcpxAgent(model); + const permFlag = permission === "deny" ? "--deny-all" : "--approve-all"; + const modelArg = agentModel === null ? "" : ` --model ${shellQuote(agentModel)}`; + const command = + `acpx --cwd ${shellQuote(root)} ${permFlag} --format json --json-strict` + + `${modelArg} ${shellQuote(agent)} exec --file -`; + const result = await runCommandRaw(command, root, buildAcpxPrompt(prompt, schema)); + if (result.exitCode !== 0) { + throw new ClawpatchError( + `acpx provider failed: ${result.stderr || result.stdout}`, + acpxExitCode(result.stderr), + "provider-failure", + ); + } + return extractAcpxJson(result.stdout); +} + +function buildAcpxPrompt(prompt: string, schema: object): string { + return ( + `${prompt}\n\n` + + "Return ONLY a JSON object matching this schema. No prose preamble, no markdown fences, " + + "no thinking-out-loud text before the JSON. " + + `Schema:\n${JSON.stringify(schema)}\n` + ); +} + +export function extractAcpxJson(stdout: string): unknown { + const candidates: string[] = []; + const chunkBuf: string[] = []; + const observedKinds = new Set(); + for (const line of stdout.split("\n")) { + const trimmed = line.trim(); + if (trimmed.length === 0) { + continue; + } + let env: { + method?: string; + params?: { + update?: { + sessionUpdate?: string; + content?: { type?: string; text?: string }; + output?: unknown; + }; + }; + }; + try { + env = JSON.parse(trimmed); + } catch { + continue; + } + if (env.method !== "session/update") { + continue; + } + const update = env.params?.update; + if (update?.sessionUpdate === undefined) { + continue; + } + observedKinds.add(update.sessionUpdate); + if ( + (update.sessionUpdate === "agent_message_chunk" || + update.sessionUpdate === "agent_thought_chunk") && + update.content?.type === "text" && + typeof update.content.text === "string" + ) { + chunkBuf.push(update.content.text); + } else if (update.sessionUpdate === "tool_call_result" && typeof update.output === "string") { + candidates.push(update.output); + } + } + if (chunkBuf.length > 0) { + candidates.push(chunkBuf.join("")); + } + if (candidates.length === 0) { + throw new ClawpatchError( + `acpx provider produced no extractable text. 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, + "malformed-output", + ); + } + + let lastErr: unknown; + for (let i = candidates.length - 1; i >= 0; i--) { + let text = candidates[i]!.trim(); + if (text.startsWith("```")) { + const firstNl = text.indexOf("\n"); + if (firstNl > 0) { + text = text.slice(firstNl + 1); + } + if (text.endsWith("```")) { + text = text.slice(0, text.length - 3).trim(); + } + } + const firstBrace = text.indexOf("{"); + if (firstBrace > 0) { + text = text.slice(firstBrace); + } + try { + return JSON.parse(text); + } catch (err) { + lastErr = err; + } + } + throw new ClawpatchError( + `acpx provider produced unparseable JSON: ${(lastErr as Error).message}. ` + + `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, + "malformed-output", + ); +} + function shellQuote(value: string): string { return `'${value.replace(/'/gu, "'\\''")}'`; } @@ -179,6 +348,22 @@ function providerExitCode(stderr: string): number { return 1; } +function acpxExitCode(stderr: string): number { + if (/auth|login|api key|not authenticated/iu.test(stderr)) { + return 4; + } + if (/quota|rate.?limit/iu.test(stderr)) { + return 5; + } + if (/acpx: command not found|spawn acpx ENOENT/iu.test(stderr)) { + return 4; + } + return 1; +} + +// eslint-disable-next-line no-underscore-dangle +export const __testing = { extractAcpxJson, parseAcpxAgent }; + const reviewJsonSchema = { type: "object", additionalProperties: false, From b5ca8830a131d73f2a8fe2f60e64196efce61911 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 16 May 2026 08:34:15 -0700 Subject: [PATCH 2/3] fix(acpx): enforce read-only via prompt directive and document agent-mode caveat acpx --deny-all denies ACP permission prompts but does not force the underlying agent into a read-only sandbox. Agents in their own full-access mode that bypass ACP permissions could still write to the workspace during review/revalidate. Add an explicit READ-ONLY directive to the prompt body when permission is deny, and document the limitation in safety.md and providers.md so the existing read-only contract is preserved where the agent honors prompt directives, and the residual risk is named for agents that don't. --- docs/providers.md | 2 +- docs/safety.md | 6 +++++- src/provider.ts | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/docs/providers.md b/docs/providers.md index 47d1438..fd60630 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -35,7 +35,7 @@ Routes through `acpx exec`, where `` is any ACP-compatible coding - review / revalidate: `--deny-all` (auto-deny all permission prompts; effectively read-only) - fix: `--approve-all` (auto-approve all permission prompts) -**Permission model caveat.** `acpx --approve-all` is not the same as `codex --sandbox workspace-write`. Codex's workspace-write is an enforced sandbox: the runtime confines filesystem writes to the workspace and blocks network. acpx's `--approve-all` is a permission-prompt auto-approver. The underlying agent still has whatever filesystem and network access its own runtime grants. When running `clawpatch fix --provider acpx` on code you do not control, run inside a dedicated git worktree so the agent's blast radius is bounded by the filesystem you exposed. +**Permission model caveat.** `acpx --approve-all` is not the same as `codex --sandbox workspace-write`. Codex's workspace-write is an enforced sandbox: the runtime confines filesystem writes to the workspace and blocks network. acpx's `--approve-all` is a permission-prompt auto-approver. The underlying agent still has whatever filesystem and network access its own runtime grants. When running `clawpatch fix --provider acpx` on code you do not control, run inside a dedicated git worktree so the agent's blast radius is bounded by the filesystem you exposed. For `review` and `revalidate`, the provider also embeds an explicit read-only directive in the prompt. This still relies on the underlying agent's cooperation; an agent running in its own full-access mode that bypasses ACP permissions is NOT strictly read-only. ### Agent selection diff --git a/docs/safety.md b/docs/safety.md index 5c78e91..43a274a 100644 --- a/docs/safety.md +++ b/docs/safety.md @@ -12,7 +12,11 @@ Current safety rules: - `fix` requires explicit `--finding `. - `fix` refuses a dirty source worktree by default. - `.clawpatch/` state changes are allowed during runs. -- review and revalidate provider calls use a read-only sandbox. +- review and revalidate provider calls use a read-only sandbox for the `codex` + provider. The `acpx` provider relies on `acpx --deny-all` plus an explicit + read-only prompt directive; underlying agents that bypass ACP permissions (e.g. + agents running in their own full-access mode) may not be strictly sandboxed. + See docs/providers.md. - provider output must pass runtime schema validation. - feature locks are stored in feature records and can be cleared with `clawpatch clean-locks`. diff --git a/src/provider.ts b/src/provider.ts index 5b4eb39..dd1b2b1 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -225,7 +225,7 @@ async function runAcpxJson( const command = `acpx --cwd ${shellQuote(root)} ${permFlag} --format json --json-strict` + `${modelArg} ${shellQuote(agent)} exec --file -`; - const result = await runCommandRaw(command, root, buildAcpxPrompt(prompt, schema)); + const result = await runCommandRaw(command, root, buildAcpxPrompt(prompt, schema, permission)); if (result.exitCode !== 0) { throw new ClawpatchError( `acpx provider failed: ${result.stderr || result.stdout}`, @@ -236,9 +236,18 @@ async function runAcpxJson( return extractAcpxJson(result.stdout); } -function buildAcpxPrompt(prompt: string, schema: object): string { +function buildAcpxPrompt(prompt: string, schema: object, permission: "deny" | "approve"): string { + const promptBody = + permission === "deny" + ? "READ-ONLY REVIEW MODE.\n" + + "Do not modify, create, or delete any files.\n" + + "Do not make any tool calls that write to the workspace.\n" + + "Only read files and report findings in the JSON output below.\n\n" + + prompt + : prompt; + return ( - `${prompt}\n\n` + + `${promptBody}\n\n` + "Return ONLY a JSON object matching this schema. No prose preamble, no markdown fences, " + "no thinking-out-loud text before the JSON. " + `Schema:\n${JSON.stringify(schema)}\n` From 40df062a0dbcfe080a367dc1ad44182ab156ae04 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 17 May 2026 03:35:41 +0100 Subject: [PATCH 3/3] fix(provider): harden acpx review mode --- CHANGELOG.md | 4 -- docs/providers.md | 10 ++-- docs/safety.md | 2 +- src/provider.test.ts | 50 +++++++++++++++-- src/provider.ts | 125 ++++++++++++++++++++++++++++++++++--------- 5 files changed, 151 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6abe72..8685814 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,10 +29,6 @@ - Fixed Codex provider execution on Windows paths with spaces and npm `.cmd` shims, thanks @1berto. - Fixed `clawpatch fix` so feature-specific validation commands run during dry-run previews and applied fix validation, thanks @rohitjavvadi. -### Changed - -- Improved Node/TypeScript mapping for large workspaces by splitting package source trees into bounded review groups with package-local tests. - ## 0.1.0 - 2026-05-15 - Added the initial strict TypeScript `clawpatch` CLI scaffold with `init`, `map`, `status`, `review`, `report`, `fix`, `revalidate`, `doctor`, and `clean-locks`. diff --git a/docs/providers.md b/docs/providers.md index 189bfd7..6828a78 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -41,9 +41,9 @@ CLAWPATCH_MODEL= clawpatch review The `acpx` provider routes through `acpx exec`, where `` is any ACP-compatible coding agent. -- review / revalidate: `--deny-all` plus an explicit read-only prompt directive +- review / revalidate: `--approve-reads` plus an explicit read-only prompt directive - fix: `--approve-all` -- output: `--format json --json-strict`, parsed from known ACP NDJSON envelope kinds +- output: `--format json --json-strict --suppress-reads`, parsed from known ACP NDJSON envelope kinds - tested envelope shape: `acpx@^0.8.0` Permission caveat: `acpx --approve-all` is not the same as `codex --sandbox @@ -52,16 +52,16 @@ approval flags control ACP permission prompts; the underlying agent still has whatever filesystem and network access its own runtime grants. For untrusted code, run `clawpatch fix --provider acpx` inside an isolated checkout. For review and revalidate, strict read-only behavior still depends on the underlying -agent honoring the denied permissions and prompt directive. +agent honoring read-only permissions and the prompt directive. Agent selection uses `--model` as `` or `:`, split on the -last colon: +first colon: - unset: agent `codex`, default model - `codex`: agent `codex`, default model - `claude`: agent `claude`, default model - `claude:sonnet-4-5`: agent `claude`, model `sonnet-4-5` -- `ollama:llama3:70b`: agent `ollama:llama3`, model `70b` +- `ollama:llama3:70b`: agent `ollama`, model `llama3:70b` Migration note: `--provider codex --model gpt-5-codex` is not equivalent to `--provider acpx --model gpt-5-codex`; the latter selects an ACP agent named diff --git a/docs/safety.md b/docs/safety.md index 117ac1e..9b25fb5 100644 --- a/docs/safety.md +++ b/docs/safety.md @@ -13,7 +13,7 @@ Current safety rules: - `fix` refuses a dirty source worktree by default. - `.clawpatch/` state changes are allowed during runs. - review and revalidate provider calls use a read-only sandbox for the `codex` - provider. The `acpx` provider relies on `acpx --deny-all` plus an explicit + provider. The `acpx` provider relies on `acpx --approve-reads` plus an explicit read-only prompt directive; underlying agents that bypass ACP permissions (e.g. agents running in their own full-access mode) may not be strictly sandboxed. See docs/providers.md. diff --git a/src/provider.test.ts b/src/provider.test.ts index 5b71e2a..e120e0a 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -3,7 +3,7 @@ import { ClawpatchError } from "./errors.js"; import { __testing, extractJson, providerByName } from "./provider.js"; // eslint-disable-next-line no-underscore-dangle -const { extractAcpxJson, parseAcpxAgent } = __testing; +const { acpxFailureMessage, extractAcpxJson, parseAcpxAgent } = __testing; function updateEnvelope(update: object): string { return JSON.stringify({ @@ -90,10 +90,10 @@ describe("parseAcpxAgent", () => { }); }); - it("splits on the last colon for model ids that contain colons", () => { + it("splits on the first colon so model ids may contain colons", () => { expect(parseAcpxAgent("ollama:llama3:70b")).toEqual({ - agent: "ollama:llama3", - agentModel: "70b", + agent: "ollama", + agentModel: "llama3:70b", }); }); }); @@ -139,6 +139,15 @@ describe("extractAcpxJson", () => { }); }); + it("prefers final message chunks over thought chunks", () => { + const stdout = [ + textChunk("agent_thought_chunk", '{"note":"not final"}'), + textChunk("agent_message_chunk", '{"ok":true}'), + ].join("\n"); + + expect(extractAcpxJson(stdout)).toEqual({ ok: true }); + }); + it("strips json markdown fences", () => { const stdout = textChunk("agent_message_chunk", '```json\n{"ok":true}\n```'); @@ -200,6 +209,39 @@ describe("extractAcpxJson", () => { }); }); +describe("acpxFailureMessage", () => { + it("does not include raw prompt envelopes from ACPX stdout", () => { + const secretPrompt = "SOURCE_CONTEXT_SECRET"; + const stdout = [ + JSON.stringify({ + jsonrpc: "2.0", + id: 2, + method: "session/prompt", + params: { + prompt: [{ type: "text", text: secretPrompt }], + }, + }), + JSON.stringify({ + jsonrpc: "2.0", + id: null, + error: { + code: -32070, + message: "Timed out after 500ms", + data: { acpxCode: "TIMEOUT", origin: "cli", sessionId: "session-1" }, + }, + }), + ].join("\n"); + + const message = acpxFailureMessage(stdout, "", 3); + + expect(message).toContain("acpx provider failed"); + expect(message).toContain("acpxCode=TIMEOUT"); + expect(message).toContain("message=Timed out after 500ms"); + expect(message).not.toContain(secretPrompt); + expect(message).not.toContain("session/prompt"); + }); +}); + describe("providerByName", () => { it("returns provider instances for optional CLI-backed providers", () => { expect(providerByName("acpx").name).toBe("acpx"); diff --git a/src/provider.ts b/src/provider.ts index 091e5d1..7a88014 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -79,7 +79,7 @@ const acpxProvider: Provider = { return `${version} (tested against ${ACPX_TESTED_VERSIONS})`; }, async review(root: string, prompt: string, model: string | null): Promise { - const output = await runAcpxJson(root, prompt, model, reviewJsonSchema, "deny"); + const output = await runAcpxJson(root, prompt, model, reviewJsonSchema, "read"); return reviewOutputSchema.parse(output); }, async fix(root: string, prompt: string, model: string | null): Promise { @@ -87,7 +87,7 @@ const acpxProvider: Provider = { return fixPlanOutputSchema.parse(output); }, async revalidate(root: string, prompt: string, model: string | null): Promise { - const output = await runAcpxJson(root, prompt, model, revalidateJsonSchema, "deny"); + const output = await runAcpxJson(root, prompt, model, revalidateJsonSchema, "read"); return revalidateOutputSchema.parse(output); }, }; @@ -244,7 +244,7 @@ export function parseAcpxAgent(model: string | null): { if (model === null) { return { agent: "codex", agentModel: null }; } - const idx = model.lastIndexOf(":"); + const idx = model.indexOf(":"); if (idx === -1) { return { agent: model, agentModel: null }; } @@ -256,11 +256,11 @@ async function runAcpxJson( prompt: string, model: string | null, schema: object, - permission: "deny" | "approve", + permission: "read" | "approve", ): Promise { const { agent, agentModel } = parseAcpxAgent(model); - const permFlag = permission === "deny" ? "--deny-all" : "--approve-all"; - const args = ["--cwd", root, permFlag, "--format", "json", "--json-strict"]; + const permFlag = permission === "read" ? "--approve-reads" : "--approve-all"; + const args = ["--cwd", root, permFlag, "--format", "json", "--json-strict", "--suppress-reads"]; if (agentModel !== null) { args.push("--model", agentModel); } @@ -274,17 +274,17 @@ async function runAcpxJson( ); if (result.exitCode !== 0) { throw new ClawpatchError( - `acpx provider failed: ${result.stderr || result.stdout}`, - acpxExitCode(result.stderr), + acpxFailureMessage(result.stdout, result.stderr, result.exitCode), + acpxExitCode(result.stdout, result.stderr, result.exitCode), "provider-failure", ); } return extractAcpxJson(result.stdout); } -function buildAcpxPrompt(prompt: string, schema: object, permission: "deny" | "approve"): string { +function buildAcpxPrompt(prompt: string, schema: object, permission: "read" | "approve"): string { const promptBody = - permission === "deny" + permission === "read" ? "READ-ONLY REVIEW MODE.\n" + "Do not modify, create, or delete any files.\n" + "Do not make any tool calls that write to the workspace.\n" + @@ -301,8 +301,9 @@ function buildAcpxPrompt(prompt: string, schema: object, permission: "deny" | "a } export function extractAcpxJson(stdout: string): unknown { - const candidates: string[] = []; - const chunkBuf: string[] = []; + const toolCandidates: string[] = []; + const messageChunks: string[] = []; + const thoughtChunks: string[] = []; const observedKinds = new Set(); for (const line of stdout.split("\n")) { const trimmed = line.trim(); @@ -333,19 +334,26 @@ export function extractAcpxJson(stdout: string): unknown { } observedKinds.add(update.sessionUpdate); if ( - (update.sessionUpdate === "agent_message_chunk" || - update.sessionUpdate === "agent_thought_chunk") && + update.sessionUpdate === "agent_message_chunk" && update.content?.type === "text" && typeof update.content.text === "string" ) { - chunkBuf.push(update.content.text); + messageChunks.push(update.content.text); + } else if ( + update.sessionUpdate === "agent_thought_chunk" && + update.content?.type === "text" && + typeof update.content.text === "string" + ) { + thoughtChunks.push(update.content.text); } else if (update.sessionUpdate === "tool_call_result" && typeof update.output === "string") { - candidates.push(update.output); + toolCandidates.push(update.output); } } - if (chunkBuf.length > 0) { - candidates.push(chunkBuf.join("")); - } + const candidates = [ + ...(messageChunks.length > 0 ? [messageChunks.join("")] : []), + ...toolCandidates.toReversed(), + ...(thoughtChunks.length > 0 ? [thoughtChunks.join("")] : []), + ]; if (candidates.length === 0) { throw new ClawpatchError( `acpx provider produced no extractable text. Observed envelope kinds: ` + @@ -358,8 +366,8 @@ export function extractAcpxJson(stdout: string): unknown { } let lastErr: unknown; - for (let i = candidates.length - 1; i >= 0; i--) { - const text = candidates[i]!.trim(); + for (const candidate of candidates) { + const text = candidate.trim(); try { const parsed = extractJson(text); if (parsed !== null) { @@ -535,21 +543,86 @@ function providerExitCode(stderr: string): number { return 1; } -function acpxExitCode(stderr: string): number { - if (/auth|login|api key|not authenticated/iu.test(stderr)) { +function acpxFailureMessage(stdout: string, stderr: string, exitCode: number | null): string { + const error = extractAcpxError(stdout); + if (error !== null) { + return `acpx provider failed: ${error}`; + } + const stderrPreview = safeProviderPreview(stderr); + if (stderrPreview.length > 0) { + return `acpx provider failed: ${stderrPreview}`; + } + return `acpx provider failed with exit code ${exitCode ?? "unknown"}`; +} + +function extractAcpxError(stdout: string): string | null { + for (const line of stdout.split("\n")) { + const trimmed = line.trim(); + if (trimmed.length === 0) { + continue; + } + let env: unknown; + try { + env = JSON.parse(trimmed) as unknown; + } catch { + continue; + } + if (typeof env !== "object" || env === null) { + continue; + } + const error = (env as Record)["error"]; + if (typeof error !== "object" || error === null) { + continue; + } + const errorRecord = error as Record; + const data = errorRecord["data"]; + const dataRecord = + typeof data === "object" && data !== null ? (data as Record) : {}; + const parts = [ + stringPart("code", errorRecord["code"]), + stringPart("acpxCode", dataRecord["acpxCode"]), + stringPart("detail", dataRecord["detailCode"]), + stringPart("origin", dataRecord["origin"]), + stringPart("message", errorRecord["message"], 160), + ].filter((part) => part.length > 0); + if (parts.length > 0) { + return parts.join("; "); + } + } + return null; +} + +function stringPart(label: string, value: unknown, maxLength = 80): string { + if (typeof value !== "string" && typeof value !== "number") { + return ""; + } + const preview = safeProviderPreview(String(value), maxLength); + return preview.length === 0 ? "" : `${label}=${preview}`; +} + +function safeProviderPreview(value: string, maxLength = 200): string { + return value.replace(/\s+/gu, " ").trim().slice(0, maxLength); +} + +function acpxExitCode(stdout: string, stderr: string, exitCode: number | null): number { + const combined = `${stderr}\n${extractAcpxError(stdout) ?? ""}`; + if (/auth|login|api key|not authenticated|AUTH_REQUIRED/iu.test(combined)) { return 4; } - if (/quota|rate.?limit/iu.test(stderr)) { + if (/quota|rate.?limit/iu.test(combined)) { return 5; } - if (/acpx: command not found|spawn acpx ENOENT/iu.test(stderr)) { + if (/acpx: command not found|spawn acpx ENOENT/iu.test(combined)) { return 4; } + if (exitCode === 3 || /TIMEOUT/iu.test(combined)) { + return 1; + } return 1; } // eslint-disable-next-line no-underscore-dangle -export const __testing = { extractAcpxJson, parseAcpxAgent }; +export const __testing = { acpxFailureMessage, extractAcpxJson, parseAcpxAgent }; const reviewJsonSchema = { type: "object",