Disable checkpoint if nested git repos are detected (#4509)

Author: cte

src/core/assistant-message/presentAssistantMessage.ts

@@ -51,7 +51,7 @@ import { codebaseSearchTool } from "../tools/codebaseSearchTool"
 
 export async function presentAssistantMessage(cline: Task) {
 	if (cline.abort) {
-		throw new Error(`[Cline#presentAssistantMessage] task ${cline.taskId}.${cline.instanceId} aborted`)
+		throw new Error(`[Task#presentAssistantMessage] task ${cline.taskId}.${cline.instanceId} aborted`)
 	}
 
 	if (cline.presentAssistantMessageLocked) {

src/core/checkpoints/index.ts

@@ -24,7 +24,7 @@ export function getCheckpointService(cline: Task) {
 	}
 
 	if (cline.checkpointServiceInitializing) {
-		console.log("[Cline#getCheckpointService] checkpoint service is still initializing")
+		console.log("[Task#getCheckpointService] checkpoint service is still initializing")
 		return undefined
 	}
 
@@ -40,21 +40,21 @@ export function getCheckpointService(cline: Task) {
 		}
 	}
 
-	console.log("[Cline#getCheckpointService] initializing checkpoints service")
+	console.log("[Task#getCheckpointService] initializing checkpoints service")
 
 	try {
 		const workspaceDir = getWorkspacePath()
 
 		if (!workspaceDir) {
-			log("[Cline#getCheckpointService] workspace folder not found, disabling checkpoints")
+			log("[Task#getCheckpointService] workspace folder not found, disabling checkpoints")
 			cline.enableCheckpoints = false
 			return undefined
 		}
 
 		const globalStorageDir = provider?.context.globalStorageUri.fsPath
 
 		if (!globalStorageDir) {
-			log("[Cline#getCheckpointService] globalStorageDir not found, disabling checkpoints")
+			log("[Task#getCheckpointService] globalStorageDir not found, disabling checkpoints")
 			cline.enableCheckpoints = false
 			return undefined
 		}
@@ -71,7 +71,7 @@ export function getCheckpointService(cline: Task) {
 		cline.checkpointServiceInitializing = true
 
 		service.on("initialize", () => {
-			log("[Cline#getCheckpointService] service initialized")
+			log("[Task#getCheckpointService] service initialized")
 
 			try {
 				const isCheckpointNeeded =
@@ -81,11 +81,11 @@ export function getCheckpointService(cline: Task) {
 				cline.checkpointServiceInitializing = false
 
 				if (isCheckpointNeeded) {
-					log("[Cline#getCheckpointService] no checkpoints found, saving initial checkpoint")
+					log("[Task#getCheckpointService] no checkpoints found, saving initial checkpoint")
 					checkpointSave(cline)
 				}
 			} catch (err) {
-				log("[Cline#getCheckpointService] caught error in on('initialize'), disabling checkpoints")
+				log("[Task#getCheckpointService] caught error in on('initialize'), disabling checkpoints")
 				cline.enableCheckpoints = false
 			}
 		})
@@ -99,30 +99,26 @@ export function getCheckpointService(cline: Task) {
 						isNonInteractive: true,
 					})
 					.catch((err) => {
-						log("[Cline#getCheckpointService] caught unexpected error in say('checkpoint_saved')")
+						log("[Task#getCheckpointService] caught unexpected error in say('checkpoint_saved')")
 						console.error(err)
 					})
 			} catch (err) {
-				log("[Cline#getCheckpointService] caught unexpected error in on('checkpoint'), disabling checkpoints")
+				log("[Task#getCheckpointService] caught unexpected error in on('checkpoint'), disabling checkpoints")
 				console.error(err)
 				cline.enableCheckpoints = false
 			}
 		})
 
-		log("[Cline#getCheckpointService] initializing shadow git")
+		log("[Task#getCheckpointService] initializing shadow git")
 
 		service.initShadowGit().catch((err) => {
-			log(
-				`[Cline#getCheckpointService] caught unexpected error in initShadowGit, disabling checkpoints (${err.message})`,
-			)
-
-			console.error(err)
+			log(`[Task#getCheckpointService] initShadowGit -> ${err.message}`)
 			cline.enableCheckpoints = false
 		})
 
 		return service
 	} catch (err) {
-		log("[Cline#getCheckpointService] caught unexpected error, disabling checkpoints")
+		log(`[Task#getCheckpointService] ${err.message}`)
 		cline.enableCheckpoints = false
 		return undefined
 	}
