From 7504bf62d25b3fcbad2b2cceaa5fb4f669d621f1 Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 15:26:42 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A4=96=20fix:=20prevent=20double=20ne?= =?UTF-8?q?wlines=20in=20file=5Fedit=5Finsert=20tool?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file_edit_insert tool was adding an extra newline when content already ended with \n, causing failures on simple tasks like terminal-bench's 'hello-world' where precision matters. Root cause: The tool splits content into lines and joins with "\n", which always adds a trailing newline. When content already includes "\n", this produces double newlines. Fix: Detect if content ends with \n and normalize before joining. This matches the agent's natural expectation - if they explicitly add \n, it should be preserved as-is without doubling. Test coverage: - Added regression test for content with trailing newline - Added test for multiline content with trailing newline - All 14 tests pass including existing behavior tests Impact: Fixes terminal-bench hello-world task and likely improves accuracy on other tasks where exact file content matters (configs, data files). Analysis: terminal-bench-results/analysis_run_18894357631.md _Generated with `cmux`_ --- src/services/tools/file_edit_insert.test.ts | 49 +++++++++++++++++++++ src/services/tools/file_edit_insert.ts | 11 ++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index c056e5e8dc..0618c3b2ce 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -329,4 +329,53 @@ describe("file_edit_insert tool", () => { expect(result.error).toContain("beyond file length"); } }); + + it("should handle content with trailing newline correctly (no double newlines)", async () => { + // This test verifies the fix for the terminal-bench "hello-world" bug + // where content with \n at the end was getting an extra newline added + using testEnv = createTestFileEditInsertTool({ cwd: testDir }); + const tool = testEnv.tool; + const args: FileEditInsertToolArgs = { + file_path: "newfile.txt", + line_offset: 0, + content: "Hello, world!\n", // Content already has trailing newline + create: true, + }; + + // Execute + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + + // Assert + expect(result.success).toBe(true); + + const fileContent = await fs.readFile(path.join(testDir, "newfile.txt"), "utf-8"); + // Should NOT have double newline - the trailing \n in content should be preserved as-is + expect(fileContent).toBe("Hello, world!\n"); + expect(fileContent).not.toBe("Hello, world!\n\n"); + }); + + it("should handle multiline content with trailing newline", async () => { + // Setup + const initialContent = "line1\nline2"; + await fs.writeFile(testFilePath, initialContent); + + using testEnv = createTestFileEditInsertTool({ cwd: testDir }); + const tool = testEnv.tool; + const args: FileEditInsertToolArgs = { + file_path: "test.txt", + line_offset: 1, + content: "INSERTED1\nINSERTED2\n", // Multiline with trailing newline + }; + + // Execute + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + + // Assert + expect(result.success).toBe(true); + + const updatedContent = await fs.readFile(testFilePath, "utf-8"); + // Should respect the trailing newline in content + expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2"); + }); + }); diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 340dc838a4..7af9ca99ba 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -93,7 +93,16 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; } - const newLines = [...lines.slice(0, line_offset), content, ...lines.slice(line_offset)]; + // Normalize content: if it already ends with \n, treat it as having its own newline + // Otherwise, it will get a newline when joined with other lines + const contentEndsWithNewline = content.endsWith("\n"); + const normalizedContent = contentEndsWithNewline ? content.slice(0, -1) : content; + + const newLines = [ + ...lines.slice(0, line_offset), + normalizedContent, + ...lines.slice(line_offset), + ]; const newContent = newLines.join("\n"); return { From cd612b2ba9e0ca81b1222f58868a0bf87bea25b7 Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 15:28:40 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A4=96=20fix:=20apply=20prettier=20fo?= =?UTF-8?q?rmatting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/tools/file_edit_insert.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 0618c3b2ce..2b235990f6 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -377,5 +377,4 @@ describe("file_edit_insert tool", () => { // Should respect the trailing newline in content expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2"); }); - }); From a8a1ed996d39ad05d268e3b04eb59d840483f2e9 Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 15:32:36 +0000 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=A4=96=20fix:=20handle=20EOF=20edge?= =?UTF-8?q?=20case=20in=20newline=20normalization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex caught a regression: when inserting at EOF, the previous fix would strip the trailing newline from content, but join() wouldn't add it back since there's nothing after it. Example: File: "line1\nline2" (no trailing newline) Insert "line3\n" at offset 2 (end of file) Previous: "line1\nline2\nline3" (newline lost!) Now: "line1\nline2\nline3\n" (preserved ✓) Solution: Only strip trailing newline if we're NOT inserting at EOF. When at EOF, preserve the content's trailing newline as-is. Added regression test for this case. Co-authored-by: chatgpt-codex-connector --- src/services/tools/file_edit_insert.test.ts | 25 +++++++++++++++++++++ src/services/tools/file_edit_insert.ts | 10 ++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 2b235990f6..01b14dc87d 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -377,4 +377,29 @@ describe("file_edit_insert tool", () => { // Should respect the trailing newline in content expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2"); }); + + it("should preserve trailing newline when appending to file without trailing newline", async () => { + // Regression test for Codex feedback: when inserting at EOF with content ending in \n, + // the newline should be preserved even if the original file doesn't end with one + const initialContent = "line1\nline2"; // No trailing newline + await fs.writeFile(testFilePath, initialContent); + + using testEnv = createTestFileEditInsertTool({ cwd: testDir }); + const tool = testEnv.tool; + const args: FileEditInsertToolArgs = { + file_path: "test.txt", + line_offset: 2, // Append at end + content: "line3\n", // With trailing newline + }; + + // Execute + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + + // Assert + expect(result.success).toBe(true); + + const updatedContent = await fs.readFile(testFilePath, "utf-8"); + // Should preserve the trailing newline from content even at EOF + expect(updatedContent).toBe("line1\nline2\nline3\n"); + }); }); diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 7af9ca99ba..6a2b049525 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -93,10 +93,14 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; } - // Normalize content: if it already ends with \n, treat it as having its own newline - // Otherwise, it will get a newline when joined with other lines + // Handle newline behavior: + // - If content ends with \n and we're not at EOF, strip it (join will add it back) + // - If content ends with \n and we're at EOF, keep it (join won't add trailing newline) + // - If content doesn't end with \n, keep as-is (join will add newlines between lines) const contentEndsWithNewline = content.endsWith("\n"); - const normalizedContent = contentEndsWithNewline ? content.slice(0, -1) : content; + const insertingAtEnd = line_offset === lines.length; + const shouldStripTrailingNewline = contentEndsWithNewline && !insertingAtEnd; + const normalizedContent = shouldStripTrailingNewline ? content.slice(0, -1) : content; const newLines = [ ...lines.slice(0, line_offset),