Improve file excludes for checkpoints service

Author: cte

src/services/checkpoints/ShadowCheckpointService.ts

@@ -6,8 +6,11 @@ import EventEmitter from "events"
 import simpleGit, { SimpleGit } from "simple-git"
 import { globby } from "globby"
 
-import { GIT_DISABLED_SUFFIX, GIT_EXCLUDES } from "./constants"
+import { fileExistsAtPath } from "../../utils/fs"
+
+import { GIT_DISABLED_SUFFIX } from "./constants"
 import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types"
+import { getExcludePatterns } from "./excludes"
 
 export abstract class ShadowCheckpointService extends EventEmitter {
 	public readonly taskId: string
@@ -65,12 +68,6 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 		const gitVersion = await git.version()
 		this.log(`[${this.constructor.name}#create] git = ${gitVersion}`)
 
-		const fileExistsAtPath = (path: string) =>
-			fs
-				.access(path)
-				.then(() => true)
-				.catch(() => false)
-
 		let created = false
 		const startTime = Date.now()
 
@@ -84,48 +81,24 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 				)
 			}
 
+			await this.writeExcludeFile()
 			this.baseHash = await git.revparse(["HEAD"])
 		} else {
 			this.log(`[${this.constructor.name}#initShadowGit] creating shadow git repo at ${this.checkpointsDir}`)
-
 			await git.init()
 			await git.addConfig("core.worktree", this.workspaceDir) // Sets the working tree to the current workspace.
 			await git.addConfig("commit.gpgSign", "false") // Disable commit signing for shadow repo.
 			await git.addConfig("user.name", "Roo Code")
 			await git.addConfig("user.email", "[email protected]")
-
-			let lfsPatterns: string[] = [] // Get LFS patterns from workspace if they exist.
-
-			try {
-				const attributesPath = path.join(this.workspaceDir, ".gitattributes")
-
-				if (await fileExistsAtPath(attributesPath)) {
-					lfsPatterns = (await fs.readFile(attributesPath, "utf8"))
-						.split("\n")
-						.filter((line) => line.includes("filter=lfs"))
-						.map((line) => line.split(" ")[0].trim())
-				}
-			} catch (error) {
-				this.log(
-					`[${this.constructor.name}#initShadowGit] failed to read .gitattributes: ${error instanceof Error ? error.message : String(error)}`,
-				)
-			}
-
-			// Add basic excludes directly in git config, while respecting any
-			// .gitignore in the workspace.
-			// .git/info/exclude is local to the shadow git repo, so it's not
-			// shared with the main repo - and won't conflict with user's
-			// .gitignore.
-			await fs.mkdir(path.join(this.dotGitDir, "info"), { recursive: true })
-			const excludesPath = path.join(this.dotGitDir, "info", "exclude")
-			await fs.writeFile(excludesPath, [...GIT_EXCLUDES, ...lfsPatterns].join("\n"))
+			await this.writeExcludeFile()
 			await this.stageAll(git)
 			const { commit } = await git.commit("initial commit", { "--allow-empty": null })
 			this.baseHash = commit
 			created = true
 		}
 
 		const duration = Date.now() - startTime
+
 		this.log(
 			`[${this.constructor.name}#initShadowGit] initialized shadow repo with base commit ${this.baseHash} in ${duration}ms`,
 		)
@@ -145,8 +118,18 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 		return { created, duration }
 	}
 