@@ -141,7 +137,7 @@ async function getInitializedCheckpointService(
 	try {
 		await pWaitFor(
 			() => {
-				console.log("[Cline#getCheckpointService] waiting for service to initialize")
+				console.log("[Task#getCheckpointService] waiting for service to initialize")
 				return service.isInitialized
 			},
 			{ interval, timeout },
@@ -171,7 +167,7 @@ export async function checkpointSave(cline: Task, force = false) {
 
 	// Start the checkpoint process in the background.
 	return service.saveCheckpoint(`Task: ${cline.taskId}, Time: ${Date.now()}`, { allowEmpty: force }).catch((err) => {
-		console.error("[Cline#checkpointSave] caught unexpected error, disabling checkpoints", err)
+		console.error("[Task#checkpointSave] caught unexpected error, disabling checkpoints", err)
 		cline.enableCheckpoints = false
 	})
 }

src/core/environment/getEnvironmentDetails.ts

@@ -153,7 +153,7 @@ export async function getEnvironmentDetails(cline: Task, includeFileDetails: boo
 		}
 	}
 
-	// console.log(`[Cline#getEnvironmentDetails] terminalDetails: ${terminalDetails}`)
+	// console.log(`[Task#getEnvironmentDetails] terminalDetails: ${terminalDetails}`)
 
 	// Add recently modified files section.
 	const recentlyModifiedFiles = cline.fileContextTracker.getAndClearRecentlyModifiedFiles()

src/services/checkpoints/ShadowCheckpointService.ts

@@ -10,7 +10,6 @@ import pWaitFor from "p-wait-for"
 import { fileExistsAtPath } from "../../utils/fs"
 import { executeRipgrep } from "../../services/search/file-search"
 
-import { GIT_DISABLED_SUFFIX } from "./constants"
 import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types"
 import { getExcludePatterns } from "./excludes"
 
@@ -65,6 +64,15 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 			throw new Error("Shadow git repo already initialized")
 		}
 
+		const hasNestedGitRepos = await this.hasNestedGitRepositories()
+
+		if (hasNestedGitRepos) {
+			throw new Error(
+				"Checkpoints are disabled because nested git repositories were detected in the workspace. " +
+					"Please remove or relocate nested git repositories to use the checkpoints feature.",
+			)
+		}
+
 		await fs.mkdir(this.checkpointsDir, { recursive: true })
 		const git = simpleGit(this.checkpointsDir)
 		const gitVersion = await git.version()
@@ -132,71 +140,43 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 	}
 
 	private async stageAll(git: SimpleGit) {
-		await this.renameNestedGitRepos(true)
-
 		try {
 			await git.add(".")
 		} catch (error) {
 			this.log(
 				`[${this.constructor.name}#stageAll] failed to add files to git: ${error instanceof Error ? error.message : String(error)}`,
 			)
-		} finally {
-			await this.renameNestedGitRepos(false)
 		}
 	}
 
