mirror of https://github.com/openclaw/openclaw.git
fix: support edit tool edits[] payloads
This commit is contained in:
parent
ae730d9a86
commit
693d17c4a2
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> | undefined, ...keys: s
|
|||
return undefined;
|
||||
}
|
||||
|
||||
function readEditReplacements(record: Record<string, unknown> | undefined): EditReplacement[] {
|
||||
if (!Array.isArray(record?.edits)) {
|
||||
return [];
|
||||
}
|
||||
return record.edits.flatMap((entry) => {
|
||||
if (!entry || typeof entry !== "object") {
|
||||
return [];
|
||||
}
|
||||
const replacement = entry as Record<string, unknown>;
|
||||
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<string, unknown>) : undefined;
|
||||
normalized ??
|
||||
(params && typeof params === "object" ? (params as Record<string, unknown>) : 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<unknown> {
|
||||
function buildEditSuccessResult(pathParam: string, editCount: number): AgentToolResult<unknown> {
|
||||
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<unknown>,
|
||||
) => {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ export type RequiredParamGroup = {
|
|||
keys: readonly string[];
|
||||
allowEmpty?: boolean;
|
||||
label?: string;
|
||||
validator?: (record: Record<string, unknown>) => 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<string, unknown>, key: string) {
|
|||
}
|
||||
}
|
||||
|
||||
function normalizeEditReplacement(value: unknown): EditReplacement | undefined {
|
||||
if (!value || typeof value !== "object") {
|
||||
return undefined;
|
||||
}
|
||||
const normalized = { ...(value as Record<string, unknown>) };
|
||||
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<string, unknown>) {
|
||||
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<string, unknown>): boolean {
|
||||
const edits = record.edits;
|
||||
return (
|
||||
Array.isArray(edits) &&
|
||||
edits.length > 0 &&
|
||||
edits.every((entry) => normalizeEditReplacement(entry) !== undefined)
|
||||
);
|
||||
}
|
||||
|
||||
function normalizeClaudeParamAliases(record: Record<string, unknown>) {
|
||||
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<string, unknown> |
|
|||
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 ");
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue