mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-09 15:35:17 +00:00
fix(agents): treat host edit tool as success when file contains newText after upstream throw (fixes #32333)
This commit is contained in:
committed by
Peter Steinberger
parent
0fd77c9856
commit
bf6aa7ca67
78
src/agents/pi-tools.read.host-edit-recovery.test.ts
Normal file
78
src/agents/pi-tools.read.host-edit-recovery.test.ts
Normal file
@@ -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<typeof import("@mariozechner/pi-coding-agent")>();
|
||||
return {
|
||||
...actual,
|
||||
createEditTool: (cwd: string, options?: { operations?: unknown }) => {
|
||||
const base = actual.createEditTool(cwd, options);
|
||||
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);
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -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<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;
|
||||
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<unknown>;
|
||||
}
|
||||
} 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(
|
||||
|
||||
Reference in New Issue
Block a user