fix(vcs): VcsGit IsCloned validates HEAD; Clone recovers atomically#207
Open
gmoigneu wants to merge 1 commit into
Open
fix(vcs): VcsGit IsCloned validates HEAD; Clone recovers atomically#207gmoigneu wants to merge 1 commit into
gmoigneu wants to merge 1 commit into
Conversation
Two related git robustness gaps closed. A. IsCloned no longer returns true for any directory containing a .git entry. It now opens the repository via gogit.PlainOpen and verifies a resolvable HEAD; broken or partially-cloned destinations report not-cloned, and the manager naturally falls back to Clone. B. Clone now stages into a sibling temp dir and atomically renames into place. If the destination already exists (left behind by a previously broken clone) it is removed before the swap, so the recovery path is automatic — no --repair flag needed. A crash mid-clone leaves either the previous tree or the new one, never a half-written hybrid. The same staging+rename pattern matches #121 (skill install) and #124 (archive update); the file system semantics are now consistent across the three backends that overlay-wrote before. Tests: - TestVcsGit_IsCloned_FakeDotGit: a directory with an empty .git/ reports false (was true before). - TestVcsGit_IsCloned_PartialClone: a half-fetched repo with HEAD pointing at an unresolvable ref reports false. - TestVcsGit_Clone_RecoversFromBrokenDest: clone over a leftover broken dir succeeds, removes the stale leftover file, installs the source content. - TestVcsGit_IsCloned_True updated to use makeLocalRepo (which commits) instead of bare PlainInit (which has no HEAD and now correctly reports false). - skill TestManager_Sync_PreCachedRemoteSource updated: instead of faking a .git directory it uses a new helper that initialises a real git repo with one commit. Closes #125
3 tasks
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
1 participant
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.
Summary
Why
The historical `IsCloned` returned true for any directory containing a `.git` entry — including non-git folders that happened to have one, half-cloned destinations from a Ctrl-C / network-drop, and renamed-into-place dirs. `Update` then opened the broken state and surfaced a cryptic go-git error with no recovery path.
Pattern parity
This matches the staging-temp-dir + atomic rename used in #121 (skill install) and #124 (archive update). The three backends that overlay-wrote before now have consistent file-system semantics.
Test plan
Closes #125