Skip to content

Commit 3e1cbbf

Browse files
committed
fix(checkpoints): isolate shadow repo operations from
GIT_DIR environment Fixes issue where checkpoint commits were written to the wrong repository when GIT_DIR environment variable was set in Dev Containers. Problem: When developers use VS Code Dev Containers with remoteEnv setting GIT_DIR, Roo's checkpoint commits would land in the GIT_DIR location instead of the shadow repository, causing unintended commits and confusion. Root cause: The simple-git library inherits the parent process environment, and git gives precedence to GIT_DIR over local .git directories. Even though the shadow repo was correctly initialized with core.worktree, the inherited GIT_DIR environment variable overrode repository detection. Solution: - Created createSanitizedGit() function that explicitly unsets git-related environment variables (GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, etc.) before creating SimpleGit instances for checkpoint operations - Applied sanitization at both shadow repo creation and cleanup operations - Added logging to help debug environment issues in Dev Containers - Environment isolation is scoped only to checkpoint operations, preserving other workflows that may rely on GIT_DIR Testing: Added comprehensive test that simulates GIT_DIR scenario and verifies: - Commits don't leak to external repositories - Checkpoint save/restore operations work correctly - Environment state is properly cleaned up
1 parent 97331bc commit 3e1cbbf

File tree

2 files changed

+125
-3
lines changed

2 files changed

+125
-3
lines changed

src/services/checkpoints/ShadowCheckpointService.ts

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as path from "path"
44
import crypto from "crypto"
55
import EventEmitter from "events"
66

7-
import simpleGit, { SimpleGit } from "simple-git"
7+
import simpleGit, { SimpleGit, SimpleGitOptions } from "simple-git"
88
import pWaitFor from "p-wait-for"
99
import * as vscode from "vscode"
1010

@@ -15,6 +15,65 @@ import { t } from "../../i18n"
1515
import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types"
1616
import { getExcludePatterns } from "./excludes"
1717

18+
/**
19+
* Creates a SimpleGit instance with sanitized environment variables to prevent
20+
* interference from inherited git environment variables like GIT_DIR and GIT_WORK_TREE.
21+
* This ensures checkpoint operations always target the intended shadow repository.
22+
*
23+
* @param baseDir - The directory where git operations should be executed
24+
* @returns A SimpleGit instance with sanitized environment
25+
*/
26+
function createSanitizedGit(baseDir: string): SimpleGit {
27+
// Create a clean environment by explicitly unsetting git-related environment variables
28+
// that could interfere with checkpoint operations
29+
const sanitizedEnv: Record<string, string> = {}
30+
const removedVars: string[] = []
31+
32+
// Copy all environment variables except git-specific ones
33+
for (const [key, value] of Object.entries(process.env)) {
34+
// Skip git environment variables that would override repository location
35+
if (
36+
key === "GIT_DIR" ||
37+
key === "GIT_WORK_TREE" ||
38+
key === "GIT_INDEX_FILE" ||
39+
key === "GIT_OBJECT_DIRECTORY" ||
40+
key === "GIT_ALTERNATE_OBJECT_DIRECTORIES" ||
41+
key === "GIT_CEILING_DIRECTORIES"
42+
) {
43+
removedVars.push(`${key}=${value}`)
44+
continue
45+
}
46+
47+
// Only include defined values
48+
if (value !== undefined) {
49+
sanitizedEnv[key] = value
50+
}
51+
}
52+
53+
// Log which git env vars were removed (helps with debugging Dev Container issues)
54+
if (removedVars.length > 0) {
55+
console.log(
56+
`[createSanitizedGit] Removed git environment variables for checkpoint isolation: ${removedVars.join(", ")}`,
57+
)
58+
}
59+
60+
const options: Partial<SimpleGitOptions> = {
61+
baseDir,
62+
config: [],
63+
}
64+
65+
// Create git instance and set the sanitized environment
66+
const git = simpleGit(options)
67+
68+
// Use the .env() method to set the complete sanitized environment
69+
// This replaces the inherited environment with our sanitized version
70+
git.env(sanitizedEnv)
71+
72+
console.log(`[createSanitizedGit] Created git instance for baseDir: ${baseDir}`)
73+
74+
return git
75+
}
76+
1877
export abstract class ShadowCheckpointService extends EventEmitter {
1978
public readonly taskId: string
2079
public readonly checkpointsDir: string
@@ -85,7 +144,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
85144
}
86145

