From c1d3644b9a8fdee555a4139b113e284bc59b0fc4 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Tue, 6 Jan 2026 14:59:51 -0800 Subject: [PATCH 1/2] test(init): verify --branch flag persistence - Add test case for --branch flag persistence in config.yaml Coverage: init command configuration logic --- cmd/bd/init_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/cmd/bd/init_test.go b/cmd/bd/init_test.go index db0632b2..c8df8ad8 100644 --- a/cmd/bd/init_test.go +++ b/cmd/bd/init_test.go @@ -1138,6 +1138,56 @@ func TestSetupClaudeSettings_NoExistingFile(t *testing.T) { } } +// TestInitBranchPersistsToConfigYaml verifies that --branch flag persists to config.yaml +// GH#927 Bug 3: The --branch flag sets sync.branch in database but NOT in config.yaml. +// This matters because config.yaml is version-controlled and shared across clones, +// while the database is local and gitignored. +func TestInitBranchPersistsToConfigYaml(t *testing.T) { + // Reset global state + origDBPath := dbPath + defer func() { dbPath = origDBPath }() + dbPath = "" + + // Reset Cobra flags + initCmd.Flags().Set("branch", "") + + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + // Initialize git repo first (needed for sync branch) + if err := runCommandInDir(tmpDir, "git", "init", "--initial-branch=dev"); err != nil { + t.Fatalf("Failed to init git: %v", err) + } + + // Run bd init with --branch flag + rootCmd.SetArgs([]string{"init", "--prefix", "test", "--branch", "beads-sync", "--quiet"}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("Init with --branch failed: %v", err) + } + + // Read config.yaml and verify sync-branch is uncommented + configPath := filepath.Join(tmpDir, ".beads", "config.yaml") + content, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read config.yaml: %v", err) + } + + configStr := string(content) + + // The bug: sync-branch remains commented as "# sync-branch:" instead of "sync-branch:" + // This test should FAIL on the current codebase to prove the bug exists + if strings.Contains(configStr, "# sync-branch:") && !strings.Contains(configStr, "\nsync-branch:") { + t.Errorf("BUG: --branch flag did not persist to config.yaml\n"+ + "Expected uncommented 'sync-branch: \"beads-sync\"'\n"+ + "Got commented '# sync-branch:' (only set in database, not config.yaml)") + } + + // Verify the uncommented line exists with correct value + if !strings.Contains(configStr, "sync-branch: \"beads-sync\"") { + t.Errorf("config.yaml should contain 'sync-branch: \"beads-sync\"', got:\n%s", configStr) + } +} + // TestSetupGlobalGitIgnore_ReadOnly verifies graceful handling when the // gitignore file cannot be written (prints manual instructions instead of failing). func TestSetupGlobalGitIgnore_ReadOnly(t *testing.T) { From 362b07d74f8dd56df63ffc441add6de8447b90f3 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Tue, 6 Jan 2026 15:22:15 -0800 Subject: [PATCH 2/2] fix(init): ensure sync branch persistence on init Problem: - Sync branch setup occurred before the config file was initialized - Persistence only targeted the database, leading to loss on re-init Solution: - Reorder initialization to create the config file before sync setup - Synchronize sync branch state to both config file and database Impact: - Settings are preserved across re-initialization and DB clears - Better consistency between file and database state Fixes: #927 (Bug 3) --- cmd/bd/init.go | 39 +++++++------- cmd/bd/init_test.go | 58 +++++++++++++++++++++ internal/syncbranch/syncbranch.go | 18 ++++++- internal/syncbranch/syncbranch_test.go | 72 ++++++++++++++++++++++++++ 4 files changed, 168 insertions(+), 19 deletions(-) diff --git a/cmd/bd/init.go b/cmd/bd/init.go index b89bacd7..b0238465 100644 --- a/cmd/bd/init.go +++ b/cmd/bd/init.go @@ -287,24 +287,6 @@ With --stealth: configures per-repository git settings for invisible beads usage os.Exit(1) } - // Set sync.branch only if explicitly specified via --branch flag - // GH#807: Do NOT auto-detect current branch - if sync.branch is set to main/master, - // the worktree created by bd sync will check out main, preventing the user from - // checking out main in their working directory (git error: "'main' is already checked out") - // - // When --branch is not specified, bd sync will commit directly to the current branch - // (the original behavior before sync branch feature) - 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) - } - } - // === TRACKING METADATA (Pattern B: Warn and Continue) === // Tracking metadata enhances functionality (diagnostics, version checks, collision detection) // but the system works without it. Failures here degrade gracefully - we warn but continue. @@ -386,6 +368,27 @@ With --stealth: configures per-repository git settings for invisible beads usage } } + // Set sync.branch only if explicitly specified via --branch flag + // GH#807: Do NOT auto-detect current branch - if sync.branch is set to main/master, + // the worktree created by bd sync will check out main, preventing the user from + // checking out main in their working directory (git error: "'main' is already checked out") + // + // When --branch is not specified, bd sync will commit directly to the current branch + // (the original behavior before sync branch feature) + // + // GH#927: This must run AFTER createConfigYaml() so that config.yaml exists + // and syncbranch.Set() can update it via config.SetYamlConfig() (PR#910 mechanism) + 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) + } + } + // Check if git has existing issues to import (fresh clone scenario) // With --from-jsonl: import from local file instead of git history if fromJSONL { diff --git a/cmd/bd/init_test.go b/cmd/bd/init_test.go index c8df8ad8..3d1a0ee9 100644 --- a/cmd/bd/init_test.go +++ b/cmd/bd/init_test.go @@ -1188,6 +1188,64 @@ func TestInitBranchPersistsToConfigYaml(t *testing.T) { } } +// TestInitReinitWithBranch verifies that --branch flag works on reinit +// GH#927: When reinitializing with --branch, config.yaml should be updated even if it exists +func TestInitReinitWithBranch(t *testing.T) { + // Reset global state + origDBPath := dbPath + defer func() { dbPath = origDBPath }() + dbPath = "" + + // Reset Cobra flags + initCmd.Flags().Set("branch", "") + initCmd.Flags().Set("force", "false") + + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + // Initialize git repo first + if err := runCommandInDir(tmpDir, "git", "init", "--initial-branch=dev"); err != nil { + t.Fatalf("Failed to init git: %v", err) + } + + // First init WITHOUT --branch (creates config.yaml with commented sync-branch) + rootCmd.SetArgs([]string{"init", "--prefix", "test", "--quiet"}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("First init failed: %v", err) + } + + // Verify config.yaml has commented sync-branch initially + configPath := filepath.Join(tmpDir, ".beads", "config.yaml") + content, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read config.yaml: %v", err) + } + if !strings.Contains(string(content), "# sync-branch:") { + t.Errorf("Initial config.yaml should have commented sync-branch") + } + + // Reset Cobra flags for reinit + initCmd.Flags().Set("branch", "") + initCmd.Flags().Set("force", "false") + + // Reinit WITH --branch (should update existing config.yaml) + rootCmd.SetArgs([]string{"init", "--prefix", "test", "--branch", "beads-sync", "--force", "--quiet"}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("Reinit with --branch failed: %v", err) + } + + // Verify config.yaml now has uncommented sync-branch + content, err = os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read config.yaml after reinit: %v", err) + } + + configStr := string(content) + if !strings.Contains(configStr, "sync-branch: \"beads-sync\"") { + t.Errorf("After reinit with --branch, config.yaml should contain uncommented 'sync-branch: \"beads-sync\"', got:\n%s", configStr) + } +} + // TestSetupGlobalGitIgnore_ReadOnly verifies graceful handling when the // gitignore file cannot be written (prints manual instructions instead of failing). func TestSetupGlobalGitIgnore_ReadOnly(t *testing.T) { diff --git a/internal/syncbranch/syncbranch.go b/internal/syncbranch/syncbranch.go index c1577305..57bec273 100644 --- a/internal/syncbranch/syncbranch.go +++ b/internal/syncbranch/syncbranch.go @@ -193,13 +193,29 @@ func getConfigFromDB(dbPath string, key string) string { return value } -// Set stores the sync branch configuration in the database +// Set stores the sync branch configuration in both config.yaml AND the database. +// GH#909: Writing to both ensures bd doctor and migrate detection work correctly. +// +// Config precedence on read (from Get function): +// 1. BEADS_SYNC_BRANCH env var +// 2. sync-branch in config.yaml (recommended, version controlled) +// 3. sync.branch in database (legacy, for backward compatibility) func Set(ctx context.Context, store storage.Storage, branch string) error { // GH#807: Use sync-specific validation that rejects main/master if err := ValidateSyncBranchName(branch); err != nil { return err } + // GH#909: Write to config.yaml first (primary source for doctor/migration checks) + // This also handles uncommenting if the key was commented out + if err := config.SetYamlConfig(ConfigYAMLKey, branch); err != nil { + // Log warning but don't fail - database write is still valuable + // This can fail if config.yaml doesn't exist yet (pre-init state) + // In that case, the database config still works for backward compatibility + fmt.Fprintf(os.Stderr, "Warning: could not update config.yaml: %v\n", err) + } + + // Write to database for backward compatibility return store.SetConfig(ctx, ConfigKey, branch) } diff --git a/internal/syncbranch/syncbranch_test.go b/internal/syncbranch/syncbranch_test.go index 5a53d550..20e47126 100644 --- a/internal/syncbranch/syncbranch_test.go +++ b/internal/syncbranch/syncbranch_test.go @@ -252,6 +252,78 @@ func TestSet(t *testing.T) { }) } +// TestSetUpdatesConfigYAML verifies GH#909 fix: Set() writes to config.yaml +func TestSetUpdatesConfigYAML(t *testing.T) { + ctx := context.Background() + + t.Run("updates config.yaml when it exists", func(t *testing.T) { + // Create temp directory with .beads/config.yaml + tmpDir, err := os.MkdirTemp("", "test-syncbranch-yaml-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + beadsDir := tmpDir + "/.beads" + if err := os.MkdirAll(beadsDir, 0750); err != nil { + t.Fatalf("Failed to create .beads dir: %v", err) + } + + // Create initial config.yaml with sync-branch commented out + configPath := beadsDir + "/config.yaml" + initialConfig := `# beads configuration +# sync-branch: "" +auto-start-daemon: true +` + if err := os.WriteFile(configPath, []byte(initialConfig), 0600); err != nil { + t.Fatalf("Failed to create config.yaml: %v", err) + } + + // Change to temp dir so findProjectConfigYaml can find it + origWd, _ := os.Getwd() + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to chdir: %v", err) + } + defer os.Chdir(origWd) + + // Create test store + store := newTestStore(t) + defer store.Close() + + // Call Set() which should update both database and config.yaml + if err := Set(ctx, store, "beads-sync"); err != nil { + t.Fatalf("Set() error = %v", err) + } + + // Verify database was updated + dbValue, err := store.GetConfig(ctx, ConfigKey) + if err != nil { + t.Fatalf("GetConfig() error = %v", err) + } + if dbValue != "beads-sync" { + t.Errorf("Database value = %q, want %q", dbValue, "beads-sync") + } + + // Verify config.yaml was updated (key uncommented and value set) + yamlContent, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read config.yaml: %v", err) + } + + yamlStr := string(yamlContent) + if !strings.Contains(yamlStr, "sync-branch:") { + t.Error("config.yaml should contain 'sync-branch:' (uncommented)") + } + if !strings.Contains(yamlStr, "beads-sync") { + t.Errorf("config.yaml should contain 'beads-sync', got:\n%s", yamlStr) + } + // Should NOT contain the commented version anymore + if strings.Contains(yamlStr, "# sync-branch:") { + t.Error("config.yaml still has commented '# sync-branch:', should be uncommented") + } + }) +} + func TestUnset(t *testing.T) { ctx := context.Background()