fix: prevent sync.branch from being set to main/master (#807)
Two issues fixed: 1. `bd init` was auto-detecting current branch (e.g., main) as sync.branch when no --branch flag was specified. This caused worktree conflicts. 2. Added validation to reject main/master as sync.branch values. When sync.branch is set to main, the worktree mechanism creates a checkout of main at .git/beads-worktrees/main/, which prevents git checkout main from working in the user's working directory. The sync branch feature should use a dedicated branch like 'beads-sync'. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Executed-By: beads/crew/dave Rig: beads Role: crew
This commit is contained in:
@@ -287,16 +287,13 @@ With --stealth: configures per-repository git settings for invisible beads usage
|
|||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set sync.branch: use explicit --branch flag, or auto-detect current branch
|
// Set sync.branch only if explicitly specified via --branch flag
|
||||||
// This ensures bd sync --status works after bd init
|
// GH#807: Do NOT auto-detect current branch - if sync.branch is set to main/master,
|
||||||
if branch == "" && isGitRepo() {
|
// the worktree created by bd sync will check out main, preventing the user from
|
||||||
// Auto-detect current branch if not specified
|
// checking out main in their working directory (git error: "'main' is already checked out")
|
||||||
currentBranch, err := getGitBranch()
|
//
|
||||||
if err == nil && currentBranch != "" {
|
// When --branch is not specified, bd sync will commit directly to the current branch
|
||||||
branch = currentBranch
|
// (the original behavior before sync branch feature)
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if branch != "" {
|
if branch != "" {
|
||||||
if err := syncbranch.Set(ctx, store, branch); err != nil {
|
if err := syncbranch.Set(ctx, store, branch); err != nil {
|
||||||
fmt.Fprintf(os.Stderr, "Error: failed to set sync branch: %v\n", err)
|
fmt.Fprintf(os.Stderr, "Error: failed to set sync branch: %v\n", err)
|
||||||
|
|||||||
@@ -65,6 +65,23 @@ func ValidateBranchName(name string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ValidateSyncBranchName checks if a branch name is valid for use as sync.branch.
|
||||||
|
// GH#807: Setting sync.branch to 'main' or 'master' causes problems because the
|
||||||
|
// worktree mechanism will check out that branch, preventing the user from checking
|
||||||
|
// it out in their working directory.
|
||||||
|
func ValidateSyncBranchName(name string) error {
|
||||||
|
if err := ValidateBranchName(name); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// GH#807: Reject main/master as sync branch - these cause worktree conflicts
|
||||||
|
if name == "main" || name == "master" {
|
||||||
|
return fmt.Errorf("cannot use '%s' as sync branch: git worktrees prevent checking out the same branch in multiple locations. Use a dedicated branch like 'beads-sync' instead", name)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// Get retrieves the sync branch configuration with the following precedence:
|
// Get retrieves the sync branch configuration with the following precedence:
|
||||||
// 1. BEADS_SYNC_BRANCH environment variable
|
// 1. BEADS_SYNC_BRANCH environment variable
|
||||||
// 2. sync-branch from config.yaml (version controlled, shared across clones)
|
// 2. sync-branch from config.yaml (version controlled, shared across clones)
|
||||||
@@ -178,7 +195,8 @@ func getConfigFromDB(dbPath string, key string) string {
|
|||||||
|
|
||||||
// Set stores the sync branch configuration in the database
|
// Set stores the sync branch configuration in the database
|
||||||
func Set(ctx context.Context, store storage.Storage, branch string) error {
|
func Set(ctx context.Context, store storage.Storage, branch string) error {
|
||||||
if err := ValidateBranchName(branch); err != nil {
|
// GH#807: Use sync-specific validation that rejects main/master
|
||||||
|
if err := ValidateSyncBranchName(branch); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -48,6 +48,36 @@ func TestValidateBranchName(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestValidateSyncBranchName(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
branch string
|
||||||
|
wantErr bool
|
||||||
|
}{
|
||||||
|
// Valid sync branches
|
||||||
|
{"beads-sync is valid", "beads-sync", false},
|
||||||
|
{"feature branch is valid", "feature-branch", false},
|
||||||
|
{"empty is valid", "", false},
|
||||||
|
|
||||||
|
// GH#807: main and master should be rejected for sync branch
|
||||||
|
{"main is invalid for sync", "main", true},
|
||||||
|
{"master is invalid for sync", "master", true},
|
||||||
|
|
||||||
|
// Standard branch name validation still applies
|
||||||
|
{"invalid: HEAD", "HEAD", true},
|
||||||
|
{"invalid: contains ..", "feature..branch", true},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
err := ValidateSyncBranchName(tt.branch)
|
||||||
|
if (err != nil) != tt.wantErr {
|
||||||
|
t.Errorf("ValidateSyncBranchName(%q) error = %v, wantErr %v", tt.branch, err, tt.wantErr)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func newTestStore(t *testing.T) *sqlite.SQLiteStorage {
|
func newTestStore(t *testing.T) *sqlite.SQLiteStorage {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
store, err := sqlite.New(context.Background(), "file::memory:?mode=memory&cache=private")
|
store, err := sqlite.New(context.Background(), "file::memory:?mode=memory&cache=private")
|
||||||
|
|||||||
Reference in New Issue
Block a user