From e7d543af550dfa0c2171d32dbf151c91e2f2d2d7 Mon Sep 17 00:00:00 2001
From: Peter Chanthamynavong <peterkc@hey.com>
Date: Mon, 5 Jan 2026 19:11:55 -0800
Subject: [PATCH] fix(sync): persist sync branch to yaml and database (GH#909)
 (#910)

Problem:
- Sync branch configuration was not consistently saved across storage types
- State divergence between database and config.yaml files

Solution:
- Update Set to write configuration to both yaml and database backends
- Add regression test for yaml configuration persistence

Impact:
- Guarantees configuration consistency across system reboots
- Prevents sync settings from being lost when reading from yaml
---
 internal/syncbranch/syncbranch.go      | 18 ++++++-
 internal/syncbranch/syncbranch_test.go | 72 ++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)

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()