From bf6aa7ca67b79329f546427a88acbbb2f889b5ea 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:30:57 +0800 Subject: [PATCH] fix(agents): treat host edit tool as success when file contains newText after upstream throw (fixes #32333) --- .../pi-tools.read.host-edit-recovery.test.ts | 78 +++++++++++++++++++ src/agents/pi-tools.read.ts | 66 +++++++++++++++- 2 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 src/agents/pi-tools.read.host-edit-recovery.test.ts diff --git a/src/agents/pi-tools.read.host-edit-recovery.test.ts b/src/agents/pi-tools.read.host-edit-recovery.test.ts new file mode 100644 index 00000000000..d8b5d0ad94f --- /dev/null +++ b/src/agents/pi-tools.read.host-edit-recovery.test.ts @@ -0,0 +1,78 @@ +/** + * 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 { afterEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + executeThrows: true, +})); + +vi.mock("@mariozechner/pi-coding-agent", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createEditTool: (cwd: string, options?: { operations?: unknown }) => { + const base = actual.createEditTool(cwd, options); + return { + ...base, + execute: async (...args: Parameters) => { + if (mocks.executeThrows) { + throw new Error("Simulated post-write failure (e.g. generateDiffString)"); + } + return base.execute(...args); + }, + }; + }, + }; +}); + +const { createHostWorkspaceEditTool } = await import("./pi-tools.read.js"); + +describe("createHostWorkspaceEditTool post-write recovery", () => { + let tmpDir = ""; + + afterEach(async () => { + mocks.executeThrows = true; + if (tmpDir) { + await fs.rm(tmpDir, { recursive: true, force: true }); + tmpDir = ""; + } + }); + + it("returns success when upstream throws but file on disk contains newText", async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-")); + const filePath = path.join(tmpDir, "MEMORY.md"); + const newText = "Blog Writing"; + await fs.writeFile(filePath, `# Memory\n\n${newText}\n`, "utf-8"); + + const tool = createHostWorkspaceEditTool(tmpDir); + const result = await tool.execute( + "call-1", + { path: filePath, oldText: "# Memory", 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"); + }); + + it("rethrows when file on disk does not contain newText", 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 tool = createHostWorkspaceEditTool(tmpDir); + await expect( + tool.execute("call-1", { path: filePath, oldText: "x", newText: "never-written" }, undefined), + ).rejects.toThrow("Simulated post-write failure"); + }); +}); diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index 1c5ce11c0f0..20200d0ce11 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; @@ -680,11 +681,74 @@ export function createHostWorkspaceWriteTool(root: string, options?: { workspace return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write); } +/** Resolve path for host edit: expand ~ and resolve relative paths against root. */ +function resolveHostEditPath(root: string, pathParam: string): string { + const expanded = + pathParam.startsWith("~/") || pathParam === "~" + ? pathParam.replace(/^~/, os.homedir()) + : pathParam; + return path.isAbsolute(expanded) ? path.resolve(expanded) : path.resolve(root, expanded); +} + +/** + * 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). + */ +function wrapHostEditToolWithPostWriteRecovery(base: AnyAgentTool, root: string): AnyAgentTool { + return { + ...base, + execute: async ( + toolCallId: string, + params: unknown, + signal: AbortSignal | undefined, + onUpdate?: (update: unknown) => void, + ) => { + try { + return await base.execute(toolCallId, params, signal, onUpdate); + } catch (err) { + const record = + params && typeof params === "object" ? (params as Record) : 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; + if (!pathParam || !newText) { + throw err; + } + try { + const absolutePath = resolveHostEditPath(root, pathParam); + const content = await fs.readFile(absolutePath, "utf-8"); + if (content.includes(newText)) { + return { + content: [ + { + type: "text", + text: `Successfully replaced text in ${pathParam}.`, + }, + ], + details: { diff: "", firstChangedLine: undefined }, + } as AgentToolResult; + } + } catch { + // File read failed or path invalid; rethrow original error. + } + throw err; + } + }, + }; +} + export function createHostWorkspaceEditTool(root: string, options?: { workspaceOnly?: boolean }) { const base = createEditTool(root, { operations: createHostEditOperations(root, options), }) as unknown as AnyAgentTool; - return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit); + const withRecovery = wrapHostEditToolWithPostWriteRecovery(base, root); + return wrapToolParamNormalization(withRecovery, CLAUDE_PARAM_GROUPS.edit); } export function createOpenClawReadTool(