+	// Add basic excludes directly in git config, while respecting any
+	// .gitignore in the workspace.
+	// .git/info/exclude is local to the shadow git repo, so it's not
+	// shared with the main repo - and won't conflict with user's
+	// .gitignore.
+	protected async writeExcludeFile() {
+		await fs.mkdir(path.join(this.dotGitDir, "info"), { recursive: true })
+		const patterns = await getExcludePatterns(this.workspaceDir)
+		await fs.writeFile(path.join(this.dotGitDir, "info", "exclude"), patterns.join("\n"))
+	}
+
 	private async stageAll(git: SimpleGit) {
-		// await writeExcludesFile(gitPath, await getLfsPatterns(this.cwd)).
 		await this.renameNestedGitRepos(true)
 
 		try {
@@ -188,6 +171,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 
 			try {
 				await fs.rename(fullPath, newPath)
+
 				this.log(
 					`[${this.constructor.name}#renameNestedGitRepos] ${disable ? "disabled" : "enabled"} nested git repo ${gitPath}`,
 				)

src/services/checkpoints/__tests__/ShadowCheckpointService.test.ts

@@ -7,6 +7,7 @@ import { EventEmitter } from "events"
 
 import { simpleGit, SimpleGit } from "simple-git"
 
+import { fileExistsAtPath } from "../../../utils/fs"
 import { RepoPerTaskCheckpointService } from "../RepoPerTaskCheckpointService"
 import { RepoPerWorkspaceCheckpointService } from "../RepoPerWorkspaceCheckpointService"
 
@@ -16,7 +17,7 @@ jest.mock("globby", () => ({
 
 const tmpDir = path.join(os.tmpdir(), "CheckpointService")
 
-const initRepo = async ({
+const initWorkspaceRepo = async ({
 	workspaceDir,
 	userName = "Roo Code",
 	userEmail = "[email protected]",
@@ -64,7 +65,7 @@ describe.each([
 
 		const shadowDir = path.join(tmpDir, `${prefix}-${Date.now()}`)
 		const workspaceDir = path.join(tmpDir, `workspace-${Date.now()}`)
-		const repo = await initRepo({ workspaceDir })
+		const repo = await initWorkspaceRepo({ workspaceDir })
 
 		workspaceGit = repo.git
 		testFile = repo.testFile
@@ -298,6 +299,52 @@ describe.each([
 			await expect(fs.readFile(testFile, "utf-8")).rejects.toThrow()
 			await expect(fs.readFile(untrackedFile, "utf-8")).rejects.toThrow()
 		})
+
+		it("does not create a checkpoint for ignored files", async () => {
+			// Create a file that matches an ignored pattern (e.g., .log file).
+			const ignoredFile = path.join(service.workspaceDir, "ignored.log")
+			await fs.writeFile(ignoredFile, "Initial ignored content")
+
+			const commit = await service.saveCheckpoint("Ignored file checkpoint")
+			expect(commit?.commit).toBeFalsy()
+
+			await fs.writeFile(ignoredFile, "Modified ignored content")
+
+			const commit2 = await service.saveCheckpoint("Ignored file modified checkpoint")
+			expect(commit2?.commit).toBeFalsy()
+
+			expect(await fs.readFile(ignoredFile, "utf-8")).toBe("Modified ignored content")
+		})
+
+		it("does not create a checkpoint for LFS files", async () => {
+			// Create a .gitattributes file with LFS patterns.
+			const gitattributesPath = path.join(service.workspaceDir, ".gitattributes")
+			await fs.writeFile(gitattributesPath, "*.lfs filter=lfs diff=lfs merge=lfs -text")
+
+			// Re-initialize the service to trigger a write to .git/info/exclude.
+			service = new klass(service.taskId, service.checkpointsDir, service.workspaceDir, () => {})
+			const excludesPath = path.join(service.checkpointsDir, ".git", "info", "exclude")
+			expect((await fs.readFile(excludesPath, "utf-8")).split("\n")).not.toContain("*.lfs")
+			await service.initShadowGit()
+			expect((await fs.readFile(excludesPath, "utf-8")).split("\n")).toContain("*.lfs")
+
+			const commit0 = await service.saveCheckpoint("Add gitattributes")
+			expect(commit0?.commit).toBeTruthy()
+
+			// Create a file that matches an LFS pattern.
+			const lfsFile = path.join(service.workspaceDir, "foo.lfs")
+			await fs.writeFile(lfsFile, "Binary file content simulation")
+
+			const commit = await service.saveCheckpoint("LFS file checkpoint")
+			expect(commit?.commit).toBeFalsy()
+
+			await fs.writeFile(lfsFile, "Modified binary content")
+
+			const commit2 = await service.saveCheckpoint("LFS file modified checkpoint")
+			expect(commit2?.commit).toBeFalsy()
+
+			expect(await fs.readFile(lfsFile, "utf-8")).toBe("Modified binary content")
+		})
 	})
 
 	describe(`${klass.name}#create`, () => {
@@ -337,6 +384,106 @@ describe.each([
 		})
 	})
 
+	describe(`${klass.name}#renameNestedGitRepos`, () => {
+		it("handles nested git repositories during initialization", async () => {
+			// Create a new temporary workspace and service for this test.
+			const shadowDir = path.join(tmpDir, `${prefix}-nested-git-${Date.now()}`)
+			const workspaceDir = path.join(tmpDir, `workspace-nested-git-${Date.now()}`)
+
+			// Create a primary workspace repo.
+			await fs.mkdir(workspaceDir, { recursive: true })
+			const mainGit = simpleGit(workspaceDir)
+			await mainGit.init()
+			await mainGit.addConfig("user.name", "Roo Code")
+			await mainGit.addConfig("user.email", "[email protected]")
+
+			// Create a nested repo inside the workspace.
+			const nestedRepoPath = path.join(workspaceDir, "nested-project")
+			await fs.mkdir(nestedRepoPath, { recursive: true })
+			const nestedGit = simpleGit(nestedRepoPath)
+			await nestedGit.init()
+			await nestedGit.addConfig("user.name", "Roo Code")
+			await nestedGit.addConfig("user.email", "[email protected]")
+
+			// Add a file to the nested repo.
+			const nestedFile = path.join(nestedRepoPath, "nested-file.txt")
+			await fs.writeFile(nestedFile, "Content in nested repo")
+			await nestedGit.add(".")
+			await nestedGit.commit("Initial commit in nested repo")
+
+			// Create a test file in the main workspace.
+			const mainFile = path.join(workspaceDir, "main-file.txt")
+			await fs.writeFile(mainFile, "Content in main repo")
+			await mainGit.add(".")
+			await mainGit.commit("Initial commit in main repo")
+
+			// Confirm nested git directory exists before initialization.
+			const nestedGitDir = path.join(nestedRepoPath, ".git")
+			const nestedGitDisabledDir = `${nestedGitDir}_disabled`
+			expect(await fileExistsAtPath(nestedGitDir)).toBe(true)
+			expect(await fileExistsAtPath(nestedGitDisabledDir)).toBe(false)
+
+			// Configure globby mock to return our nested git repository.
+			const relativeGitPath = path.relative(workspaceDir, nestedGitDir)
+
+			jest.mocked(require("globby").globby).mockImplementation((pattern: string | string[]) => {
+				if (pattern === "**/.git") {
+					return Promise.resolve([relativeGitPath])
+				} else if (pattern === "**/.git_disabled") {
+					return Promise.resolve([`${relativeGitPath}_disabled`])
+				}
+
+				return Promise.resolve([])
+			})
+
+			// Create a spy on fs.rename to track when it's called.
+			const renameSpy = jest.spyOn(fs, "rename")
+
+			// Initialize the shadow git service.
+			const service = new klass(taskId, shadowDir, workspaceDir, () => {})
+
+			// Override renameNestedGitRepos to track calls.
+			const originalRenameMethod = service["renameNestedGitRepos"].bind(service)
+			let disableCall = false
+			let enableCall = false
+
+			service["renameNestedGitRepos"] = async (disable: boolean) => {
+				if (disable) {
+					disableCall = true
+				} else {
+					enableCall = true
+				}
+
+				return originalRenameMethod(disable)
+			}
+
+			// Initialize the shadow git repo.
+			await service.initShadowGit()
+
+			// Verify both disable and enable were called.
+			expect(disableCall).toBe(true)
+			expect(enableCall).toBe(true)
+
+			// Verify rename was called with correct paths.
+			const renameCallsArgs = renameSpy.mock.calls.map((call) => call[0] + " -> " + call[1])
+			expect(
+				renameCallsArgs.some((args) => args.includes(nestedGitDir) && args.includes(nestedGitDisabledDir)),
+			).toBe(true)
+			expect(
+				renameCallsArgs.some((args) => args.includes(nestedGitDisabledDir) && args.includes(nestedGitDir)),
+			).toBe(true)
+
+			// Verify the nested git directory is back to normal after initialization.
+			expect(await fileExistsAtPath(nestedGitDir)).toBe(true)
+			expect(await fileExistsAtPath(nestedGitDisabledDir)).toBe(false)
+
+			// Clean up.
+			renameSpy.mockRestore()
+			await fs.rm(shadowDir, { recursive: true, force: true })
+			await fs.rm(workspaceDir, { recursive: true, force: true })
+		})
+	})
+
 	describe(`${klass.name}#events`, () => {
 		it("emits initialize event when service is created", async () => {
 			const shadowDir = path.join(tmpDir, `${prefix}3-${Date.now()}`)