-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: allow Git repositories in non-Git workspaces for checkpointing #8166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Modified getNestedGitRepository() to first check if workspace is a Git repo - Only flag repositories as nested if workspace itself is a Git repository - Added test case for the specific scenario in issue #8164 - This fixes the false positive where legitimate Git repos in non-Git workspaces were incorrectly flagged as nested Fixes #8164
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backward but somehow still broken.
private async getNestedGitRepository(): Promise<string | null> { | ||
try { | ||
// First check if the workspace root itself is a Git repository | ||
const workspaceIsGitRepo = await fileExistsAtPath(path.join(this.workspaceDir, ".git")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check robust enough? The .git
path could also be a file (in the case of Git worktrees or submodules pointing to .git/worktrees/...
). Should we consider checking if it's actually a directory?
Or perhaps we could use the simpleGit library to check if it's a valid repo?
// that happen to be in the workspace. This is a valid use case. | ||
if (!workspaceIsGitRepo) { | ||
this.log( | ||
`[${this.constructor.name}#getNestedGitRepository] workspace is not a git repository, allowing child repositories`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestion: Could this log message be more descriptive? Something like:
workspace is not a git repository, skipping nested repository check
This would make it clearer that we're intentionally skipping the check rather than just "allowing" repositories.
}) | ||
|
||
// Mock fileExistsAtPath to return false for workspace/.git (workspace is not a git repo) | ||
vitest.spyOn(fsUtils, "fileExistsAtPath").mockImplementation((filePath: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mock implementation is clever, but could we make it more explicit about what we're testing? Consider adding a comment explaining why we're only mocking the workspace/.git path and using real implementation for others to maintain test integrity.
Description
This PR fixes issue #8164 where checkpointing was incorrectly disabled when the workspace root was not a Git repository but contained cloned repositories as subdirectories.
Problem
When users had a workspace folder that wasn't a Git repository but contained cloned Git repositories as child folders, RooCode would incorrectly flag these as "nested" repositories and disable checkpointing. This was a false positive since these repositories weren't actually nested (a Git repo inside another Git repo), but rather legitimate repositories within a non-Git workspace.
Solution
Modified the
getNestedGitRepository()
method inShadowCheckpointService.ts
to:Changes
Testing
allows git repositories in non-git workspace (issue #8164)
that validates the fixChecklist
Fixes #8164
Important
Fixes issue #8164 by allowing checkpointing in non-Git workspaces with Git subdirectories, updating logic in
ShadowCheckpointService.ts
.getNestedGitRepository()
inShadowCheckpointService.ts
to check if the workspace is a Git repo before flagging nested repos.allows git repositories in non-git workspace (issue #8164)
inShadowCheckpointService.spec.ts
to validate the fix.This description was created by
for 0034b35. You can customize this summary. It will automatically update as commits are pushed.