Skip to content

🤖 Remove lease concept from file operations #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ammario merged 3 commits into from
Oct 10, 2025
Merged

🤖 Remove lease concept from file operations #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16ad58e
🤖 Remove lease concept from file operations
ammar-agent Oct 10, 2025
7728577
Fix lint errors: remove empty blocks and unused variables
ammar-agent Oct 10, 2025
4ec39f3
Format code with Prettier
ammar-agent Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 1 addition & 68 deletions src/services/tools/fileCommon.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down
111 changes: 0 additions & 111 deletions src/services/tools/file_edit_insert.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -33,26 +32,18 @@ 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
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;

// 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");
Expand All @@ -63,25 +54,18 @@ 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
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;

// 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");
Expand All @@ -92,25 +76,18 @@ 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
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;

// 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");
Expand All @@ -121,25 +98,18 @@ 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
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;

// 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");
Expand All @@ -150,25 +120,18 @@ 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
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;

// 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");
Expand All @@ -178,25 +141,18 @@ 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
const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult;

// 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");
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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");
}
});
});
Loading