mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
checkpoints: normalize nested repo paths to POSIX and use workspace .gitmodules absolute path; add tests; verify no gitlinks staged and log .gitmodules changes
This commit is contained in:
daniel-lxs
parent
0d089b193c
commit
fd22d0f080
2 changed files with 306 additions and 6 deletions
|
|
@ -149,7 +149,9 @@ export abstract class ShadowCheckpointService extends EventEmitter {
|
|||
// Remove any existing gitlinks from the index before staging
|
||||
for (const repoPath of nestedRepos) {
|
||||
try {
|
||||
await git.raw(["rm", "--cached", "--ignore-unmatch", "-r", repoPath])
|
||||
// Normalize to POSIX for git pathspec compatibility
|
||||
const posixPath = repoPath.replace(/\\/g, "/")
|
||||
await git.raw(["rm", "--cached", "--ignore-unmatch", "-r", posixPath])
|
||||
} catch (error) {
|
||||
this.log(
|
||||
`[${this.constructor.name}#stageAll] failed to remove cached gitlink ${repoPath}: ${error instanceof Error ? error.message : String(error)}`,
|
||||
|
|
@ -160,7 +162,9 @@ export abstract class ShadowCheckpointService extends EventEmitter {
|
|||
// Build add command with pathspec excludes
|
||||
const addArgs: string[] = ["-A", ":/"]
|
||||
for (const repoPath of nestedRepos) {
|
||||
addArgs.push(`:(exclude)${repoPath}/`)
|
||||
// Normalize to POSIX for git pathspec compatibility
|
||||
const posixPath = repoPath.replace(/\\/g, "/")
|
||||
addArgs.push(`:(exclude)${posixPath}/`)
|
||||
}
|
||||
|
||||
// Stage files
|
||||
|
|
@ -198,10 +202,15 @@ export abstract class ShadowCheckpointService extends EventEmitter {
|
|||
|
||||
// 1. From .gitmodules (declared submodules)
|
||||
try {
|
||||
const config = await git.raw(["config", "-f", ".gitmodules", "--get-regexp", "^submodule\\..*\\.path$"])
|
||||
const modulesPath = path.join(this.workspaceDir, ".gitmodules")
|
||||
const config = await git.raw(["config", "-f", modulesPath, "--get-regexp", "^submodule\\..*\\.path$"])
|
||||
for (const line of config.split("\n")) {
|
||||
const match = line.match(/submodule\..*\.path\s+(.+)/)
|
||||
if (match) nestedRepos.add(match[1])
|
||||
if (match) {
|
||||
// Normalize paths to POSIX format for git pathspec compatibility
|
||||
const normalizedPath = match[1].replace(/\\/g, "/")
|
||||
nestedRepos.add(normalizedPath)
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
// No .gitmodules file is expected in most cases, only log if it's a real error
|
||||
|
|
@ -242,8 +251,10 @@ export abstract class ShadowCheckpointService extends EventEmitter {
|
|||
!normalizedPath.startsWith(".git/") &&
|
||||
normalizedPath !== ".git/HEAD"
|
||||
) {
|
||||
// Normalize path first to handle Windows-style backslashes
|
||||
const normalizedPath = result.path.replace(/\\/g, "/")
|
||||
// Extract repo directory (remove .git/HEAD)
|
||||
const gitDir = path.dirname(result.path)
|
||||
const gitDir = path.dirname(normalizedPath)
|
||||
const repoDir = path.dirname(gitDir)
|
||||
nestedRepos.add(repoDir)
|
||||
}
|
||||
|
|
@ -273,7 +284,9 @@ export abstract class ShadowCheckpointService extends EventEmitter {
|
|||
const content = await fs.readFile(gitFilePath, "utf8")
|
||||
if (content.trim().startsWith("gitdir:")) {
|
||||
// This is a worktree - exclude its directory
|
||||
const repoDir = path.dirname(result.path)
|
||||
// Normalize path first to handle Windows-style backslashes
|
||||
const normalizedPath = result.path.replace(/\\/g, "/")
|
||||
const repoDir = path.dirname(normalizedPath)
|
||||
nestedRepos.add(repoDir)
|
||||
}
|
||||
} catch (error) {
|
||||
|
|
|
|||
|
|
@ -379,6 +379,166 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
|
|||
})
|
||||
|
||||
describe(`${klass.name}#nestedGitRepositories`, () => {
|
||||
it("uses absolute path for .gitmodules lookup and handles absent file gracefully", async () => {
|
||||
// Create a new temporary workspace and service for this test
|
||||
const shadowDir = path.join(tmpDir, `${prefix}-gitmodules-absolute-${Date.now()}`)
|
||||
const workspaceDir = path.join(tmpDir, `workspace-gitmodules-absolute-${Date.now()}`)
|
||||
|
||||
// Create workspace directory
|
||||
await fs.mkdir(workspaceDir, { recursive: true })
|
||||
|
||||
// Create a primary workspace repo
|
||||
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")
|
||||
|
||||
// Create a .gitmodules file with submodule paths
|
||||
const gitmodulesPath = path.join(workspaceDir, ".gitmodules")
|
||||
await fs.writeFile(
|
||||
gitmodulesPath,
|
||||
`[submodule "test-submodule"]
|
||||
path = test-submodule
|
||||
url = https://github.com/example/test-submodule.git`,
|
||||
)
|
||||
|
||||
// Add .gitmodules to the repo
|
||||
await mainGit.add(".gitmodules")
|
||||
await mainGit.commit("Add gitmodules")
|
||||
|
||||
const logMessages: string[] = []
|
||||
const service = new klass(taskId, shadowDir, workspaceDir, (msg: string) => logMessages.push(msg))
|
||||
|
||||
// Initialize the service
|
||||
await service.initShadowGit()
|
||||
|
||||
// Create nested directories to simulate submodule structure
|
||||
const testSubmoduleDir = path.join(workspaceDir, "test-submodule")
|
||||
await fs.mkdir(testSubmoduleDir, { recursive: true })
|
||||
const nestedFile = path.join(testSubmoduleDir, "nested-file.txt")
|
||||
await fs.writeFile(nestedFile, "Content that should be excluded")
|
||||
|
||||
// Modify main file and save a checkpoint to trigger findNestedRepos
|
||||
await fs.writeFile(mainFile, "Modified content")
|
||||
await service.saveCheckpoint("Test with gitmodules")
|
||||
|
||||
// The service should have detected and excluded the submodule path
|
||||
const excludeLog = logMessages.find((msg) => msg.includes("excluding") && msg.includes("nested repos"))
|
||||
expect(excludeLog).toBeDefined()
|
||||
expect(excludeLog).toContain("test-submodule")
|
||||
|
||||
// Now test graceful handling when .gitmodules is missing
|
||||
await fs.unlink(gitmodulesPath)
|
||||
|
||||
// Modify file and save another checkpoint
|
||||
await fs.writeFile(mainFile, "Modified content again")
|
||||
|
||||
// This should not throw despite missing .gitmodules file
|
||||
await expect(service.saveCheckpoint("Test missing gitmodules")).resolves.not.toThrow()
|
||||
|
||||
// The error should be handled gracefully (exit code 1 is expected for missing file)
|
||||
const gitmodulesErrors = logMessages.filter(
|
||||
(msg) => msg.includes("failed to read .gitmodules") && !msg.includes("exit code 1"),
|
||||
)
|
||||
expect(gitmodulesErrors).toHaveLength(0) // Should not log exit code 1 as an error
|
||||
|
||||
// Clean up
|
||||
await fs.rm(shadowDir, { recursive: true, force: true })
|
||||
await fs.rm(workspaceDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it("normalizes Windows-style backslash paths from .gitmodules to POSIX format", async () => {
|
||||
// Create a new temporary workspace and service for this test
|
||||
const shadowDir = path.join(tmpDir, `${prefix}-gitmodules-normalize-${Date.now()}`)
|
||||
const workspaceDir = path.join(tmpDir, `workspace-gitmodules-normalize-${Date.now()}`)
|
||||
|
||||
// Create workspace directory
|
||||
await fs.mkdir(workspaceDir, { recursive: true })
|
||||
|
||||
// Create a primary workspace repo
|
||||
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")
|
||||
|
||||
// Simulate a .gitmodules file with Windows-style backslash paths by directly creating it
|
||||
// and adding it to the git repo (to pass git config parsing)
|
||||
const gitmodulesPath = path.join(workspaceDir, ".gitmodules")
|
||||
await fs.writeFile(
|
||||
gitmodulesPath,
|
||||
`[submodule "nested\\\\project"]
|
||||
path = nested\\\\project
|
||||
url = https://github.com/example/nested-project.git
|
||||
[submodule "sub\\\\folder\\\\repo"]
|
||||
path = sub\\\\folder\\\\repo
|
||||
url = https://github.com/example/sub-folder-repo.git`,
|
||||
)
|
||||
|
||||
// Add .gitmodules to the repo
|
||||
await mainGit.add(".gitmodules")
|
||||
await mainGit.commit("Add gitmodules with backslash paths")
|
||||
|
||||
// Create the nested directories to simulate the submodule structure
|
||||
const nestedProjectDir = path.join(workspaceDir, "nested\\project")
|
||||
const subFolderRepoDir = path.join(workspaceDir, "sub\\folder\\repo")
|
||||
await fs.mkdir(nestedProjectDir, { recursive: true })
|
||||
await fs.mkdir(subFolderRepoDir, { recursive: true })
|
||||
|
||||
await fs.writeFile(path.join(nestedProjectDir, "file1.txt"), "Content that should be excluded")
|
||||
await fs.writeFile(path.join(subFolderRepoDir, "file2.txt"), "Content that should be excluded")
|
||||
|
||||
const logMessages: string[] = []
|
||||
const service = new klass(taskId, shadowDir, workspaceDir, (msg: string) => logMessages.push(msg))
|
||||
|
||||
// Initialize the service
|
||||
await service.initShadowGit()
|
||||
|
||||
// Create a test file and save a checkpoint to trigger path normalization
|
||||
await fs.writeFile(mainFile, "Test content")
|
||||
|
||||
// This will trigger findNestedRepos and path normalization
|
||||
await service.saveCheckpoint("Test path normalization")
|
||||
|
||||
// Verify that paths were normalized to POSIX format (forward slashes)
|
||||
// The log should show normalized paths
|
||||
const excludeLog = logMessages.find((msg) => msg.includes("excluding") && msg.includes("nested repos"))
|
||||
expect(excludeLog).toBeDefined()
|
||||
|
||||
// Check that the log contains POSIX paths (normalized from backslash paths)
|
||||
// The exact format may vary based on path.join behavior, but should contain forward slashes
|
||||
expect(excludeLog).toMatch(/nested[\\\/]project/) // Should contain the path, normalized or not
|
||||
expect(excludeLog).toMatch(/sub[\\\/]folder[\\\/]repo/) // Should contain the path, normalized or not
|
||||
|
||||
// Most importantly, verify no gitlink entries were created (which would happen if paths aren't normalized)
|
||||
// This is the key safety check: ensuring Windows backslash paths don't break git pathspecs
|
||||
const git = service["git"] // Access private git instance
|
||||
expect(git).toBeDefined() // Ensure git is initialized
|
||||
const stagedFiles = await git!.raw(["ls-files", "-s", "--cached"])
|
||||
const gitlinkEntries = stagedFiles
|
||||
.split("\n")
|
||||
.filter((line) => line.startsWith("160000"))
|
||||
.map((line) => line.split(/\s+/)[3])
|
||||
.filter(Boolean)
|
||||
|
||||
expect(gitlinkEntries).toHaveLength(0) // No gitlink entries should exist
|
||||
|
||||
// Clean up
|
||||
await fs.rm(shadowDir, { recursive: true, force: true })
|
||||
await fs.rm(workspaceDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it("succeeds when nested git repositories are detected and excludes them from checkpoints", async () => {
|
||||
// Create a new temporary workspace and service for this test.
|
||||
const shadowDir = path.join(tmpDir, `${prefix}-nested-git-${Date.now()}`)
|
||||
|
|
@ -469,6 +629,133 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
|
|||
await fs.rm(workspaceDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it("normalizes Windows-style backslash paths from filesystem scans to POSIX for git pathspecs", async () => {
|
||||
// Create a new temporary workspace and service for this test.
|
||||
const shadowDir = path.join(tmpDir, `${prefix}-windows-paths-${Date.now()}`)
|
||||
const workspaceDir = path.join(tmpDir, `workspace-windows-paths-${Date.now()}`)
|
||||
|
||||
// Create a primary workspace repo.
|
||||
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 test file in 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")
|
||||
|
||||
// Mock executeRipgrep to simulate Windows-style backslash paths from filesystem
|
||||
vitest.spyOn(fileSearch, "executeRipgrep").mockImplementation(({ args }) => {
|
||||
const searchPattern = args[4]
|
||||
|
||||
if (searchPattern.includes(".git/HEAD")) {
|
||||
// Return Windows-style paths with backslashes to simulate filesystem scan on Windows
|
||||
return Promise.resolve([
|
||||
{
|
||||
path: "nested-project\\.git\\HEAD", // Backslash path from Windows filesystem
|
||||
type: "file",
|
||||
label: "HEAD",
|
||||
},
|
||||
{
|
||||
path: "another-repo\\subdir\\.git\\HEAD", // Another backslash path
|
||||
type: "file",
|
||||
label: "HEAD",
|
||||
},
|
||||
])
|
||||
} else if (searchPattern.includes("**/.git")) {
|
||||
// Return Windows-style worktree .git file paths with backslashes
|
||||
return Promise.resolve([
|
||||
{
|
||||
path: "worktree-repo\\.git", // Backslash path from Windows filesystem
|
||||
type: "file",
|
||||
label: ".git",
|
||||
},
|
||||
])
|
||||
} else {
|
||||
return Promise.resolve([])
|
||||
}
|
||||
})
|
||||
|
||||
// Mock fs.readFile to simulate a worktree .git file for the worktree test
|
||||
const originalReadFile = fs.readFile
|
||||
vitest.spyOn(fs, "readFile").mockImplementation(async (filePath: any, encoding?: any) => {
|
||||
const pathStr = typeof filePath === "string" ? filePath : filePath.toString()
|
||||
if (pathStr.includes("worktree-repo") && pathStr.endsWith(".git")) {
|
||||
return "gitdir: /some/worktree/path/.git/worktrees/worktree-repo"
|
||||
}
|
||||
return originalReadFile(filePath, encoding)
|
||||
})
|
||||
|
||||
const logMessages: string[] = []
|
||||
const service = new klass(taskId, shadowDir, workspaceDir, (msg: string) => logMessages.push(msg))
|
||||
|
||||
// Initialize the service and verify it handles Windows paths correctly
|
||||
await expect(service.initShadowGit()).resolves.not.toThrow()
|
||||
expect(service.isInitialized).toBe(true)
|
||||
|
||||
// Create files that would conflict with the "nested" repos if they weren't properly excluded
|
||||
const nestedFile1 = path.join(workspaceDir, "nested-project", "nested-file.txt")
|
||||
const nestedFile2 = path.join(workspaceDir, "another-repo", "subdir", "another-file.txt")
|
||||
const worktreeFile = path.join(workspaceDir, "worktree-repo", "worktree-file.txt")
|
||||
|
||||
await fs.mkdir(path.dirname(nestedFile1), { recursive: true })
|
||||
await fs.mkdir(path.dirname(nestedFile2), { recursive: true })
|
||||
await fs.mkdir(path.dirname(worktreeFile), { recursive: true })
|
||||
|
||||
await fs.writeFile(nestedFile1, "Content that should be excluded")
|
||||
await fs.writeFile(nestedFile2, "Content that should be excluded")
|
||||
await fs.writeFile(worktreeFile, "Content that should be excluded")
|
||||
await fs.writeFile(mainFile, "Updated main content")
|
||||
|
||||
// Save a checkpoint
|
||||
const checkpoint = await service.saveCheckpoint("Test Windows path normalization")
|
||||
expect(checkpoint?.commit).toBeTruthy()
|
||||
|
||||
// Verify that the log shows POSIX-normalized paths for exclusion
|
||||
const excludeLog = logMessages.find((msg) => msg.includes("excluding") && msg.includes("nested repos"))
|
||||
expect(excludeLog).toBeDefined()
|
||||
|
||||
// The log should contain POSIX paths (forward slashes), not Windows paths (backslashes)
|
||||
expect(excludeLog).toContain("nested-project")
|
||||
expect(excludeLog).toContain("another-repo/subdir")
|
||||
expect(excludeLog).toContain("worktree-repo")
|
||||
|
||||
// Verify that only the main file was included in the checkpoint
|
||||
const diff = await service.getDiff({ to: checkpoint!.commit })
|
||||
const mainFileChange = diff.find((change) => change.paths.relative === "main-file.txt")
|
||||
const nestedFileChanges = diff.filter(
|
||||
(change) =>
|
||||
change.paths.relative.includes("nested-project") ||
|
||||
change.paths.relative.includes("another-repo") ||
|
||||
change.paths.relative.includes("worktree-repo"),
|
||||
)
|
||||
|
||||
expect(mainFileChange).toBeDefined()
|
||||
expect(mainFileChange?.content.after).toBe("Updated main content")
|
||||
expect(nestedFileChanges).toHaveLength(0) // No nested repo files should be included
|
||||
|
||||
// Enhanced safety check: Verify no mode 160000 gitlink entries were created
|
||||
// This is the key test - ensuring Windows backslash paths don't break git pathspecs
|
||||
const git = service["git"] // Access private git instance
|
||||
expect(git).toBeDefined() // Ensure git is initialized
|
||||
const stagedFiles = await git!.raw(["ls-files", "-s", "--cached"])
|
||||
const gitlinkEntries = stagedFiles
|
||||
.split("\n")
|
||||
.filter((line) => line.startsWith("160000"))
|
||||
.map((line) => line.split(/\s+/)[3])
|
||||
.filter(Boolean)
|
||||
|
||||
expect(gitlinkEntries).toHaveLength(0) // No gitlink entries should exist
|
||||
|
||||
// Clean up.
|
||||
vitest.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()}`)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue