Skip to content

Commit 4f3c123

Browse files
authored
🤖 Remove lease concept from file operations (#136)
Leases were leading to many errors and forcing models to re-read files repeatedly, which drains context unnecessarily. ## Problem The lease system added complexity and friction: - Models had to track lease values across tool calls - Any external file modification (IDE, git, etc.) invalidated leases - Failed operations required re-reading files to get new leases - This created error loops that consumed context tokens rapidly ## Solution Remove the lease concept entirely: - File operations no longer require or return leases - Tools read files directly when needed for editing - Simpler API with fewer failure modes - Models often fallback to `bash` + `sed` anyway when tool errors occur ## Changes - ✅ Remove `lease` parameter from `file_edit_replace` and `file_edit_insert` - ✅ Remove `lease` from `file_read` results - ✅ Remove lease validation logic from all file operations - ✅ Update type definitions - ✅ Update tests (all 87 tests passing) ## Why This Works This tool environment is designed for isolated changes: - Each workspace is a separate git worktree - Changes are contained and easily reviewable - Models can use `bash` for complex file operations if needed - Reduces cognitive overhead for both models and developers _Generated with `cmux`_
1 parent 9932d8b commit 4f3c123

File tree

9 files changed

+20
-389
lines changed

9 files changed

+20
-389
lines changed

src/services/tools/fileCommon.test.ts

Lines changed: 1 addition & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,8 @@
11
import { describe, it, expect } from "bun:test";
22
import type * as fs from "fs";
3-
import { leaseFromContent, validatePathInCwd, validateFileSize, MAX_FILE_SIZE } from "./fileCommon";
3+
import { validatePathInCwd, validateFileSize, MAX_FILE_SIZE } from "./fileCommon";
44

55
describe("fileCommon", () => {
6-
describe("leaseFromContent", () => {
7-
it("should return a 6-character hexadecimal string", () => {
8-
const content = "Hello, world!";
9-
const lease = leaseFromContent(content);
10-
11-
expect(lease).toMatch(/^[0-9a-f]{6}$/);
12-
expect(lease.length).toBe(6);
13-
});
14-
15-
it("should be deterministic for same content", () => {
16-
const content = "Hello, world!";
17-
const lease1 = leaseFromContent(content);
18-
const lease2 = leaseFromContent(content);
19-
20-
expect(lease1).toBe(lease2);
21-
});
22-
23-
it("should produce different leases for different content", () => {
24-
const content1 = "Hello, world!";
25-
const content2 = "Hello, world!!";
26-
27-
const lease1 = leaseFromContent(content1);
28-
const lease2 = leaseFromContent(content2);
29-
30-
expect(lease1).not.toBe(lease2);
31-
});
32-
33-
it("should work with Buffer input", () => {
34-
const buffer = Buffer.from("Hello, world!", "utf-8");
35-
const lease = leaseFromContent(buffer);
36-
37-
expect(lease).toMatch(/^[0-9a-f]{6}$/);
38-
expect(lease.length).toBe(6);
39-
});
40-
41-
it("should produce same lease for string and equivalent Buffer", () => {
42-
const content = "Hello, world!";
43-
const buffer = Buffer.from(content, "utf-8");
44-
45-
const lease1 = leaseFromContent(content);
46-
const lease2 = leaseFromContent(buffer);
47-
48-
expect(lease1).toBe(lease2);
49-
});
50-
51-
it("should produce different leases for empty vs non-empty content", () => {
52-
const lease1 = leaseFromContent("");
53-
const lease2 = leaseFromContent("a");
54-
55-
expect(lease1).not.toBe(lease2);
56-
});
57-
58-
it("should produce identical lease for same content regardless of external factors", () => {
59-
// This test verifies that content-based leases are immune to mtime changes
60-
// that could be triggered by external processes (e.g., IDE, git, filesystem tools)
61-
const content = "const x = 42;\n";
62-
const lease1 = leaseFromContent(content);
63-
64-
// Simulate same content but different metadata (like mtime)
65-
// In the old mtime-based system, this would produce a different lease
66-
// With content-based leases, it produces the same lease
67-
const lease2 = leaseFromContent(content);
68-
69-
expect(lease1).toBe(lease2);
70-
});
71-
});
72-
736
describe("validateFileSize", () => {
747
it("should return null for files within size limit", () => {
758
const stats = {

src/services/tools/file_edit_insert.test.ts

Lines changed: 0 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import * as fs from "fs/promises";
33
import * as path from "path";
44
import * as os from "os";
55
import { createFileEditInsertTool } from "./file_edit_insert";
6-
import { leaseFromContent } from "./fileCommon";
76
import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/types/tools";
87
import type { ToolCallOptions } from "ai";
98

@@ -33,26 +32,18 @@ describe("file_edit_insert tool", () => {
3332
const initialContent = "line1\nline2\nline3";
3433
await fs.writeFile(testFilePath, initialContent);
3534

36-
const content = await fs.readFile(testFilePath, "utf-8");
37-
const lease = leaseFromContent(content);
38-
3935
const tool = createFileEditInsertTool({ cwd: testDir });
4036
const args: FileEditInsertToolArgs = {
4137
file_path: testFilePath,
4238
line_offset: 0,
4339
content: "INSERTED",
44-
lease,
4540
};
4641

4742
// Execute
4843
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
4944

5045
// Assert
5146
expect(result.success).toBe(true);
52-
if (result.success) {
53-
expect(result.lease).toMatch(/^[0-9a-f]{6}$/);
54-
expect(result.lease).not.toBe(lease); // New lease should be different
55-
}
5647

5748
const updatedContent = await fs.readFile(testFilePath, "utf-8");
5849
expect(updatedContent).toBe("INSERTED\nline1\nline2\nline3");
@@ -63,25 +54,18 @@ describe("file_edit_insert tool", () => {
6354
const initialContent = "line1\nline2\nline3";
6455
await fs.writeFile(testFilePath, initialContent);
6556

66-
const content = await fs.readFile(testFilePath, "utf-8");
67-
const lease = leaseFromContent(content);
68-
6957
const tool = createFileEditInsertTool({ cwd: testDir });
7058
const args: FileEditInsertToolArgs = {
7159
file_path: testFilePath,
7260
line_offset: 1,
7361
content: "INSERTED",
74-
lease,
7562
};
7663

7764
// Execute
7865
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
7966

8067
// Assert
8168
expect(result.success).toBe(true);
82-
if (result.success) {
83-
expect(result.lease).toMatch(/^[0-9a-f]{6}$/);
84-
}
8569

8670
const updatedContent = await fs.readFile(testFilePath, "utf-8");
8771
expect(updatedContent).toBe("line1\nINSERTED\nline2\nline3");
@@ -92,25 +76,18 @@ describe("file_edit_insert tool", () => {
9276
const initialContent = "line1\nline2\nline3";
9377
await fs.writeFile(testFilePath, initialContent);
9478

95-
const content = await fs.readFile(testFilePath, "utf-8");
96-
const lease = leaseFromContent(content);
97-
9879
const tool = createFileEditInsertTool({ cwd: testDir });
9980
const args: FileEditInsertToolArgs = {
10081
file_path: testFilePath,
10182
line_offset: 2,
10283
content: "INSERTED",
103-
lease,
10484
};
10585

10686
// Execute
10787
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
10888

10989
// Assert
11090
expect(result.success).toBe(true);
111-
if (result.success) {
112-
expect(result.lease).toMatch(/^[0-9a-f]{6}$/);
113-
}
11491

11592
const updatedContent = await fs.readFile(testFilePath, "utf-8");
11693
expect(updatedContent).toBe("line1\nline2\nINSERTED\nline3");
@@ -121,25 +98,18 @@ describe("file_edit_insert tool", () => {
12198
const initialContent = "line1\nline2\nline3";
12299
await fs.writeFile(testFilePath, initialContent);
123100

124-
const content = await fs.readFile(testFilePath, "utf-8");
125-
const lease = leaseFromContent(content);
126-
127101
const tool = createFileEditInsertTool({ cwd: testDir });
128102
const args: FileEditInsertToolArgs = {
129103
file_path: testFilePath,
130104
line_offset: 3,
131105
content: "INSERTED",
132-
lease,
133106
};
134107

135108
// Execute
136109
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
137110

138111
// Assert
139112
expect(result.success).toBe(true);
140-
if (result.success) {
141-
expect(result.lease).toMatch(/^[0-9a-f]{6}$/);
142-
}
143113

144114
const updatedContent = await fs.readFile(testFilePath, "utf-8");
145115
expect(updatedContent).toBe("line1\nline2\nline3\nINSERTED");
@@ -150,25 +120,18 @@ describe("file_edit_insert tool", () => {
150120
const initialContent = "line1\nline2";
151121
await fs.writeFile(testFilePath, initialContent);
152122

153-
const content = await fs.readFile(testFilePath, "utf-8");
154-
const lease = leaseFromContent(content);
155-
156123
const tool = createFileEditInsertTool({ cwd: testDir });
157124
const args: FileEditInsertToolArgs = {
158125
file_path: testFilePath,
159126
line_offset: 1,
160127
content: "INSERTED1\nINSERTED2",
161-
lease,
162128
};
163129

164130
// Execute
165131
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
166132

167133
// Assert
168134
expect(result.success).toBe(true);
169-
if (result.success) {
170-
expect(result.lease).toMatch(/^[0-9a-f]{6}$/);
171-
}
172135

173136
const updatedContent = await fs.readFile(testFilePath, "utf-8");
174137
expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2");
@@ -178,25 +141,18 @@ describe("file_edit_insert tool", () => {
178141
// Setup
179142
await fs.writeFile(testFilePath, "");
180143

181-
const content = await fs.readFile(testFilePath, "utf-8");
182-
const lease = leaseFromContent(content);
183-
184144
const tool = createFileEditInsertTool({ cwd: testDir });
185145
const args: FileEditInsertToolArgs = {
186146
file_path: testFilePath,
187147
line_offset: 0,
188148
content: "INSERTED",
189-
lease,
190149
};
191150

192151
// Execute
193152
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
194153

195154
// Assert
196155
expect(result.success).toBe(true);
197-
if (result.success) {
198-
expect(result.lease).toMatch(/^[0-9a-f]{6}$/);
199-
}
200156

201157
const updatedContent = await fs.readFile(testFilePath, "utf-8");
202158
expect(updatedContent).toBe("INSERTED\n");
@@ -211,7 +167,6 @@ describe("file_edit_insert tool", () => {
211167
file_path: nonExistentPath,
212168
line_offset: 0,
213169
content: "INSERTED",
214-
lease: "000000", // Doesn't matter, file doesn't exist
215170
};
216171

217172
// Execute
@@ -229,15 +184,11 @@ describe("file_edit_insert tool", () => {
229184
const initialContent = "line1\nline2";
230185
await fs.writeFile(testFilePath, initialContent);
231186

232-
const content = await fs.readFile(testFilePath, "utf-8");
233-
const lease = leaseFromContent(content);
234-
235187
const tool = createFileEditInsertTool({ cwd: testDir });
236188
const args: FileEditInsertToolArgs = {
237189
file_path: testFilePath,
238190
line_offset: -1,
239191
content: "INSERTED",
240-
lease,
241192
};
242193

243194
// Execute
@@ -255,15 +206,11 @@ describe("file_edit_insert tool", () => {
255206
const initialContent = "line1\nline2";
256207
await fs.writeFile(testFilePath, initialContent);
257208

258-
const content = await fs.readFile(testFilePath, "utf-8");
259-
const lease = leaseFromContent(content);
260-
261209
const tool = createFileEditInsertTool({ cwd: testDir });
262210
const args: FileEditInsertToolArgs = {
263211
file_path: testFilePath,
264212
line_offset: 10, // File only has 2 lines
265213
content: "INSERTED",
266-
lease,
267214
};
268215

269216
// Execute
@@ -275,62 +222,4 @@ describe("file_edit_insert tool", () => {
275222
expect(result.error).toContain("beyond file length");
276223
}
277224
});
278-
279-
it("should reject edit with incorrect lease", async () => {
280-
// Setup
281-
const initialContent = "line1\nline2";
282-
await fs.writeFile(testFilePath, initialContent);
283-
284-
const tool = createFileEditInsertTool({ cwd: testDir });
285-
const args: FileEditInsertToolArgs = {
286-
file_path: testFilePath,
287-
line_offset: 1,
288-
content: "INSERTED",
289-
lease: "ffffff", // Incorrect lease
290-
};
291-
292-
// Execute
293-
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
294-
295-
// Assert
296-
expect(result.success).toBe(false);
297-
if (!result.success) {
298-
expect(result.error).toContain("lease mismatch");
299-
expect(result.error).toContain("obtain a new lease from tool");
300-
}
301-
302-
// File should remain unchanged
303-
const content = await fs.readFile(testFilePath, "utf-8");
304-
expect(content).toBe(initialContent);
305-
});
306-
307-
it("should detect file modified between read and insert", async () => {
308-
// Setup - create initial file
309-
const initialContent = "line1\nline2";
310-
await fs.writeFile(testFilePath, initialContent);
311-
312-
// Get initial lease
313-
const content = await fs.readFile(testFilePath, "utf-8");
314-
const lease = leaseFromContent(content);
315-
316-
// Modify file to simulate concurrent edit
317-
await fs.writeFile(testFilePath, "Modified content");
318-
319-
const tool = createFileEditInsertTool({ cwd: testDir });
320-
const args: FileEditInsertToolArgs = {
321-
file_path: testFilePath,
322-
line_offset: 1,
323-
content: "INSERTED",
324-
lease, // This lease is now stale
325-
};
326-
327-
// Execute
328-
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;
329-
330-
// Assert
331-
expect(result.success).toBe(false);
332-
if (!result.success) {
333-
expect(result.error).toContain("lease mismatch");
334-
}
335-
});
336225
});

0 commit comments

Comments
 (0)