87146
await fs.mkdir(this.checkpointsDir, { recursive: true })
88-
const git = simpleGit(this.checkpointsDir)
147+
const git = createSanitizedGit(this.checkpointsDir)
89148
const gitVersion = await git.version()
90149
this.log(`[${this.constructor.name}#create] git = ${gitVersion}`)
91150

@@ -391,7 +450,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
391450
}) {
392451
const workspaceRepoDir = this.workspaceRepoDir({ globalStorageDir, workspaceDir })
393452
const branchName = `roo-${taskId}`
394-
const git = simpleGit(workspaceRepoDir)
453+
const git = createSanitizedGit(workspaceRepoDir)
395454
const success = await this.deleteBranch(git, branchName)
396455

397456
if (success) {

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,69 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
823823
// File should be back to original state
824824
expect(await fs.readFile(testFile, "utf-8")).toBe("Hello, world!")
825825
})
826+
827+
it("isolates checkpoint operations from GIT_DIR environment variable", async () => {
828+
// This test verifies the fix for the issue where GIT_DIR environment variable
829+
// causes checkpoint commits to go to the wrong repository
830+
831+
// Create a separate git directory to simulate GIT_DIR pointing elsewhere
832+
const externalGitDir = path.join(tmpDir, `external-git-${Date.now()}`)
833+
await fs.mkdir(externalGitDir, { recursive: true })
834+
const externalGit = simpleGit(externalGitDir)
835+
await externalGit.init()
836+
await externalGit.addConfig("user.name", "External User")
837+
await externalGit.addConfig("user.email", "[email protected]")
838+
839+
// Create and commit a file in the external repo
840+
const externalFile = path.join(externalGitDir, "external.txt")
841+
await fs.writeFile(externalFile, "External content")
842+
await externalGit.add(".")
843+
await externalGit.commit("External commit")
844+
845+
// Store the original commit count in the external repo
846+
const externalLogBefore = await externalGit.log()
847+
const externalCommitCountBefore = externalLogBefore.total
848+
849+
// Set GIT_DIR to point to the external repository
850+
const originalGitDir = process.env.GIT_DIR
851+
const externalDotGit = path.join(externalGitDir, ".git")
852+
process.env.GIT_DIR = externalDotGit
853+
854+
try {
855+
// Make a change in the workspace and save a checkpoint
856+
await fs.writeFile(testFile, "Modified with GIT_DIR set")
857+
const commit = await service.saveCheckpoint("Checkpoint with GIT_DIR set")
858+
expect(commit?.commit).toBeTruthy()
859+
860+
// Verify the checkpoint was saved in the shadow repo, not the external repo
861+
const externalLogAfter = await externalGit.log()
862+
const externalCommitCountAfter = externalLogAfter.total
863+
864+
// External repo should have the same number of commits (no new commits)
865+
expect(externalCommitCountAfter).toBe(externalCommitCountBefore)
866+
867+
// Verify the checkpoint is accessible in the shadow repo
868+
const diff = await service.getDiff({ to: commit!.commit })
869+
expect(diff).toHaveLength(1)
870+
expect(diff[0].paths.relative).toBe("test.txt")
871+
expect(diff[0].content.after).toBe("Modified with GIT_DIR set")
872+
873+
// Verify we can restore the checkpoint
874+
await fs.writeFile(testFile, "Another modification")
875+
await service.restoreCheckpoint(commit!.commit)
876+
expect(await fs.readFile(testFile, "utf-8")).toBe("Modified with GIT_DIR set")
877+
} finally {
878+
// Restore original GIT_DIR
879+
if (originalGitDir !== undefined) {
880+
process.env.GIT_DIR = originalGitDir
881+
} else {
882+
delete process.env.GIT_DIR
883+
}
884+
885+
// Clean up external git directory
886+
await fs.rm(externalGitDir, { recursive: true, force: true })
887+
}
888+
})
826889
})
827890
},
828891
)

0 commit comments

Comments
 (0)