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
This commit is contained in:
Peter Chanthamynavong
2026-01-05 19:11:55 -08:00
committed by GitHub
parent 504e980669
commit e7d543af55
2 changed files with 89 additions and 1 deletions

View File

@@ -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)
}

View File

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