fix: resolve push-files fallback against the remote-tracking ref - #1485
Open
hamodywe wants to merge 1 commit into
Open
fix: resolve push-files fallback against the remote-tracking ref#1485hamodywe wants to merge 1 commit into
hamodywe wants to merge 1 commit into
Conversation
resolveHeadBranch() read origin/HEAD and returned the bare branch name
(e.g. "main"), which PushFiles() then diffed against directly. In a
clone that never checked out a local branch of that name (e.g. someone
who always branches straight off origin/main), that local ref doesn't
exist, so the diff fails with "fatal: bad revision 'main'" and the
error propagates instead of falling through to the existing ls-tree
fallback. Since pre-push always computes push-files internally to
gate the {push_files}/{files} templates, this hard-fails the whole
hook even for jobs that never reference either template.
Return the remote-tracking ref ("origin/main") instead, matching what
the git-branch--remotes fallback path already produced. The
remote-tracking ref always exists once origin/HEAD does, and reflects
what was actually fetched rather than a possibly stale or entirely
absent local branch of the same name.
Fixes evilmartians#1474
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fs := afero.NewMemMapFs() | ||
| root := "/repo" | ||
| gitPath := filepath.Join(root, ".git") | ||
| originHead := filepath.Join(gitPath, "refs", "remotes", "origin", "HEAD") |
There was a problem hiding this comment.
The new regression case uses a standalone t.Run block and direct t.Fatalf assertions instead of the repository-required table-driven map[string]struct structure and testify/assert, making related cases harder to extend consistently.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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.
resolveHeadBranch() read origin/HEAD and returned the bare branch name (e.g. "main"), which PushFiles() then diffed against directly. In a clone that never checked out a local branch of that name (e.g. someone who always branches straight off origin/main), that local ref doesn't exist, so the diff fails with "fatal: bad revision 'main'" and the error propagates instead of falling through to the existing ls-tree fallback. Since pre-push always computes push-files internally to gate the {push_files}/{files} templates, this hard-fails the whole hook even for jobs that never reference either template.
Return the remote-tracking ref ("origin/main") instead, matching what the git-branch--remotes fallback path already produced. The remote-tracking ref always exists once origin/HEAD does, and reflects what was actually fetched rather than a possibly stale or entirely absent local branch of the same name.
Fixes #1474
Context
Pre-push hooks that gate {push_files}/{files} templates were hard-failing entirely in clones without a local branch matching the remote's default branch name.
Changes
internal/git/repo.go: resolveHeadBranch() now returns the remote-tracking ref instead of the bare branch name, consistent with the existing git-branch--remotes fallback path. Added a regression test in internal/git/repo_test.go.