fix: prefer main for worktree base branches (#1532)
This commit is contained in:
@@ -4,11 +4,14 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
githubsvc "github.com/kandev/kandev/internal/github"
|
||||
"github.com/kandev/kandev/internal/repoclone"
|
||||
taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite"
|
||||
taskservice "github.com/kandev/kandev/internal/task/service"
|
||||
)
|
||||
@@ -21,6 +24,268 @@ func newTestLogger() *logger.Logger {
|
||||
return log
|
||||
}
|
||||
|
||||
func TestDetectGitDefaultBranchDetachedHEADReturnsEmpty(t *testing.T) {
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(gitDir, 0o755); err != nil {
|
||||
t.Fatalf("mkdir git dir: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "HEAD"),
|
||||
[]byte("3a3f2d3b00000000000000000000000000000000\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write detached HEAD: %v", err)
|
||||
}
|
||||
|
||||
if got := detectGitDefaultBranch(repoPath); got != "" {
|
||||
t.Fatalf("detectGitDefaultBranch = %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveReviewBaseBranchRedetectsStoredMasterWhenMainExists(t *testing.T) {
|
||||
harness := newBootStateTestHarness(t)
|
||||
ctx := context.Background()
|
||||
workspace, err := harness.taskSvc.CreateWorkspace(ctx, &taskservice.CreateWorkspaceRequest{
|
||||
Name: "Workspace",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin refs: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "refs", "remotes", "origin", "HEAD"),
|
||||
[]byte("ref: refs/remotes/origin/master\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write origin HEAD: %v", err)
|
||||
}
|
||||
for _, branch := range []string{"main", "master"} {
|
||||
refPath := filepath.Join(gitDir, "refs", "remotes", "origin", branch)
|
||||
if err := os.WriteFile(refPath, []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write %s ref: %v", branch, err)
|
||||
}
|
||||
}
|
||||
repo, err := harness.taskSvc.CreateRepository(ctx, &taskservice.CreateRepositoryRequest{
|
||||
WorkspaceID: workspace.ID,
|
||||
Name: "owner/repo",
|
||||
SourceType: "provider",
|
||||
LocalPath: repoPath,
|
||||
Provider: "github",
|
||||
ProviderOwner: "owner",
|
||||
ProviderName: "repo",
|
||||
DefaultBranch: "master",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
|
||||
adapter := &repositoryResolverAdapter{
|
||||
taskSvc: harness.taskSvc,
|
||||
logger: newTestLogger(),
|
||||
}
|
||||
if got := adapter.resolveReviewBaseBranch(ctx, repo, repoPath, ""); got != "main" {
|
||||
t.Fatalf("resolveReviewBaseBranch = %q, want main", got)
|
||||
}
|
||||
|
||||
stored, err := harness.taskSvc.GetRepository(ctx, repo.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetRepository: %v", err)
|
||||
}
|
||||
if stored.DefaultBranch != "main" {
|
||||
t.Fatalf("stored default_branch = %q, want main", stored.DefaultBranch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveReviewBaseBranchSkipsNoopPersistForStoredMaster(t *testing.T) {
|
||||
harness := newBootStateTestHarness(t)
|
||||
ctx := context.Background()
|
||||
workspace, err := harness.taskSvc.CreateWorkspace(ctx, &taskservice.CreateWorkspaceRequest{
|
||||
Name: "Workspace",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin refs: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "refs", "remotes", "origin", "HEAD"),
|
||||
[]byte("ref: refs/remotes/origin/master\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write origin HEAD: %v", err)
|
||||
}
|
||||
refPath := filepath.Join(gitDir, "refs", "remotes", "origin", "master")
|
||||
if err := os.WriteFile(refPath, []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write master ref: %v", err)
|
||||
}
|
||||
repo, err := harness.taskSvc.CreateRepository(ctx, &taskservice.CreateRepositoryRequest{
|
||||
WorkspaceID: workspace.ID,
|
||||
Name: "owner/repo",
|
||||
SourceType: "provider",
|
||||
LocalPath: repoPath,
|
||||
Provider: "github",
|
||||
ProviderOwner: "owner",
|
||||
ProviderName: "repo",
|
||||
DefaultBranch: "master",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
|
||||
adapter := &repositoryResolverAdapter{
|
||||
taskSvc: harness.taskSvc,
|
||||
logger: newTestLogger(),
|
||||
}
|
||||
if got := adapter.resolveReviewBaseBranch(ctx, repo, repoPath, ""); got != "master" {
|
||||
t.Fatalf("resolveReviewBaseBranch = %q, want master", got)
|
||||
}
|
||||
|
||||
stored, err := harness.taskSvc.GetRepository(ctx, repo.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetRepository: %v", err)
|
||||
}
|
||||
if !stored.UpdatedAt.Equal(repo.UpdatedAt) {
|
||||
t.Fatalf("stored updated_at = %v, want unchanged %v", stored.UpdatedAt, repo.UpdatedAt)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveReviewBaseBranchKeepsStoredMasterOnHeadFallback(t *testing.T) {
|
||||
harness := newBootStateTestHarness(t)
|
||||
ctx := context.Background()
|
||||
workspace, err := harness.taskSvc.CreateWorkspace(ctx, &taskservice.CreateWorkspaceRequest{
|
||||
Name: "Workspace",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(gitDir, 0o755); err != nil {
|
||||
t.Fatalf("mkdir git dir: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
repo, err := harness.taskSvc.CreateRepository(ctx, &taskservice.CreateRepositoryRequest{
|
||||
WorkspaceID: workspace.ID,
|
||||
Name: "owner/repo",
|
||||
SourceType: "provider",
|
||||
LocalPath: repoPath,
|
||||
Provider: "github",
|
||||
ProviderOwner: "owner",
|
||||
ProviderName: "repo",
|
||||
DefaultBranch: "master",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
|
||||
adapter := &repositoryResolverAdapter{
|
||||
taskSvc: harness.taskSvc,
|
||||
logger: newTestLogger(),
|
||||
}
|
||||
if got := adapter.resolveReviewBaseBranch(ctx, repo, repoPath, ""); got != "master" {
|
||||
t.Fatalf("resolveReviewBaseBranch = %q, want master", got)
|
||||
}
|
||||
|
||||
stored, err := harness.taskSvc.GetRepository(ctx, repo.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetRepository: %v", err)
|
||||
}
|
||||
if stored.DefaultBranch != "master" {
|
||||
t.Fatalf("stored default_branch = %q, want master", stored.DefaultBranch)
|
||||
}
|
||||
if !stored.UpdatedAt.Equal(repo.UpdatedAt) {
|
||||
t.Fatalf("stored updated_at = %v, want unchanged %v", stored.UpdatedAt, repo.UpdatedAt)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveForReviewRedetectsStoredMasterAfterClonePath(t *testing.T) {
|
||||
harness := newBootStateTestHarness(t)
|
||||
ctx := context.Background()
|
||||
workspace, err := harness.taskSvc.CreateWorkspace(ctx, &taskservice.CreateWorkspaceRequest{
|
||||
Name: "Workspace",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
basePath := t.TempDir()
|
||||
repoPath := filepath.Join(basePath, "owner", "repo")
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin refs: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "refs", "remotes", "origin", "HEAD"),
|
||||
[]byte("ref: refs/remotes/origin/master\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write origin HEAD: %v", err)
|
||||
}
|
||||
for _, branch := range []string{"main", "master"} {
|
||||
refPath := filepath.Join(gitDir, "refs", "remotes", "origin", branch)
|
||||
if err := os.WriteFile(refPath, []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write %s ref: %v", branch, err)
|
||||
}
|
||||
}
|
||||
repo, err := harness.taskSvc.CreateRepository(ctx, &taskservice.CreateRepositoryRequest{
|
||||
WorkspaceID: workspace.ID,
|
||||
Name: "owner/repo",
|
||||
SourceType: "provider",
|
||||
Provider: "github",
|
||||
ProviderOwner: "owner",
|
||||
ProviderName: "repo",
|
||||
DefaultBranch: "master",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
|
||||
adapter := &repositoryResolverAdapter{
|
||||
cloner: repoclone.NewCloner(repoclone.Config{BasePath: basePath}, repoclone.ProtocolHTTPS, "", newTestLogger()),
|
||||
taskSvc: harness.taskSvc,
|
||||
logger: newTestLogger(),
|
||||
}
|
||||
repoID, baseBranch, err := adapter.ResolveForReview(ctx, workspace.ID, "github", "owner", "repo", "")
|
||||
if err != nil {
|
||||
t.Fatalf("ResolveForReview: %v", err)
|
||||
}
|
||||
if repoID != repo.ID {
|
||||
t.Fatalf("repository ID = %q, want %q", repoID, repo.ID)
|
||||
}
|
||||
if baseBranch != "main" {
|
||||
t.Fatalf("base branch = %q, want main", baseBranch)
|
||||
}
|
||||
|
||||
stored, err := harness.taskSvc.GetRepository(ctx, repo.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetRepository: %v", err)
|
||||
}
|
||||
if stored.LocalPath != repoPath {
|
||||
t.Fatalf("stored local_path = %q, want %q", stored.LocalPath, repoPath)
|
||||
}
|
||||
if stored.DefaultBranch != "main" {
|
||||
t.Fatalf("stored default_branch = %q, want main", stored.DefaultBranch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetSessionModel_Caching(t *testing.T) {
|
||||
// We can't easily instantiate a full taskservice.Service here without a DB,
|
||||
// so we test the caching mechanism directly on the adapter struct.
|
||||
|
||||
@@ -18,6 +18,7 @@ import (
|
||||
agentsettingscontroller "github.com/kandev/kandev/internal/agent/settings/controller"
|
||||
settingsstore "github.com/kandev/kandev/internal/agent/settings/store"
|
||||
"github.com/kandev/kandev/internal/common/config"
|
||||
"github.com/kandev/kandev/internal/common/gitref"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/db"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
@@ -37,6 +38,11 @@ import (
|
||||
workflowservice "github.com/kandev/kandev/internal/workflow/service"
|
||||
)
|
||||
|
||||
const (
|
||||
defaultMainBranch = "main"
|
||||
defaultMasterBranch = "master"
|
||||
)
|
||||
|
||||
const defaultEventNamespace = "default"
|
||||
|
||||
func provideOrchestrator(
|
||||
@@ -661,13 +667,7 @@ func (a *repositoryResolverAdapter) ResolveForReview(
|
||||
return "", "", fmt.Errorf("lookup repository by provider info: %w", err)
|
||||
}
|
||||
if existing != nil && existing.LocalPath != "" {
|
||||
baseBranch := defaultBranch
|
||||
if baseBranch == "" {
|
||||
baseBranch = existing.DefaultBranch
|
||||
}
|
||||
if baseBranch == "" {
|
||||
baseBranch = a.detectAndPersistDefaultBranch(ctx, existing, existing.LocalPath)
|
||||
}
|
||||
baseBranch := a.resolveReviewBaseBranch(ctx, existing, existing.LocalPath, defaultBranch)
|
||||
return existing.ID, baseBranch, nil
|
||||
}
|
||||
|
||||
@@ -693,18 +693,31 @@ func (a *repositoryResolverAdapter) ResolveForReview(
|
||||
return "", "", fmt.Errorf("find/create repository: %w", err)
|
||||
}
|
||||
|
||||
baseBranch := defaultBranch
|
||||
if baseBranch == "" {
|
||||
baseBranch = repo.DefaultBranch
|
||||
}
|
||||
// When no default branch is known (e.g. issue watch with no PR context),
|
||||
// detect it from the cloned repo's HEAD.
|
||||
if baseBranch == "" && localPath != "" {
|
||||
baseBranch = a.detectAndPersistDefaultBranch(ctx, repo, localPath)
|
||||
}
|
||||
baseBranch := a.resolveReviewBaseBranch(ctx, repo, localPath, defaultBranch)
|
||||
return repo.ID, baseBranch, nil
|
||||
}
|
||||
|
||||
func (a *repositoryResolverAdapter) resolveReviewBaseBranch(
|
||||
ctx context.Context,
|
||||
repo *taskmodels.Repository,
|
||||
localPath string,
|
||||
requestedBranch string,
|
||||
) string {
|
||||
if requestedBranch != "" {
|
||||
return requestedBranch
|
||||
}
|
||||
stored := strings.TrimSpace(repo.DefaultBranch)
|
||||
if stored == defaultMasterBranch && localPath != "" {
|
||||
if detected := detectGitDefaultBranch(localPath); detected == defaultMainBranch {
|
||||
return a.persistDetectedDefaultBranch(ctx, repo, detected)
|
||||
}
|
||||
}
|
||||
if stored != "" {
|
||||
return stored
|
||||
}
|
||||
return a.detectAndPersistDefaultBranch(ctx, repo, localPath)
|
||||
}
|
||||
|
||||
// detectAndPersistDefaultBranch reads the default branch from the local clone
|
||||
// and persists it to the repository record for future lookups.
|
||||
func (a *repositoryResolverAdapter) detectAndPersistDefaultBranch(
|
||||
@@ -714,6 +727,17 @@ func (a *repositoryResolverAdapter) detectAndPersistDefaultBranch(
|
||||
if detected == "" {
|
||||
return ""
|
||||
}
|
||||
return a.persistDetectedDefaultBranch(ctx, repo, detected)
|
||||
}
|
||||
|
||||
func (a *repositoryResolverAdapter) persistDetectedDefaultBranch(
|
||||
ctx context.Context,
|
||||
repo *taskmodels.Repository,
|
||||
detected string,
|
||||
) string {
|
||||
if strings.TrimSpace(repo.DefaultBranch) == detected {
|
||||
return detected
|
||||
}
|
||||
if _, err := a.taskSvc.UpdateRepository(ctx, repo.ID, &taskservice.UpdateRepositoryRequest{
|
||||
DefaultBranch: &detected,
|
||||
}); err != nil {
|
||||
@@ -726,26 +750,11 @@ func (a *repositoryResolverAdapter) detectAndPersistDefaultBranch(
|
||||
}
|
||||
|
||||
// detectGitDefaultBranch reads the default branch of a git repository.
|
||||
// It first checks refs/remotes/origin/HEAD (set by `git clone`), then
|
||||
// falls back to .git/HEAD. Returns empty string on any failure.
|
||||
// Returns empty string on any failure.
|
||||
func detectGitDefaultBranch(repoPath string) string {
|
||||
// Prefer the remote default branch pointer set by `git clone`.
|
||||
originHead := filepath.Join(repoPath, ".git", "refs", "remotes", "origin", "HEAD")
|
||||
if content, err := os.ReadFile(originHead); err == nil {
|
||||
trimmed := strings.TrimSpace(string(content))
|
||||
if after, ok := strings.CutPrefix(trimmed, "ref: refs/remotes/origin/"); ok {
|
||||
return after
|
||||
}
|
||||
}
|
||||
// Fall back to the local HEAD (works for fresh clones that lack origin/HEAD).
|
||||
headPath := filepath.Join(repoPath, ".git", "HEAD")
|
||||
content, err := os.ReadFile(headPath)
|
||||
branch, err := gitref.DefaultBranchOrEmpty(repoPath)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
trimmed := strings.TrimSpace(string(content))
|
||||
if after, ok := strings.CutPrefix(trimmed, "ref: refs/heads/"); ok {
|
||||
return after
|
||||
}
|
||||
return ""
|
||||
return branch
|
||||
}
|
||||
|
||||
@@ -22,10 +22,13 @@ const headFile = "HEAD"
|
||||
// to the wrong ref.
|
||||
//
|
||||
// Resolution order:
|
||||
// 1. refs/remotes/origin/HEAD when set (the upstream's declared default)
|
||||
// 2. The first ref that exists from
|
||||
// {origin/main, origin/master, main, master}
|
||||
// 3. The current HEAD as a last resort, so brand-new repos with only a
|
||||
// 1. refs/remotes/origin/HEAD when set, except origin/HEAD=master yields to
|
||||
// origin/main when it exists
|
||||
// 2. origin/main
|
||||
// 3. origin/master
|
||||
// 4. local main
|
||||
// 5. local master
|
||||
// 6. The current HEAD as a last resort, so brand-new repos with only a
|
||||
// feature branch still produce a value — callers that care about
|
||||
// correctness can override.
|
||||
func DefaultBranch(repoPath string) (string, error) {
|
||||
@@ -38,15 +41,44 @@ func DefaultBranch(repoPath string) (string, error) {
|
||||
return "", err
|
||||
}
|
||||
commonDir := ResolveCommonGitDir(gitDir)
|
||||
commonDir, err = guardRepoPath(commonDir)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if branch := readOriginHEAD(commonDir); branch != "" {
|
||||
if branch == "master" {
|
||||
if refExists(commonDir, "refs/remotes/origin/main") {
|
||||
return "main", nil
|
||||
}
|
||||
}
|
||||
return branch, nil
|
||||
}
|
||||
if branch := pickFirstExistingBranch(commonDir); branch != "" {
|
||||
return branch, nil
|
||||
for _, candidate := range []struct {
|
||||
ref string
|
||||
branch string
|
||||
}{
|
||||
{"refs/remotes/origin/main", "main"},
|
||||
{"refs/remotes/origin/master", "master"},
|
||||
{"refs/heads/main", "main"},
|
||||
{"refs/heads/master", "master"},
|
||||
} {
|
||||
if refExists(commonDir, candidate.ref) {
|
||||
return candidate.branch, nil
|
||||
}
|
||||
}
|
||||
return readHEADBranchFallback(gitDir)
|
||||
}
|
||||
|
||||
// DefaultBranchOrEmpty returns DefaultBranch, but collapses detached HEAD's
|
||||
// literal "HEAD" sentinel to empty for callers that persist branch names.
|
||||
func DefaultBranchOrEmpty(repoPath string) (string, error) {
|
||||
branch, err := DefaultBranch(repoPath)
|
||||
if err != nil || branch == headFile {
|
||||
return "", err
|
||||
}
|
||||
return branch, nil
|
||||
}
|
||||
|
||||
// guardRepoPath rejects relative paths and any path containing `..` segments
|
||||
// before passing the value into the file-reading helpers below. Callers
|
||||
// already validate against an allowlist (see service.resolveAllowedLocalPath),
|
||||
@@ -128,8 +160,8 @@ func readOriginHEAD(commonDir string) string {
|
||||
// origin/HEAD only lives in packed-refs after a `git pack-refs --all`,
|
||||
// and the on-disk symref format there is gnarly to parse correctly.
|
||||
// We deliberately skip that case rather than maintain a broken parser:
|
||||
// pickFirstExistingBranch's origin/main → origin/master → main →
|
||||
// master fallbacks cover every realistic clone in practice.
|
||||
// the named main/master fallbacks cover every realistic clone in
|
||||
// practice.
|
||||
return ""
|
||||
}
|
||||
return parseSymbolicRefToBranch(strings.TrimSpace(string(content)))
|
||||
@@ -149,23 +181,13 @@ func parseSymbolicRefToBranch(line string) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
func pickFirstExistingBranch(commonDir string) string {
|
||||
for _, candidate := range []struct{ ref, branch string }{
|
||||
{"refs/remotes/origin/main", "main"},
|
||||
{"refs/remotes/origin/master", "master"},
|
||||
{"refs/heads/main", "main"},
|
||||
{"refs/heads/master", "master"},
|
||||
} {
|
||||
if refExists(commonDir, candidate.ref) {
|
||||
return candidate.branch
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func refExists(commonDir, ref string) bool {
|
||||
if _, err := os.Stat(filepath.Join(commonDir, ref)); err == nil {
|
||||
return true
|
||||
refPath, ok := safeRefPath(commonDir, ref)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
if info, err := os.Stat(refPath); err == nil {
|
||||
return !info.IsDir()
|
||||
}
|
||||
content, err := os.ReadFile(filepath.Join(commonDir, "packed-refs"))
|
||||
if err != nil {
|
||||
@@ -187,6 +209,26 @@ func refExists(commonDir, ref string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
func safeRefPath(commonDir, ref string) (string, bool) {
|
||||
if ref == "" || filepath.IsAbs(ref) {
|
||||
return "", false
|
||||
}
|
||||
for _, part := range strings.FieldsFunc(ref, func(r rune) bool {
|
||||
return r == '/' || r == filepath.Separator
|
||||
}) {
|
||||
if part == "" || part == "." || part == ".." {
|
||||
return "", false
|
||||
}
|
||||
}
|
||||
cleanCommon := filepath.Clean(commonDir)
|
||||
candidate := filepath.Join(cleanCommon, filepath.Clean(ref))
|
||||
rel, err := filepath.Rel(cleanCommon, candidate)
|
||||
if err != nil || rel == "." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || rel == ".." {
|
||||
return "", false
|
||||
}
|
||||
return candidate, true
|
||||
}
|
||||
|
||||
func readHEADBranchFallback(gitDir string) (string, error) {
|
||||
headPath := filepath.Join(gitDir, headFile)
|
||||
content, err := os.ReadFile(headPath)
|
||||
|
||||
@@ -65,3 +65,162 @@ func TestDefaultBranch_AcceptsAbsolutePathToValidRepo(t *testing.T) {
|
||||
t.Fatalf("expected main, got %q", branch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultBranch_PrefersMainWhenOriginHeadPointsAtMaster(t *testing.T) {
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin refs: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "refs", "remotes", "origin", "HEAD"),
|
||||
[]byte("ref: refs/remotes/origin/master\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write origin HEAD: %v", err)
|
||||
}
|
||||
for _, branch := range []string{"main", "master"} {
|
||||
refPath := filepath.Join(gitDir, "refs", "remotes", "origin", branch)
|
||||
if err := os.WriteFile(refPath, []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write %s ref: %v", branch, err)
|
||||
}
|
||||
}
|
||||
|
||||
branch, err := DefaultBranch(repoPath)
|
||||
if err != nil {
|
||||
t.Fatalf("DefaultBranch error: %v", err)
|
||||
}
|
||||
if branch != "main" {
|
||||
t.Fatalf("DefaultBranch = %q, want main", branch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultBranch_PreservesNonMainOriginHeadWhenMainExists(t *testing.T) {
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin refs: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "refs", "remotes", "origin", "HEAD"),
|
||||
[]byte("ref: refs/remotes/origin/trunk\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write origin HEAD: %v", err)
|
||||
}
|
||||
for _, branch := range []string{"main", "trunk"} {
|
||||
refPath := filepath.Join(gitDir, "refs", "remotes", "origin", branch)
|
||||
if err := os.WriteFile(refPath, []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write %s ref: %v", branch, err)
|
||||
}
|
||||
}
|
||||
|
||||
branch, err := DefaultBranch(repoPath)
|
||||
if err != nil {
|
||||
t.Fatalf("DefaultBranch error: %v", err)
|
||||
}
|
||||
if branch != "trunk" {
|
||||
t.Fatalf("DefaultBranch = %q, want trunk", branch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultBranch_LocalMainDoesNotOverrideOriginHeadMaster(t *testing.T) {
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin refs: %v", err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "heads"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir local refs: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "refs", "remotes", "origin", "HEAD"),
|
||||
[]byte("ref: refs/remotes/origin/master\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write origin HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "refs", "remotes", "origin", "master"), []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write origin master ref: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "refs", "heads", "main"), []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write local main ref: %v", err)
|
||||
}
|
||||
|
||||
branch, err := DefaultBranch(repoPath)
|
||||
if err != nil {
|
||||
t.Fatalf("DefaultBranch error: %v", err)
|
||||
}
|
||||
if branch != "master" {
|
||||
t.Fatalf("DefaultBranch = %q, want master", branch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultBranch_RemoteMasterBeatsLocalMainWithoutOriginHead(t *testing.T) {
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin refs: %v", err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "heads"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir local refs: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "refs", "remotes", "origin", "master"), []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write origin master ref: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "refs", "heads", "main"), []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write local main ref: %v", err)
|
||||
}
|
||||
|
||||
branch, err := DefaultBranch(repoPath)
|
||||
if err != nil {
|
||||
t.Fatalf("DefaultBranch error: %v", err)
|
||||
}
|
||||
if branch != "master" {
|
||||
t.Fatalf("DefaultBranch = %q, want master", branch)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultBranch_MainNamespaceDoesNotOverrideOriginHeadMaster(t *testing.T) {
|
||||
repoPath := t.TempDir()
|
||||
gitDir := filepath.Join(repoPath, ".git")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin", "main"), 0o755); err != nil {
|
||||
t.Fatalf("mkdir origin main namespace: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "HEAD"), []byte("ref: refs/heads/feature/x\n"), 0o644); err != nil {
|
||||
t.Fatalf("write HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(
|
||||
filepath.Join(gitDir, "refs", "remotes", "origin", "HEAD"),
|
||||
[]byte("ref: refs/remotes/origin/master\n"),
|
||||
0o644,
|
||||
); err != nil {
|
||||
t.Fatalf("write origin HEAD: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "refs", "remotes", "origin", "main", "foo"), []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write origin main/foo ref: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(gitDir, "refs", "remotes", "origin", "master"), []byte("0000000\n"), 0o644); err != nil {
|
||||
t.Fatalf("write origin master ref: %v", err)
|
||||
}
|
||||
|
||||
branch, err := DefaultBranch(repoPath)
|
||||
if err != nil {
|
||||
t.Fatalf("DefaultBranch error: %v", err)
|
||||
}
|
||||
if branch != "master" {
|
||||
t.Fatalf("DefaultBranch = %q, want master", branch)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -199,7 +199,7 @@ func (e *Executor) backfillRepoDefaultBranch(ctx context.Context, repo *models.R
|
||||
if repo.DefaultBranch != "" || localPath == "" {
|
||||
return
|
||||
}
|
||||
detected, err := gitref.DefaultBranch(localPath)
|
||||
detected, err := gitref.DefaultBranchOrEmpty(localPath)
|
||||
if err != nil || detected == "" {
|
||||
e.logger.Debug("could not detect default branch from clone; leaving empty",
|
||||
zap.String("repository_id", repo.ID),
|
||||
|
||||
@@ -147,11 +147,11 @@ func seedBareGitDir(t *testing.T, repoPath, headContent string) string {
|
||||
return gitDir
|
||||
}
|
||||
|
||||
// TestReadGitDefaultBranch_PrefersOriginHEAD — the upstream's declared
|
||||
// default beats every other heuristic. Regression for the bug where the
|
||||
// repo's stored default_branch latched onto whatever feature branch the user
|
||||
// happened to be on at task-creation time.
|
||||
func TestReadGitDefaultBranch_PrefersOriginHEAD(t *testing.T) {
|
||||
// TestReadGitDefaultBranch_UsesOriginHEADWhenMainRefAbsent — origin/HEAD still
|
||||
// beats the checked-out branch when no main ref exists. Regression for the bug
|
||||
// where the repo's stored default_branch latched onto whatever feature branch
|
||||
// the user happened to be on at task-creation time.
|
||||
func TestReadGitDefaultBranch_UsesOriginHEADWhenMainRefAbsent(t *testing.T) {
|
||||
repoPath := t.TempDir()
|
||||
gitDir := seedBareGitDir(t, repoPath, "ref: refs/heads/feature/x\n")
|
||||
if err := os.MkdirAll(filepath.Join(gitDir, "refs", "remotes", "origin"), 0o755); err != nil {
|
||||
|
||||
@@ -473,7 +473,7 @@ func (s *Service) resolveRepoInputLocal(
|
||||
// and feeds into os.Stat/ReadFile inside gitref.DefaultBranch, so
|
||||
// without this guard a caller could traverse the filesystem.
|
||||
if safePath, pathErr := s.resolveAllowedLocalPath(repoInput.LocalPath); pathErr == nil {
|
||||
if probed, err := gitref.DefaultBranch(safePath); err == nil && probed != "" && probed != "HEAD" {
|
||||
if probed, err := gitref.DefaultBranchOrEmpty(safePath); err == nil && probed != "" {
|
||||
defaultBranch = probed
|
||||
}
|
||||
}
|
||||
|
||||
@@ -55,7 +55,7 @@ describe("selectPreferredBranch", () => {
|
||||
expect(selectPreferredBranch(branches)).toBe("main");
|
||||
});
|
||||
|
||||
it("prefers local master over origin/main", () => {
|
||||
it("keeps local master ahead of origin/main", () => {
|
||||
const branches = [
|
||||
{ name: "master", type: "local" },
|
||||
{ name: "main", type: "remote", remote: "origin" },
|
||||
@@ -63,6 +63,15 @@ describe("selectPreferredBranch", () => {
|
||||
expect(selectPreferredBranch(branches)).toBe("master");
|
||||
});
|
||||
|
||||
it("keeps local master ahead of remote conventional branches", () => {
|
||||
const branches = [
|
||||
{ name: "master", type: "local" },
|
||||
{ name: "master", type: "remote", remote: "origin" },
|
||||
{ name: "main", type: "remote", remote: "origin" },
|
||||
];
|
||||
expect(selectPreferredBranch(branches)).toBe("master");
|
||||
});
|
||||
|
||||
it("falls back to origin/main when no local main/master", () => {
|
||||
const branches = [
|
||||
{ name: "main", type: "remote", remote: "origin" },
|
||||
|
||||
Reference in New Issue
Block a user