mirror of https://github.com/openclaw/openclaw.git
fix(agents): harden edit tool recovery (#52516)
Merged via squash.
Prepared head SHA: e23bde893a
Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com>
Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com>
Reviewed-by: @mbelinky
This commit is contained in:
parent
60cd98a841
commit
922f4e66ea
|
|
@ -407,6 +407,10 @@ Docs: https://docs.openclaw.ai
|
|||
- Slack/embedded delivery: suppress transcript-only `delivery-mirror` assistant messages before embedded re-delivery and raise the default Slack chunk fallback so messages just over 4000 characters stay in a single post. (#45489) Thanks @theo674.
|
||||
- Slack/embedded delivery: suppress transcript-only `delivery-mirror` assistant messages before embedded re-delivery and raise the default Slack chunk fallback so messages just over 4000 characters stay in a single post. (#45489) Thanks @theo674.
|
||||
|
||||
### Fixes
|
||||
|
||||
- Agents/edit tool: accept common path/text alias spellings, show current file contents on exact-match failures, and avoid false edit failures after successful writes. (#52516) thanks @mbelinky.
|
||||
|
||||
## 2026.3.13
|
||||
|
||||
### Changes
|
||||
|
|
|
|||
|
|
@ -178,6 +178,66 @@ describe("handleToolExecutionEnd cron.add commitment tracking", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("handleToolExecutionEnd mutating failure recovery", () => {
|
||||
it("clears edit failure when the retry succeeds through common file path aliases", async () => {
|
||||
const { ctx } = createTestContext();
|
||||
|
||||
await handleToolExecutionStart(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_start",
|
||||
toolName: "edit",
|
||||
toolCallId: "tool-edit-1",
|
||||
args: {
|
||||
file_path: "/tmp/demo.txt",
|
||||
old_string: "beta stale",
|
||||
new_string: "beta fixed",
|
||||
},
|
||||
} as never,
|
||||
);
|
||||
|
||||
await handleToolExecutionEnd(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_end",
|
||||
toolName: "edit",
|
||||
toolCallId: "tool-edit-1",
|
||||
isError: true,
|
||||
result: { error: "Could not find the exact text in /tmp/demo.txt" },
|
||||
} as never,
|
||||
);
|
||||
|
||||
expect(ctx.state.lastToolError?.toolName).toBe("edit");
|
||||
|
||||
await handleToolExecutionStart(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_start",
|
||||
toolName: "edit",
|
||||
toolCallId: "tool-edit-2",
|
||||
args: {
|
||||
file: "/tmp/demo.txt",
|
||||
oldText: "beta",
|
||||
newText: "beta fixed",
|
||||
},
|
||||
} as never,
|
||||
);
|
||||
|
||||
await handleToolExecutionEnd(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_end",
|
||||
toolName: "edit",
|
||||
toolCallId: "tool-edit-2",
|
||||
isError: false,
|
||||
result: { ok: true },
|
||||
} as never,
|
||||
);
|
||||
|
||||
expect(ctx.state.lastToolError).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("handleToolExecutionEnd exec approval prompts", () => {
|
||||
it("emits a deterministic approval payload and marks assistant output suppressed", async () => {
|
||||
const { ctx } = createTestContext();
|
||||
|
|
|
|||
|
|
@ -39,6 +39,37 @@ describe("createOpenClawCodingTools", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("accepts broader file/edit alias variants", async () => {
|
||||
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-alias-broad-"));
|
||||
try {
|
||||
const tools = createOpenClawCodingTools({ workspaceDir: tmpDir });
|
||||
const { readTool, writeTool, editTool } = expectReadWriteEditTools(tools);
|
||||
|
||||
await writeTool?.execute("tool-alias-broad-1", {
|
||||
file: "broad-alias.txt",
|
||||
content: "hello old value",
|
||||
});
|
||||
|
||||
await editTool?.execute("tool-alias-broad-2", {
|
||||
filePath: "broad-alias.txt",
|
||||
old_text: "old",
|
||||
newString: "new",
|
||||
});
|
||||
|
||||
const result = await readTool?.execute("tool-alias-broad-3", {
|
||||
file: "broad-alias.txt",
|
||||
});
|
||||
|
||||
const textBlocks = result?.content?.filter((block) => block.type === "text") as
|
||||
| Array<{ text?: string }>
|
||||
| undefined;
|
||||
const combinedText = textBlocks?.map((block) => block.text ?? "").join("\n");
|
||||
expect(combinedText).toContain("hello new value");
|
||||
} finally {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("coerces structured content blocks for write", async () => {
|
||||
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-structured-write-"));
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -1,11 +1,24 @@
|
|||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { AgentToolResult, AgentToolUpdateCallback } from "@mariozechner/pi-agent-core";
|
||||
import type { AnyAgentTool } from "./pi-tools.types.js";
|
||||
|
||||
/** Resolve path for host edit: expand ~ and resolve relative paths against root. */
|
||||
function resolveHostEditPath(root: string, pathParam: string): string {
|
||||
type EditToolRecoveryOptions = {
|
||||
root: string;
|
||||
readFile: (absolutePath: string) => Promise<string>;
|
||||
};
|
||||
|
||||
type EditToolParams = {
|
||||
pathParam?: string;
|
||||
oldText?: string;
|
||||
newText?: string;
|
||||
};
|
||||
|
||||
const EDIT_MISMATCH_MESSAGE = "Could not find the exact text in";
|
||||
const EDIT_MISMATCH_HINT_LIMIT = 800;
|
||||
|
||||
/** Resolve path for edit recovery: expand ~ and resolve relative paths against root. */
|
||||
function resolveEditPath(root: string, pathParam: string): string {
|
||||
const expanded =
|
||||
pathParam.startsWith("~/") || pathParam === "~"
|
||||
? pathParam.replace(/^~/, os.homedir())
|
||||
|
|
@ -13,15 +26,103 @@ function resolveHostEditPath(root: string, pathParam: string): string {
|
|||
return path.isAbsolute(expanded) ? path.resolve(expanded) : path.resolve(root, expanded);
|
||||
}
|
||||
|
||||
function readStringParam(record: Record<string, unknown> | undefined, ...keys: string[]) {
|
||||
for (const key of keys) {
|
||||
const value = record?.[key];
|
||||
if (typeof value === "string") {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function readEditToolParams(params: unknown): EditToolParams {
|
||||
const record =
|
||||
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"),
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeToLF(value: string): string {
|
||||
return value.replace(/\r\n?/g, "\n");
|
||||
}
|
||||
|
||||
function removeExactOccurrences(content: string, needle: string): string {
|
||||
return needle.length > 0 ? content.split(needle).join("") : content;
|
||||
}
|
||||
|
||||
function didEditLikelyApply(params: {
|
||||
originalContent?: string;
|
||||
currentContent: string;
|
||||
oldText?: string;
|
||||
newText: string;
|
||||
}) {
|
||||
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;
|
||||
|
||||
if (normalizedOriginal !== undefined && normalizedOriginal === normalizedCurrent) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (normalizedNew.length > 0 && !normalizedCurrent.includes(normalizedNew)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const withoutInsertedNewText =
|
||||
normalizedNew.length > 0
|
||||
? removeExactOccurrences(normalizedCurrent, normalizedNew)
|
||||
: normalizedCurrent;
|
||||
if (normalizedOld && withoutInsertedNewText.includes(normalizedOld)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
function buildEditSuccessResult(pathParam: string): AgentToolResult<unknown> {
|
||||
return {
|
||||
isError: false,
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: `Successfully replaced text in ${pathParam}.`,
|
||||
},
|
||||
],
|
||||
details: { diff: "", firstChangedLine: undefined },
|
||||
} as AgentToolResult<unknown>;
|
||||
}
|
||||
|
||||
function shouldAddMismatchHint(error: unknown) {
|
||||
return error instanceof Error && error.message.includes(EDIT_MISMATCH_MESSAGE);
|
||||
}
|
||||
|
||||
function appendMismatchHint(error: Error, currentContent: string): Error {
|
||||
const snippet =
|
||||
currentContent.length <= EDIT_MISMATCH_HINT_LIMIT
|
||||
? currentContent
|
||||
: `${currentContent.slice(0, EDIT_MISMATCH_HINT_LIMIT)}\n... (truncated)`;
|
||||
const enhanced = new Error(`${error.message}\nCurrent file contents:\n${snippet}`);
|
||||
enhanced.stack = error.stack;
|
||||
return enhanced;
|
||||
}
|
||||
|
||||
/**
|
||||
* When the upstream edit tool throws after having already written (e.g. generateDiffString fails),
|
||||
* the file may be correctly updated but the tool reports failure. This wrapper catches errors and
|
||||
* if the target file on disk contains the intended newText, returns success so we don't surface
|
||||
* a false "edit failed" to the user (fixes #32333, same pattern as #30773 for write).
|
||||
* Recover from two edit-tool failure classes without changing edit semantics:
|
||||
* - exact-match mismatch errors become actionable by including current file contents
|
||||
* - post-write throws are converted back to success only if the file actually changed
|
||||
*/
|
||||
export function wrapHostEditToolWithPostWriteRecovery(
|
||||
export function wrapEditToolWithRecovery(
|
||||
base: AnyAgentTool,
|
||||
root: string,
|
||||
options: EditToolRecoveryOptions,
|
||||
): AnyAgentTool {
|
||||
return {
|
||||
...base,
|
||||
|
|
@ -31,50 +132,54 @@ export function wrapHostEditToolWithPostWriteRecovery(
|
|||
signal: AbortSignal | undefined,
|
||||
onUpdate?: AgentToolUpdateCallback<unknown>,
|
||||
) => {
|
||||
const { pathParam, oldText, newText } = readEditToolParams(params);
|
||||
const absolutePath =
|
||||
typeof pathParam === "string" ? resolveEditPath(options.root, pathParam) : undefined;
|
||||
let originalContent: string | undefined;
|
||||
|
||||
if (absolutePath && newText !== undefined) {
|
||||
try {
|
||||
originalContent = await options.readFile(absolutePath);
|
||||
} catch {
|
||||
// Best-effort snapshot only; recovery should still proceed without it.
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
return await base.execute(toolCallId, params, signal, onUpdate);
|
||||
} catch (err) {
|
||||
const record =
|
||||
params && typeof params === "object" ? (params as Record<string, unknown>) : undefined;
|
||||
const pathParam = record && typeof record.path === "string" ? record.path : undefined;
|
||||
const newText =
|
||||
record && typeof record.newText === "string"
|
||||
? record.newText
|
||||
: record && typeof record.new_string === "string"
|
||||
? record.new_string
|
||||
: undefined;
|
||||
const oldText =
|
||||
record && typeof record.oldText === "string"
|
||||
? record.oldText
|
||||
: record && typeof record.old_string === "string"
|
||||
? record.old_string
|
||||
: undefined;
|
||||
if (!pathParam || !newText) {
|
||||
if (!absolutePath) {
|
||||
throw err;
|
||||
}
|
||||
|
||||
let currentContent: string | undefined;
|
||||
try {
|
||||
const absolutePath = resolveHostEditPath(root, pathParam);
|
||||
const content = await fs.readFile(absolutePath, "utf-8");
|
||||
// Only recover when the replacement likely occurred: newText is present and oldText
|
||||
// is no longer present. This avoids false success when upstream threw before writing
|
||||
// (e.g. oldText not found) but the file already contained newText (review feedback).
|
||||
const hasNew = content.includes(newText);
|
||||
const stillHasOld =
|
||||
oldText !== undefined && oldText.length > 0 && content.includes(oldText);
|
||||
if (hasNew && !stillHasOld) {
|
||||
return {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: `Successfully replaced text in ${pathParam}.`,
|
||||
},
|
||||
],
|
||||
details: { diff: "", firstChangedLine: undefined },
|
||||
} as AgentToolResult<unknown>;
|
||||
}
|
||||
currentContent = await options.readFile(absolutePath);
|
||||
} catch {
|
||||
// File read failed or path invalid; rethrow original error.
|
||||
// Fall through to the original error if readback fails.
|
||||
}
|
||||
|
||||
if (typeof currentContent === "string" && newText !== undefined) {
|
||||
if (
|
||||
didEditLikelyApply({
|
||||
originalContent,
|
||||
currentContent,
|
||||
oldText,
|
||||
newText,
|
||||
})
|
||||
) {
|
||||
return buildEditSuccessResult(pathParam ?? absolutePath);
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
typeof currentContent === "string" &&
|
||||
err instanceof Error &&
|
||||
shouldAddMismatchHint(err)
|
||||
) {
|
||||
throw appendMismatchHint(err, currentContent);
|
||||
}
|
||||
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -13,20 +13,20 @@ function parameterValidationError(message: string): Error {
|
|||
}
|
||||
|
||||
export const CLAUDE_PARAM_GROUPS = {
|
||||
read: [{ keys: ["path", "file_path"], label: "path (path or file_path)" }],
|
||||
read: [{ keys: ["path", "file_path", "filePath", "file"], label: "path alias" }],
|
||||
write: [
|
||||
{ keys: ["path", "file_path"], label: "path (path or file_path)" },
|
||||
{ keys: ["path", "file_path", "filePath", "file"], label: "path alias" },
|
||||
{ keys: ["content"], label: "content" },
|
||||
],
|
||||
edit: [
|
||||
{ keys: ["path", "file_path"], label: "path (path or file_path)" },
|
||||
{ keys: ["path", "file_path", "filePath", "file"], label: "path alias" },
|
||||
{
|
||||
keys: ["oldText", "old_string"],
|
||||
label: "oldText (oldText or old_string)",
|
||||
keys: ["oldText", "old_string", "old_text", "oldString"],
|
||||
label: "oldText alias",
|
||||
},
|
||||
{
|
||||
keys: ["newText", "new_string"],
|
||||
label: "newText (newText or new_string)",
|
||||
keys: ["newText", "new_string", "new_text", "newString"],
|
||||
label: "newText alias",
|
||||
allowEmpty: true,
|
||||
},
|
||||
],
|
||||
|
|
@ -91,20 +91,22 @@ export function normalizeToolParams(params: unknown): Record<string, unknown> |
|
|||
}
|
||||
const record = params as Record<string, unknown>;
|
||||
const normalized = { ...record };
|
||||
// file_path → path (read, write, edit)
|
||||
if ("file_path" in normalized && !("path" in normalized)) {
|
||||
normalized.path = normalized.file_path;
|
||||
delete normalized.file_path;
|
||||
}
|
||||
// old_string → oldText (edit)
|
||||
if ("old_string" in normalized && !("oldText" in normalized)) {
|
||||
normalized.oldText = normalized.old_string;
|
||||
delete normalized.old_string;
|
||||
}
|
||||
// new_string → newText (edit)
|
||||
if ("new_string" in normalized && !("newText" in normalized)) {
|
||||
normalized.newText = normalized.new_string;
|
||||
delete normalized.new_string;
|
||||
const aliasPairs: Array<{ original: string; alias: string }> = [
|
||||
{ original: "path", alias: "file_path" },
|
||||
{ original: "path", alias: "filePath" },
|
||||
{ original: "path", alias: "file" },
|
||||
{ original: "oldText", alias: "old_string" },
|
||||
{ original: "oldText", alias: "old_text" },
|
||||
{ original: "oldText", alias: "oldString" },
|
||||
{ original: "newText", alias: "new_string" },
|
||||
{ original: "newText", alias: "new_text" },
|
||||
{ original: "newText", alias: "newString" },
|
||||
];
|
||||
for (const { original, alias } of aliasPairs) {
|
||||
if (alias in normalized && !(original in normalized)) {
|
||||
normalized[original] = normalized[alias];
|
||||
}
|
||||
delete normalized[alias];
|
||||
}
|
||||
// Some providers/models emit text payloads as structured blocks instead of raw strings.
|
||||
// Normalize these for write/edit so content matching and writes stay deterministic.
|
||||
|
|
@ -132,8 +134,14 @@ export function patchToolSchemaForClaudeCompatibility(tool: AnyAgentTool): AnyAg
|
|||
|
||||
const aliasPairs: Array<{ original: string; alias: string }> = [
|
||||
{ original: "path", alias: "file_path" },
|
||||
{ original: "path", alias: "filePath" },
|
||||
{ original: "path", alias: "file" },
|
||||
{ original: "oldText", alias: "old_string" },
|
||||
{ original: "oldText", alias: "old_text" },
|
||||
{ original: "oldText", alias: "oldString" },
|
||||
{ original: "newText", alias: "new_string" },
|
||||
{ original: "newText", alias: "new_text" },
|
||||
{ original: "newText", alias: "newString" },
|
||||
];
|
||||
|
||||
for (const { original, alias } of aliasPairs) {
|
||||
|
|
|
|||
|
|
@ -1,95 +1,216 @@
|
|||
/**
|
||||
* Tests for edit tool post-write recovery: when the upstream library throws after
|
||||
* having already written the file (e.g. generateDiffString fails), we catch and
|
||||
* if the file on disk contains the intended newText we return success (#32333).
|
||||
*/
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { EditToolOptions } from "@mariozechner/pi-coding-agent";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { wrapEditToolWithRecovery } from "./pi-tools.host-edit.js";
|
||||
import type { AnyAgentTool } from "./pi-tools.types.js";
|
||||
import type { SandboxFsBridge, SandboxFsStat } from "./sandbox/fs-bridge.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
executeThrows: true,
|
||||
}));
|
||||
function createInMemoryBridge(root: string, files: Map<string, string>): SandboxFsBridge {
|
||||
const resolveAbsolute = (filePath: string, cwd?: string) =>
|
||||
path.isAbsolute(filePath) ? path.resolve(filePath) : path.resolve(cwd ?? root, filePath);
|
||||
|
||||
const readStat = (absolutePath: string): SandboxFsStat | null => {
|
||||
const content = files.get(absolutePath);
|
||||
if (typeof content !== "string") {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
type: "file",
|
||||
size: Buffer.byteLength(content, "utf8"),
|
||||
mtimeMs: 0,
|
||||
};
|
||||
};
|
||||
|
||||
vi.mock("@mariozechner/pi-coding-agent", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("@mariozechner/pi-coding-agent")>();
|
||||
return {
|
||||
...actual,
|
||||
createEditTool: (cwd: string, options?: EditToolOptions) => {
|
||||
const base = actual.createEditTool(cwd, options);
|
||||
resolvePath: ({ filePath, cwd }) => {
|
||||
const absolutePath = resolveAbsolute(filePath, cwd);
|
||||
return {
|
||||
...base,
|
||||
execute: async (...args: Parameters<typeof base.execute>) => {
|
||||
if (mocks.executeThrows) {
|
||||
throw new Error("Simulated post-write failure (e.g. generateDiffString)");
|
||||
}
|
||||
return base.execute(...args);
|
||||
},
|
||||
hostPath: absolutePath,
|
||||
relativePath: path.relative(root, absolutePath),
|
||||
containerPath: absolutePath,
|
||||
};
|
||||
},
|
||||
readFile: async ({ filePath, cwd }) => {
|
||||
const absolutePath = resolveAbsolute(filePath, cwd);
|
||||
const content = files.get(absolutePath);
|
||||
if (typeof content !== "string") {
|
||||
throw new Error(`ENOENT: ${absolutePath}`);
|
||||
}
|
||||
return Buffer.from(content, "utf8");
|
||||
},
|
||||
writeFile: async ({ filePath, cwd, data }) => {
|
||||
const absolutePath = resolveAbsolute(filePath, cwd);
|
||||
files.set(absolutePath, typeof data === "string" ? data : Buffer.from(data).toString("utf8"));
|
||||
},
|
||||
mkdirp: async () => {},
|
||||
remove: async ({ filePath, cwd }) => {
|
||||
files.delete(resolveAbsolute(filePath, cwd));
|
||||
},
|
||||
rename: async ({ from, to, cwd }) => {
|
||||
const fromPath = resolveAbsolute(from, cwd);
|
||||
const toPath = resolveAbsolute(to, cwd);
|
||||
const content = files.get(fromPath);
|
||||
if (typeof content !== "string") {
|
||||
throw new Error(`ENOENT: ${fromPath}`);
|
||||
}
|
||||
files.set(toPath, content);
|
||||
files.delete(fromPath);
|
||||
},
|
||||
stat: async ({ filePath, cwd }) => readStat(resolveAbsolute(filePath, cwd)),
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
let createHostWorkspaceEditTool: typeof import("./pi-tools.read.js").createHostWorkspaceEditTool;
|
||||
|
||||
describe("createHostWorkspaceEditTool post-write recovery", () => {
|
||||
describe("edit tool recovery hardening", () => {
|
||||
let tmpDir = "";
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.resetModules();
|
||||
({ createHostWorkspaceEditTool } = await import("./pi-tools.read.js"));
|
||||
mocks.executeThrows = true;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
mocks.executeThrows = true;
|
||||
if (tmpDir) {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
tmpDir = "";
|
||||
}
|
||||
});
|
||||
|
||||
it("returns success when upstream throws but file has newText and no longer has oldText", async () => {
|
||||
function createRecoveredEditTool(params: {
|
||||
root: string;
|
||||
readFile: (absolutePath: string) => Promise<string>;
|
||||
execute: AnyAgentTool["execute"];
|
||||
}) {
|
||||
const base = {
|
||||
name: "edit",
|
||||
execute: params.execute,
|
||||
} as unknown as AnyAgentTool;
|
||||
return wrapEditToolWithRecovery(base, {
|
||||
root: params.root,
|
||||
readFile: params.readFile,
|
||||
});
|
||||
}
|
||||
|
||||
it("adds current file contents to exact-match mismatch errors", async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-"));
|
||||
const filePath = path.join(tmpDir, "MEMORY.md");
|
||||
const oldText = "# Memory";
|
||||
const newText = "Blog Writing";
|
||||
await fs.writeFile(filePath, `\n\n${newText}\n`, "utf-8");
|
||||
const filePath = path.join(tmpDir, "demo.txt");
|
||||
await fs.writeFile(filePath, "actual current content", "utf-8");
|
||||
|
||||
const tool = createHostWorkspaceEditTool(tmpDir);
|
||||
const result = await tool.execute("call-1", { path: filePath, oldText, newText }, undefined);
|
||||
|
||||
expect(result).toBeDefined();
|
||||
const content = Array.isArray((result as { content?: unknown }).content)
|
||||
? (result as { content: Array<{ type?: string; text?: string }> }).content
|
||||
: [];
|
||||
const textBlock = content.find((b) => b?.type === "text" && typeof b.text === "string");
|
||||
expect(textBlock?.text).toContain("Successfully replaced text");
|
||||
const tool = createRecoveredEditTool({
|
||||
root: tmpDir,
|
||||
readFile: (absolutePath) => fs.readFile(absolutePath, "utf-8"),
|
||||
execute: async () => {
|
||||
throw new Error(
|
||||
"Could not find the exact text in demo.txt. The old text must match exactly including all whitespace and newlines.",
|
||||
);
|
||||
},
|
||||
});
|
||||
await expect(
|
||||
tool.execute(
|
||||
"call-1",
|
||||
{ path: filePath, oldText: "missing", newText: "replacement" },
|
||||
undefined,
|
||||
),
|
||||
).rejects.toThrow(/Current file contents:\nactual current content/);
|
||||
});
|
||||
|
||||
it("rethrows when file on disk does not contain newText", async () => {
|
||||
it("recovers success after a post-write throw when CRLF output contains newText and oldText is only a substring", async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-"));
|
||||
const filePath = path.join(tmpDir, "other.md");
|
||||
await fs.writeFile(filePath, "unchanged content", "utf-8");
|
||||
const filePath = path.join(tmpDir, "demo.txt");
|
||||
await fs.writeFile(filePath, 'const value = "foo";\r\n', "utf-8");
|
||||
|
||||
const tool = createHostWorkspaceEditTool(tmpDir);
|
||||
const tool = createRecoveredEditTool({
|
||||
root: tmpDir,
|
||||
readFile: (absolutePath) => fs.readFile(absolutePath, "utf-8"),
|
||||
execute: async () => {
|
||||
await fs.writeFile(filePath, 'const value = "foobar";\r\n', "utf-8");
|
||||
throw new Error("Simulated post-write failure (e.g. generateDiffString)");
|
||||
},
|
||||
});
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{
|
||||
path: filePath,
|
||||
oldText: 'const value = "foo";\n',
|
||||
newText: 'const value = "foobar";\n',
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({ isError: false });
|
||||
expect(result.content[0]).toMatchObject({
|
||||
type: "text",
|
||||
text: `Successfully replaced text in ${filePath}.`,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not recover false success when the file never changed", async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-"));
|
||||
const filePath = path.join(tmpDir, "demo.txt");
|
||||
await fs.writeFile(filePath, "replacement already present", "utf-8");
|
||||
|
||||
const tool = createRecoveredEditTool({
|
||||
root: tmpDir,
|
||||
readFile: (absolutePath) => fs.readFile(absolutePath, "utf-8"),
|
||||
execute: async () => {
|
||||
throw new Error("Simulated post-write failure (e.g. generateDiffString)");
|
||||
},
|
||||
});
|
||||
await expect(
|
||||
tool.execute("call-1", { path: filePath, oldText: "x", newText: "never-written" }, undefined),
|
||||
tool.execute(
|
||||
"call-1",
|
||||
{ path: filePath, oldText: "missing", newText: "replacement already present" },
|
||||
undefined,
|
||||
),
|
||||
).rejects.toThrow("Simulated post-write failure");
|
||||
});
|
||||
|
||||
it("rethrows when file still contains oldText (pre-write failure; avoid false success)", async () => {
|
||||
it("recovers deletion edits when the file changed and oldText is gone", async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-"));
|
||||
const filePath = path.join(tmpDir, "pre-write-fail.md");
|
||||
const oldText = "replace me";
|
||||
const newText = "new content";
|
||||
await fs.writeFile(filePath, `before ${oldText} after ${newText}`, "utf-8");
|
||||
const filePath = path.join(tmpDir, "demo.txt");
|
||||
await fs.writeFile(filePath, "before delete me after\n", "utf-8");
|
||||
|
||||
const tool = createHostWorkspaceEditTool(tmpDir);
|
||||
await expect(
|
||||
tool.execute("call-1", { path: filePath, oldText, newText }, undefined),
|
||||
).rejects.toThrow("Simulated post-write failure");
|
||||
const tool = createRecoveredEditTool({
|
||||
root: tmpDir,
|
||||
readFile: (absolutePath) => fs.readFile(absolutePath, "utf-8"),
|
||||
execute: async () => {
|
||||
await fs.writeFile(filePath, "before after\n", "utf-8");
|
||||
throw new Error("Simulated post-write failure (e.g. generateDiffString)");
|
||||
},
|
||||
});
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{ path: filePath, oldText: "delete me", newText: "" },
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({ isError: false });
|
||||
expect(result.content[0]).toMatchObject({
|
||||
type: "text",
|
||||
text: `Successfully replaced text 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");
|
||||
const files = new Map<string, string>([[filePath, "before old text after\n"]]);
|
||||
|
||||
const bridge = createInMemoryBridge(tmpDir, files);
|
||||
const tool = createRecoveredEditTool({
|
||||
root: tmpDir,
|
||||
readFile: async (absolutePath: string) =>
|
||||
(await bridge.readFile({ filePath: absolutePath, cwd: tmpDir })).toString("utf8"),
|
||||
execute: async () => {
|
||||
files.set(filePath, "before new text after\n");
|
||||
throw new Error("Simulated post-write failure (e.g. generateDiffString)");
|
||||
},
|
||||
});
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{ path: filePath, oldText: "old text", newText: "new text" },
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({ isError: false });
|
||||
expect(result.content[0]).toMatchObject({
|
||||
type: "text",
|
||||
text: `Successfully replaced text in ${filePath}.`,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import { detectMime } from "../media/mime.js";
|
|||
import { sniffMimeFromBase64 } from "../media/sniff-mime-from-base64.js";
|
||||
import type { ImageSanitizationLimits } from "./image-sanitization.js";
|
||||
import { toRelativeWorkspacePath } from "./path-policy.js";
|
||||
import { wrapHostEditToolWithPostWriteRecovery } from "./pi-tools.host-edit.js";
|
||||
import { wrapEditToolWithRecovery } from "./pi-tools.host-edit.js";
|
||||
import {
|
||||
CLAUDE_PARAM_GROUPS,
|
||||
assertRequiredParams,
|
||||
|
|
@ -607,7 +607,12 @@ export function createSandboxedEditTool(params: SandboxToolParams) {
|
|||
const base = createEditTool(params.root, {
|
||||
operations: createSandboxEditOperations(params),
|
||||
}) as unknown as AnyAgentTool;
|
||||
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
|
||||
const withRecovery = wrapEditToolWithRecovery(base, {
|
||||
root: params.root,
|
||||
readFile: async (absolutePath: string) =>
|
||||
(await params.bridge.readFile({ filePath: absolutePath, cwd: params.root })).toString("utf8"),
|
||||
});
|
||||
return wrapToolParamNormalization(withRecovery, CLAUDE_PARAM_GROUPS.edit);
|
||||
}
|
||||
|
||||
export function createHostWorkspaceWriteTool(root: string, options?: { workspaceOnly?: boolean }) {
|
||||
|
|
@ -621,7 +626,10 @@ export function createHostWorkspaceEditTool(root: string, options?: { workspaceO
|
|||
const base = createEditTool(root, {
|
||||
operations: createHostEditOperations(root, options),
|
||||
}) as unknown as AnyAgentTool;
|
||||
const withRecovery = wrapHostEditToolWithPostWriteRecovery(base, root);
|
||||
const withRecovery = wrapEditToolWithRecovery(base, {
|
||||
root,
|
||||
readFile: (absolutePath: string) => fs.readFile(absolutePath, "utf-8"),
|
||||
});
|
||||
return wrapToolParamNormalization(withRecovery, CLAUDE_PARAM_GROUPS.edit);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -37,6 +37,22 @@ describe("tool mutation helpers", () => {
|
|||
expect(readFingerprint).toBeUndefined();
|
||||
});
|
||||
|
||||
it("treats coding-tool path aliases as the same stable target", () => {
|
||||
const filePathFingerprint = buildToolActionFingerprint("edit", {
|
||||
file_path: "/tmp/demo.txt",
|
||||
old_string: "before",
|
||||
new_string: "after",
|
||||
});
|
||||
const fileAliasFingerprint = buildToolActionFingerprint("edit", {
|
||||
file: "/tmp/demo.txt",
|
||||
oldText: "before",
|
||||
newText: "after again",
|
||||
});
|
||||
|
||||
expect(filePathFingerprint).toBe("tool=edit|path=/tmp/demo.txt");
|
||||
expect(fileAliasFingerprint).toBe("tool=edit|path=/tmp/demo.txt");
|
||||
});
|
||||
|
||||
it("exposes mutation state for downstream payload rendering", () => {
|
||||
expect(
|
||||
buildToolMutationState("message", { action: "send", to: "telegram:1" }).mutatingAction,
|
||||
|
|
|
|||
|
|
@ -82,6 +82,23 @@ function normalizeFingerprintValue(value: unknown): string | undefined {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
function appendFingerprintAlias(
|
||||
parts: string[],
|
||||
record: Record<string, unknown> | undefined,
|
||||
label: string,
|
||||
keys: string[],
|
||||
): boolean {
|
||||
for (const key of keys) {
|
||||
const value = normalizeFingerprintValue(record?.[key]);
|
||||
if (!value) {
|
||||
continue;
|
||||
}
|
||||
parts.push(`${label}=${value}`);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export function isLikelyMutatingToolName(toolName: string): boolean {
|
||||
const normalized = toolName.trim().toLowerCase();
|
||||
if (!normalized) {
|
||||
|
|
@ -152,25 +169,30 @@ export function buildToolActionFingerprint(
|
|||
parts.push(`action=${action}`);
|
||||
}
|
||||
let hasStableTarget = false;
|
||||
for (const key of [
|
||||
"path",
|
||||
"filePath",
|
||||
"oldPath",
|
||||
"newPath",
|
||||
"to",
|
||||
"target",
|
||||
"messageId",
|
||||
"sessionKey",
|
||||
"jobId",
|
||||
"id",
|
||||
"model",
|
||||
]) {
|
||||
const value = normalizeFingerprintValue(record?.[key]);
|
||||
if (value) {
|
||||
parts.push(`${key.toLowerCase()}=${value}`);
|
||||
hasStableTarget = true;
|
||||
}
|
||||
}
|
||||
hasStableTarget =
|
||||
appendFingerprintAlias(parts, record, "path", [
|
||||
"path",
|
||||
"file_path",
|
||||
"filePath",
|
||||
"filepath",
|
||||
"file",
|
||||
]) || hasStableTarget;
|
||||
hasStableTarget =
|
||||
appendFingerprintAlias(parts, record, "oldpath", ["oldPath", "old_path"]) || hasStableTarget;
|
||||
hasStableTarget =
|
||||
appendFingerprintAlias(parts, record, "newpath", ["newPath", "new_path"]) || hasStableTarget;
|
||||
hasStableTarget =
|
||||
appendFingerprintAlias(parts, record, "to", ["to", "target"]) || hasStableTarget;
|
||||
hasStableTarget =
|
||||
appendFingerprintAlias(parts, record, "messageid", ["messageId", "message_id"]) ||
|
||||
hasStableTarget;
|
||||
hasStableTarget =
|
||||
appendFingerprintAlias(parts, record, "sessionkey", ["sessionKey", "session_key"]) ||
|
||||
hasStableTarget;
|
||||
hasStableTarget =
|
||||
appendFingerprintAlias(parts, record, "jobid", ["jobId", "job_id"]) || hasStableTarget;
|
||||
hasStableTarget = appendFingerprintAlias(parts, record, "id", ["id"]) || hasStableTarget;
|
||||
hasStableTarget = appendFingerprintAlias(parts, record, "model", ["model"]) || hasStableTarget;
|
||||
const normalizedMeta = meta?.trim().replace(/\s+/g, " ").toLowerCase();
|
||||
// Meta text often carries volatile details (for example "N chars").
|
||||
// Prefer stable arg-derived keys for matching; only fall back to meta
|
||||
|
|
|
|||
Loading…
Reference in New Issue