-	// Since we use git to track checkpoints, we need to temporarily disable
-	// nested git repos to work around git's requirement of using submodules for
-	// nested repos.
-	private async renameNestedGitRepos(disable: boolean) {
+	private async hasNestedGitRepositories(): Promise<boolean> {
 		try {
 			// Find all .git directories that are not at the root level.
-			const gitDir = ".git" + (disable ? "" : GIT_DISABLED_SUFFIX)
-			const args = ["--files", "--hidden", "--follow", "-g", `**/${gitDir}/HEAD`, this.workspaceDir]
-
-			const gitPaths = await (
-				await executeRipgrep({ args, workspacePath: this.workspaceDir })
-			).filter(({ type, path }) => type === "folder" && path.includes(".git") && !path.startsWith(".git"))
-
-			// For each nested .git directory, rename it based on operation.
-			for (const gitPath of gitPaths) {
-				if (gitPath.path.startsWith(".git")) {
-					continue
-				}
-
-				const currentPath = path.join(this.workspaceDir, gitPath.path)
-				let newPath: string
-
-				if (disable) {
-					newPath = !currentPath.endsWith(GIT_DISABLED_SUFFIX)
-						? currentPath + GIT_DISABLED_SUFFIX
-						: currentPath
-				} else {
-					newPath = currentPath.endsWith(GIT_DISABLED_SUFFIX)
-						? currentPath.slice(0, -GIT_DISABLED_SUFFIX.length)
-						: currentPath
-				}
+			const args = ["--files", "--hidden", "--follow", "-g", "**/.git/HEAD", this.workspaceDir]
 
-				if (currentPath === newPath) {
-					continue
-				}
+			const gitPaths = await executeRipgrep({ args, workspacePath: this.workspaceDir })
 
-				try {
-					await fs.rename(currentPath, newPath)
+			// Filter to only include nested git directories (not the root .git).
+			const nestedGitPaths = gitPaths.filter(
+				({ type, path }) =>
+					type === "folder" && path.includes(".git") && !path.startsWith(".git") && path !== ".git",
+			)
 
-					this.log(
-						`[${this.constructor.name}#renameNestedGitRepos] ${disable ? "disabled" : "enabled"} nested git repo ${currentPath}`,
-					)
-				} catch (error) {
-					this.log(
-						`[${this.constructor.name}#renameNestedGitRepos] failed to ${disable ? "disable" : "enable"} nested git repo ${currentPath}: ${error instanceof Error ? error.message : String(error)}`,
-					)
-				}
+			if (nestedGitPaths.length > 0) {
+				this.log(
+					`[${this.constructor.name}#hasNestedGitRepositories] found ${nestedGitPaths.length} nested git repositories: ${nestedGitPaths.map((p) => p.path).join(", ")}`,
+				)
+				return true
 			}
+
+			return false
 		} catch (error) {
 			this.log(
-				`[${this.constructor.name}#renameNestedGitRepos] failed to ${disable ? "disable" : "enable"} nested git repos: ${error instanceof Error ? error.message : String(error)}`,
+				`[${this.constructor.name}#hasNestedGitRepositories] failed to check for nested git repos: ${error instanceof Error ? error.message : String(error)}`,
 			)
+
+			// If we can't check, assume there are no nested repos to avoid blocking the feature.
+			return false
 		}
 	}
 

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

@@ -380,8 +380,8 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
 			})
 		})
 
