fix: allow checkpoints with nested git repos by excluding them #8434
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Modified ShadowCheckpointService to exclude nested git repos instead of disabling checkpoints - Added getNestedGitRepositories method to find all nested repos - Updated writeExcludeFile to add nested repos to exclude patterns - Added info messages to notify users when nested repos are excluded - Updated tests to reflect new behavior - Fixes #8433
dosubot
bot
added
size:M
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| await git.addConfig("commit.gpgSign", "false") // Disable commit signing for shadow repo. | ||
| await git.addConfig("user.name", "Roo Code") | ||
| await git.addConfig("user.email", "[email protected]") | ||
| // Write exclude file which will include nested git repos |
Contributor
There was a problem hiding this comment.
Typo/wording concern: The comment on line 101 says "Write exclude file which will include nested git repos", but based on the commit message, the intention is to exclude nested git repos. Please verify if "include" should be changed to "exclude" to maintain consistency.
Suggested change
| // Write exclude file which will include nested git repos | |
| // Write exclude file which will exclude nested git repos |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| for (const gitPath of nestedGitPaths) { | ||
| const relativePath = path.relative(this.workspaceDir, gitPath) | ||
| // Add the directory and all its contents to exclude patterns | ||
| patterns.push(relativePath + "/") |
Contributor
Author
There was a problem hiding this comment.
P1: Normalize to POSIX-style separators before writing to exclude to ensure Git matching across platforms.
Suggested change
| patterns.push(relativePath + "/") | |
| const relativePath = path | |
| .relative(this.workspaceDir, gitPath) | |
| .split(path.sep) | |
| .join('/'); | |
| patterns.push(`${relativePath}/`) |
| this.log(`[${this.constructor.name}#writeExcludeFile] excluding nested git repo: ${relativePath}`) | ||
| } | ||
|
|
||
| await fs.writeFile(path.join(this.dotGitDir, "info", "exclude"), patterns.join("\n")) |
Contributor
Author
There was a problem hiding this comment.
P3: Add a trailing newline when writing .git/info/exclude for consistency with POSIX tools and Git defaults.
Suggested change
| await fs.writeFile(path.join(this.dotGitDir, "info", "exclude"), patterns.join("\n")) | |
| await fs.writeFile(path.join(this.dotGitDir, "info", "exclude"), patterns.join("\n") + "\n") |
| private async getNestedGitRepositories(): Promise<string[]> { | ||
| try { | ||
| // Find all .git/HEAD files that are not at the root level. | ||
| const args = ["--files", "--hidden", "--follow", "-g", "**/.git/HEAD", this.workspaceDir] |
Contributor
Author
There was a problem hiding this comment.
P2: This only finds nested repos with a .git directory containing HEAD; submodules/worktrees often have .git as a file pointing to a gitdir. Consider also detecting .git files and resolving their gitdir targets to exclude them.
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the issue where the checkpoint feature was completely disabled when nested git repositories were detected in the project. Instead of throwing an error and disabling checkpoints, the implementation now excludes nested git repositories from checkpoint tracking, allowing users to continue using checkpoints in projects with nested repos.
Changes
ShadowCheckpointServiceto exclude nested git repos instead of disabling checkpoints entirelygetNestedGitRepositorytogetNestedGitRepositoriesto find all nested repos (not just the first one)writeExcludeFileto add nested repos to the exclude patternsTesting
Related Issue
Fixes #8433
Type of Change
Checklist
Important
ShadowCheckpointServicenow excludes nested git repos from checkpoints, allowing main repo tracking, with updated tests and user notifications.ShadowCheckpointServicenow excludes nested git repos from checkpoints instead of disabling them.common.json.getNestedGitRepositorytogetNestedGitRepositoriesto handle multiple nested repos.writeExcludeFileto add nested repos to exclude patterns.ShadowCheckpointService.spec.tsto verify exclusion of nested repos.This description was created by
for a995d0e. You can customize this summary. It will automatically update as commits are pushed.