diff --git a/src/services/tools/fileCommon.test.ts b/src/services/tools/fileCommon.test.ts index 34fea621ed..983e48ed9a 100644 --- a/src/services/tools/fileCommon.test.ts +++ b/src/services/tools/fileCommon.test.ts @@ -1,75 +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", () => { const stats = { diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 857956338a..85d6c969f0 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,11 @@ 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 @@ -49,10 +44,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"); expect(updatedContent).toBe("INSERTED\nline1\nline2\nline3"); @@ -63,15 +54,11 @@ 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 @@ -79,9 +66,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"); expect(updatedContent).toBe("line1\nINSERTED\nline2\nline3"); @@ -92,15 +76,11 @@ 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 @@ -108,9 +88,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"); expect(updatedContent).toBe("line1\nline2\nINSERTED\nline3"); @@ -121,15 +98,11 @@ 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 @@ -137,9 +110,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"); expect(updatedContent).toBe("line1\nline2\nline3\nINSERTED"); @@ -150,15 +120,11 @@ 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 @@ -166,9 +132,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"); expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2"); @@ -178,15 +141,11 @@ 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 @@ -194,9 +153,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"); expect(updatedContent).toBe("INSERTED\n"); @@ -211,7 +167,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 +184,11 @@ 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 +206,11 @@ 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 +222,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..e8d84d4897 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 @@ -16,12 +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, - lease, - }): 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); @@ -57,16 +52,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 +78,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..3ab3cb19be 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,45 +46,34 @@ 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 - ); + const result = await executeReplace(tool, testFilePath, [ + { 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( - tool, - testFilePath, - [ - { old_string: "foo", new_string: "FOO" }, - { old_string: "bar", new_string: "BAR" }, - { old_string: "baz", new_string: "BAZ" }, - ], - lease - ); + 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) { expect(result.edits_applied).toBe(3); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } expect(await readFile(testFilePath)).toBe("FOO BAR BAZ"); @@ -100,9 +86,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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -118,7 +101,6 @@ describe("file_edit_replace tool", () => { new_string: "qux", }, ], - lease, }; // Execute @@ -143,9 +125,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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -156,7 +135,6 @@ describe("file_edit_replace tool", () => { replace_count: -1, }, ], - lease, }; // Execute @@ -166,7 +144,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"); @@ -178,9 +155,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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -191,7 +165,6 @@ describe("file_edit_replace tool", () => { // replace_count omitted, defaults to 1 }, ], - lease, }; // Execute @@ -201,7 +174,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"); @@ -213,9 +185,6 @@ describe("file_edit_replace tool", () => { const initialContent = "Hello world"; 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, @@ -225,7 +194,6 @@ describe("file_edit_replace tool", () => { new_string: "replacement", }, ], - lease, }; // Execute @@ -247,9 +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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -260,7 +225,6 @@ describe("file_edit_replace tool", () => { replace_count: 1, // Explicitly set to 1 }, ], - lease, }; // Execute @@ -284,9 +248,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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -297,7 +258,6 @@ describe("file_edit_replace tool", () => { replace_count: 2, // Replace first 2 occurrences }, ], - lease, }; // Execute @@ -307,7 +267,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"); @@ -319,9 +278,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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -332,7 +288,6 @@ describe("file_edit_replace tool", () => { replace_count: 5, // Only 1 occurrence exists }, ], - lease, }; // Execute @@ -363,7 +318,6 @@ describe("file_edit_replace tool", () => { new_string: "bar", }, ], - lease: "000000", // Doesn't matter, file doesn't exist }; // Execute @@ -381,9 +335,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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -393,7 +344,6 @@ describe("file_edit_replace tool", () => { new_string: "REPLACED", }, ], - lease, }; // Execute @@ -403,7 +353,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"); @@ -415,9 +364,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 lease = leaseFromContent(content); - const tool = createFileEditReplaceTool({ cwd: testDir }); const args: FileEditReplaceToolArgs = { file_path: testFilePath, @@ -427,7 +373,6 @@ describe("file_edit_replace tool", () => { new_string: "", }, ], - lease, }; // Execute @@ -437,7 +382,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"); @@ -449,9 +393,6 @@ describe("file_edit_replace tool", () => { const initialContent = "step1"; 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, @@ -465,7 +406,6 @@ describe("file_edit_replace tool", () => { new_string: "step3", }, ], - lease, }; // Execute @@ -475,87 +415,17 @@ 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, @@ -565,7 +435,6 @@ describe("file_edit_replace tool", () => { new_string: "LINE5_MODIFIED", }, ], - lease, }; // Execute 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..03ba0a7256 100644 --- a/src/services/tools/file_read.test.ts +++ b/src/services/tools/file_read.test.ts @@ -46,7 +46,6 @@ describe("file_read tool", () => { 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}$/); } }); @@ -69,7 +68,6 @@ describe("file_read tool", () => { 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}$/); } }); @@ -92,7 +90,6 @@ describe("file_read tool", () => { 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}$/); } }); @@ -116,7 +113,6 @@ describe("file_read tool", () => { 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}$/); } }); @@ -138,7 +134,6 @@ describe("file_read tool", () => { if (result.success) { expect(result.lines_read).toBe(1); expect(result.content).toBe("1\tsingle line"); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } }); @@ -159,7 +154,6 @@ describe("file_read tool", () => { if (result.success) { expect(result.lines_read).toBe(0); expect(result.content).toBe(""); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } }); @@ -226,7 +220,6 @@ describe("file_read tool", () => { 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}$/); } }); @@ -294,7 +287,6 @@ describe("file_read tool", () => { expect(result.success).toBe(true); if (result.success) { expect(result.lines_read).toBe(500); - expect(result.lease).toMatch(/^[0-9a-f]{6}$/); } }); 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..b9434346f0 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -25,13 +25,6 @@ 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 * Key = tool name, Value = { description, schema } @@ -112,7 +105,6 @@ export const TOOL_DEFINITIONS = { ) .min(1) .describe("Array of edits to apply sequentially"), - lease: leaseSchema, }), }, file_edit_insert: { @@ -126,7 +118,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: {