-		describe(`${klass.name}#renameNestedGitRepos`, () => {
-			it("handles nested git repositories during initialization", async () => {
+		describe(`${klass.name}#hasNestedGitRepositories`, () => {
+			it("throws error when nested git repositories are detected 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()}`)
@@ -417,11 +417,7 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
 				const nestedGitDir = path.join(nestedRepoPath, ".git")
 				const headFile = path.join(nestedGitDir, "HEAD")
 				await fs.writeFile(headFile, "HEAD")
-				const nestedGitDisabledDir = `${nestedGitDir}_disabled`
 				expect(await fileExistsAtPath(nestedGitDir)).toBe(true)
-				expect(await fileExistsAtPath(nestedGitDisabledDir)).toBe(false)
-
-				const renameSpy = jest.spyOn(fs, "rename")
 
 				jest.spyOn(fileSearch, "executeRipgrep").mockImplementation(({ args }) => {
 					const searchPattern = args[4]
@@ -440,29 +436,48 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
 				})
 
 				const service = new klass(taskId, shadowDir, workspaceDir, () => {})
-				await service.initShadowGit()
 
-				// Verify rename was called with correct paths.
-				expect(renameSpy.mock.calls).toHaveLength(1)
-				expect(renameSpy.mock.calls[0][0]).toBe(nestedGitDir)
-				expect(renameSpy.mock.calls[0][1]).toBe(nestedGitDisabledDir)
+				// Verify that initialization throws an error when nested git repos are detected
+				await expect(service.initShadowGit()).rejects.toThrow(
+					"Checkpoints are disabled because nested git repositories were detected in the workspace",
+				)
 
-				jest.spyOn(require("../../../utils/fs"), "fileExistsAtPath").mockImplementation((path) => {
-					if (path === nestedGitDir) {
-						return Promise.resolve(true)
-					} else if (path === nestedGitDisabledDir) {
-						return Promise.resolve(false)
-					}
+				// Clean up.
+				jest.restoreAllMocks()
+				await fs.rm(shadowDir, { recursive: true, force: true })
+				await fs.rm(workspaceDir, { recursive: true, force: true })
+			})
+
+			it("succeeds when no nested git repositories are detected", async () => {
+				// Create a new temporary workspace and service for this test.
+				const shadowDir = path.join(tmpDir, `${prefix}-no-nested-git-${Date.now()}`)
+				const workspaceDir = path.join(tmpDir, `workspace-no-nested-git-${Date.now()}`)
+
+				// Create a primary workspace repo without any nested repos.
+				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]")
 
-					return Promise.resolve(false)
+				// 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")
+
+				jest.spyOn(fileSearch, "executeRipgrep").mockImplementation(() => {
+					// Return empty array to simulate no nested git repos found
+					return Promise.resolve([])
 				})
 
-				// Verify the nested git directory is back to normal after initialization.
-				expect(await fileExistsAtPath(nestedGitDir)).toBe(true)
-				expect(await fileExistsAtPath(nestedGitDisabledDir)).toBe(false)
+				const service = new klass(taskId, shadowDir, workspaceDir, () => {})
+
+				// Verify that initialization succeeds when no nested git repos are detected
+				await expect(service.initShadowGit()).resolves.not.toThrow()
+				expect(service.isInitialized).toBe(true)
 
 				// Clean up.
-				renameSpy.mockRestore()
 				jest.restoreAllMocks()
 				await fs.rm(shadowDir, { recursive: true, force: true })
 				await fs.rm(workspaceDir, { recursive: true, force: true })

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

@@ -6,7 +6,6 @@ import { join } from "path"
 import { fileExistsAtPath } from "../../../utils/fs"
 
 import { getExcludePatterns } from "../excludes"
-import { GIT_DISABLED_SUFFIX } from "../constants"
 
 jest.mock("fs/promises")
 
@@ -54,7 +53,6 @@ readme.md text
 
 			// Verify all normal patterns also exist
 			expect(excludePatterns).toContain(".git/")
-			expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
 		})
 
 		it("should handle .gitattributes with no LFS patterns", async () => {
@@ -89,7 +87,6 @@ readme.md text
 
 			// Verify default patterns are included
 			expect(excludePatterns).toContain(".git/")
-			expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
 		})
 
 		it("should handle missing .gitattributes file", async () => {
@@ -107,7 +104,6 @@ readme.md text
 
 			// Verify standard patterns are included
 			expect(excludePatterns).toContain(".git/")
-			expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
 
 			// Verify we have standard patterns but no LFS patterns
 			// Check for a few known patterns from different categories
@@ -139,7 +135,6 @@ readme.md text
 
 			// Verify standard patterns are included
 			expect(excludePatterns).toContain(".git/")
-			expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
 
 			// Verify we have standard patterns but no LFS patterns
 			// Check for a few known patterns from different categories

src/services/checkpoints/constants.ts

@@ -3,8 +3,6 @@ import { join } from "path"
 
 import { fileExistsAtPath } from "../../utils/fs"
 
-import { GIT_DISABLED_SUFFIX } from "./constants"
-
 const getBuildArtifactPatterns = () => [
 	".gradle/",
 	".idea/",
@@ -200,7 +198,6 @@ const getLfsPatterns = async (workspacePath: string) => {
 
 export const getExcludePatterns = async (workspacePath: string) => [
 	".git/",
-	`.git${GIT_DISABLED_SUFFIX}/`,
 	...getBuildArtifactPatterns(),
 	...getMediaFilePatterns(),
 	...getCacheFilePatterns(),