From 16ad58e6a433edeadb20790b7e003f63d8fd175d Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 9 Oct 2025 21:53:15 -0500 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A4=96=20Remove=20lease=20concept=20f?= =?UTF-8?q?rom=20file=20operations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Leases were causing unnecessary errors, forcing models to re-read files repeatedly and draining context. Models often fallback to sed anyway, and this tool is designed to keep changes isolated within worktrees. Changes: - Remove lease parameter from file_edit_replace and file_edit_insert tools - Remove lease from file_read tool results - Remove lease validation logic from all file operations - Update type definitions to remove lease fields - Update and clean up tests to remove lease-related assertions - Remove lease-specific tests that are no longer relevant The file operations now work without lease validation, simplifying the API and reducing context drain. --- src/services/tools/fileCommon.test.ts | 68 +------- src/services/tools/file_edit_insert.test.ts | 99 +----------- src/services/tools/file_edit_insert.ts | 17 +- src/services/tools/file_edit_replace.test.ts | 162 +++---------------- src/services/tools/file_edit_replace.ts | 18 +-- src/services/tools/file_read.test.ts | 32 +--- src/services/tools/file_read.ts | 9 +- src/types/tools.ts | 5 - src/utils/tools/toolDefinitions.ts | 9 +- 9 files changed, 41 insertions(+), 378 deletions(-) diff --git a/src/services/tools/fileCommon.test.ts b/src/services/tools/fileCommon.test.ts index 34fea621ed..9cfd2ddbbd 100644 --- a/src/services/tools/fileCommon.test.ts +++ b/src/services/tools/fileCommon.test.ts @@ -1,74 +1,8 @@ import { describe, it, expect } from "bun:test"; import type * as fs from "fs"; -import { leaseFromContent, validatePathInCwd, validateFileSize, MAX_FILE_SIZE } from "./fileCommon"; +import { validatePathInCwd, validateFileSize, MAX_FILE_SIZE } from "./fileCommon"; describe("fileCommon", () => { - describe("leaseFromContent", () => { - it("should return a 6-character hexadecimal string", () => { - const content = "Hello, world!"; - const lease = leaseFromContent(content); - - expect(lease).toMatch(/^[0-9a-f]{6}$/); - expect(lease.length).toBe(6); - }); - - it("should be deterministic for same content", () => { - const content = "Hello, world!"; - const lease1 = leaseFromContent(content); - const lease2 = leaseFromContent(content); - - expect(lease1).toBe(lease2); - }); - - it("should produce different leases for different content", () => { - const content1 = "Hello, world!"; - const content2 = "Hello, world!!"; - - const lease1 = leaseFromContent(content1); - const lease2 = leaseFromContent(content2); - - expect(lease1).not.toBe(lease2); - }); - - it("should work with Buffer input", () => { - const buffer = Buffer.from("Hello, world!", "utf-8"); - const lease = leaseFromContent(buffer); - - expect(lease).toMatch(/^[0-9a-f]{6}$/); - expect(lease.length).toBe(6); - }); - - it("should produce same lease for string and equivalent Buffer", () => { - const content = "Hello, world!"; - const buffer = Buffer.from(content, "utf-8"); - - const lease1 = leaseFromContent(content); - const lease2 = leaseFromContent(buffer); - - expect(lease1).toBe(lease2); - }); - - it("should produce different leases for empty vs non-empty content", () => { - const lease1 = leaseFromContent(""); - const lease2 = leaseFromContent("a"); - - expect(lease1).not.toBe(lease2); - }); - - it("should produce identical lease for same content regardless of external factors", () => { - // This test verifies that content-based leases are immune to mtime changes - // that could be triggered by external processes (e.g., IDE, git, filesystem tools) - const content = "const x = 42;\n"; - const lease1 = leaseFromContent(content); - - // Simulate same content but different metadata (like mtime) - // In the old mtime-based system, this would produce a different lease - // With content-based leases, it produces the same lease - const lease2 = leaseFromContent(content); - - expect(lease1).toBe(lease2); - }); - }); describe("validateFileSize", () => { it("should return null for files within size limit", () => { diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 857956338a..1ae5b737ac 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -3,7 +3,6 @@ import * as fs from "fs/promises"; import * as path from "path"; import * as os from "os"; import { createFileEditInsertTool } from "./file_edit_insert"; -import { leaseFromContent } from "./fileCommon"; import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/types/tools"; import type { ToolCallOptions } from "ai"; @@ -33,15 +32,13 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 0, content: "INSERTED", - lease, + }; // Execute @@ -50,8 +47,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - expect(result.lease).not.toBe(lease); // New lease should be different } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -63,15 +58,13 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 1, content: "INSERTED", - lease, + }; // Execute @@ -80,7 +73,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -92,15 +84,13 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 2, content: "INSERTED", - lease, + }; // Execute @@ -109,7 +99,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -121,15 +110,13 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 3, content: "INSERTED", - lease, + }; // Execute @@ -138,7 +125,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -150,15 +136,13 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 1, content: "INSERTED1\nINSERTED2", - lease, + }; // Execute @@ -167,7 +151,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -178,15 +161,13 @@ describe("file_edit_insert tool", () => { // Setup await fs.writeFile(testFilePath, ""); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 0, content: "INSERTED", - lease, + }; // Execute @@ -195,7 +176,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -211,7 +191,6 @@ describe("file_edit_insert tool", () => { file_path: nonExistentPath, line_offset: 0, content: "INSERTED", - lease: "000000", // Doesn't matter, file doesn't exist }; // Execute @@ -229,15 +208,13 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: -1, content: "INSERTED", - lease, + }; // Execute @@ -255,15 +232,13 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 10, // File only has 2 lines content: "INSERTED", - lease, + }; // Execute @@ -275,62 +250,4 @@ describe("file_edit_insert tool", () => { expect(result.error).toContain("beyond file length"); } }); - - it("should reject edit with incorrect lease", async () => { - // Setup - const initialContent = "line1\nline2"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditInsertTool({ cwd: testDir }); - const args: FileEditInsertToolArgs = { - file_path: testFilePath, - line_offset: 1, - content: "INSERTED", - lease: "ffffff", // Incorrect lease - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("lease mismatch"); - expect(result.error).toContain("obtain a new lease from tool"); - } - - // File should remain unchanged - const content = await fs.readFile(testFilePath, "utf-8"); - expect(content).toBe(initialContent); - }); - - it("should detect file modified between read and insert", async () => { - // Setup - create initial file - const initialContent = "line1\nline2"; - await fs.writeFile(testFilePath, initialContent); - - // Get initial lease - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - - // Modify file to simulate concurrent edit - await fs.writeFile(testFilePath, "Modified content"); - - const tool = createFileEditInsertTool({ cwd: testDir }); - const args: FileEditInsertToolArgs = { - file_path: testFilePath, - line_offset: 1, - content: "INSERTED", - lease, // This lease is now stale - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("lease mismatch"); - } - }); }); diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 019ae33a0f..44f05f70cd 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -5,7 +5,7 @@ import writeFileAtomic from "write-file-atomic"; import type { FileEditInsertToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { leaseFromContent, generateDiff, validatePathInCwd, validateFileSize } from "./fileCommon"; +import { generateDiff, validatePathInCwd, validateFileSize } from "./fileCommon"; /** * File edit insert tool factory for AI assistant @@ -20,7 +20,6 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) file_path, line_offset, content, - lease, }): Promise => { try { // Validate that the path is within the working directory @@ -57,16 +56,6 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) // Read file content const originalContent = await fs.readFile(resolvedPath, { encoding: "utf-8" }); - - // Validate lease to prevent editing stale file state - // Use content-based lease to avoid mtime issues with external processes - const currentLease = leaseFromContent(originalContent); - if (currentLease !== lease) { - return { - success: false, - error: `WRITE DENIED: File lease mismatch. The file has been modified since it was read. Please obtain a new lease from tool \`file_read\`.`, - }; - } const lines = originalContent.split("\n"); // Validate line_offset @@ -93,15 +82,11 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) // Write the modified content back to file atomically await writeFileAtomic(resolvedPath, newContent, { encoding: "utf-8" }); - // Compute new lease from modified content - const newLease = leaseFromContent(newContent); - // Generate diff const diff = generateDiff(resolvedPath, originalContent, newContent); return { success: true, - lease: newLease, diff, }; } catch (error) { diff --git a/src/services/tools/file_edit_replace.test.ts b/src/services/tools/file_edit_replace.test.ts index de8c68e842..8cce4b50b3 100644 --- a/src/services/tools/file_edit_replace.test.ts +++ b/src/services/tools/file_edit_replace.test.ts @@ -3,7 +3,6 @@ import * as fs from "fs/promises"; import * as path from "path"; import * as os from "os"; import { createFileEditReplaceTool } from "./file_edit_replace"; -import { leaseFromContent } from "./fileCommon"; import type { FileEditReplaceToolArgs, FileEditReplaceToolResult } from "@/types/tools"; import type { ToolCallOptions } from "ai"; @@ -14,9 +13,8 @@ const mockToolCallOptions: ToolCallOptions = { }; // Test helpers -const setupFile = async (filePath: string, content: string): Promise => { +const setupFile = async (filePath: string, content: string): Promise => { await fs.writeFile(filePath, content); - return leaseFromContent(content); }; const readFile = async (filePath: string): Promise => { @@ -26,10 +24,9 @@ const readFile = async (filePath: string): Promise => { const executeReplace = async ( tool: ReturnType, filePath: string, - edits: FileEditReplaceToolArgs["edits"], - lease: string + edits: FileEditReplaceToolArgs["edits"] ): Promise => { - const args: FileEditReplaceToolArgs = { file_path: filePath, edits, lease }; + const args: FileEditReplaceToolArgs = { file_path: filePath, edits }; return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; }; @@ -49,28 +46,24 @@ describe("file_edit_replace tool", () => { }); it("should apply a single edit successfully", async () => { - const lease = await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); + await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); const tool = createFileEditReplaceTool({ cwd: testDir }); const result = await executeReplace( tool, testFilePath, - [{ old_string: "Hello world", new_string: "Hello universe" }], - lease - ); + [{ old_string: "Hello world", new_string: "Hello universe" }]); expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(1); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - expect(result.lease).not.toBe(lease); } expect(await readFile(testFilePath)).toBe("Hello universe\nThis is a test\nGoodbye world"); }); it("should apply multiple edits sequentially", async () => { - const lease = await setupFile(testFilePath, "foo bar baz"); + await setupFile(testFilePath, "foo bar baz"); const tool = createFileEditReplaceTool({ cwd: testDir }); const result = await executeReplace( @@ -80,14 +73,11 @@ describe("file_edit_replace tool", () => { { old_string: "foo", new_string: "FOO" }, { old_string: "bar", new_string: "BAR" }, { old_string: "baz", new_string: "BAZ" }, - ], - lease - ); + ]); expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(3); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } expect(await readFile(testFilePath)).toBe("FOO BAR BAZ"); @@ -101,8 +91,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -117,9 +105,7 @@ describe("file_edit_replace tool", () => { old_string: "foo", new_string: "qux", }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -144,8 +130,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -155,9 +139,7 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: -1, }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -166,7 +148,6 @@ describe("file_edit_replace tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(3); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -179,8 +160,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -190,9 +169,7 @@ describe("file_edit_replace tool", () => { new_string: "mouse", // replace_count omitted, defaults to 1 }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -201,7 +178,6 @@ describe("file_edit_replace tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(1); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -214,8 +190,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -224,9 +198,7 @@ describe("file_edit_replace tool", () => { old_string: "nonexistent", new_string: "replacement", }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -248,8 +220,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -259,9 +229,7 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: 1, // Explicitly set to 1 }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -285,8 +253,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -296,9 +262,7 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: 2, // Replace first 2 occurrences }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -307,7 +271,6 @@ describe("file_edit_replace tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(2); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -320,8 +283,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -331,9 +292,7 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: 5, // Only 1 occurrence exists }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -382,8 +341,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -392,9 +349,7 @@ describe("file_edit_replace tool", () => { old_string: "line2\nline3", new_string: "REPLACED", }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -403,7 +358,6 @@ describe("file_edit_replace tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(1); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -416,8 +370,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -426,9 +378,7 @@ describe("file_edit_replace tool", () => { old_string: "[DELETE_ME] ", new_string: "", }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -437,7 +387,6 @@ describe("file_edit_replace tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(1); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -450,8 +399,6 @@ describe("file_edit_replace tool", () => { await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -464,9 +411,7 @@ describe("file_edit_replace tool", () => { old_string: "step2", new_string: "step3", }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -475,87 +420,18 @@ describe("file_edit_replace tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(2); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); expect(updatedContent).toBe("step3"); }); - it("should reject edit with incorrect lease", async () => { - // Setup - const initialContent = "Hello world"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { - file_path: testFilePath, - edits: [ - { - old_string: "world", - new_string: "universe", - }, - ], - lease: "ffffff", // Incorrect lease - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; - - // Assert - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("lease mismatch"); - expect(result.error).toContain("obtain a new lease from tool"); - } - - // File should remain unchanged - const unchangedContent = await fs.readFile(testFilePath, "utf-8"); - expect(unchangedContent).toBe(initialContent); - }); - - it("should detect file modified between read and edit", async () => { - // Setup - create initial file - const initialContent = "Hello world"; - await fs.writeFile(testFilePath, initialContent); - - // Get initial lease - const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - - // Modify file to simulate concurrent edit - await fs.writeFile(testFilePath, "Modified content"); - - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { - file_path: testFilePath, - edits: [ - { - old_string: "world", - new_string: "universe", - }, - ], - lease, // This lease is now stale - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; - - // Assert - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("lease mismatch"); - } - }); - it("should return unified diff with context of 3", async () => { // Setup - create a file with multiple lines const initialContent = "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9"; await fs.writeFile(testFilePath, initialContent); const content = await fs.readFile(testFilePath, "utf-8"); - const lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -564,9 +440,7 @@ describe("file_edit_replace tool", () => { old_string: "line5", new_string: "LINE5_MODIFIED", }, - ], - lease, - }; + ] }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; diff --git a/src/services/tools/file_edit_replace.ts b/src/services/tools/file_edit_replace.ts index 7c161077f7..f0face101e 100644 --- a/src/services/tools/file_edit_replace.ts +++ b/src/services/tools/file_edit_replace.ts @@ -5,7 +5,7 @@ import writeFileAtomic from "write-file-atomic"; import type { FileEditReplaceToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { leaseFromContent, generateDiff, validatePathInCwd, validateFileSize } from "./fileCommon"; +import { generateDiff, validatePathInCwd, validateFileSize } from "./fileCommon"; /** * File edit replace tool factory for AI assistant @@ -17,7 +17,7 @@ export const createFileEditReplaceTool: ToolFactory = (config: ToolConfiguration description: TOOL_DEFINITIONS.file_edit_replace.description, inputSchema: TOOL_DEFINITIONS.file_edit_replace.schema, execute: async ( - { file_path, edits, lease }, + { file_path, edits }, { abortSignal: _abortSignal } ): Promise => { // Note: abortSignal available but not used - file operations are fast and complete quickly @@ -56,16 +56,6 @@ export const createFileEditReplaceTool: ToolFactory = (config: ToolConfiguration // Read file content const originalContent = await fs.readFile(resolvedPath, { encoding: "utf-8" }); - - // Validate lease to prevent editing stale file state - // Use content-based lease to avoid mtime issues with external processes - const currentLease = leaseFromContent(originalContent); - if (currentLease !== lease) { - return { - success: false, - error: `WRITE DENIED: File lease mismatch. The file has been modified since it was read. Please obtain a new lease from tool \`file_read\`.`, - }; - } let content = originalContent; // Apply each edit sequentially @@ -143,16 +133,12 @@ export const createFileEditReplaceTool: ToolFactory = (config: ToolConfiguration // Write the modified content back to file atomically await writeFileAtomic(resolvedPath, content, { encoding: "utf-8" }); - // Compute new lease from modified content - const newLease = leaseFromContent(content); - // Generate diff const diff = generateDiff(resolvedPath, originalContent, content); return { success: true, edits_applied: editsApplied, - lease: newLease, diff, }; } catch (error) { diff --git a/src/services/tools/file_read.test.ts b/src/services/tools/file_read.test.ts index a92f9c0907..ed8dab1479 100644 --- a/src/services/tools/file_read.test.ts +++ b/src/services/tools/file_read.test.ts @@ -45,9 +45,7 @@ describe("file_read tool", () => { if (result.success) { expect(result.lines_read).toBe(3); expect(result.content).toBe("1\tline one\n2\tline two\n3\tline three"); - expect(result.file_size).toBeGreaterThan(0); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(result.file_size).toBeGreaterThan(0); } }); it("should read file with offset", async () => { @@ -68,9 +66,7 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(3); - expect(result.content).toBe("3\tline3\n4\tline4\n5\tline5"); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(result.content).toBe("3\tline3\n4\tline4\n5\tline5"); } }); it("should read file with limit", async () => { @@ -91,9 +87,7 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(2); - expect(result.content).toBe("1\tline1\n2\tline2"); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(result.content).toBe("1\tline1\n2\tline2"); } }); it("should read file with offset and limit", async () => { @@ -115,9 +109,7 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(2); - expect(result.content).toBe("2\tline2\n3\tline3"); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(result.content).toBe("2\tline2\n3\tline3"); } }); it("should handle single line file", async () => { @@ -137,9 +129,7 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(1); - expect(result.content).toBe("1\tsingle line"); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(result.content).toBe("1\tsingle line"); } }); it("should handle empty file", async () => { @@ -158,9 +148,7 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(0); - expect(result.content).toBe(""); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(result.content).toBe(""); } }); it("should fail when file does not exist", async () => { @@ -225,9 +213,7 @@ describe("file_read tool", () => { expect(lines[0]).toBe("1\tshort line"); expect(lines[1]).toContain("... [truncated]"); expect(Buffer.byteLength(lines[1], "utf-8")).toBeLessThan(1100); // Should be around 1024 + prefix + truncation marker - expect(lines[2]).toBe("3\tanother short line"); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(lines[2]).toBe("3\tanother short line"); } }); it("should fail when reading more than 1000 lines", async () => { @@ -293,9 +279,7 @@ describe("file_read tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lines_read).toBe(500); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); - } + expect(result.lines_read).toBe(500); } }); it("should reject reading files outside cwd using ..", async () => { diff --git a/src/services/tools/file_read.ts b/src/services/tools/file_read.ts index d17bed5417..3c1227da63 100644 --- a/src/services/tools/file_read.ts +++ b/src/services/tools/file_read.ts @@ -4,7 +4,7 @@ import * as path from "path"; import type { FileReadToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { leaseFromContent, validatePathInCwd, validateFileSize } from "./fileCommon"; +import { validatePathInCwd, validateFileSize } from "./fileCommon"; /** * File read tool factory for AI assistant @@ -53,10 +53,8 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { }; } - // Read full file content once for both display and lease computation - // This ensures lease matches the exact content snapshot being returned + // Read full file content const fullContent = await fs.readFile(resolvedPath, { encoding: "utf-8" }); - const lease = leaseFromContent(fullContent); const startLineNumber = offset ?? 1; @@ -133,15 +131,12 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { const content = numberedLines.join("\n"); // Return file info and content - // IMPORTANT: lease must be last in the return object so it remains fresh in the LLM's context - // when it's reading this tool result. The LLM needs the lease value to perform subsequent edits. return { success: true, file_size: stats.size, modifiedTime: stats.mtime.toISOString(), lines_read: numberedLines.length, content, - lease, // Must be last - see comment above }; } catch (error) { // Handle specific errors diff --git a/src/types/tools.ts b/src/types/tools.ts index 7d52e4a02a..2bab15c9c7 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -44,7 +44,6 @@ export type FileReadToolResult = modifiedTime: string; lines_read: number; content: string; - lease: string; } | { success: false; @@ -61,14 +60,12 @@ export interface FileEditReplaceEdit { export interface FileEditReplaceToolArgs { file_path: string; edits: FileEditReplaceEdit[]; - lease: string; } export type FileEditReplaceToolResult = | { success: true; edits_applied: number; - lease: string; diff: string; } | { @@ -81,13 +78,11 @@ export interface FileEditInsertToolArgs { file_path: string; line_offset: number; // 1-indexed line position (0 = insert at top, N = insert after line N) content: string; - lease: string; } export type FileEditInsertToolResult = | { success: true; - lease: string; diff: string; } | { diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 14cd4094e7..55d82e3c0a 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -25,12 +25,7 @@ interface ToolSchema { }; } -/** - * Shared schema definitions - */ -const leaseSchema = z - .string() - .describe("The lease from the file_read result. Used to prevent edits on stale file state."); + /** * Tool definitions: single source of truth @@ -112,7 +107,6 @@ export const TOOL_DEFINITIONS = { ) .min(1) .describe("Array of edits to apply sequentially"), - lease: leaseSchema, }), }, file_edit_insert: { @@ -126,7 +120,6 @@ export const TOOL_DEFINITIONS = { .min(0) .describe("1-indexed line position (0 = insert at top, N = insert after line N)"), content: z.string().describe("The content to insert"), - lease: leaseSchema, }), }, propose_plan: { From 7728577a756356adf7083ed69f6ad2b272acd8ee Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 9 Oct 2025 21:55:59 -0500 Subject: [PATCH 2/3] Fix lint errors: remove empty blocks and unused variables --- src/services/tools/file_edit_insert.test.ts | 12 ------------ src/services/tools/file_edit_replace.test.ts | 12 ------------ 2 files changed, 24 deletions(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 1ae5b737ac..67a3b908d7 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -46,8 +46,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - if (result.success) { - } const updatedContent = await fs.readFile(testFilePath, "utf-8"); expect(updatedContent).toBe("INSERTED\nline1\nline2\nline3"); @@ -72,8 +70,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - if (result.success) { - } const updatedContent = await fs.readFile(testFilePath, "utf-8"); expect(updatedContent).toBe("line1\nINSERTED\nline2\nline3"); @@ -98,8 +94,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - if (result.success) { - } const updatedContent = await fs.readFile(testFilePath, "utf-8"); expect(updatedContent).toBe("line1\nline2\nINSERTED\nline3"); @@ -124,8 +118,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - if (result.success) { - } const updatedContent = await fs.readFile(testFilePath, "utf-8"); expect(updatedContent).toBe("line1\nline2\nline3\nINSERTED"); @@ -150,8 +142,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - if (result.success) { - } const updatedContent = await fs.readFile(testFilePath, "utf-8"); expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2"); @@ -175,8 +165,6 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - if (result.success) { - } const updatedContent = await fs.readFile(testFilePath, "utf-8"); expect(updatedContent).toBe("INSERTED\n"); diff --git a/src/services/tools/file_edit_replace.test.ts b/src/services/tools/file_edit_replace.test.ts index 8cce4b50b3..0a61d5b866 100644 --- a/src/services/tools/file_edit_replace.test.ts +++ b/src/services/tools/file_edit_replace.test.ts @@ -90,7 +90,6 @@ describe("file_edit_replace tool", () => { const initialContent = "foo bar baz"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -129,7 +128,6 @@ describe("file_edit_replace tool", () => { const initialContent = "cat dog cat bird cat"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -159,7 +157,6 @@ describe("file_edit_replace tool", () => { const initialContent = "cat dog bird"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -189,7 +186,6 @@ describe("file_edit_replace tool", () => { const initialContent = "Hello world"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -219,7 +215,6 @@ describe("file_edit_replace tool", () => { const initialContent = "cat dog cat bird cat"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -252,7 +247,6 @@ describe("file_edit_replace tool", () => { const initialContent = "cat dog cat bird cat"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -282,7 +276,6 @@ describe("file_edit_replace tool", () => { const initialContent = "cat dog bird"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -322,7 +315,6 @@ describe("file_edit_replace tool", () => { new_string: "bar", }, ], - lease: "000000", // Doesn't matter, file doesn't exist }; // Execute @@ -340,7 +332,6 @@ describe("file_edit_replace tool", () => { const initialContent = "line1\nline2\nline3\nline4"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -369,7 +360,6 @@ describe("file_edit_replace tool", () => { const initialContent = "Hello [DELETE_ME] world"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -398,7 +388,6 @@ describe("file_edit_replace tool", () => { const initialContent = "step1"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -431,7 +420,6 @@ describe("file_edit_replace tool", () => { const initialContent = "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9"; await fs.writeFile(testFilePath, initialContent); - const content = await fs.readFile(testFilePath, "utf-8"); const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, From 4ec39f3b6bc85598a620ad8a187529cb9ed75598 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 9 Oct 2025 21:57:41 -0500 Subject: [PATCH 3/3] Format code with Prettier --- src/services/tools/fileCommon.test.ts | 1 - src/services/tools/file_edit_insert.test.ts | 16 ------ src/services/tools/file_edit_insert.ts | 6 +-- src/services/tools/file_edit_replace.test.ts | 53 +++++++++++--------- src/services/tools/file_read.test.ts | 24 ++++++--- src/utils/tools/toolDefinitions.ts | 2 - 6 files changed, 47 insertions(+), 55 deletions(-) diff --git a/src/services/tools/fileCommon.test.ts b/src/services/tools/fileCommon.test.ts index 9cfd2ddbbd..983e48ed9a 100644 --- a/src/services/tools/fileCommon.test.ts +++ b/src/services/tools/fileCommon.test.ts @@ -3,7 +3,6 @@ import type * as fs from "fs"; import { validatePathInCwd, validateFileSize, MAX_FILE_SIZE } from "./fileCommon"; describe("fileCommon", () => { - describe("validateFileSize", () => { it("should return null for files within size limit", () => { const stats = { diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 67a3b908d7..85d6c969f0 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -32,13 +32,11 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 0, content: "INSERTED", - }; // Execute @@ -56,13 +54,11 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 1, content: "INSERTED", - }; // Execute @@ -80,13 +76,11 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 2, content: "INSERTED", - }; // Execute @@ -104,13 +98,11 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2\nline3"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 3, content: "INSERTED", - }; // Execute @@ -128,13 +120,11 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 1, content: "INSERTED1\nINSERTED2", - }; // Execute @@ -151,13 +141,11 @@ describe("file_edit_insert tool", () => { // Setup await fs.writeFile(testFilePath, ""); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 0, content: "INSERTED", - }; // Execute @@ -196,13 +184,11 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: -1, content: "INSERTED", - }; // Execute @@ -220,13 +206,11 @@ describe("file_edit_insert tool", () => { const initialContent = "line1\nline2"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditInsertTool({ cwd: testDir }); const args: FileEditInsertToolArgs = { file_path: testFilePath, line_offset: 10, // File only has 2 lines content: "INSERTED", - }; // Execute diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 44f05f70cd..e8d84d4897 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -16,11 +16,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) return tool({ description: TOOL_DEFINITIONS.file_edit_insert.description, inputSchema: TOOL_DEFINITIONS.file_edit_insert.schema, - execute: async ({ - file_path, - line_offset, - content, - }): Promise => { + execute: async ({ file_path, line_offset, content }): Promise => { try { // Validate that the path is within the working directory const pathValidation = validatePathInCwd(file_path, config.cwd); diff --git a/src/services/tools/file_edit_replace.test.ts b/src/services/tools/file_edit_replace.test.ts index 0a61d5b866..3ab3cb19be 100644 --- a/src/services/tools/file_edit_replace.test.ts +++ b/src/services/tools/file_edit_replace.test.ts @@ -49,10 +49,9 @@ describe("file_edit_replace tool", () => { await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); const tool = createFileEditReplaceTool({ cwd: testDir }); - const result = await executeReplace( - tool, - testFilePath, - [{ old_string: "Hello world", new_string: "Hello universe" }]); + const result = await executeReplace(tool, testFilePath, [ + { old_string: "Hello world", new_string: "Hello universe" }, + ]); expect(result.success).toBe(true); if (result.success) { @@ -66,14 +65,11 @@ describe("file_edit_replace tool", () => { await setupFile(testFilePath, "foo bar baz"); const tool = createFileEditReplaceTool({ cwd: testDir }); - const result = await executeReplace( - tool, - testFilePath, - [ - { old_string: "foo", new_string: "FOO" }, - { old_string: "bar", new_string: "BAR" }, - { old_string: "baz", new_string: "BAZ" }, - ]); + const result = await executeReplace(tool, testFilePath, [ + { old_string: "foo", new_string: "FOO" }, + { old_string: "bar", new_string: "BAR" }, + { old_string: "baz", new_string: "BAZ" }, + ]); expect(result.success).toBe(true); if (result.success) { @@ -104,7 +100,8 @@ describe("file_edit_replace tool", () => { old_string: "foo", new_string: "qux", }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -137,7 +134,8 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: -1, }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -166,7 +164,8 @@ describe("file_edit_replace tool", () => { new_string: "mouse", // replace_count omitted, defaults to 1 }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -194,7 +193,8 @@ describe("file_edit_replace tool", () => { old_string: "nonexistent", new_string: "replacement", }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -224,7 +224,8 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: 1, // Explicitly set to 1 }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -256,7 +257,8 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: 2, // Replace first 2 occurrences }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -285,7 +287,8 @@ describe("file_edit_replace tool", () => { new_string: "mouse", replace_count: 5, // Only 1 occurrence exists }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -340,7 +343,8 @@ describe("file_edit_replace tool", () => { old_string: "line2\nline3", new_string: "REPLACED", }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -368,7 +372,8 @@ describe("file_edit_replace tool", () => { old_string: "[DELETE_ME] ", new_string: "", }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -400,7 +405,8 @@ describe("file_edit_replace tool", () => { old_string: "step2", new_string: "step3", }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; @@ -428,7 +434,8 @@ describe("file_edit_replace tool", () => { old_string: "line5", new_string: "LINE5_MODIFIED", }, - ] }; + ], + }; // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; diff --git a/src/services/tools/file_read.test.ts b/src/services/tools/file_read.test.ts index ed8dab1479..03ba0a7256 100644 --- a/src/services/tools/file_read.test.ts +++ b/src/services/tools/file_read.test.ts @@ -45,7 +45,8 @@ describe("file_read tool", () => { if (result.success) { expect(result.lines_read).toBe(3); expect(result.content).toBe("1\tline one\n2\tline two\n3\tline three"); - expect(result.file_size).toBeGreaterThan(0); } + expect(result.file_size).toBeGreaterThan(0); + } }); it("should read file with offset", async () => { @@ -66,7 +67,8 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(3); - expect(result.content).toBe("3\tline3\n4\tline4\n5\tline5"); } + expect(result.content).toBe("3\tline3\n4\tline4\n5\tline5"); + } }); it("should read file with limit", async () => { @@ -87,7 +89,8 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(2); - expect(result.content).toBe("1\tline1\n2\tline2"); } + expect(result.content).toBe("1\tline1\n2\tline2"); + } }); it("should read file with offset and limit", async () => { @@ -109,7 +112,8 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(2); - expect(result.content).toBe("2\tline2\n3\tline3"); } + expect(result.content).toBe("2\tline2\n3\tline3"); + } }); it("should handle single line file", async () => { @@ -129,7 +133,8 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(1); - expect(result.content).toBe("1\tsingle line"); } + expect(result.content).toBe("1\tsingle line"); + } }); it("should handle empty file", async () => { @@ -148,7 +153,8 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(0); - expect(result.content).toBe(""); } + expect(result.content).toBe(""); + } }); it("should fail when file does not exist", async () => { @@ -213,7 +219,8 @@ describe("file_read tool", () => { expect(lines[0]).toBe("1\tshort line"); expect(lines[1]).toContain("... [truncated]"); expect(Buffer.byteLength(lines[1], "utf-8")).toBeLessThan(1100); // Should be around 1024 + prefix + truncation marker - expect(lines[2]).toBe("3\tanother short line"); } + expect(lines[2]).toBe("3\tanother short line"); + } }); it("should fail when reading more than 1000 lines", async () => { @@ -279,7 +286,8 @@ describe("file_read tool", () => { // Assert expect(result.success).toBe(true); if (result.success) { - expect(result.lines_read).toBe(500); } + expect(result.lines_read).toBe(500); + } }); it("should reject reading files outside cwd using ..", async () => { diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 55d82e3c0a..b9434346f0 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -25,8 +25,6 @@ interface ToolSchema { }; } - - /** * Tool definitions: single source of truth * Key = tool name, Value = { description, schema }