Skip to content

Commit b02bf48

Browse files
committed
fix: handle nested git repos gracefully in checkpoints
Instead of blocking checkpoints when nested git repos are detected, now excludes them from staging to prevent submodule-related errors. Changes: - Removed initialization-time nested repo check that blocked feature - Added findNestedRepos() to detect repos from multiple sources: * .gitmodules (declared submodules) * git index (gitlinks with mode 160000) * filesystem (.git directories and worktrees) - Modified stageAll() to exclude nested repos using pathspec - Added safety check to prevent submodule changes in staging - Updated tests to verify nested repos are excluded from checkpoints This allows checkpoints to work in monorepos and projects with submodules while preventing git errors from nested repo changes.
1 parent b92a22b commit b02bf48

File tree

2 files changed

+142
-63
lines changed

2 files changed

+142
-63
lines changed

src/services/checkpoints/ShadowCheckpointService.ts

Lines changed: 103 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import * as vscode from "vscode"
1010

1111
import { fileExistsAtPath } from "../../utils/fs"
1212
import { executeRipgrep } from "../../services/search/file-search"
13-
import { t } from "../../i18n"
1413

1514
import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types"
1615
import { getExcludePatterns } from "./excludes"
@@ -70,20 +69,6 @@ export abstract class ShadowCheckpointService extends EventEmitter {
7069
throw new Error("Shadow git repo already initialized")
7170
}
7271

