Skip to content

Commit 64dbcc8

Browse files
ctecte
authored
Disable checkpoint if nested git repos are detected (RooCodeInc#4509)
1 parent 8d5dab3 commit 64dbcc8

File tree

8 files changed

+81
-99
lines changed

8 files changed

+81
-99
lines changed

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ import { codebaseSearchTool } from "../tools/codebaseSearchTool"
5151

5252
export async function presentAssistantMessage(cline: Task) {
5353
if (cline.abort) {
54-
throw new Error(`[Cline#presentAssistantMessage] task ${cline.taskId}.${cline.instanceId} aborted`)
54+
throw new Error(`[Task#presentAssistantMessage] task ${cline.taskId}.${cline.instanceId} aborted`)
5555
}
5656

5757
if (cline.presentAssistantMessageLocked) {

‎src/core/checkpoints/index.ts‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export function getCheckpointService(cline: Task) {
2424
}
2525

2626
if (cline.checkpointServiceInitializing) {
27-
console.log("[Cline#getCheckpointService] checkpoint service is still initializing")
27+
console.log("[Task#getCheckpointService] checkpoint service is still initializing")
2828
return undefined
2929
}
3030

@@ -40,21 +40,21 @@ export function getCheckpointService(cline: Task) {
4040
}
4141
}
4242

43-
console.log("[Cline#getCheckpointService] initializing checkpoints service")
43+
console.log("[Task#getCheckpointService] initializing checkpoints service")
4444

4545
try {
4646
const workspaceDir = getWorkspacePath()
4747

4848
if (!workspaceDir) {
49-
log("[Cline#getCheckpointService] workspace folder not found, disabling checkpoints")
49+
log("[Task#getCheckpointService] workspace folder not found, disabling checkpoints")
5050
cline.enableCheckpoints = false
5151
return undefined
5252
}
5353

5454
const globalStorageDir = provider?.context.globalStorageUri.fsPath
5555

5656
if (!globalStorageDir) {
57-
log("[Cline#getCheckpointService] globalStorageDir not found, disabling checkpoints")
57+
log("[Task#getCheckpointService] globalStorageDir not found, disabling checkpoints")
5858
cline.enableCheckpoints = false
5959
return undefined
6060
}
@@ -71,7 +71,7 @@ export function getCheckpointService(cline: Task) {
7171
cline.checkpointServiceInitializing = true
7272

7373
service.on("initialize", () => {
74-
log("[Cline#getCheckpointService] service initialized")
74+
log("[Task#getCheckpointService] service initialized")
7575

7676
try {
7777
const isCheckpointNeeded =
@@ -81,11 +81,11 @@ export function getCheckpointService(cline: Task) {
8181
cline.checkpointServiceInitializing = false
8282

8383
if (isCheckpointNeeded) {
84-
log("[Cline#getCheckpointService] no checkpoints found, saving initial checkpoint")
84+
log("[Task#getCheckpointService] no checkpoints found, saving initial checkpoint")
8585
checkpointSave(cline)
8686
}
8787
} catch (err) {
88-
log("[Cline#getCheckpointService] caught error in on('initialize'), disabling checkpoints")
88+
log("[Task#getCheckpointService] caught error in on('initialize'), disabling checkpoints")
8989
cline.enableCheckpoints = false
9090
}
9191
})
@@ -99,30 +99,26 @@ export function getCheckpointService(cline: Task) {
9999
isNonInteractive: true,
100100
})
101101
.catch((err) => {
102-
log("[Cline#getCheckpointService] caught unexpected error in say('checkpoint_saved')")
102+
log("[Task#getCheckpointService] caught unexpected error in say('checkpoint_saved')")
103103
console.error(err)
104104
})
105105
} catch (err) {
106-
log("[Cline#getCheckpointService] caught unexpected error in on('checkpoint'), disabling checkpoints")
106+
log("[Task#getCheckpointService] caught unexpected error in on('checkpoint'), disabling checkpoints")
107107
console.error(err)
108108
cline.enableCheckpoints = false
109109
}
110110
})
111111

112-
log("[Cline#getCheckpointService] initializing shadow git")
112+
log("[Task#getCheckpointService] initializing shadow git")
113113

114114
service.initShadowGit().catch((err) => {
115-
log(
116-
`[Cline#getCheckpointService] caught unexpected error in initShadowGit, disabling checkpoints (${err.message})`,
117-
)
118-
119-
console.error(err)
115+
log(`[Task#getCheckpointService] initShadowGit -> ${err.message}`)
120116
cline.enableCheckpoints = false
121117
})
122118

123119
return service
124120
} catch (err) {
125-
log("[Cline#getCheckpointService] caught unexpected error, disabling checkpoints")
121+
log(`[Task#getCheckpointService] ${err.message}`)
126122
cline.enableCheckpoints = false
127123
return undefined
128124
}
@@ -141,7 +137,7 @@ async function getInitializedCheckpointService(
141137
try {
142138
await pWaitFor(
143139
() => {
144-
console.log("[Cline#getCheckpointService] waiting for service to initialize")
140+
console.log("[Task#getCheckpointService] waiting for service to initialize")
145141
return service.isInitialized
146142
},
147143
{ interval, timeout },
@@ -171,7 +167,7 @@ export async function checkpointSave(cline: Task, force = false) {
171167

172168
// Start the checkpoint process in the background.
173169
return service.saveCheckpoint(`Task: ${cline.taskId}, Time: ${Date.now()}`, { allowEmpty: force }).catch((err) => {
174-
console.error("[Cline#checkpointSave] caught unexpected error, disabling checkpoints", err)
170+
console.error("[Task#checkpointSave] caught unexpected error, disabling checkpoints", err)
175171
cline.enableCheckpoints = false
176172
})
177173
}

‎src/core/environment/getEnvironmentDetails.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export async function getEnvironmentDetails(cline: Task, includeFileDetails: boo
153153
}
154154
}
155155

156-
// console.log(`[Cline#getEnvironmentDetails] terminalDetails: ${terminalDetails}`)
156+
// console.log(`[Task#getEnvironmentDetails] terminalDetails: ${terminalDetails}`)
157157

158158
// Add recently modified files section.
159159
const recentlyModifiedFiles = cline.fileContextTracker.getAndClearRecentlyModifiedFiles()

‎src/services/checkpoints/ShadowCheckpointService.ts‎

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import pWaitFor from "p-wait-for"
1010
import { fileExistsAtPath } from "../../utils/fs"
1111
import { executeRipgrep } from "../../services/search/file-search"
1212

13-
import { GIT_DISABLED_SUFFIX } from "./constants"
1413
import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types"
1514
import { getExcludePatterns } from "./excludes"
1615

@@ -65,6 +64,15 @@ export abstract class ShadowCheckpointService extends EventEmitter {
6564
throw new Error("Shadow git repo already initialized")
6665
}
6766

67+
const hasNestedGitRepos = await this.hasNestedGitRepositories()
68+
69+
if (hasNestedGitRepos) {
70+
throw new Error(
71+
"Checkpoints are disabled because nested git repositories were detected in the workspace. " +
72+
"Please remove or relocate nested git repositories to use the checkpoints feature.",
73+
)
74+
}
75+
6876
await fs.mkdir(this.checkpointsDir, { recursive: true })
6977
const git = simpleGit(this.checkpointsDir)
7078
const gitVersion = await git.version()
@@ -132,71 +140,43 @@ export abstract class ShadowCheckpointService extends EventEmitter {
132140
}
133141

134142
private async stageAll(git: SimpleGit) {
135-
await this.renameNestedGitRepos(true)
136-
137143
try {
138144
await git.add(".")
139145
} catch (error) {
140146
this.log(
141147
`[${this.constructor.name}#stageAll] failed to add files to git: ${error instanceof Error ? error.message : String(error)}`,
142148
)
143-
} finally {
144-
await this.renameNestedGitRepos(false)
145149
}
146150
}
147151

148-
// Since we use git to track checkpoints, we need to temporarily disable
149-
// nested git repos to work around git's requirement of using submodules for
150-
// nested repos.
151-
private async renameNestedGitRepos(disable: boolean) {
152+
private async hasNestedGitRepositories(): Promise<boolean> {
152153
try {
153154
// Find all .git directories that are not at the root level.
154-
const gitDir = ".git" + (disable ? "" : GIT_DISABLED_SUFFIX)
155-
const args = ["--files", "--hidden", "--follow", "-g", `**/${gitDir}/HEAD`, this.workspaceDir]
156-
157-
const gitPaths = await (
158-
await executeRipgrep({ args, workspacePath: this.workspaceDir })
159-
).filter(({ type, path }) => type === "folder" && path.includes(".git") && !path.startsWith(".git"))
160-
161-
// For each nested .git directory, rename it based on operation.
162-
for (const gitPath of gitPaths) {
163-
if (gitPath.path.startsWith(".git")) {
164-
continue
165-
}
166-
167-
const currentPath = path.join(this.workspaceDir, gitPath.path)
168-
let newPath: string
169-
170-
if (disable) {
171-
newPath = !currentPath.endsWith(GIT_DISABLED_SUFFIX)
172-
? currentPath + GIT_DISABLED_SUFFIX
173-
: currentPath
174-
} else {
175-
newPath = currentPath.endsWith(GIT_DISABLED_SUFFIX)
176-
? currentPath.slice(0, -GIT_DISABLED_SUFFIX.length)
177-
: currentPath
178-
}
155+
const args = ["--files", "--hidden", "--follow", "-g", "**/.git/HEAD", this.workspaceDir]
179156

180-
if (currentPath === newPath) {
181-
continue
182-
}
157+
const gitPaths = await executeRipgrep({ args, workspacePath: this.workspaceDir })
183158

184-
try {
185-
await fs.rename(currentPath, newPath)
159+
// Filter to only include nested git directories (not the root .git).
160+
const nestedGitPaths = gitPaths.filter(
161+
({ type, path }) =>
162+
type === "folder" && path.includes(".git") && !path.startsWith(".git") && path !== ".git",
163+
)
186164

187-
this.log(
188-
`[${this.constructor.name}#renameNestedGitRepos] ${disable ? "disabled" : "enabled"} nested git repo ${currentPath}`,
189-
)
190-
} catch (error) {
191-
this.log(
192-
`[${this.constructor.name}#renameNestedGitRepos] failed to ${disable ? "disable" : "enable"} nested git repo ${currentPath}: ${error instanceof Error ? error.message : String(error)}`,
193-
)
194-
}
165+
if (nestedGitPaths.length > 0) {
166+
this.log(
167+
`[${this.constructor.name}#hasNestedGitRepositories] found ${nestedGitPaths.length} nested git repositories: ${nestedGitPaths.map((p) => p.path).join(", ")}`,
168+
)
169+
return true
195170
}
171+
172+
return false
196173
} catch (error) {
197174
this.log(
198-
`[${this.constructor.name}#renameNestedGitRepos] failed to ${disable ? "disable" : "enable"} nested git repos: ${error instanceof Error ? error.message : String(error)}`,
175+
`[${this.constructor.name}#hasNestedGitRepositories] failed to check for nested git repos: ${error instanceof Error ? error.message : String(error)}`,
199176
)
177+
178+
// If we can't check, assume there are no nested repos to avoid blocking the feature.
179+
return false
200180
}
201181
}
202182

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

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,8 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
380380
})
381381
})
382382

