Skip to content

Commit bbc0045

Browse files
committed
fix(tools): honor Git attributes for new file line endings
1 parent 837860e commit bbc0045

5 files changed

Lines changed: 173 additions & 33 deletions

File tree

‎packages/core/src/common/file-utils.ts‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import childProcess from "node:child_process";
12
import * as fs from "fs";
23
import * as os from "os";
34
import * as path from "path";
@@ -18,20 +19,39 @@ export function detectLineEndings(value: string): FileLineEnding {
1819
return value.includes("\r\n") ? "CRLF" : "LF";
1920
}
2021

21-
/**
22-
* Line ending a newly created file should use: the platform-native one.
23-
*
24-
* Created files have no existing EOL to preserve, and models emit LF-only text.
25-
* Writing that verbatim produces LF files on Windows, where native tooling (and
26-
* the files the user's editor creates) use CRLF. Existing files are unaffected:
27-
* their recorded line endings still win in the write handler.
28-
*
29-
* @param eol platform line ending, injectable for tests
30-
*/
3122
export function platformLineEnding(eol: string = os.EOL): FileLineEnding {
3223
return eol === "\r\n" ? "CRLF" : "LF";
3324
}
3425

26+
/** Resolve Git's eol attribute for a new file, falling back to the platform default. */
27+
export function newFileLineEnding(filePath: string, eol: string = os.EOL): FileLineEnding {
28+
try {
29+
// The caller creates the parent directory first. Resolve from the target's
30+
// directory so nested attributes, worktrees, and files outside the session's
31+
// project root use the correct repository and Git's own matching rules.
32+
const output = childProcess.execFileSync(
33+
"git",
34+
["check-attr", "-z", "text", "eol", "--", path.basename(filePath)],
35+
{
36+
cwd: path.dirname(filePath),
37+
encoding: "utf8",
38+
stdio: ["ignore", "pipe", "ignore"],
39+
timeout: 5000,
40+
windowsHide: true,
41+
}
42+
);
43+
const [, , textAttribute, , , eolAttribute] = output.split("\0");
44+
// Git ignores eol when text conversion is explicitly disabled (-text/binary).
45+
if (textAttribute !== "unset") {
46+
if (eolAttribute === "lf") return "LF";
47+
if (eolAttribute === "crlf") return "CRLF";
48+
}
49+
} catch {
50+
// Missing Git, non-repository paths, or failed lookups must not block writes.
51+
}
52+
return platformLineEnding(eol);
53+
}
54+
3555
export function detectEncoding(buffer: Buffer): BufferEncoding {
3656
if (buffer.length >= 2 && buffer[0] === 0xff && buffer[1] === 0xfe) {
3757
return "utf16le";

‎packages/core/src/tests/session.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2192,7 +2192,7 @@ test("Write checkpoints restore tool-touched files outside the workspace and lea
21922192
const sessionId = await manager.createSession({ text: "create an outside file" });
21932193
const userMessage = manager.listSessionMessages(sessionId).find((message) => message.role === "user");
21942194
assert.ok(userMessage?.checkpointHash);
2195-
assert.equal(fs.readFileSync(outsideFilePath, "utf8"), "outside\n");
2195+
assert.equal(fs.readFileSync(outsideFilePath, "utf8"), `outside${os.EOL}`);
21962196

21972197
fs.writeFileSync(unrelatedWorkspaceFilePath, "keep\n", "utf8");
21982198
manager.restoreSessionCode(sessionId, userMessage.id);
@@ -2236,7 +2236,7 @@ test("missing git executable does not block sessions or Write tool calls", async
22362236
const sessionId = await manager.createSession({ text: "create an index page" });
22372237
const userMessage = manager.listSessionMessages(sessionId).find((message) => message.role === "user");
22382238

2239-
assert.equal(fs.readFileSync(filePath, "utf8"), "<h1>No Git</h1>\n");
2239+
assert.equal(fs.readFileSync(filePath, "utf8"), `<h1>No Git</h1>${os.EOL}`);
22402240
assert.equal(userMessage?.checkpointHash, undefined);
22412241
assert.equal(manager.getSession(sessionId)?.status, "completed");
22422242
} finally {

‎packages/core/src/tests/tool-handlers.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,10 +1196,10 @@ test("Write repairs JSON object content for .json files", async () => {
11961196
assert.equal(writeResult.metadata?.type, "create");
11971197
assert.equal(writeResult.metadata?.file_path, filePath);
11981198
assert.equal(writeResult.metadata?.cache_refreshed, true);
1199-
assert.equal(writeResult.metadata?.line_endings, "LF");
1199+
assert.equal(writeResult.metadata?.line_endings, os.EOL === "\r\n" ? "CRLF" : "LF");
12001200
assert.equal(writeResult.metadata?.input_repaired, true);
12011201
assert.match(String(writeResult.metadata?.diff_preview ?? ""), /\+\s*"name": "demo"|^\+\{/m);
1202-
assert.equal(fs.readFileSync(filePath, "utf8"), '{\n "name": "demo",\n "private": true\n}');
1202+
assert.equal(fs.readFileSync(filePath, "utf8"), ["{", ' "name": "demo",', ' "private": true', "}"].join(os.EOL));
12031203
});
12041204

12051205
test("Edit requires snippet_id even after Write refreshes file state", async () => {
@@ -1229,7 +1229,7 @@ test("Edit requires snippet_id even after Write refreshes file state", async ()
12291229

12301230
assert.equal(editResult.ok, false);
12311231
assert.match(editResult.error ?? "", /snippet_id/);
1232-
assert.equal(fs.readFileSync(filePath, "utf8"), "alpha\nbeta\n");
1232+
assert.equal(fs.readFileSync(filePath, "utf8"), `alpha${os.EOL}beta${os.EOL}`);
12331233
});
12341234

12351235
test("Edit allows empty old_string when the file is empty", async () => {

‎packages/core/src/tests/write-handler-line-endings.test.ts‎

Lines changed: 136 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { afterEach, test } from "node:test";
22
import assert from "node:assert/strict";
3+
import childProcess from "node:child_process";
34
import * as fs from "fs";
45
import * as os from "os";
56
import * as path from "path";
67
import type { ToolExecutionContext } from "../tools/executor";
78
import { handleReadTool } from "../tools/read-handler";
89
import { handleWriteTool } from "../tools/write-handler";
9-
import { platformLineEnding } from "../common/file-utils";
10+
import { newFileLineEnding, platformLineEnding } from "../common/file-utils";
1011

1112
const tempDirs: string[] = [];
1213

@@ -25,6 +26,15 @@ function createTempWorkspace(): string {
2526
return dir;
2627
}
2728

29+
function createRepository(attributes: string): string {
30+
const workspace = createTempWorkspace();
31+
childProcess.execFileSync("git", ["init", "--quiet", workspace]);
32+
// Keep the fixture independent of the developer's global Git attributes.
33+
childProcess.execFileSync("git", ["config", "core.attributesFile", os.devNull], { cwd: workspace });
34+
fs.writeFileSync(path.join(workspace, ".gitattributes"), attributes);
35+
return workspace;
36+
}
37+
2838
function createContext(sessionId: string, projectRoot: string): ToolExecutionContext {
2939
return {
3040
sessionId,
@@ -45,26 +55,137 @@ test("platformLineEnding reports CRLF only on Windows-style platforms", () => {
4555
assert.equal(platformLineEnding("\n"), "LF");
4656
});
4757

48-
test("a newly created file uses the platform-native line ending", async () => {
49-
// Models emit LF-only text. A created file has no existing EOL to preserve, so it
50-
// should follow the platform: CRLF on Windows, matching what native tooling writes.
58+
test("a new file without Git attributes uses the platform ending regardless of model content", async () => {
5159
const workspace = createTempWorkspace();
52-
const filePath = path.join(workspace, "created.txt");
60+
fs.writeFileSync(path.join(workspace, ".editorconfig"), "root = true\n[*]\nend_of_line = crlf\n");
5361

54-
await handleWriteTool({ file_path: filePath, content: "one\ntwo" }, createContext("create-eol", workspace));
62+
for (const [index, content] of ["one\ntwo", "one\r\ntwo"].entries()) {
63+
const filePath = path.join(workspace, `created-${index}.txt`);
64+
const result = await handleWriteTool({ file_path: filePath, content }, createContext("create-eol", workspace));
5565

56-
const expected = platformLineEnding() === "CRLF" ? "one\r\ntwo" : "one\ntwo";
57-
assert.equal(fs.readFileSync(filePath, "utf8"), expected);
66+
assert.equal(result.ok, true, result.error);
67+
assert.equal(fs.readFileSync(filePath, "utf8"), `one${os.EOL}two`);
68+
assert.equal(result.metadata?.bytes, Buffer.byteLength(`one${os.EOL}two`));
69+
assert.equal(result.metadata?.line_endings, os.EOL === "\r\n" ? "CRLF" : "LF");
70+
}
5871
});
5972

60-
test("an existing CRLF file keeps its line endings when rewritten with LF content", async () => {
61-
const workspace = createTempWorkspace();
62-
const filePath = path.join(workspace, "existing.txt");
63-
fs.writeFileSync(filePath, "one\r\ntwo\r\n", "utf8");
64-
const context = createContext("keep-eol", workspace);
73+
for (const eol of ["lf", "crlf"] as const) {
74+
test(`new files honor eol=${eol} over both platform defaults and model content`, async () => {
75+
const workspace = createRepository(`* text=auto eol=${eol}\n`);
76+
const filePath = path.join(workspace, "new directory", "你好 file.txt");
77+
// The handler must create missing parent directories before asking Git.
78+
const content = eol === "lf" ? "one\r\ntwo\r\n" : "one\ntwo\n";
79+
const result = await handleWriteTool({ file_path: filePath, content }, createContext(`create-${eol}`, workspace));
80+
const ending = eol === "lf" ? "\n" : "\r\n";
81+
const expected = `one${ending}two${ending}`;
82+
83+
assert.equal(result.ok, true, result.error);
84+
assert.equal(fs.readFileSync(filePath, "utf8"), expected);
85+
assert.equal(result.metadata?.bytes, Buffer.byteLength(expected));
86+
assert.equal(result.metadata?.line_endings, eol.toUpperCase());
87+
for (const platformEol of ["\n", "\r\n"]) {
88+
assert.equal(newFileLineEnding(filePath, platformEol), eol.toUpperCase());
89+
}
90+
});
91+
92+
test(`existing ${eol} files retain their encoding and endings despite conflicting attributes`, async () => {
93+
const workspace = createRepository(`* text eol=${eol === "lf" ? "crlf" : "lf"}\n`);
94+
for (const encoding of ["utf8", "utf16le"] as const) {
95+
const filePath = path.join(workspace, `existing-${encoding}.txt`);
96+
const ending = eol === "lf" ? "\n" : "\r\n";
97+
const bom = encoding === "utf16le" ? "\uFEFF" : "";
98+
fs.writeFileSync(filePath, `${bom}one${ending}two${ending}`, encoding);
99+
const context = createContext(`keep-${eol}-${encoding}`, workspace);
100+
const readResult = await handleReadTool({ file_path: filePath }, context);
101+
assert.equal(readResult.ok, true, readResult.error);
102+
const content = eol === "lf" ? `${bom}one\r\nchanged` : `${bom}one\nchanged`;
103+
const result = await handleWriteTool({ file_path: filePath, content }, context);
104+
105+
assert.equal(result.ok, true, result.error);
106+
assert.deepEqual(fs.readFileSync(filePath), Buffer.from(`${bom}one${ending}changed`, encoding));
107+
assert.equal(result.metadata?.encoding, encoding);
108+
assert.equal(result.metadata?.line_endings, eol.toUpperCase());
109+
}
110+
});
111+
}
65112

66-
await handleReadTool({ file_path: filePath }, context);
67-
await handleWriteTool({ file_path: filePath, content: "one\ntwo\n" }, context);
113+
test("nested attributes and later matching rules follow Git precedence", async () => {
114+
const workspace = createRepository("* text eol=lf\n*.cmd eol=crlf\n");
115+
const nested = path.join(workspace, "nested");
116+
fs.mkdirSync(nested);
117+
fs.writeFileSync(path.join(nested, ".gitattributes"), '* eol=crlf\n*.txt eol=lf\n"space name.txt" eol=crlf\n');
118+
119+
for (const [relativePath, ending] of [
120+
["root.txt", "\n"],
121+
["root.cmd", "\r\n"],
122+
["nested/file.ts", "\r\n"],
123+
["nested/file.txt", "\n"],
124+
["nested/space name.txt", "\r\n"],
125+
]) {
126+
const filePath = path.join(workspace, relativePath);
127+
const result = await handleWriteTool(
128+
{ file_path: filePath, content: "one\ntwo\n" },
129+
createContext("nested-attributes", workspace)
130+
);
131+
assert.equal(result.ok, true, result.error);
132+
assert.equal(fs.readFileSync(filePath, "utf8"), `one${ending}two${ending}`, relativePath);
133+
}
134+
});
135+
136+
test("unspecified, unset, invalid, and binary attributes fall back to the platform", () => {
137+
const workspace = createRepository(
138+
[
139+
"*.txt text eol=crlf",
140+
"unspecified.txt !eol",
141+
"unset.txt -eol",
142+
"invalid.txt eol=native",
143+
"binary.txt binary",
144+
"no-text.txt -text",
145+
"auto.txt text=auto !eol",
146+
].join("\n") + "\n"
147+
);
148+
childProcess.execFileSync("git", ["config", "core.eol", "crlf"], { cwd: workspace });
149+
150+
for (const fileName of [
151+
"unmatched.ts",
152+
"unspecified.txt",
153+
"unset.txt",
154+
"invalid.txt",
155+
"binary.txt",
156+
"no-text.txt",
157+
"auto.txt",
158+
]) {
159+
const filePath = path.join(workspace, fileName);
160+
assert.equal(newFileLineEnding(filePath, "\n"), "LF", fileName);
161+
assert.equal(newFileLineEnding(filePath, "\r\n"), "CRLF", fileName);
162+
}
163+
});
164+
165+
test("Git lookup failures fall back without blocking file creation", async (t) => {
166+
const workspace = createTempWorkspace();
167+
t.mock.method(childProcess, "execFileSync", () => {
168+
throw Object.assign(new Error("spawnSync git ENOENT"), { code: "ENOENT" });
169+
});
170+
const filePath = path.join(workspace, "created.txt");
171+
assert.equal(newFileLineEnding(filePath, "\n"), "LF");
172+
assert.equal(newFileLineEnding(filePath, "\r\n"), "CRLF");
173+
const result = await handleWriteTool(
174+
{ file_path: filePath, content: "one\ntwo\n" },
175+
createContext("no-git", workspace)
176+
);
177+
assert.equal(result.ok, true, result.error);
178+
assert.equal(fs.readFileSync(filePath, "utf8"), `one${os.EOL}two${os.EOL}`);
179+
});
68180

181+
test("new files outside the session project use their own repository attributes", async () => {
182+
const projectRoot = createRepository("* text eol=lf\n");
183+
const targetRoot = createRepository("* text eol=crlf\n");
184+
const filePath = path.join(targetRoot, "outside.txt");
185+
const result = await handleWriteTool(
186+
{ file_path: filePath, content: "one\ntwo\n" },
187+
createContext("outside-project", projectRoot)
188+
);
189+
assert.equal(result.ok, true, result.error);
69190
assert.equal(fs.readFileSync(filePath, "utf8"), "one\r\ntwo\r\n");
70191
});

‎packages/core/src/tools/write-handler.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
ensureParentDirectory,
77
hasFileChangedSinceState,
88
normalizeContent,
9-
platformLineEnding,
9+
newFileLineEnding,
1010
readTextFileWithMetadata,
1111
writeTextFile,
1212
} from "../common/file-utils";
@@ -97,8 +97,7 @@ export async function handleWriteTool(
9797

9898
const existingMetadata = existingFile ? readTextFileWithMetadata(filePath) : null;
9999
const encoding = existingMetadata?.encoding ?? "utf8";
100-
const lineEndings =
101-
existingMetadata?.lineEndings ?? (input.content.includes("\r\n") ? "CRLF" : platformLineEnding());
100+
const lineEndings = existingMetadata?.lineEndings ?? newFileLineEnding(filePath);
102101
const diffPreview = buildDiffPreview(filePath, existingMetadata?.content ?? null, normalizedContent);
103102
context.signal?.throwIfAborted();
104103
context.onBeforeFileMutation?.(filePath);

0 commit comments

Comments
 (0)