diff --git a/app/src/agent-tools.test.ts b/app/src/agent-tools.test.ts index cbb4031..f0c5c76 100644 --- a/app/src/agent-tools.test.ts +++ b/app/src/agent-tools.test.ts @@ -387,6 +387,88 @@ describe("agent-tools", () => { const patch = ["--- /dev/null", "+++ b/../../etc/evil.txt", "@@ -0,0 +1,1 @@", "+pwned", ""].join("\n"); expect(() => applyPatch(workspace, patch)).toThrow(/outside the workspace/); }); + + // The patch base used to come from readFile(), which caps its result + // at MAX_READ_CHARS and appends a "[truncated ...]" marker — so + // patching a file past that cap silently destroyed its tail and wrote + // the marker into the source. + it("preserves content past the read-truncation cap", () => { + const body = Array.from({ length: 12_000 }, (_, i) => `line ${i}`).join("\n"); + const original = `first\n${body}\n`; + expect(original.length).toBeGreaterThan(100_000); + fs.writeFileSync(path.join(workspace, "big.txt"), original); + + const patch = ["--- a/big.txt", "+++ b/big.txt", "@@ -1,1 +1,1 @@", "-first", "+FIRST", ""].join("\n"); + applyPatch(workspace, patch); + + const updated = fs.readFileSync(path.join(workspace, "big.txt"), "utf-8"); + expect(updated).toBe(`FIRST\n${body}\n`); + expect(updated).not.toContain("truncated"); + }); + + // Writing as we went left earlier files modified when a later file in + // the same patch failed to align. + it("leaves every file untouched when one file in the patch fails to apply", () => { + fs.writeFileSync(path.join(workspace, "first.txt"), "alpha\n"); + fs.writeFileSync(path.join(workspace, "second.txt"), "actual content\n"); + const patch = [ + "--- a/first.txt", + "+++ b/first.txt", + "@@ -1,1 +1,1 @@", + "-alpha", + "+ALPHA", + "--- a/second.txt", + "+++ b/second.txt", + "@@ -1,1 +1,1 @@", + "-expected content", + "+new content", + "", + ].join("\n"); + + expect(() => applyPatch(workspace, patch)).toThrow(/Context mismatch/); + expect(fs.readFileSync(path.join(workspace, "first.txt"), "utf-8")).toBe("alpha\n"); + expect(fs.readFileSync(path.join(workspace, "second.txt"), "utf-8")).toBe("actual content\n"); + }); + + // Resolving every file before writing must not mean every file is read + // from disk first: two sections for one path have to chain, or the + // second silently discards the first. + it("chains two sections that touch the same file", () => { + fs.writeFileSync(path.join(workspace, "twice.txt"), "one\ntwo\n"); + const patch = [ + "--- a/twice.txt", + "+++ b/twice.txt", + "@@ -1,1 +1,1 @@", + "-one", + "+ONE", + "--- a/twice.txt", + "+++ b/twice.txt", + "@@ -2,1 +2,1 @@", + "-two", + "+TWO", + "", + ].join("\n"); + const result = applyPatch(workspace, patch); + expect(fs.readFileSync(path.join(workspace, "twice.txt"), "utf-8")).toBe("ONE\nTWO\n"); + expect(result.filesChanged).toEqual(["twice.txt"]); + }); + + it("patches a file the same diff created", () => { + const patch = [ + "--- /dev/null", + "+++ b/made.txt", + "@@ -0,0 +1,1 @@", + "+hello", + "--- a/made.txt", + "+++ b/made.txt", + "@@ -1,1 +1,1 @@", + "-hello", + "+HELLO", + "", + ].join("\n"); + applyPatch(workspace, patch); + expect(fs.readFileSync(path.join(workspace, "made.txt"), "utf-8")).toBe("HELLO"); + }); }); describe("executeTool", () => { diff --git a/app/src/agent-tools.ts b/app/src/agent-tools.ts index 94e8a08..77361fd 100644 --- a/app/src/agent-tools.ts +++ b/app/src/agent-tools.ts @@ -1235,25 +1235,59 @@ function applyHunksToContent(content: string, hunks: PatchHunk[], filePath: stri return result.join("\n"); } +// Reads a file's complete content for use as the base of an edit. readFile() +// is display-oriented — it caps its result at MAX_READ_CHARS and appends a +// "[truncated ...]" marker — which makes it unsafe to write back: everything +// past the cap would be destroyed and the marker itself saved into the file. +function readFileForEdit(workspaceRoot: string, relativePath: string): string { + const target = resolveSafePath(workspaceRoot, relativePath); + if (fs.statSync(target).isDirectory()) throw new Error(`"${relativePath}" is a directory, not a file.`); + return fs.readFileSync(target, "utf-8"); +} + export function applyPatch(workspaceRoot: string, patchText: string): { filesChanged: string[] } { const files = parseUnifiedDiff(patchText); if (files.length === 0) throw new Error("No valid file patches found in the given diff."); - const filesChanged: string[] = []; + // Every file's outcome is resolved in memory before anything is written. + // Writing as we went meant a patch whose later file failed to align left + // the earlier ones already modified — a half-applied patch, which is the + // exact outcome this parser's refusal to fuzzy-match exists to avoid. + // + // `pending` (rather than a plain list) keeps resolution sequential: a diff + // with two sections for the same path, or one that creates a file and then + // patches it, has to see the earlier section's result instead of the stale + // copy on disk. null means the path is pending deletion. + const pending = new Map(); + const order: string[] = []; + const remember = (relativePath: string, content: string | null): void => { + if (!pending.has(relativePath)) order.push(relativePath); + pending.set(relativePath, content); + }; + for (const file of files) { if (file.newPath === null) { if (!file.oldPath) throw new Error("Patch deletes a file but its path (/dev/null on both sides) is missing."); - deletePath(workspaceRoot, file.oldPath, false); - filesChanged.push(file.oldPath); + remember(file.oldPath, null); continue; } - const isNewFile = file.oldPath === null; - const existingContent = isNewFile ? "" : readFile(workspaceRoot, file.newPath); - const newContent = applyHunksToContent(existingContent, file.hunks, file.newPath); - writeFile(workspaceRoot, file.newPath, newContent); - filesChanged.push(file.newPath); + let existingContent: string; + if (pending.has(file.newPath)) { + // Already touched by an earlier section of this same patch — a + // pending deletion reads back as empty, i.e. as a fresh file. + existingContent = pending.get(file.newPath) ?? ""; + } else { + existingContent = file.oldPath === null ? "" : readFileForEdit(workspaceRoot, file.newPath); + } + remember(file.newPath, applyHunksToContent(existingContent, file.hunks, file.newPath)); + } + + for (const relativePath of order) { + const content = pending.get(relativePath) ?? null; + if (content === null) deletePath(workspaceRoot, relativePath, false); + else writeFile(workspaceRoot, relativePath, content); } - return { filesChanged }; + return { filesChanged: order }; } export interface WebSearchResult {