fix: handle nested git repos gracefully in checkpoints#8863
Closed
daniel-lxs wants to merge 5 commits intomainfrom
Closed
fix: handle nested git repos gracefully in checkpoints#8863daniel-lxs wants to merge 5 commits intomainfrom
daniel-lxs wants to merge 5 commits intomainfrom
Conversation
Instead of blocking checkpoints when nested git repos are detected, now excludes them from staging to prevent submodule-related errors. Changes: - Removed initialization-time nested repo check that blocked feature - Added findNestedRepos() to detect repos from multiple sources: * .gitmodules (declared submodules) * git index (gitlinks with mode 160000) * filesystem (.git directories and worktrees) - Modified stageAll() to exclude nested repos using pathspec - Added safety check to prevent submodule changes in staging - Updated tests to verify nested repos are excluded from checkpoints This allows checkpoints to work in monorepos and projects with submodules while preventing git errors from nested repo changes.
Contributor
Reviewed commit c98980d. All previously flagged issues have been resolved.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
Improvements based on reviewer bot feedback: 1. Enhanced safety check (HIGH priority): - Now uses 'git ls-files -s --cached' to detect mode 160000 entries - Catches newly added/removed gitlinks, not just pointer changes - Added check for .gitmodules changes with warning log 2. Improved pathspec exclusion (MEDIUM priority): - Added 'git rm --cached' to remove existing gitlinks before staging - Prevents staging of gitlink entries already in the index - Ensures complete exclusion of nested repo changes 3. Better error logging (LOW priority): - Added warning logs for git command failures in findNestedRepos() - Helps debugging while avoiding feature breakage - Distinguishes expected errors (no .gitmodules) from real issues All tests continue to pass with these enhancements.
Contributor
|
Posted 2 inline review comments focusing on correctness across platforms and config resolution. Both are minor and straightforward to address.
Mention @roomote in a comment to trigger your PR Fixer agent and make changes to this pull request. |
Collaborator
|
@daniel-lxs do you buy the Oroocle's comments? |
Member
Author
|
Moving back to Changes Requested |
…gitmodules absolute path; add tests; verify no gitlinks staged and log .gitmodules changes
Collaborator
|
stale |
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
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.
Fixes #8433
Summary
Instead of blocking the checkpoints feature when nested git repositories are detected, this PR modifies the behavior to exclude nested repos from staging, allowing checkpoints to work in monorepos and projects with submodules.
Changes
Benefits
Testing
All existing tests pass, including the updated test that now verifies:
Important
Modifies checkpoints to exclude nested git repos from staging, allowing functionality in monorepos and submodules.
ShadowCheckpointService.ts.stageAll().findNestedRepos()detects nested repos from.gitmodules, git index, and filesystem.ShadowCheckpointService.spec.tsto verify nested repos are excluded..gitmodules, path normalization, and gitlink entries with spaces.This description was created by
for c98980d. You can customize this summary. It will automatically update as commits are pushed.