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

This commit is contained in:
Chris Estreich 2025-06-10 13:29:19 -07:00 committed by GitHub
parent 8d5dab3518
commit 64dbcc8221
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 85 additions and 103 deletions

View file

@ -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) {

View file

@ -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,13 +40,13 @@ 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
}
@ -54,7 +54,7 @@ export function getCheckpointService(cline: Task) {
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
})
}

View file

@ -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()

View file

@ -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 args = ["--files", "--hidden", "--follow", "-g", "**/.git/HEAD", this.workspaceDir]
const gitPaths = await (
await executeRipgrep({ args, workspacePath: this.workspaceDir })
).filter(({ type, path }) => type === "folder" && path.includes(".git") && !path.startsWith(".git"))
const gitPaths = await executeRipgrep({ args, workspacePath: this.workspaceDir })
// For each nested .git directory, rename it based on operation.
for (const gitPath of gitPaths) {
if (gitPath.path.startsWith(".git")) {
continue
}
// 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",
)
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
}
if (currentPath === newPath) {
continue
}
try {
await fs.rename(currentPath, newPath)
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
}
}

View file

@ -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)
jest.spyOn(require("../../../utils/fs"), "fileExistsAtPath").mockImplementation((path) => {
if (path === nestedGitDir) {
return Promise.resolve(true)
} else if (path === nestedGitDisabledDir) {
return Promise.resolve(false)
}
return Promise.resolve(false)
})
// Verify the nested git directory is back to normal after initialization.
expect(await fileExistsAtPath(nestedGitDir)).toBe(true)
expect(await fileExistsAtPath(nestedGitDisabledDir)).toBe(false)
// 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",
)
// 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", "support@roocode.com")
// 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([])
})
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 })

View file

@ -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

View file

@ -1 +0,0 @@
export const GIT_DISABLED_SUFFIX = "_disabled"

View file

@ -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(),