Merge pull request #360 from prathamdby/fix/edit-crlf-windows

Fix edit tool failing on Windows due to CRLF line endings
This commit is contained in:
Mario Zechner 2025-12-29 23:28:55 +01:00 committed by GitHub
commit c214a33405
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 113 additions and 10 deletions

7
package-lock.json generated
View file

@ -4013,6 +4013,7 @@
"resolved": "https://registry.npmjs.org/hono/-/hono-4.11.2.tgz", "resolved": "https://registry.npmjs.org/hono/-/hono-4.11.2.tgz",
"integrity": "sha512-o+avdUAD1v94oHkjGBhiMhBV4WBHxhbu0+CUVH78hhphKy/OKQLxtKjkmmNcrMlbYAhAbsM/9F+l3KnYxyD3Lg==", "integrity": "sha512-o+avdUAD1v94oHkjGBhiMhBV4WBHxhbu0+CUVH78hhphKy/OKQLxtKjkmmNcrMlbYAhAbsM/9F+l3KnYxyD3Lg==",
"license": "MIT", "license": "MIT",
"peer": true,
"engines": { "engines": {
"node": ">=16.9.0" "node": ">=16.9.0"
} }
@ -4582,6 +4583,7 @@
"resolved": "https://registry.npmjs.org/lit/-/lit-3.3.2.tgz", "resolved": "https://registry.npmjs.org/lit/-/lit-3.3.2.tgz",
"integrity": "sha512-NF9zbsP79l4ao2SNrH3NkfmFgN/hBYSQo90saIVI1o5GpjAdCPVstVzO1MrLOakHoEhYkrtRjPK6Ob521aoYWQ==", "integrity": "sha512-NF9zbsP79l4ao2SNrH3NkfmFgN/hBYSQo90saIVI1o5GpjAdCPVstVzO1MrLOakHoEhYkrtRjPK6Ob521aoYWQ==",
"license": "BSD-3-Clause", "license": "BSD-3-Clause",
"peer": true,
"dependencies": { "dependencies": {
"@lit/reactive-element": "^2.1.0", "@lit/reactive-element": "^2.1.0",
"lit-element": "^4.2.0", "lit-element": "^4.2.0",
@ -5702,6 +5704,7 @@
"resolved": "https://registry.npmjs.org/tailwind-merge/-/tailwind-merge-3.4.0.tgz", "resolved": "https://registry.npmjs.org/tailwind-merge/-/tailwind-merge-3.4.0.tgz",
"integrity": "sha512-uSaO4gnW+b3Y2aWoWfFpX62vn2sR3skfhbjsEnaBI81WD1wBLlHZe5sWf0AqjksNdYTbGBEd0UasQMT3SNV15g==", "integrity": "sha512-uSaO4gnW+b3Y2aWoWfFpX62vn2sR3skfhbjsEnaBI81WD1wBLlHZe5sWf0AqjksNdYTbGBEd0UasQMT3SNV15g==",
"license": "MIT", "license": "MIT",
"peer": true,
"funding": { "funding": {
"type": "github", "type": "github",
"url": "https://github.com/sponsors/dcastil" "url": "https://github.com/sponsors/dcastil"
@ -5730,7 +5733,8 @@
"version": "4.1.18", "version": "4.1.18",
"resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-4.1.18.tgz", "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-4.1.18.tgz",
"integrity": "sha512-4+Z+0yiYyEtUVCScyfHCxOYP06L5Ne+JiHhY2IjR2KWMIWhJOYZKLSGZaP5HkZ8+bY0cxfzwDE5uOmzFXyIwxw==", "integrity": "sha512-4+Z+0yiYyEtUVCScyfHCxOYP06L5Ne+JiHhY2IjR2KWMIWhJOYZKLSGZaP5HkZ8+bY0cxfzwDE5uOmzFXyIwxw==",
"license": "MIT" "license": "MIT",
"peer": true
}, },
"node_modules/tapable": { "node_modules/tapable": {
"version": "2.3.0", "version": "2.3.0",
@ -5995,6 +5999,7 @@
"resolved": "https://registry.npmjs.org/vite/-/vite-7.3.0.tgz", "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.0.tgz",
"integrity": "sha512-dZwN5L1VlUBewiP6H9s2+B3e3Jg96D0vzN+Ry73sOefebhYr9f94wwkMNN/9ouoU8pV1BqA1d1zGk8928cx0rg==", "integrity": "sha512-dZwN5L1VlUBewiP6H9s2+B3e3Jg96D0vzN+Ry73sOefebhYr9f94wwkMNN/9ouoU8pV1BqA1d1zGk8928cx0rg==",
"license": "MIT", "license": "MIT",
"peer": true,
"dependencies": { "dependencies": {
"esbuild": "^0.27.0", "esbuild": "^0.27.0",
"fdir": "^6.5.0", "fdir": "^6.5.0",

View file

@ -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. - **`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 ## [0.30.2] - 2025-12-26
### Changed ### Changed

View file

@ -5,6 +5,22 @@ import { constants } from "fs";
import { access, readFile, writeFile } from "fs/promises"; import { access, readFile, writeFile } from "fs/promises";
import { resolveToCwd } from "./path-utils.js"; 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 * Generate a unified diff string with line numbers and context
*/ */
@ -169,8 +185,13 @@ export function createEditTool(cwd: string): AgentTool<typeof editSchema> {
return; return;
} }
const originalEnding = detectLineEnding(content);
const normalizedContent = normalizeToLF(content);
const normalizedOldText = normalizeToLF(oldText);
const normalizedNewText = normalizeToLF(newText);
// Check if old text exists // Check if old text exists
if (!content.includes(oldText)) { if (!normalizedContent.includes(normalizedOldText)) {
if (signal) { if (signal) {
signal.removeEventListener("abort", onAbort); signal.removeEventListener("abort", onAbort);
} }
@ -183,7 +204,7 @@ export function createEditTool(cwd: string): AgentTool<typeof editSchema> {
} }
// Count occurrences // Count occurrences
const occurrences = content.split(oldText).length - 1; const occurrences = normalizedContent.split(normalizedOldText).length - 1;
if (occurrences > 1) { if (occurrences > 1) {
if (signal) { if (signal) {
@ -204,11 +225,14 @@ export function createEditTool(cwd: string): AgentTool<typeof editSchema> {
// Perform replacement using indexOf + substring (raw string replace, no special character interpretation) // Perform replacement using indexOf + substring (raw string replace, no special character interpretation)
// String.replace() interprets $ in the replacement string, so we do manual replacement // String.replace() interprets $ in the replacement string, so we do manual replacement
const index = content.indexOf(oldText); const index = normalizedContent.indexOf(normalizedOldText);
const newContent = content.substring(0, index) + newText + content.substring(index + oldText.length); const normalizedNewContent =
normalizedContent.substring(0, index) +
normalizedNewText +
normalizedContent.substring(index + normalizedOldText.length);
// Verify the replacement actually changed something // Verify the replacement actually changed something
if (content === newContent) { if (normalizedContent === normalizedNewContent) {
if (signal) { if (signal) {
signal.removeEventListener("abort", onAbort); signal.removeEventListener("abort", onAbort);
} }
@ -220,7 +244,8 @@ export function createEditTool(cwd: string): AgentTool<typeof editSchema> {
return; return;
} }
await writeFile(absolutePath, newContent, "utf-8"); const finalContent = restoreLineEndings(normalizedNewContent, originalEnding);
await writeFile(absolutePath, finalContent, "utf-8");
// Check if aborted after writing // Check if aborted after writing
if (aborted) { if (aborted) {
@ -236,10 +261,10 @@ export function createEditTool(cwd: string): AgentTool<typeof editSchema> {
content: [ content: [
{ {
type: "text", 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) { } catch (error: any) {
// Clean up abort handler // Clean up abort handler

View file

@ -1,4 +1,4 @@
import { mkdirSync, rmSync, writeFileSync } from "fs"; import { mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
import { tmpdir } from "os"; import { tmpdir } from "os";
import { join } from "path"; import { join } from "path";
import { afterEach, beforeEach, describe, expect, it } from "vitest"; 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/);
});
});