fix: improve shallow clone guidance for synthetic history#105
fix: improve shallow clone guidance for synthetic history#105Kelvinoppong wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
@reubeno this pr is ready for review. This change adds detection for shallow project repositories prior to synthetic history generation. When a shallow repository is encountered, the system now returns a clear and actionable error message instructing users to run git fetch --unshallow or re-clone the repository without --depth. The PR also includes a regression test to cover the shallow clone scenario. Closes #99. |
Thanks for tackling another item! Could you rebase your changes against latest |
Kelvinoppong
force-pushed
the
issue-99-shallow-clone-message
branch
from
120dda5 to
bedfc3e
Compare
@reubeno Thanks. Rebasing against latest main is done. The PR should now only include the shallow clone message fix. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a pre-check to detect shallow Git repositories before attempting synthetic history generation, returning an actionable error and introducing a regression test for the shallow-clone path (closes #99).
Changes:
- Detect shallow repositories in
FindAffectsCommitsand fail fast with guidance to unshallow/re-clone. - Add a regression test that marks an in-memory repo as shallow and asserts the new error.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/app/azldev/core/sources/synthistory.go | Adds shallow-clone detection and an actionable error before walking the commit log. |
| internal/app/azldev/core/sources/synthistory_test.go | Adds a regression test ensuring shallow repos return the expected error behavior. |
| shallowCommits, err := repo.Storer.Shallow() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to inspect repository history depth:\n%w", err) | ||
| } |
There was a problem hiding this comment.
repo.Storer.Shallow() can return an "unsupported" error depending on the underlying go-git storage implementation; failing hard here would incorrectly break synthetic history for storers that don't support shallow metadata (even when the repo is effectively "full"). Consider special-casing the go-git "shallow unsupported" error to treat it as non-shallow, and only error for unexpected failures.
| if len(shallowCommits) > 0 { | ||
| return nil, fmt.Errorf( | ||
| "repository is a shallow clone; synthetic history requires a full clone. " + | ||
| "Run `git fetch --unshallow` or re-clone without `--depth`", | ||
| ) | ||
| } |
There was a problem hiding this comment.
This introduces a new, user-actionable failure mode that callers may want to detect programmatically (and tests currently can only validate via substring checks). Consider introducing a sentinel/typed error (e.g., ErrShallowClone) and wrapping it (so callers can errors.Is(err, ErrShallowClone)), while keeping the detailed guidance in the message.
| results, err := sources.FindAffectsCommits(repo, "curl") | ||
| require.Error(t, err) | ||
| assert.Nil(t, results) | ||
| assert.Contains(t, err.Error(), "shallow clone") |
There was a problem hiding this comment.
These assertions are tightly coupled to the exact error phrasing and may become brittle if the message is later tweaked. If you adopt a sentinel/typed error for shallow clones, prefer asserting with errors.Is (or require.ErrorIs) and keep only one message substring assertion for the user guidance.
| assert.Contains(t, err.Error(), "shallow clone") |
Tonisal-byte
left a comment
Tonisal-byte
left a comment
There was a problem hiding this comment.
Looks good to me, make sure it passes the linting errors first :)
I will work on it asap |
Kelvinoppong
force-pushed
the
issue-99-shallow-clone-message
branch
from
bedfc3e to
ce0b44c
Compare
|
@reubeno The PR is ready to be merged, all the tests has pass and looks clean |
|
@reubeno I have fix everything and all test has passed, have a final look at it |
Summary
Detects shallow project repositories before synthetic history generation.
Returns a clear actionable error telling users to run
git fetch --unshallowor re-clone without--depth.Adds a regression test covering the shallow clone case.
Closes #99
Test plan
go test ./internal/app/azldev/core/sources -run TestFindAffectsCommits -v passes