fix(agents): only recover edit when oldText no longer in file (review feedback)
This commit is contained in:
committed by
Peter Steinberger
parent
bf6aa7ca67
commit
0fb3f188b2
@@ -44,18 +44,15 @@ describe("createHostWorkspaceEditTool post-write recovery", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("returns success when upstream throws but file on disk contains newText", async () => {
|
||||
it("returns success when upstream throws but file has newText and no longer has oldText", 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, `# Memory\n\n${newText}\n`, "utf-8");
|
||||
await fs.writeFile(filePath, `\n\n${newText}\n`, "utf-8");
|
||||
|
||||
const tool = createHostWorkspaceEditTool(tmpDir);
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{ path: filePath, oldText: "# Memory", newText },
|
||||
undefined,
|
||||
);
|
||||
const result = await tool.execute("call-1", { path: filePath, oldText, newText }, undefined);
|
||||
|
||||
expect(result).toBeDefined();
|
||||
const content = Array.isArray((result as { content?: unknown }).content)
|
||||
@@ -75,4 +72,17 @@ describe("createHostWorkspaceEditTool post-write recovery", () => {
|
||||
tool.execute("call-1", { path: filePath, oldText: "x", newText: "never-written" }, undefined),
|
||||
).rejects.toThrow("Simulated post-write failure");
|
||||
});
|
||||
|
||||
it("rethrows when file still contains oldText (pre-write failure; avoid false success)", 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 tool = createHostWorkspaceEditTool(tmpDir);
|
||||
await expect(
|
||||
tool.execute("call-1", { path: filePath, oldText, newText }, undefined),
|
||||
).rejects.toThrow("Simulated post-write failure");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -717,13 +717,25 @@ function wrapHostEditToolWithPostWriteRecovery(base: AnyAgentTool, root: string)
|
||||
: 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) {
|
||||
throw err;
|
||||
}
|
||||
try {
|
||||
const absolutePath = resolveHostEditPath(root, pathParam);
|
||||
const content = await fs.readFile(absolutePath, "utf-8");
|
||||
if (content.includes(newText)) {
|
||||
// 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: [
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user