From 693d17c4a2ae21b345db41c321b915d723df163d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 31 Mar 2026 19:05:44 +0100 Subject: [PATCH] fix: support edit tool edits[] payloads --- ...pi-tool-definition-adapter.logging.test.ts | 41 +++++++++ src/agents/pi-tools.host-edit.ts | 90 +++++++++++++------ src/agents/pi-tools.params.ts | 89 +++++++++++++++--- .../pi-tools.read.host-edit-recovery.test.ts | 32 +++++++ src/agents/pi-tools.workspace-paths.test.ts | 29 ++++++ 5 files changed, 239 insertions(+), 42 deletions(-) diff --git a/src/agents/pi-tool-definition-adapter.logging.test.ts b/src/agents/pi-tool-definition-adapter.logging.test.ts index cc442abc3b9..a959cb64c48 100644 --- a/src/agents/pi-tool-definition-adapter.logging.test.ts +++ b/src/agents/pi-tool-definition-adapter.logging.test.ts @@ -64,4 +64,45 @@ describe("pi tool definition adapter logging", () => { ), ); }); + + it("accepts nested edits arrays for the current edit schema", async () => { + const execute = vi.fn(async (_toolCallId: string, params: unknown) => ({ + content: [{ type: "text" as const, text: JSON.stringify(params) }], + details: { ok: true }, + })); + const baseTool = { + name: "edit", + label: "Edit", + description: "edits files", + parameters: Type.Object({ + path: Type.String(), + edits: Type.Array( + Type.Object({ + oldText: Type.String(), + newText: Type.String(), + }), + ), + }), + execute, + } satisfies AgentTool; + + const tool = wrapToolParamNormalization(baseTool, CLAUDE_PARAM_GROUPS.edit); + const [def] = toToolDefinitions([tool]); + if (!def) { + throw new Error("missing tool definition"); + } + + const payload = { + path: "notes.txt", + edits: [ + { oldText: "alpha", newText: "beta" }, + { oldText: "gamma", newText: "" }, + ], + }; + + await def.execute("call-edit-batch", payload, undefined, undefined, extensionContext); + + expect(execute).toHaveBeenCalledWith("call-edit-batch", payload, undefined, undefined); + expect(logError).not.toHaveBeenCalled(); + }); }); diff --git a/src/agents/pi-tools.host-edit.ts b/src/agents/pi-tools.host-edit.ts index bc8ab0c2054..aaa7b367f97 100644 --- a/src/agents/pi-tools.host-edit.ts +++ b/src/agents/pi-tools.host-edit.ts @@ -1,6 +1,7 @@ import os from "node:os"; import path from "node:path"; import type { AgentToolResult, AgentToolUpdateCallback } from "@mariozechner/pi-agent-core"; +import { normalizeToolParams } from "./pi-tools.params.js"; import type { AnyAgentTool } from "./pi-tools.types.js"; type EditToolRecoveryOptions = { @@ -10,8 +11,12 @@ type EditToolRecoveryOptions = { type EditToolParams = { pathParam?: string; - oldText?: string; - newText?: string; + edits: EditReplacement[]; +}; + +type EditReplacement = { + oldText: string; + newText: string; }; const EDIT_MISMATCH_MESSAGE = "Could not find the exact text in"; @@ -36,13 +41,33 @@ function readStringParam(record: Record | undefined, ...keys: s return undefined; } +function readEditReplacements(record: Record | undefined): EditReplacement[] { + if (!Array.isArray(record?.edits)) { + return []; + } + return record.edits.flatMap((entry) => { + if (!entry || typeof entry !== "object") { + return []; + } + const replacement = entry as Record; + if (typeof replacement.oldText !== "string" || replacement.oldText.trim().length === 0) { + return []; + } + if (typeof replacement.newText !== "string") { + return []; + } + return [{ oldText: replacement.oldText, newText: replacement.newText }]; + }); +} + function readEditToolParams(params: unknown): EditToolParams { + const normalized = normalizeToolParams(params); const record = - params && typeof params === "object" ? (params as Record) : undefined; + normalized ?? + (params && typeof params === "object" ? (params as Record) : undefined); return { - pathParam: readStringParam(record, "path", "file_path", "file"), - oldText: readStringParam(record, "oldText", "old_string", "old_text", "oldString"), - newText: readStringParam(record, "newText", "new_string", "new_text", "newString"), + pathParam: readStringParam(record, "path", "file_path", "filePath", "file"), + edits: readEditReplacements(record), }; } @@ -57,15 +82,12 @@ function removeExactOccurrences(content: string, needle: string): string { function didEditLikelyApply(params: { originalContent?: string; currentContent: string; - oldText?: string; - newText: string; + edits: EditReplacement[]; }) { + if (params.edits.length === 0) { + return false; + } const normalizedCurrent = normalizeToLF(params.currentContent); - const normalizedNew = normalizeToLF(params.newText); - const normalizedOld = - typeof params.oldText === "string" && params.oldText.length > 0 - ? normalizeToLF(params.oldText) - : undefined; const normalizedOriginal = typeof params.originalContent === "string" ? normalizeToLF(params.originalContent) : undefined; @@ -73,28 +95,39 @@ function didEditLikelyApply(params: { return false; } - if (normalizedNew.length > 0 && !normalizedCurrent.includes(normalizedNew)) { - return false; + let withoutInsertedNewText = normalizedCurrent; + for (const edit of params.edits) { + const normalizedNew = normalizeToLF(edit.newText); + if (normalizedNew.length > 0 && !normalizedCurrent.includes(normalizedNew)) { + return false; + } + withoutInsertedNewText = + normalizedNew.length > 0 + ? removeExactOccurrences(withoutInsertedNewText, normalizedNew) + : withoutInsertedNewText; } - const withoutInsertedNewText = - normalizedNew.length > 0 - ? removeExactOccurrences(normalizedCurrent, normalizedNew) - : normalizedCurrent; - if (normalizedOld && withoutInsertedNewText.includes(normalizedOld)) { - return false; + for (const edit of params.edits) { + const normalizedOld = normalizeToLF(edit.oldText); + if (withoutInsertedNewText.includes(normalizedOld)) { + return false; + } } return true; } -function buildEditSuccessResult(pathParam: string): AgentToolResult { +function buildEditSuccessResult(pathParam: string, editCount: number): AgentToolResult { + const text = + editCount > 1 + ? `Successfully replaced ${editCount} block(s) in ${pathParam}.` + : `Successfully replaced text in ${pathParam}.`; return { isError: false, content: [ { type: "text", - text: `Successfully replaced text in ${pathParam}.`, + text, }, ], details: { diff: "", firstChangedLine: undefined }, @@ -132,12 +165,12 @@ export function wrapEditToolWithRecovery( signal: AbortSignal | undefined, onUpdate?: AgentToolUpdateCallback, ) => { - const { pathParam, oldText, newText } = readEditToolParams(params); + const { pathParam, edits } = readEditToolParams(params); const absolutePath = typeof pathParam === "string" ? resolveEditPath(options.root, pathParam) : undefined; let originalContent: string | undefined; - if (absolutePath && newText !== undefined) { + if (absolutePath && edits.length > 0) { try { originalContent = await options.readFile(absolutePath); } catch { @@ -159,16 +192,15 @@ export function wrapEditToolWithRecovery( // Fall through to the original error if readback fails. } - if (typeof currentContent === "string" && newText !== undefined) { + if (typeof currentContent === "string" && edits.length > 0) { if ( didEditLikelyApply({ originalContent, currentContent, - oldText, - newText, + edits, }) ) { - return buildEditSuccessResult(pathParam ?? absolutePath); + return buildEditSuccessResult(pathParam ?? absolutePath, edits.length); } } diff --git a/src/agents/pi-tools.params.ts b/src/agents/pi-tools.params.ts index a752154f889..23f34f779c2 100644 --- a/src/agents/pi-tools.params.ts +++ b/src/agents/pi-tools.params.ts @@ -4,6 +4,7 @@ export type RequiredParamGroup = { keys: readonly string[]; allowEmpty?: boolean; label?: string; + validator?: (record: Record) => boolean; }; const RETRY_GUIDANCE_SUFFIX = " Supply correct parameters before retrying."; @@ -23,11 +24,13 @@ export const CLAUDE_PARAM_GROUPS = { { keys: ["oldText", "old_string", "old_text", "oldString"], label: "oldText alias", + validator: hasValidEditReplacements, }, { keys: ["newText", "new_string", "new_text", "newString"], label: "newText alias", allowEmpty: true, + validator: hasValidEditReplacements, }, ], } as const; @@ -49,6 +52,11 @@ const CLAUDE_PARAM_ALIASES: ClaudeParamAlias[] = [ { original: "newText", alias: "newString" }, ]; +type EditReplacement = { + oldText: string; + newText: string; +}; + function extractStructuredText(value: unknown, depth = 0): string | undefined { if (depth > 6) { return undefined; @@ -99,6 +107,58 @@ function normalizeTextLikeParam(record: Record, key: string) { } } +function normalizeEditReplacement(value: unknown): EditReplacement | undefined { + if (!value || typeof value !== "object") { + return undefined; + } + const normalized = { ...(value as Record) }; + normalizeClaudeParamAliases(normalized); + normalizeTextLikeParam(normalized, "oldText"); + normalizeTextLikeParam(normalized, "newText"); + if (typeof normalized.oldText !== "string" || normalized.oldText.trim().length === 0) { + return undefined; + } + if (typeof normalized.newText !== "string") { + return undefined; + } + return { + oldText: normalized.oldText, + newText: normalized.newText, + }; +} + +function normalizeEditReplacements(record: Record) { + const replacements: EditReplacement[] = []; + if (Array.isArray(record.edits)) { + for (const entry of record.edits) { + const normalized = normalizeEditReplacement(entry); + if (normalized) { + replacements.push(normalized); + } + } + } + if (typeof record.oldText === "string" && record.oldText.trim().length > 0) { + if (typeof record.newText === "string") { + replacements.push({ + oldText: record.oldText, + newText: record.newText, + }); + } + } + if (replacements.length > 0) { + record.edits = replacements; + } +} + +function hasValidEditReplacements(record: Record): boolean { + const edits = record.edits; + return ( + Array.isArray(edits) && + edits.length > 0 && + edits.every((entry) => normalizeEditReplacement(entry) !== undefined) + ); +} + function normalizeClaudeParamAliases(record: Record) { for (const { original, alias } of CLAUDE_PARAM_ALIASES) { if (alias in record && !(original in record)) { @@ -145,6 +205,7 @@ export function normalizeToolParams(params: unknown): Record | normalizeTextLikeParam(normalized, "content"); normalizeTextLikeParam(normalized, "oldText"); normalizeTextLikeParam(normalized, "newText"); + normalizeEditReplacements(normalized); return normalized; } @@ -189,19 +250,21 @@ export function assertRequiredParams( const missingLabels: string[] = []; for (const group of groups) { - const satisfied = group.keys.some((key) => { - if (!(key in record)) { - return false; - } - const value = record[key]; - if (typeof value !== "string") { - return false; - } - if (group.allowEmpty) { - return true; - } - return value.trim().length > 0; - }); + const satisfied = + group.validator?.(record) ?? + group.keys.some((key) => { + if (!(key in record)) { + return false; + } + const value = record[key]; + if (typeof value !== "string") { + return false; + } + if (group.allowEmpty) { + return true; + } + return value.trim().length > 0; + }); if (!satisfied) { const label = group.label ?? group.keys.join(" or "); diff --git a/src/agents/pi-tools.read.host-edit-recovery.test.ts b/src/agents/pi-tools.read.host-edit-recovery.test.ts index 297e47aa604..9648c5873de 100644 --- a/src/agents/pi-tools.read.host-edit-recovery.test.ts +++ b/src/agents/pi-tools.read.host-edit-recovery.test.ts @@ -186,6 +186,38 @@ describe("edit tool recovery hardening", () => { }); }); + it("recovers multi-edit payloads after a post-write throw", async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-")); + const filePath = path.join(tmpDir, "demo.txt"); + await fs.writeFile(filePath, "alpha beta gamma delta\n", "utf-8"); + + const tool = createRecoveredEditTool({ + root: tmpDir, + readFile: (absolutePath) => fs.readFile(absolutePath, "utf-8"), + execute: async () => { + await fs.writeFile(filePath, "ALPHA beta gamma DELTA\n", "utf-8"); + throw new Error("Simulated post-write failure (e.g. generateDiffString)"); + }, + }); + const result = await tool.execute( + "call-1", + { + path: filePath, + edits: [ + { oldText: "alpha", newText: "ALPHA" }, + { oldText: "delta", newText: "DELTA" }, + ], + }, + undefined, + ); + + expect(result).toMatchObject({ isError: false }); + expect(result.content[0]).toMatchObject({ + type: "text", + text: `Successfully replaced 2 block(s) in ${filePath}.`, + }); + }); + it("applies the same recovery path to sandboxed edit tools", async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-")); const filePath = path.join(tmpDir, "demo.txt"); diff --git a/src/agents/pi-tools.workspace-paths.test.ts b/src/agents/pi-tools.workspace-paths.test.ts index af17a896609..73bbb2730bb 100644 --- a/src/agents/pi-tools.workspace-paths.test.ts +++ b/src/agents/pi-tools.workspace-paths.test.ts @@ -115,6 +115,35 @@ describe("workspace path resolution", () => { }); }); + it("supports multi-edit edits[] payloads", async () => { + await withTempDir("openclaw-ws-", async (workspaceDir) => { + await withTempDir("openclaw-cwd-", async (otherDir) => { + const testFile = "batch.txt"; + await fs.writeFile(path.join(workspaceDir, testFile), "alpha beta gamma delta", "utf8"); + + const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir); + try { + const tools = createOpenClawCodingTools({ workspaceDir }); + const { editTool } = expectReadWriteEditTools(tools); + + await editTool.execute("ws-edit-batch", { + path: testFile, + edits: [ + { oldText: "alpha", newText: "ALPHA" }, + { oldText: "delta", newText: "DELTA" }, + ], + }); + + expect(await fs.readFile(path.join(workspaceDir, testFile), "utf8")).toBe( + "ALPHA beta gamma DELTA", + ); + } finally { + cwdSpy.mockRestore(); + } + }); + }); + }); + it("defaults exec cwd to workspaceDir when workdir is omitted", async () => { await withTempDir("openclaw-ws-", async (workspaceDir) => { const execTool = createExecTool(workspaceDir);