383-
describe(`${klass.name}#renameNestedGitRepos`, () => {
384-
it("handles nested git repositories during initialization", async () => {
383+
describe(`${klass.name}#hasNestedGitRepositories`, () => {
384+
it("throws error when nested git repositories are detected during initialization", async () => {
385385
// Create a new temporary workspace and service for this test.
386386
const shadowDir = path.join(tmpDir, `${prefix}-nested-git-${Date.now()}`)
387387
const workspaceDir = path.join(tmpDir, `workspace-nested-git-${Date.now()}`)
@@ -417,11 +417,7 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
417417
const nestedGitDir = path.join(nestedRepoPath, ".git")
418418
const headFile = path.join(nestedGitDir, "HEAD")
419419
await fs.writeFile(headFile, "HEAD")
420-
const nestedGitDisabledDir = `${nestedGitDir}_disabled`
421420
expect(await fileExistsAtPath(nestedGitDir)).toBe(true)
422-
expect(await fileExistsAtPath(nestedGitDisabledDir)).toBe(false)
423-
424-
const renameSpy = jest.spyOn(fs, "rename")
425421

426422
jest.spyOn(fileSearch, "executeRipgrep").mockImplementation(({ args }) => {
427423
const searchPattern = args[4]
@@ -440,29 +436,48 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
440436
})
441437

442438
const service = new klass(taskId, shadowDir, workspaceDir, () => {})
443-
await service.initShadowGit()
444439

445-
// Verify rename was called with correct paths.
446-
expect(renameSpy.mock.calls).toHaveLength(1)
447-
expect(renameSpy.mock.calls[0][0]).toBe(nestedGitDir)
448-
expect(renameSpy.mock.calls[0][1]).toBe(nestedGitDisabledDir)
440+
// Verify that initialization throws an error when nested git repos are detected
441+
await expect(service.initShadowGit()).rejects.toThrow(
442+
"Checkpoints are disabled because nested git repositories were detected in the workspace",
443+
)
449444

450-
jest.spyOn(require("../../../utils/fs"), "fileExistsAtPath").mockImplementation((path) => {
451-
if (path === nestedGitDir) {
452-
return Promise.resolve(true)
453-
} else if (path === nestedGitDisabledDir) {
454-
return Promise.resolve(false)
455-
}
445+
// Clean up.
446+
jest.restoreAllMocks()
447+
await fs.rm(shadowDir, { recursive: true, force: true })
448+
await fs.rm(workspaceDir, { recursive: true, force: true })
449+
})
450+
451+
it("succeeds when no nested git repositories are detected", async () => {
452+
// Create a new temporary workspace and service for this test.
453+
const shadowDir = path.join(tmpDir, `${prefix}-no-nested-git-${Date.now()}`)
454+
const workspaceDir = path.join(tmpDir, `workspace-no-nested-git-${Date.now()}`)
455+
456+
// Create a primary workspace repo without any nested repos.
457+
await fs.mkdir(workspaceDir, { recursive: true })
458+
const mainGit = simpleGit(workspaceDir)
459+
await mainGit.init()
460+
await mainGit.addConfig("user.name", "Roo Code")
461+
await mainGit.addConfig("user.email", "[email protected]")
456462

457-
return Promise.resolve(false)
463+
// Create a test file in the main workspace.
464+
const mainFile = path.join(workspaceDir, "main-file.txt")
465+
await fs.writeFile(mainFile, "Content in main repo")
466+
await mainGit.add(".")
467+
await mainGit.commit("Initial commit in main repo")
468+
469+
jest.spyOn(fileSearch, "executeRipgrep").mockImplementation(() => {
470+
// Return empty array to simulate no nested git repos found
471+
return Promise.resolve([])
458472
})
459473

460-
// Verify the nested git directory is back to normal after initialization.
461-
expect(await fileExistsAtPath(nestedGitDir)).toBe(true)
462-
expect(await fileExistsAtPath(nestedGitDisabledDir)).toBe(false)
474+
const service = new klass(taskId, shadowDir, workspaceDir, () => {})
475+
476+
// Verify that initialization succeeds when no nested git repos are detected
477+
await expect(service.initShadowGit()).resolves.not.toThrow()
478+
expect(service.isInitialized).toBe(true)
463479

464480
// Clean up.
465-
renameSpy.mockRestore()
466481
jest.restoreAllMocks()
467482
await fs.rm(shadowDir, { recursive: true, force: true })
468483
await fs.rm(workspaceDir, { recursive: true, force: true })

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { join } from "path"
66
import { fileExistsAtPath } from "../../../utils/fs"
77

88
import { getExcludePatterns } from "../excludes"
9-
import { GIT_DISABLED_SUFFIX } from "../constants"
109

1110
jest.mock("fs/promises")
1211

@@ -54,7 +53,6 @@ readme.md text
5453

5554
// Verify all normal patterns also exist
5655
expect(excludePatterns).toContain(".git/")
57-
expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
5856
})
5957

6058
it("should handle .gitattributes with no LFS patterns", async () => {
@@ -89,7 +87,6 @@ readme.md text
8987

9088
// Verify default patterns are included
9189
expect(excludePatterns).toContain(".git/")
92-
expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
9390
})
9491

9592
it("should handle missing .gitattributes file", async () => {
@@ -107,7 +104,6 @@ readme.md text
107104

108105
// Verify standard patterns are included
109106
expect(excludePatterns).toContain(".git/")
110-
expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
111107

112108
// Verify we have standard patterns but no LFS patterns
113109
// Check for a few known patterns from different categories
@@ -139,7 +135,6 @@ readme.md text
139135

140136
// Verify standard patterns are included
141137
expect(excludePatterns).toContain(".git/")
142-
expect(excludePatterns).toContain(`.git${GIT_DISABLED_SUFFIX}/`)
143138

144139
// Verify we have standard patterns but no LFS patterns
145140
// Check for a few known patterns from different categories

‎src/services/checkpoints/constants.ts‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎src/services/checkpoints/excludes.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import { join } from "path"
33

44
import { fileExistsAtPath } from "../../utils/fs"
55

6-
import { GIT_DISABLED_SUFFIX } from "./constants"
7-
86
const getBuildArtifactPatterns = () => [
97
".gradle/",
108
".idea/",
@@ -200,7 +198,6 @@ const getLfsPatterns = async (workspacePath: string) => {
200198

201199
export const getExcludePatterns = async (workspacePath: string) => [
202200
".git/",
203-
`.git${GIT_DISABLED_SUFFIX}/`,
204201
...getBuildArtifactPatterns(),
205202
...getMediaFilePatterns(),
206203
...getCacheFilePatterns(),

0 commit comments

Comments
 (0)