fix(git): improve parsing of worktree list (#2548)

Signed-off-by: Hidde Beydals <[email protected]>

internal/controller/git/bare_repo.go

@@ -42,6 +42,21 @@ type bareRepo struct {
 	*baseRepo
 }
 
+// workTreeInfo represents information about a working tree.
+type workTreeInfo struct {
+	// Path is the absolute path to the working tree.
+	Path string
+	// HEAD is the commit ID of the HEAD of the working tree.
+	HEAD string
+	// Branch is the name of the branch that the working tree is on,
+	// or an empty string if the working tree is in a detached HEAD state.
+	Branch string
+	// Bare is true if the working tree is a bare repository.
+	Bare bool
+	// Detached is true if the working tree is in a detached HEAD state.
+	Detached bool
+}
+
 // BareCloneOptions represents options for cloning a Git repository without a
 // working tree.
 type BareCloneOptions struct {
@@ -239,21 +254,74 @@ func (b *bareRepo) WorkTrees() ([]WorkTree, error) {
 }
 
 func (b *bareRepo) workTrees() ([]string, error) {
-	res, err := libExec.Exec(b.buildGitCommand("worktree", "list"))
+	res, err := libExec.Exec(b.buildGitCommand("worktree", "list", "--porcelain"))
 	if err != nil {
 		return nil, fmt.Errorf("error listing working trees: %w", err)
 	}
-	workTrees := []string{}
-	scanner := bufio.NewScanner(bytes.NewReader(res))
+	trees, err := b.parseWorkTreeOutput(res)
+	if err != nil {
+		return nil, fmt.Errorf("error listing repository trees: %w", err)
+	}
+	return b.filterNonBarePaths(trees), nil
+}
+
+func (b *bareRepo) parseWorkTreeOutput(output []byte) ([]workTreeInfo, error) {
+	var trees []workTreeInfo
+	var current *workTreeInfo
+
+	scanner := bufio.NewScanner(bytes.NewReader(output))
 	for scanner.Scan() {
 		line := scanner.Text()
-		if !strings.HasSuffix(line, "(bare)") {
-			fields := strings.Fields(line)
-			if len(fields) != 3 {
-				return nil, fmt.Errorf("unexpected number of fields: %q", line)
+		parts := strings.SplitN(line, " ", 2)
+
+		key := parts[0]
+		value := ""
+		if len(parts) > 1 {
+			value = parts[1]
+		}
+
+		switch key {
+		case "worktree":
+			if current != nil {
+				trees = append(trees, *current)
+			}
+			current = &workTreeInfo{Path: value}
+		case "HEAD":
+			if current != nil {
+				current.HEAD = value
+			}
+		case "branch":
+			if current != nil {
+				current.Branch = value
+			}
+		case "bare":
+			if current != nil {
+				current.Bare = true
+			}
+		case "detached":
+			if current != nil {
+				current.Detached = true
 			}
-			workTrees = append(workTrees, fields[0])
 		}
 	}
-	return workTrees, err
+
+	if current != nil {
+		trees = append(trees, *current)
+	}
+
+	if err := scanner.Err(); err != nil {
+		return nil, fmt.Errorf("error scanning worktree output: %w", err)
+	}
+
+	return trees, nil
+}
+
+func (b *bareRepo) filterNonBarePaths(trees []workTreeInfo) []string {
+	var paths []string
+	for _, info := range trees {
+		if !info.Bare {
+			paths = append(paths, info.Path)
+		}
+	}
+	return paths
 }

internal/controller/git/bare_repo_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/google/uuid"
 	"github.com/sosedoff/gitkit"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/akuity/kargo/internal/types"
@@ -159,5 +160,164 @@ func TestBareRepo(t *testing.T) {
 		require.Error(t, err)
 		require.True(t, os.IsNotExist(err))
 	})
+}
+
+func Test_bareRepo_parseWorkTreeOutput(t *testing.T) {
+	tests := []struct {
+		name       string
+		input      []byte
+		assertions func(*testing.T, []workTreeInfo, error)
+	}{
+		{
+			name: "single worktree",
+			input: []byte(`worktree /path/to/worktree
+HEAD abcdef1234567890
+branch main
+`),
+			assertions: func(t *testing.T, result []workTreeInfo, err error) {
+				assert.NoError(t, err)
+				assert.Len(t, result, 1)
+				assert.Equal(t, result, []workTreeInfo{
+					{Path: "/path/to/worktree", HEAD: "abcdef1234567890", Branch: "main"},
+				})
+			},
+		},
+		{
+			name: "multiple worktrees",
+			input: []byte(`worktree /path/to/worktree1
+HEAD abcdef1234567890
+branch main
+
+worktree /path/to/worktree2
+HEAD fedcba9876543210
+branch feature
+bare
+detached
+`),
+			assertions: func(t *testing.T, result []workTreeInfo, err error) {
+				assert.NoError(t, err)
+				assert.Len(t, result, 2)
+				assert.Equal(t, result, []workTreeInfo{
+					{
+						Path:   "/path/to/worktree1",
+						HEAD:   "abcdef1234567890",
+						Branch: "main",
+					},
+					{
+						Path:     "/path/to/worktree2",
+						HEAD:     "fedcba9876543210",
+						Branch:   "feature",
+						Bare:     true,
+						Detached: true,
+					},
+				})
+			},
+		},
+		{
+			name:  "empty input",
+			input: []byte(``),
+			assertions: func(t *testing.T, result []workTreeInfo, err error) {
+				assert.NoError(t, err)
+				assert.Empty(t, result)
+			},
+		},
+		{
+			name: "incomplete worktree info",
+			input: []byte(`worktree /path/to/incomplete
+HEAD
+branch
+
+worktree /path/to/complete
+HEAD abcdef1234567890
+branch main
+`),
+			assertions: func(t *testing.T, result []workTreeInfo, err error) {
+				assert.NoError(t, err)
+				assert.Len(t, result, 2)
+				assert.Equal(t, result, []workTreeInfo{
+					{Path: "/path/to/incomplete"},
+					{
+						Path:   "/path/to/complete",
+						HEAD:   "abcdef1234567890",
+						Branch: "main",
+					},
+				})
+			},
+		},
+		{
+			name: "invalid input",
+			input: []byte(`invalid input
+not a worktree
+`),
+			assertions: func(t *testing.T, result []workTreeInfo, err error) {
+				assert.NoError(t, err)
+				assert.Empty(t, result)
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b := &bareRepo{}
+			result, err := b.parseWorkTreeOutput(tt.input)
+			tt.assertions(t, result, err)
+		})
+	}
+}
+
+func Test_bareRepo_filterNonBarePaths(t *testing.T) {
+	tests := []struct {
+		name       string
+		input      []workTreeInfo
+		assertions func(*testing.T, []string)
+	}{
+		{
+			name: "mixed bare and non-bare worktrees",
+			input: []workTreeInfo{
+				{Path: "/path/to/worktree1", Bare: false},
+				{Path: "/path/to/worktree2", Bare: true},
+				{Path: "/path/to/worktree3", Bare: false},
+			},
+			assertions: func(t *testing.T, result []string) {
+				assert.Len(t, result, 2)
+				assert.Equal(t, result, []string{"/path/to/worktree1", "/path/to/worktree3"})
+			},
+		},
+		{
+			name: "all non-bare worktrees",
+			input: []workTreeInfo{
+				{Path: "/path/to/worktree1", Bare: false},
+				{Path: "/path/to/worktree2", Bare: false},
+			},
+			assertions: func(t *testing.T, result []string) {
+				assert.Len(t, result, 2)
+				assert.Equal(t, result, []string{"/path/to/worktree1", "/path/to/worktree2"})
+			},
+		},
+		{
+			name: "all bare worktrees",
+			input: []workTreeInfo{
+				{Path: "/path/to/worktree1", Bare: true},
+				{Path: "/path/to/worktree2", Bare: true},
+			},
+			assertions: func(t *testing.T, result []string) {
+				assert.Empty(t, result)
+			},
+		},
+		{
+			name:  "empty input",
+			input: []workTreeInfo{},
+			assertions: func(t *testing.T, result []string) {
+				assert.Empty(t, result)
+			},
+		},
+	}
 
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b := &bareRepo{}
+			result := b.filterNonBarePaths(tt.input)
+			tt.assertions(t, result)
+		})
+	}
 }