fix(checkpoints): resolve incorrect commit location when GIT_DIR set in Dev Containers

src/services/checkpoints/ShadowCheckpointService.ts

@@ -4,7 +4,7 @@ import * as path from "path"
 import crypto from "crypto"
 import EventEmitter from "events"
 
-import simpleGit, { SimpleGit } from "simple-git"
+import simpleGit, { SimpleGit, SimpleGitOptions } from "simple-git"
 import pWaitFor from "p-wait-for"
 import * as vscode from "vscode"
 
@@ -15,6 +15,65 @@ import { t } from "../../i18n"
 import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types"
 import { getExcludePatterns } from "./excludes"
 
+/**
+ * Creates a SimpleGit instance with sanitized environment variables to prevent
+ * interference from inherited git environment variables like GIT_DIR and GIT_WORK_TREE.
+ * This ensures checkpoint operations always target the intended shadow repository.
+ *
+ * @param baseDir - The directory where git operations should be executed
+ * @returns A SimpleGit instance with sanitized environment
+ */
+function createSanitizedGit(baseDir: string): SimpleGit {
+	// Create a clean environment by explicitly unsetting git-related environment variables
+	// that could interfere with checkpoint operations
+	const sanitizedEnv: Record<string, string> = {}
+	const removedVars: string[] = []
+
+	// Copy all environment variables except git-specific ones
+	for (const [key, value] of Object.entries(process.env)) {
+		// Skip git environment variables that would override repository location
+		if (
+			key === "GIT_DIR" ||
+			key === "GIT_WORK_TREE" ||
+			key === "GIT_INDEX_FILE" ||
+			key === "GIT_OBJECT_DIRECTORY" ||
+			key === "GIT_ALTERNATE_OBJECT_DIRECTORIES" ||
+			key === "GIT_CEILING_DIRECTORIES"
+		) {
+			removedVars.push(`${key}=${value}`)
+			continue
+		}
+
+		// Only include defined values
+		if (value !== undefined) {
+			sanitizedEnv[key] = value
+		}
+	}
+
+	// Log which git env vars were removed (helps with debugging Dev Container issues)
+	if (removedVars.length > 0) {
+		console.log(
+			`[createSanitizedGit] Removed git environment variables for checkpoint isolation: ${removedVars.join(", ")}`,
+		)
+	}
+
+	const options: Partial<SimpleGitOptions> = {
+		baseDir,
+		config: [],
+	}
+
+	// Create git instance and set the sanitized environment
+	const git = simpleGit(options)
+
+	// Use the .env() method to set the complete sanitized environment
+	// This replaces the inherited environment with our sanitized version
+	git.env(sanitizedEnv)
+
+	console.log(`[createSanitizedGit] Created git instance for baseDir: ${baseDir}`)
+
+	return git
+}
+
 export abstract class ShadowCheckpointService extends EventEmitter {
 	public readonly taskId: string
 	public readonly checkpointsDir: string
@@ -85,7 +144,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 		}
 
 		await fs.mkdir(this.checkpointsDir, { recursive: true })
-		const git = simpleGit(this.checkpointsDir)
+		const git = createSanitizedGit(this.checkpointsDir)
 		const gitVersion = await git.version()
 		this.log(`[${this.constructor.name}#create] git = ${gitVersion}`)
 
@@ -391,7 +450,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
 	}) {
 		const workspaceRepoDir = this.workspaceRepoDir({ globalStorageDir, workspaceDir })
 		const branchName = `roo-${taskId}`
-		const git = simpleGit(workspaceRepoDir)
+		const git = createSanitizedGit(workspaceRepoDir)
 		const success = await this.deleteBranch(git, branchName)
 
 		if (success) {

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

@@ -823,6 +823,69 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
 				// File should be back to original state
 				expect(await fs.readFile(testFile, "utf-8")).toBe("Hello, world!")
 			})
+
+			it("isolates checkpoint operations from GIT_DIR environment variable", async () => {
+				// This test verifies the fix for the issue where GIT_DIR environment variable
+				// causes checkpoint commits to go to the wrong repository
+
+				// Create a separate git directory to simulate GIT_DIR pointing elsewhere
+				const externalGitDir = path.join(tmpDir, `external-git-${Date.now()}`)
+				await fs.mkdir(externalGitDir, { recursive: true })
+				const externalGit = simpleGit(externalGitDir)
+				await externalGit.init()
+				await externalGit.addConfig("user.name", "External User")
+				await externalGit.addConfig("user.email", "external@example.com")
+
+				// Create and commit a file in the external repo
+				const externalFile = path.join(externalGitDir, "external.txt")
+				await fs.writeFile(externalFile, "External content")
+				await externalGit.add(".")
+				await externalGit.commit("External commit")
+
+				// Store the original commit count in the external repo
+				const externalLogBefore = await externalGit.log()
+				const externalCommitCountBefore = externalLogBefore.total
+
+				// Set GIT_DIR to point to the external repository
+				const originalGitDir = process.env.GIT_DIR
+				const externalDotGit = path.join(externalGitDir, ".git")
+				process.env.GIT_DIR = externalDotGit
+
+				try {
+					// Make a change in the workspace and save a checkpoint
+					await fs.writeFile(testFile, "Modified with GIT_DIR set")
+					const commit = await service.saveCheckpoint("Checkpoint with GIT_DIR set")
+					expect(commit?.commit).toBeTruthy()
+
+					// Verify the checkpoint was saved in the shadow repo, not the external repo
+					const externalLogAfter = await externalGit.log()
+					const externalCommitCountAfter = externalLogAfter.total
+
+					// External repo should have the same number of commits (no new commits)
+					expect(externalCommitCountAfter).toBe(externalCommitCountBefore)
+
+					// Verify the checkpoint is accessible in the shadow repo
+					const diff = await service.getDiff({ to: commit!.commit })
+					expect(diff).toHaveLength(1)
+					expect(diff[0].paths.relative).toBe("test.txt")
+					expect(diff[0].content.after).toBe("Modified with GIT_DIR set")
+
+					// Verify we can restore the checkpoint
+					await fs.writeFile(testFile, "Another modification")
+					await service.restoreCheckpoint(commit!.commit)
+					expect(await fs.readFile(testFile, "utf-8")).toBe("Modified with GIT_DIR set")
+				} finally {
+					// Restore original GIT_DIR
+					if (originalGitDir !== undefined) {
+						process.env.GIT_DIR = originalGitDir
+					} else {
+						delete process.env.GIT_DIR
+					}
+
+					// Clean up external git directory
+					await fs.rm(externalGitDir, { recursive: true, force: true })
+				}
+			})
 		})
 	},
 )