From 8c43a9fbc83809c033b1fa0d22b13b3e3409024b Mon Sep 17 00:00:00 2001 From: Pratham Dubey <134331217+prathamdby@users.noreply.github.com> Date: Tue, 30 Dec 2025 02:32:16 +0530 Subject: [PATCH] Fix edit tool failing on Windows due to CRLF line endings Normalize line endings to LF before matching, restore original style on write. Files with CRLF now match when LLMs send LF-only oldText. Fixes #355 --- package-lock.json | 7 +- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/core/tools/edit.ts | 41 ++++++++--- packages/coding-agent/test/tools.test.ts | 71 +++++++++++++++++++- 4 files changed, 113 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index adbd6618..7aa30116 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4013,6 +4013,7 @@ "resolved": "https://registry.npmjs.org/hono/-/hono-4.11.2.tgz", "integrity": "sha512-o+avdUAD1v94oHkjGBhiMhBV4WBHxhbu0+CUVH78hhphKy/OKQLxtKjkmmNcrMlbYAhAbsM/9F+l3KnYxyD3Lg==", "license": "MIT", + "peer": true, "engines": { "node": ">=16.9.0" } @@ -4582,6 +4583,7 @@ "resolved": "https://registry.npmjs.org/lit/-/lit-3.3.2.tgz", "integrity": "sha512-NF9zbsP79l4ao2SNrH3NkfmFgN/hBYSQo90saIVI1o5GpjAdCPVstVzO1MrLOakHoEhYkrtRjPK6Ob521aoYWQ==", "license": "BSD-3-Clause", + "peer": true, "dependencies": { "@lit/reactive-element": "^2.1.0", "lit-element": "^4.2.0", @@ -5702,6 +5704,7 @@ "resolved": "https://registry.npmjs.org/tailwind-merge/-/tailwind-merge-3.4.0.tgz", "integrity": "sha512-uSaO4gnW+b3Y2aWoWfFpX62vn2sR3skfhbjsEnaBI81WD1wBLlHZe5sWf0AqjksNdYTbGBEd0UasQMT3SNV15g==", "license": "MIT", + "peer": true, "funding": { "type": "github", "url": "https://github.com/sponsors/dcastil" @@ -5730,7 +5733,8 @@ "version": "4.1.18", "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-4.1.18.tgz", "integrity": "sha512-4+Z+0yiYyEtUVCScyfHCxOYP06L5Ne+JiHhY2IjR2KWMIWhJOYZKLSGZaP5HkZ8+bY0cxfzwDE5uOmzFXyIwxw==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/tapable": { "version": "2.3.0", @@ -5995,6 +5999,7 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.0.tgz", "integrity": "sha512-dZwN5L1VlUBewiP6H9s2+B3e3Jg96D0vzN+Ry73sOefebhYr9f94wwkMNN/9ouoU8pV1BqA1d1zGk8928cx0rg==", "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.27.0", "fdir": "^6.5.0", diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c7884797..43246cf7 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,10 @@ - **`enabledModels` setting**: Configure whitelisted models in `settings.json` (same format as `--models` CLI flag). CLI `--models` takes precedence over the setting. +### Fixed + +- **Edit tool fails on Windows due to CRLF line endings**: Files with CRLF line endings now match correctly when LLMs send LF-only text. Line endings are normalized before matching and restored to original style on write. ([#355](https://github.com/badlogic/pi-mono/issues/355)) + ## [0.30.2] - 2025-12-26 ### Changed diff --git a/packages/coding-agent/src/core/tools/edit.ts b/packages/coding-agent/src/core/tools/edit.ts index 908fd2a3..ff040091 100644 --- a/packages/coding-agent/src/core/tools/edit.ts +++ b/packages/coding-agent/src/core/tools/edit.ts @@ -5,6 +5,22 @@ import { constants } from "fs"; import { access, readFile, writeFile } from "fs/promises"; import { resolveToCwd } from "./path-utils.js"; +function detectLineEnding(content: string): "\r\n" | "\n" { + const crlfIdx = content.indexOf("\r\n"); + const lfIdx = content.indexOf("\n"); + if (lfIdx === -1) return "\n"; + if (crlfIdx === -1) return "\n"; + return crlfIdx < lfIdx ? "\r\n" : "\n"; +} + +function normalizeToLF(text: string): string { + return text.replace(/\r\n/g, "\n").replace(/\r/g, "\n"); +} + +function restoreLineEndings(text: string, ending: "\r\n" | "\n"): string { + return ending === "\r\n" ? text.replace(/\n/g, "\r\n") : text; +} + /** * Generate a unified diff string with line numbers and context */ @@ -169,8 +185,13 @@ export function createEditTool(cwd: string): AgentTool { return; } + const originalEnding = detectLineEnding(content); + const normalizedContent = normalizeToLF(content); + const normalizedOldText = normalizeToLF(oldText); + const normalizedNewText = normalizeToLF(newText); + // Check if old text exists - if (!content.includes(oldText)) { + if (!normalizedContent.includes(normalizedOldText)) { if (signal) { signal.removeEventListener("abort", onAbort); } @@ -183,7 +204,7 @@ export function createEditTool(cwd: string): AgentTool { } // Count occurrences - const occurrences = content.split(oldText).length - 1; + const occurrences = normalizedContent.split(normalizedOldText).length - 1; if (occurrences > 1) { if (signal) { @@ -204,11 +225,14 @@ export function createEditTool(cwd: string): AgentTool { // Perform replacement using indexOf + substring (raw string replace, no special character interpretation) // String.replace() interprets $ in the replacement string, so we do manual replacement - const index = content.indexOf(oldText); - const newContent = content.substring(0, index) + newText + content.substring(index + oldText.length); + const index = normalizedContent.indexOf(normalizedOldText); + const normalizedNewContent = + normalizedContent.substring(0, index) + + normalizedNewText + + normalizedContent.substring(index + normalizedOldText.length); // Verify the replacement actually changed something - if (content === newContent) { + if (normalizedContent === normalizedNewContent) { if (signal) { signal.removeEventListener("abort", onAbort); } @@ -220,7 +244,8 @@ export function createEditTool(cwd: string): AgentTool { return; } - await writeFile(absolutePath, newContent, "utf-8"); + const finalContent = restoreLineEndings(normalizedNewContent, originalEnding); + await writeFile(absolutePath, finalContent, "utf-8"); // Check if aborted after writing if (aborted) { @@ -236,10 +261,10 @@ export function createEditTool(cwd: string): AgentTool { content: [ { type: "text", - text: `Successfully replaced text in ${path}. Changed ${oldText.length} characters to ${newText.length} characters.`, + text: `Successfully replaced text in ${path}.`, }, ], - details: { diff: generateDiffString(content, newContent) }, + details: { diff: generateDiffString(normalizedContent, normalizedNewContent) }, }); } catch (error: any) { // Clean up abort handler diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 119309aa..8f18b9aa 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, rmSync, writeFileSync } from "fs"; +import { mkdirSync, readFileSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -365,3 +365,72 @@ describe("Coding Agent Tools", () => { }); }); }); + +describe("edit tool CRLF handling", () => { + let testDir: string; + + beforeEach(() => { + testDir = join(tmpdir(), `coding-agent-crlf-test-${Date.now()}`); + mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + }); + + it("should match LF oldText against CRLF file content", async () => { + const testFile = join(testDir, "crlf-test.txt"); + + writeFileSync(testFile, "line one\r\nline two\r\nline three\r\n"); + + const result = await editTool.execute("test-crlf-1", { + path: testFile, + oldText: "line two\n", + newText: "replaced line\n", + }); + + expect(getTextOutput(result)).toContain("Successfully replaced"); + }); + + it("should preserve CRLF line endings after edit", async () => { + const testFile = join(testDir, "crlf-preserve.txt"); + writeFileSync(testFile, "first\r\nsecond\r\nthird\r\n"); + + await editTool.execute("test-crlf-2", { + path: testFile, + oldText: "second\n", + newText: "REPLACED\n", + }); + + const content = readFileSync(testFile, "utf-8"); + expect(content).toBe("first\r\nREPLACED\r\nthird\r\n"); + }); + + it("should preserve LF line endings for LF files", async () => { + const testFile = join(testDir, "lf-preserve.txt"); + writeFileSync(testFile, "first\nsecond\nthird\n"); + + await editTool.execute("test-lf-1", { + path: testFile, + oldText: "second\n", + newText: "REPLACED\n", + }); + + const content = readFileSync(testFile, "utf-8"); + expect(content).toBe("first\nREPLACED\nthird\n"); + }); + + it("should detect duplicates across CRLF/LF variants", async () => { + const testFile = join(testDir, "mixed-endings.txt"); + + writeFileSync(testFile, "hello\r\nworld\r\n---\r\nhello\nworld\n"); + + await expect( + editTool.execute("test-crlf-dup", { + path: testFile, + oldText: "hello\nworld\n", + newText: "replaced\n", + }), + ).rejects.toThrow(/Found 2 occurrences/); + }); +});