From f73651a2b53c1c2798d9fcf2a34601cb1c04fba1 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sun, 2 Nov 2025 15:37:57 -0800 Subject: [PATCH] Add sync.branch configuration support (bd-b7d2) - Created internal/syncbranch package with validation and env var support - Added --branch flag to bd init command - Enhanced bd config get/set to validate sync.branch - Added BEADS_SYNC_BRANCH environment variable support - Comprehensive tests for branch name validation - Supports precedence: env var > database config > empty (current branch) --- cmd/bd/config.go | 28 ++- cmd/bd/init.go | 16 ++ internal/syncbranch/syncbranch.go | 98 +++++++++++ internal/syncbranch/syncbranch_test.go | 232 +++++++++++++++++++++++++ 4 files changed, 370 insertions(+), 4 deletions(-) create mode 100644 internal/syncbranch/syncbranch.go create mode 100644 internal/syncbranch/syncbranch_test.go diff --git a/cmd/bd/config.go b/cmd/bd/config.go index 54f1e683..e39e9c15 100644 --- a/cmd/bd/config.go +++ b/cmd/bd/config.go @@ -5,8 +5,10 @@ import ( "fmt" "os" "sort" + "strings" "github.com/spf13/cobra" + "github.com/steveyegge/beads/internal/syncbranch" ) var configCmd = &cobra.Command{ @@ -45,9 +47,18 @@ var configSetCmd = &cobra.Command{ value := args[1] ctx := context.Background() - if err := store.SetConfig(ctx, key, value); err != nil { - fmt.Fprintf(os.Stderr, "Error setting config: %v\n", err) - os.Exit(1) + + // Special handling for sync.branch to apply validation + if strings.TrimSpace(key) == syncbranch.ConfigKey { + if err := syncbranch.Set(ctx, store, value); err != nil { + fmt.Fprintf(os.Stderr, "Error setting config: %v\n", err) + os.Exit(1) + } + } else { + if err := store.SetConfig(ctx, key, value); err != nil { + fmt.Fprintf(os.Stderr, "Error setting config: %v\n", err) + os.Exit(1) + } } if jsonOutput { @@ -75,7 +86,16 @@ var configGetCmd = &cobra.Command{ key := args[0] ctx := context.Background() - value, err := store.GetConfig(ctx, key) + var value string + var err error + + // Special handling for sync.branch to support env var override + if strings.TrimSpace(key) == syncbranch.ConfigKey { + value, err = syncbranch.Get(ctx, store) + } else { + value, err = store.GetConfig(ctx, key) + } + if err != nil { fmt.Fprintf(os.Stderr, "Error getting config: %v\n", err) os.Exit(1) diff --git a/cmd/bd/init.go b/cmd/bd/init.go index a8bb1472..46af4d87 100644 --- a/cmd/bd/init.go +++ b/cmd/bd/init.go @@ -13,6 +13,7 @@ import ( "github.com/steveyegge/beads/internal/config" "github.com/steveyegge/beads/internal/configfile" "github.com/steveyegge/beads/internal/storage/sqlite" + "github.com/steveyegge/beads/internal/syncbranch" ) var initCmd = &cobra.Command{ @@ -25,6 +26,7 @@ With --no-db: creates .beads/ directory and issues.jsonl file instead of SQLite Run: func(cmd *cobra.Command, _ []string) { prefix, _ := cmd.Flags().GetString("prefix") quiet, _ := cmd.Flags().GetBool("quiet") + branch, _ := cmd.Flags().GetString("branch") // Initialize config (PersistentPreRun doesn't run for init command) if err := config.Initialize(); err != nil { @@ -178,6 +180,18 @@ bd.db os.Exit(1) } + // Set sync.branch if specified + if branch != "" { + if err := syncbranch.Set(ctx, store, branch); err != nil { + fmt.Fprintf(os.Stderr, "Error: failed to set sync branch: %v\n", err) + _ = store.Close() + os.Exit(1) + } + if !quiet { + fmt.Printf(" Sync branch: %s\n", branch) + } + } + // Store the bd version in metadata (for version mismatch detection) if err := store.SetMetadata(ctx, "bd_version", Version); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to store version metadata: %v\n", err) @@ -270,6 +284,7 @@ bd.db # - linear.api-key # - github.org # - github.repo +# - sync.branch - Git branch for beads commits (use BEADS_SYNC_BRANCH env var or bd config set) ` if err := os.WriteFile(configYamlPath, []byte(configYamlTemplate), 0600); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to create config.yaml: %v\n", err) @@ -354,6 +369,7 @@ if quiet { func init() { initCmd.Flags().StringP("prefix", "p", "", "Issue prefix (default: current directory name)") initCmd.Flags().BoolP("quiet", "q", false, "Suppress output (quiet mode)") + initCmd.Flags().StringP("branch", "b", "", "Git branch for beads commits (default: current branch)") rootCmd.AddCommand(initCmd) } diff --git a/internal/syncbranch/syncbranch.go b/internal/syncbranch/syncbranch.go new file mode 100644 index 00000000..d3cedc23 --- /dev/null +++ b/internal/syncbranch/syncbranch.go @@ -0,0 +1,98 @@ +package syncbranch + +import ( + "context" + "fmt" + "os" + "regexp" + + "github.com/steveyegge/beads/internal/storage" +) + +const ( + // ConfigKey is the database config key for sync branch + ConfigKey = "sync.branch" + + // EnvVar is the environment variable for sync branch + EnvVar = "BEADS_SYNC_BRANCH" +) + +// branchNamePattern validates git branch names +// Based on git-check-ref-format rules +var branchNamePattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._/-]*[a-zA-Z0-9]$`) + +// ValidateBranchName checks if a branch name is valid according to git rules +func ValidateBranchName(name string) error { + if name == "" { + return nil // Empty is valid (means use current branch) + } + + // Basic length check + if len(name) > 255 { + return fmt.Errorf("branch name too long (max 255 characters)") + } + + // Check pattern + if !branchNamePattern.MatchString(name) { + return fmt.Errorf("invalid branch name: must start and end with alphanumeric, can contain .-_/ in middle") + } + + // Disallow certain patterns + if name == "HEAD" || name == "." || name == ".." { + return fmt.Errorf("invalid branch name: %s is reserved", name) + } + + // No consecutive dots + if regexp.MustCompile(`\.\.`).MatchString(name) { + return fmt.Errorf("invalid branch name: cannot contain '..'") + } + + // No leading/trailing slashes + if name[0] == '/' || name[len(name)-1] == '/' { + return fmt.Errorf("invalid branch name: cannot start or end with '/'") + } + + return nil +} + +// Get retrieves the sync branch configuration with the following precedence: +// 1. BEADS_SYNC_BRANCH environment variable +// 2. sync.branch from database config +// 3. Empty string (meaning use current branch) +func Get(ctx context.Context, store storage.Storage) (string, error) { + // Check environment variable first + if envBranch := os.Getenv(EnvVar); envBranch != "" { + if err := ValidateBranchName(envBranch); err != nil { + return "", fmt.Errorf("invalid %s: %w", EnvVar, err) + } + return envBranch, nil + } + + // Check database config + dbBranch, err := store.GetConfig(ctx, ConfigKey) + if err != nil { + return "", fmt.Errorf("failed to get %s from config: %w", ConfigKey, err) + } + + if dbBranch != "" { + if err := ValidateBranchName(dbBranch); err != nil { + return "", fmt.Errorf("invalid %s in database: %w", ConfigKey, err) + } + } + + return dbBranch, nil +} + +// Set stores the sync branch configuration in the database +func Set(ctx context.Context, store storage.Storage, branch string) error { + if err := ValidateBranchName(branch); err != nil { + return err + } + + return store.SetConfig(ctx, ConfigKey, branch) +} + +// Unset removes the sync branch configuration from the database +func Unset(ctx context.Context, store storage.Storage) error { + return store.DeleteConfig(ctx, ConfigKey) +} diff --git a/internal/syncbranch/syncbranch_test.go b/internal/syncbranch/syncbranch_test.go new file mode 100644 index 00000000..8e396827 --- /dev/null +++ b/internal/syncbranch/syncbranch_test.go @@ -0,0 +1,232 @@ +package syncbranch + +import ( + "context" + "os" + "testing" + + "github.com/steveyegge/beads/internal/storage/sqlite" +) + +func TestValidateBranchName(t *testing.T) { + tests := []struct { + name string + branch string + wantErr bool + }{ + {"empty is valid", "", false}, + {"simple branch", "main", false}, + {"branch with hyphen", "feature-branch", false}, + {"branch with slash", "feature/my-feature", false}, + {"branch with underscore", "feature_branch", false}, + {"branch with dot", "release-1.0", false}, + {"complex valid branch", "feature/user-auth_v2.1", false}, + + {"invalid: HEAD", "HEAD", true}, + {"invalid: single dot", ".", true}, + {"invalid: double dot", "..", true}, + {"invalid: contains ..", "feature..branch", true}, + {"invalid: starts with slash", "/feature", true}, + {"invalid: ends with slash", "feature/", true}, + {"invalid: starts with hyphen", "-feature", true}, + {"invalid: ends with hyphen", "feature-", true}, + {"invalid: starts with dot", ".feature", true}, + {"invalid: ends with dot", "feature.", true}, + {"invalid: special char @", "feature@branch", true}, + {"invalid: special char #", "feature#branch", true}, + {"invalid: space", "feature branch", true}, + {"invalid: too long", string(make([]byte, 256)), true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateBranchName(tt.branch) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateBranchName(%q) error = %v, wantErr %v", tt.branch, err, tt.wantErr) + } + }) + } +} + +func newTestStore(t *testing.T) *sqlite.SQLiteStorage { + t.Helper() + store, err := sqlite.New("file::memory:?mode=memory&cache=private") + if err != nil { + t.Fatalf("Failed to create test database: %v", err) + } + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + _ = store.Close() + t.Fatalf("Failed to set issue_prefix: %v", err) + } + return store +} + +func TestGet(t *testing.T) { + ctx := context.Background() + + t.Run("returns empty when not set", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + branch, err := Get(ctx, store) + if err != nil { + t.Fatalf("Get() error = %v", err) + } + if branch != "" { + t.Errorf("Get() = %q, want empty string", branch) + } + }) + + t.Run("returns database config value", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + if err := store.SetConfig(ctx, ConfigKey, "beads-metadata"); err != nil { + t.Fatalf("SetConfig() error = %v", err) + } + + branch, err := Get(ctx, store) + if err != nil { + t.Fatalf("Get() error = %v", err) + } + if branch != "beads-metadata" { + t.Errorf("Get() = %q, want %q", branch, "beads-metadata") + } + }) + + t.Run("environment variable overrides database", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + // Set database config + if err := store.SetConfig(ctx, ConfigKey, "beads-metadata"); err != nil { + t.Fatalf("SetConfig() error = %v", err) + } + + // Set environment variable + os.Setenv(EnvVar, "env-branch") + defer os.Unsetenv(EnvVar) + + branch, err := Get(ctx, store) + if err != nil { + t.Fatalf("Get() error = %v", err) + } + if branch != "env-branch" { + t.Errorf("Get() = %q, want %q (env should override db)", branch, "env-branch") + } + }) + + t.Run("returns error for invalid env var", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + os.Setenv(EnvVar, "invalid..branch") + defer os.Unsetenv(EnvVar) + + _, err := Get(ctx, store) + if err == nil { + t.Error("Get() expected error for invalid env var, got nil") + } + }) + + t.Run("returns error for invalid db config", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + // Directly set invalid value (bypassing validation) + if err := store.SetConfig(ctx, ConfigKey, "invalid..branch"); err != nil { + t.Fatalf("SetConfig() error = %v", err) + } + + _, err := Get(ctx, store) + if err == nil { + t.Error("Get() expected error for invalid db config, got nil") + } + }) +} + +func TestSet(t *testing.T) { + ctx := context.Background() + + t.Run("sets valid branch name", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + if err := Set(ctx, store, "beads-metadata"); err != nil { + t.Fatalf("Set() error = %v", err) + } + + value, err := store.GetConfig(ctx, ConfigKey) + if err != nil { + t.Fatalf("GetConfig() error = %v", err) + } + if value != "beads-metadata" { + t.Errorf("GetConfig() = %q, want %q", value, "beads-metadata") + } + }) + + t.Run("allows empty string", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + if err := Set(ctx, store, ""); err != nil { + t.Fatalf("Set() error = %v for empty string", err) + } + + value, err := store.GetConfig(ctx, ConfigKey) + if err != nil { + t.Fatalf("GetConfig() error = %v", err) + } + if value != "" { + t.Errorf("GetConfig() = %q, want empty string", value) + } + }) + + t.Run("rejects invalid branch name", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + err := Set(ctx, store, "invalid..branch") + if err == nil { + t.Error("Set() expected error for invalid branch name, got nil") + } + }) +} + +func TestUnset(t *testing.T) { + ctx := context.Background() + + t.Run("removes config value", func(t *testing.T) { + store := newTestStore(t) + defer store.Close() + + // Set a value first + if err := Set(ctx, store, "beads-metadata"); err != nil { + t.Fatalf("Set() error = %v", err) + } + + // Verify it's set + value, err := store.GetConfig(ctx, ConfigKey) + if err != nil { + t.Fatalf("GetConfig() error = %v", err) + } + if value != "beads-metadata" { + t.Errorf("GetConfig() = %q, want %q", value, "beads-metadata") + } + + // Unset it + if err := Unset(ctx, store); err != nil { + t.Fatalf("Unset() error = %v", err) + } + + // Verify it's gone + value, err = store.GetConfig(ctx, ConfigKey) + if err != nil { + t.Fatalf("GetConfig() error = %v", err) + } + if value != "" { + t.Errorf("GetConfig() after Unset() = %q, want empty string", value) + } + }) +}