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/); + }); +});