mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-15 11:02:17 +00:00
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
This commit is contained in:
Pratham Dubey
parent
65fb9116fb
commit
8c43a9fbc8
4 changed files with 113 additions and 10 deletions
7
package-lock.json
generated
7
package-lock.json
generated
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<typeof editSchema> {
|
|||
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<typeof editSchema> {
|
|||
}
|
||||
|
||||
// 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<typeof editSchema> {
|
|||
|
||||
// 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<typeof editSchema> {
|
|||
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<typeof editSchema> {
|
|||
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
|
||||
|
|
|
|||
|
|
@ -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/);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue