utils-code/jesseduffield.lazygit
1
0
Fork 0
mirror of https://github.com/jesseduffield/lazygit.git synced 2026-09-10 07:36:27 -04:00
Code Issues Actions Packages Projects Releases Wiki Activity

harden deleted branch detection against reflog noise

Browse source
This commit is contained in:
Samuel Onoja 2026-08-03 11:50:54 +01:00
parent a5741831b5
commit 7da8a04187
No known key found for this signature in database
2 changed files with 248 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

93
pkg/commands/git_commands/branch_loader.go
View file

@ -494,8 +494,7 @@ func parseDifference(track string, regexStr string) string {
// HEAD). Entries are fed in the order git produces them, i.e. newest first.
type reflogEntry struct {
hash string // the commit HEAD pointed at when this reflog action occurred
subject string
timestamp int64 // commit timestamp of the `hash` commit
timestamp int64 // commit timestamp of the `hash` commit
from string // set on "checkout: moving from X to Y" lines to the source branch X; "" otherwise
to string // set to the destination branch Y on checkout lines; "" otherwise
}
@ -507,7 +506,7 @@ type reflogEntry struct {
// (reconstructed from the reflog) is the commit it pointed at when it was
// deleted.
func (self *BranchLoader) GetDeletedBranches() ([]*models.DeletedBranch, error) {
currentBranches, err := self.getCurrentBranchNames()
existingRefs, err := self.getExistingRefNames()
if err != nil {
return nil, err
}
@ -524,21 +523,40 @@ func (self *BranchLoader) GetDeletedBranches() ([]*models.DeletedBranch, error)
}
entries := parseReflogEntries(rawReflog)
return obtainDeletedBranches(entries, currentBranches), nil
return obtainDeletedBranches(entries, existingRefs, self.isValidRefFormat), nil
}
// getCurrentBranchNames returns the short names of all local branches.
func (self *BranchLoader) getCurrentBranchNames() ([]string, error) {
// isValidRefFormat reports whether git accepts `name` as a valid ref name,
// deferring the ref-name grammar (e.g. rejecting "HEAD~1", "main@{0}", trailing
// dots) to `git check-ref-format`. We pass --allow-onelevel because reflog
// checkout names are bare branch names (e.g. "master"), which are single-level
// refs. Note that git's rules only cover shape: things like tags, remote-tracking
// branches and abbreviated SHAs are all valid refs to git, so the caller still
// has to filter those out separately.
func (self *BranchLoader) isValidRefFormat(name string) bool {
return self.cmd.New(
NewGitCmd("check-ref-format").
Arg("--allow-onelevel").
Arg(name).
ToArgv(),
).DontLog().Run() == nil
}
// getExistingRefNames returns the short names of all refs (local branches,
// remote-tracking branches and tags) plus HEAD itself. A name present here is
// known not to be a deleted local branch, so it is excluded from recovery
// candidates.
func (self *BranchLoader) getExistingRefNames() ([]string, error) {
output, err := self.cmd.New(
NewGitCmd("for-each-ref").
Arg("--format=%(refname:short)").
Arg("refs/heads").
Arg("refs/heads", "refs/remotes", "refs/tags").
ToArgv(),
).DontLog().RunWithOutput()
if err != nil {
return nil, err
}
return strings.Split(strings.TrimSpace(output), "\n"), nil
return append(strings.Split(strings.TrimSpace(output), "\n"), "HEAD"), nil
}
// parseReflogEntries parses the raw output of
@ -585,8 +603,8 @@ func parseReflogCheckoutSubject(subject string) (string, string) {
// out but are no longer local branches, together with the commit they pointed
// at when last seen. The result is ordered by recency (most recently committed
// to first).
func obtainDeletedBranches(entries []*reflogEntry, currentBranchNames []string) []*models.DeletedBranch {
currentBranches := set.NewFromSlice(currentBranchNames)
func obtainDeletedBranches(entries []*reflogEntry, existingRefs []string, isValidRefFormat func(string) bool) []*models.DeletedBranch {
existing := set.NewFromSlice(existingRefs)
// currentBranch is the branch HEAD was on leading up to the current entry.
currentBranch := ""
@ -597,11 +615,21 @@ func obtainDeletedBranches(entries []*reflogEntry, currentBranchNames []string)
entry := entries[i]
if entry.from != "" && entry.to != "" {
// The hash of a checkout entry is the tip of the branch being
// moved to. Seeding it here means branches created via
// `git checkout -b` (and never committed to) are still
// recoverable. We never touch the source branch: its tip was
// recorded by the older entries that preceded this checkout, and
// overwriting it with the destination's tip would be wrong.
if isBranchName(entry.to, isValidRefFormat) {
branchTip[entry.to] = entry.hash
branchTimestamp[entry.to] = entry.timestamp
}
currentBranch = entry.to
continue
}
if currentBranch != "" && currentBranch != "HEAD" {
if currentBranch != "" && isBranchName(currentBranch, isValidRefFormat) {
branchTip[currentBranch] = entry.hash
branchTimestamp[currentBranch] = entry.timestamp
}
@ -609,7 +637,7 @@ func obtainDeletedBranches(entries []*reflogEntry, currentBranchNames []string)
deleted := make([]*models.DeletedBranch, 0, len(branchTip))
for name, tip := range branchTip {
if name == "HEAD" || currentBranches.Includes(name) {
if existing.Includes(name) {
continue
}
deleted = append(deleted, &models.DeletedBranch{
@ -637,6 +665,47 @@ func obtainDeletedBranches(entries []*reflogEntry, currentBranchNames []string)
return deleted
}
// isBranchName returns true if the given string could be a local branch name
// (as opposed to a commit hash, a tag, a remote-tracking ref, or HEAD). This
// filters out reflog noise like "checkout: moving from HEAD to abc1234" or
// tag/remote checkouts, which would otherwise show up as phantom deleted
// branches. The ref-name grammar itself is validated by isValidRefFormat
// (which defers to `git check-ref-format`).
func isBranchName(name string, isValidRefFormat func(string) bool) bool {
if name == "" || name == "HEAD" {
return false
}
if !isValidRefFormat(name) {
return false
}
// A name containing a slash is only treated as a branch if the part before
// the first slash is not a well-known remote marker (git writes
// "origin/feature" or "tags/v1.0" for remote/tag checkouts, and branch
// names can legitimately contain slashes, e.g. "feature/foo").
if strings.ContainsRune(name, '/') {
remote, _, _ := strings.Cut(name, "/")
return !lo.Contains([]string{"origin", "upstream", "fork", "tags", "remotes"}, remote)
}
return !looksLikeSha(name)
}
// looksLikeSha returns true if the string looks like a commit hash: all hex
// characters and at least as long as git's minimum abbreviation. This covers
// both abbreviated and full-length hashes that git writes for detached-head
// checkouts. A branch name that happens to be all-hex would be missed, but
// that's an acceptable trade-off since such names are vanishingly rare.
func looksLikeSha(name string) bool {
if len(name) < 7 {
return false
}
for _, c := range name {
if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) {
return false
}
}
return true
}
// TODO: only look at the new reflog commits, and otherwise store the recencies in
// int form against the branch to recalculate the time ago
func (self *BranchLoader) obtainReflogBranches(reflogCommits []*models.Commit) []*models.Branch {

195
pkg/commands/git_commands/deleted_branch_test.go
View file

@ -1,6 +1,8 @@
package git_commands
import (
"errors"
"strings"
"testing"
"github.com/jesseduffield/lazygit/pkg/commands/models"
@ -10,10 +12,10 @@ import (
func TestObtainDeletedBranches(t *testing.T) {
type scenario struct {
testName string
entries []*reflogEntry
currentBranchNames []string
expected []*models.DeletedBranch
testName string
entries []*reflogEntry
existingRefs []string
expected []*models.DeletedBranch
}
scenarios := []scenario{
@ -26,7 +28,7 @@ func TestObtainDeletedBranches(t *testing.T) {
{hash: "c", timestamp: 100, from: "main", to: "feature"},
{hash: "d", timestamp: 50},
},
currentBranchNames: []string{"main"},
existingRefs: []string{"main", "HEAD"},
expected: []*models.DeletedBranch{
{Name: "feature", CommitHash: "b", DisplayName: "feature", Recency: "56y", UnixTimestamp: 200},
},
@ -36,35 +38,35 @@ func TestObtainDeletedBranches(t *testing.T) {
// newest-first reflog
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "feat/a", to: "main"}, // newest: leave feat/a
{hash: "b", timestamp: 250}, // commit on feat/a
{hash: "b", timestamp: 250}, // commit on feat/a
{hash: "a", timestamp: 200, from: "feat/b", to: "feat/a"},
{hash: "c", timestamp: 150}, // commit on feat/b
{hash: "c", timestamp: 150}, // commit on feat/b
{hash: "a", timestamp: 100, from: "main", to: "feat/b"}, // oldest: create feat/b
},
currentBranchNames: []string{"main"},
existingRefs: []string{"main", "HEAD"},
expected: []*models.DeletedBranch{
{Name: "feat/a", CommitHash: "b", DisplayName: "feat/a", Recency: "56y", UnixTimestamp: 250},
{Name: "feat/b", CommitHash: "c", DisplayName: "feat/b", Recency: "56y", UnixTimestamp: 150},
},
},
{
testName: "existing branches are excluded",
entries: []*reflogEntry{
testName: "existing branches are excluded",
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "main", to: "other"},
{hash: "b", timestamp: 200},
{hash: "c", timestamp: 100, from: "other", to: "main"},
},
currentBranchNames: []string{"main", "other"},
expected: nil,
existingRefs: []string{"main", "other", "HEAD"},
expected: nil,
},
{
testName: "no checkout entries means nothing recoverable",
entries: []*reflogEntry{
testName: "no checkout entries means nothing recoverable",
entries: []*reflogEntry{
{hash: "a", timestamp: 300},
{hash: "b", timestamp: 200},
},
currentBranchNames: []string{"main"},
expected: nil,
existingRefs: []string{"main", "HEAD"},
expected: nil,
},
{
testName: "HEAD is not treated as a deleted branch",
@ -72,24 +74,161 @@ func TestObtainDeletedBranches(t *testing.T) {
{hash: "a", timestamp: 300, from: "main", to: "HEAD"},
{hash: "b", timestamp: 200},
},
currentBranchNames: []string{"main"},
expected: nil,
existingRefs: []string{"main", "HEAD"},
expected: nil,
},
{
testName: "branch created via checkout with no commits is recoverable",
// newest-first reflog: create feature, then immediately leave it
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "feature", to: "main"}, // leave feature
{hash: "b", timestamp: 200, from: "main", to: "feature"}, // create feature
},
existingRefs: []string{"main", "HEAD"},
expected: []*models.DeletedBranch{
{Name: "feature", CommitHash: "b", DisplayName: "feature", Recency: "56y", UnixTimestamp: 200},
},
},
{
testName: "detached head checkout is not treated as a deleted branch",
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "abc1234567890abcdefabcdefabcdefabcdefabcdef", to: "main"},
{hash: "b", timestamp: 200, from: "main", to: "abc1234567890abcdefabcdefabcdefabcdefabcdef"},
},
existingRefs: []string{"main", "HEAD"},
expected: nil,
},
{
testName: "existing remote-tracking ref is excluded from candidates",
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "origin/feature", to: "main"},
{hash: "b", timestamp: 200, from: "main", to: "origin/feature"},
},
existingRefs: []string{"main", "origin/feature", "HEAD"},
expected: nil,
},
{
testName: "existing tag checkout is not treated as a deleted branch",
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "v1.0.0", to: "main"},
{hash: "b", timestamp: 200, from: "main", to: "v1.0.0"},
},
existingRefs: []string{"main", "v1.0.0", "HEAD"},
expected: nil,
},
{
testName: "checkout to a commit expression is not treated as a deleted branch",
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "HEAD~1", to: "main"},
{hash: "b", timestamp: 200, from: "main", to: "HEAD~1"},
},
existingRefs: []string{"main", "HEAD"},
expected: nil,
},
{
testName: "checkout to a sha-256 hash is not treated as a deleted branch",
entries: []*reflogEntry{
{hash: "a", timestamp: 300, from: "8f0f1f2f3f4f5f6f7f8f9fafbfcfdfefff0f1f2f3f4f5f6f7f8f9fafbfcfdfeff", to: "main"},
{hash: "b", timestamp: 200, from: "main", to: "8f0f1f2f3f4f5f6f7f8f9fafbfcfdfefff0f1f2f3f4f5f6f7f8f9fafbfcfdfeff"},
},
existingRefs: []string{"main", "HEAD"},
expected: nil,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
result := obtainDeletedBranches(s.entries, s.currentBranchNames)
result := obtainDeletedBranches(s.entries, s.existingRefs, isValidRefFormatStub)
assert.Equal(t, s.expected, result)
})
}
}
// isValidRefFormatStub is a stand-in for `git check-ref-format` used to keep
// the pure tests independent of the subprocess. It applies the same ref-name
// grammar rules git enforces.
func isValidRefFormatStub(name string) bool {
if name == "" {
return false
}
if strings.HasPrefix(name, "-") {
return false
}
if strings.HasSuffix(name, ".") || strings.HasSuffix(name, "/") {
return false
}
if strings.Contains(name, "..") || strings.Contains(name, "@{") {
return false
}
for _, c := range name {
if c <= ' ' || strings.ContainsRune("~^:?*[\\", c) {
return false
}
}
return true
}
func TestIsValidRefFormat(t *testing.T) {
type scenario struct {
testName string
name string
isValid bool
}
scenarios := []scenario{
{
testName: "valid branch name",
name: "feature/foo",
isValid: true,
},
{
// a sha-looking string is still a valid ref to git; filtering it is
// done separately by looksLikeSha
testName: "sha-like is a valid ref to git",
name: "8f0f1f2f3f4f",
isValid: true,
},
{
testName: "rejects commit expression",
name: "HEAD~1",
},
{
testName: "rejects trailing slash",
name: "feature/",
},
{
testName: "rejects double dot",
name: "feature..other",
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
err := errors.New("invalid ref")
if s.isValid {
err = nil
}
runner := oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"check-ref-format", "--allow-onelevel", s.name}, "", err)
gitCommon := buildGitCommon(commonDeps{runner: runner})
loader := &BranchLoader{
Common: gitCommon.Common,
GitCommon: gitCommon,
cmd: gitCommon.cmd,
}
assert.Equal(t, s.isValid, loader.isValidRefFormat(s.name))
runner.CheckForMissingCalls()
})
}
}
func TestParseReflogCheckoutSubject(t *testing.T) {
type scenario struct {
testName string
subject string
expected []string
testName string
subject string
expected []string
}
scenarios := []scenario{
@ -121,9 +260,9 @@ func TestParseReflogCheckoutSubject(t *testing.T) {
func TestBranchRestoreBranch(t *testing.T) {
type scenario struct {
testName string
runner *oscommands.FakeCmdObjRunner
expectedErr bool
testName string
runner *oscommands.FakeCmdObjRunner
expectedErr bool
expectedUpstream string
}
@ -133,7 +272,7 @@ func TestBranchRestoreBranch(t *testing.T) {
runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"branch", "feature", "abc123"}, "", nil).
ExpectGitArgs([]string{"for-each-ref", "--format=%(refname:short)", "refs/remotes"}, "", nil),
expectedErr: false,
expectedErr: false,
expectedUpstream: "",
},
{
@ -142,7 +281,7 @@ func TestBranchRestoreBranch(t *testing.T) {
ExpectGitArgs([]string{"branch", "feature", "abc123"}, "", nil).
ExpectGitArgs([]string{"for-each-ref", "--format=%(refname:short)", "refs/remotes"}, "origin/feature\n", nil).
ExpectGitArgs([]string{"branch", "--set-upstream-to=origin/feature", "feature"}, "", nil),
expectedErr: false,
expectedErr: false,
expectedUpstream: "origin/feature",
},
{
@ -150,7 +289,7 @@ func TestBranchRestoreBranch(t *testing.T) {
runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"branch", "feature", "abc123"}, "", nil).
ExpectGitArgs([]string{"for-each-ref", "--format=%(refname:short)", "refs/remotes"}, "origin/feature\nfork/feature\n", nil),
expectedErr: false,
expectedErr: false,
expectedUpstream: "",
},
}