From 0fb3f188b2d20993cdb83eae0bfa5c3196959784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E6=B1=89=E6=9D=B00668001185?= Date: Tue, 3 Mar 2026 09:53:03 +0800 Subject: [PATCH] fix(agents): only recover edit when oldText no longer in file (review feedback) --- .../pi-tools.read.host-edit-recovery.test.ts | 24 +++++++++++++------ src/agents/pi-tools.read.ts | 14 ++++++++++- 2 files changed, 30 insertions(+), 8 deletions(-) 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 d8b5d0ad94f..791e2827bdf 100644 --- a/src/agents/pi-tools.read.host-edit-recovery.test.ts +++ b/src/agents/pi-tools.read.host-edit-recovery.test.ts @@ -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"); + }); }); diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index 20200d0ce11..680d99c5f1f 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -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: [ {