73-
const nestedGitPath = await this.getNestedGitRepository()
74-
75-
if (nestedGitPath) {
76-
// Show persistent error message with the offending path
77-
const relativePath = path.relative(this.workspaceDir, nestedGitPath)
78-
const message = t("common:errors.nested_git_repos_warning", { path: relativePath })
79-
vscode.window.showErrorMessage(message)
80-
81-
throw new Error(
82-
`Checkpoints are disabled because a nested git repository was detected at: ${relativePath}. ` +
83-
"Please remove or relocate nested git repositories to use the checkpoints feature.",
84-
)
85-
}
86-
8772
await fs.mkdir(this.checkpointsDir, { recursive: true })
8873
const git = simpleGit(this.checkpointsDir)
8974
const gitVersion = await git.version()
@@ -152,63 +137,126 @@ export abstract class ShadowCheckpointService extends EventEmitter {
152137

153138
private async stageAll(git: SimpleGit) {
154139
try {
155-
await git.add([".", "--ignore-errors"])
140+
// Find all nested repos to exclude
141+
const nestedRepos = await this.findNestedRepos(git)
142+
143+
if (nestedRepos.length > 0) {
144+
this.log(
145+
`[${this.constructor.name}#stageAll] excluding ${nestedRepos.length} nested repos: ${nestedRepos.join(", ")}`,
146+
)
147+
}
148+
149+
// Build add command with pathspec excludes
150+
const addArgs: string[] = ["-A", ":/"]
151+
for (const repoPath of nestedRepos) {
152+
addArgs.push(`:(exclude)${repoPath}/`)
153+
}
154+
155+
// Stage files
156+
await git.add(addArgs)
157+
158+
// Safety check: ensure no submodule changes staged
159+
const diffSummary = await git.raw(["diff", "--cached", "--summary"])
160+
if (diffSummary.includes("Submodule")) {
161+
throw new Error("Submodule changes detected in staging area - this should not happen")
162+
}
156163
} catch (error) {
157164
this.log(
158165
`[${this.constructor.name}#stageAll] failed to add files to git: ${error instanceof Error ? error.message : String(error)}`,
159166
)
167+
throw error
160168
}
161169
}
162170

163-
private async getNestedGitRepository(): Promise<string | null> {
164-
try {
165-
// Find all .git/HEAD files that are not at the root level.
166-
const args = ["--files", "--hidden", "--follow", "-g", "**/.git/HEAD", this.workspaceDir]
167-
168-
const gitPaths = await executeRipgrep({ args, workspacePath: this.workspaceDir })
169-
170-
// Filter to only include nested git directories (not the root .git).
171-
// Since we're searching for HEAD files, we expect type to be "file"
172-
const nestedGitPaths = gitPaths.filter(({ type, path: filePath }) => {
173-
// Check if it's a file and is a nested .git/HEAD (not at root)
174-
if (type !== "file") return false
175-
176-
// Ensure it's a .git/HEAD file and not the root one
177-
const normalizedPath = filePath.replace(/\\/g, "/")
178-
return (
179-
normalizedPath.includes(".git/HEAD") &&
180-
!normalizedPath.startsWith(".git/") &&
181-
normalizedPath !== ".git/HEAD"
182-
)
183-
})
171+
private async findNestedRepos(git: SimpleGit): Promise<string[]> {
172+
const nestedRepos = new Set<string>()
184173

185-
if (nestedGitPaths.length > 0) {
186-
// Get the first nested git repository path
187-
// Remove .git/HEAD from the path to get the repository directory
188-
const headPath = nestedGitPaths[0].path
174+
// 1. From .gitmodules (declared submodules)
175+
try {
176+
const config = await git.raw(["config", "-f", ".gitmodules", "--get-regexp", "^submodule\\..*\\.path$"])
177+
for (const line of config.split("\n")) {
178+
const match = line.match(/submodule\..*\.path\s+(.+)/)
179+
if (match) nestedRepos.add(match[1])
180+
}
181+
} catch {
182+
// No .gitmodules file or error reading it
183+
}
189184

190-
// Use path module to properly extract the repository directory
191-
// The HEAD file is at .git/HEAD, so we need to go up two directories
192-
const gitDir = path.dirname(headPath) // removes HEAD, gives us .git
193-
const repoDir = path.dirname(gitDir) // removes .git, gives us the repo directory
185+
// 2. From index (gitlinks with mode 160000)
186+
try {
187+
const lsFiles = await git.raw(["ls-files", "-s"])
188+
for (const line of lsFiles.split("\n")) {
189+
if (line.startsWith("160000")) {
190+
const parts = line.split(/\s+/)
191+
if (parts[3]) nestedRepos.add(parts[3])
192+
}
193+
}
194+
} catch {
195+
// Ignore errors
196+
}
194197

195-
const absolutePath = path.join(this.workspaceDir, repoDir)
198+
// 3. From filesystem (any nested .git directory or worktree)
199+
try {
200+
const gitDirs = await executeRipgrep({
201+
args: ["--files", "--hidden", "--follow", "-g", "**/.git/HEAD", this.workspaceDir],
202+
workspacePath: this.workspaceDir,
203+
})
196204

197-
this.log(
198-
`[${this.constructor.name}#getNestedGitRepository] found ${nestedGitPaths.length} nested git repositories, first at: ${repoDir}`,
199-
)
200-
return absolutePath
205+
for (const result of gitDirs) {
206+
if (result.type === "file") {
207+
const normalizedPath = result.path.replace(/\\/g, "/")
208+
if (
209+
normalizedPath.includes(".git/HEAD") &&
210+
!normalizedPath.startsWith(".git/") &&
211+
normalizedPath !== ".git/HEAD"
212+
) {
213+
// Extract repo directory (remove .git/HEAD)
214+
const gitDir = path.dirname(result.path)
215+
const repoDir = path.dirname(gitDir)
216+
nestedRepos.add(repoDir)
217+
}
218+
}
201219
}
202-
203-
return null
204220
} catch (error) {
205221
this.log(
206-
`[${this.constructor.name}#getNestedGitRepository] failed to check for nested git repos: ${error instanceof Error ? error.message : String(error)}`,
222+
`[${this.constructor.name}#findNestedRepos] failed to search filesystem: ${error instanceof Error ? error.message : String(error)}`,
207223
)
224+
}
208225

209-
// If we can't check, assume there are no nested repos to avoid blocking the feature.
210-
return null
226+
// 4. From filesystem (git worktrees - .git files pointing to worktree)
227+
try {
228+
const gitFiles = await executeRipgrep({
229+
args: ["--files", "--hidden", "--follow", "-g", "**/.git", this.workspaceDir],
230+
workspacePath: this.workspaceDir,
231+
})
232+
233+
for (const result of gitFiles) {
234+
if (result.type === "file") {
235+
const normalizedPath = result.path.replace(/\\/g, "/")
236+
// Check if this is a .git file (not directory) and not the root
237+
if (normalizedPath.endsWith("/.git") && normalizedPath !== ".git") {
238+
try {
239+
// Read the .git file to check if it's a worktree
240+
const gitFilePath = path.join(this.workspaceDir, result.path)
241+
const content = await fs.readFile(gitFilePath, "utf8")
242+
if (content.trim().startsWith("gitdir:")) {
243+
// This is a worktree - exclude its directory
244+
const repoDir = path.dirname(result.path)
245+
nestedRepos.add(repoDir)
246+
}
247+
} catch {
248+
// Ignore errors reading .git file
249+
}
250+
}
251+
}
252+
}
253+
} catch (error) {
254+
this.log(
255+
`[${this.constructor.name}#findNestedRepos] failed to search for worktrees: ${error instanceof Error ? error.message : String(error)}`,
256+
)
211257
}
258+
259+
return Array.from(nestedRepos).filter((p) => p && p !== ".")
212260
}
213261

214262
private async getShadowGitConfigWorktree(git: SimpleGit) {

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

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,8 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
378378
})
379379
})
380380

381-
describe(`${klass.name}#hasNestedGitRepositories`, () => {
382-
it("throws error when nested git repositories are detected during initialization", async () => {
381+
describe(`${klass.name}#nestedGitRepositories`, () => {
382+
it("succeeds when nested git repositories are detected and excludes them from checkpoints", async () => {
383383
// Create a new temporary workspace and service for this test.
384384
const shadowDir = path.join(tmpDir, `${prefix}-nested-git-${Date.now()}`)
385385
const workspaceDir = path.join(tmpDir, `workspace-nested-git-${Date.now()}`)
@@ -435,13 +435,33 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
435435
}
436436
})
437437

438-
const service = new klass(taskId, shadowDir, workspaceDir, () => {})
438+
const logMessages: string[] = []
439+
const service = new klass(taskId, shadowDir, workspaceDir, (msg: string) => logMessages.push(msg))
439440

440-
// Verify that initialization throws an error when nested git repos are detected
441-
// The error message now includes the specific path of the nested repository
442-
await expect(service.initShadowGit()).rejects.toThrowError(
443-
/Checkpoints are disabled because a nested git repository was detected at:/,
444-
)
441+
// Verify that initialization succeeds even with nested git repos
442+
await expect(service.initShadowGit()).resolves.not.toThrow()
443+
expect(service.isInitialized).toBe(true)
444+
445+
// Modify files in both main workspace and nested repo
446+
await fs.writeFile(mainFile, "Modified content in main repo")
447+
await fs.writeFile(nestedFile, "Modified content in nested repo")
448+
449+
// Save a checkpoint
450+
const checkpoint = await service.saveCheckpoint("Test with nested repos")
451+
expect(checkpoint?.commit).toBeTruthy()
452+
453+
// Verify that only the main file was included in the checkpoint
454+
const diff = await service.getDiff({ to: checkpoint!.commit })
455+
const mainFileChange = diff.find((change) => change.paths.relative === "main-file.txt")
456+
const nestedFileChange = diff.find((change) => change.paths.relative.includes("nested-file.txt"))
457+
458+
expect(mainFileChange).toBeDefined()
459+
expect(mainFileChange?.content.after).toBe("Modified content in main repo")
460+
expect(nestedFileChange).toBeUndefined() // Nested repo changes should be excluded
461+
462+
// Verify that the log includes information about excluding nested repos
463+
const excludeLog = logMessages.find((msg) => msg.includes("excluding") && msg.includes("nested repos"))
464+
expect(excludeLog).toBeDefined()
445465

446466
// Clean up.
447467
vitest.restoreAllMocks()
@@ -478,6 +498,17 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
478498
await expect(service.initShadowGit()).resolves.not.toThrow()
479499
expect(service.isInitialized).toBe(true)
480500

501+
// Modify the main file and save a checkpoint
502+
await fs.writeFile(mainFile, "Modified content")
503+
const checkpoint = await service.saveCheckpoint("Test without nested repos")
504+
expect(checkpoint?.commit).toBeTruthy()
505+
506+
// Verify the change was included in the checkpoint
507+
const diff = await service.getDiff({ to: checkpoint!.commit })
508+
expect(diff).toHaveLength(1)
509+
expect(diff[0].paths.relative).toBe("main-file.txt")
510+
expect(diff[0].content.after).toBe("Modified content")
511+
481512
// Clean up.
482513
vitest.restoreAllMocks()
483514
await fs.rm(shadowDir, { recursive: true, force: true })

0 commit comments

Comments
 (0)