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)
This commit is contained in:
@@ -287,24 +287,6 @@ With --stealth: configures per-repository git settings for invisible beads usage
|
|||||||
os.Exit(1)
|
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 (Pattern B: Warn and Continue) ===
|
||||||
// Tracking metadata enhances functionality (diagnostics, version checks, collision detection)
|
// Tracking metadata enhances functionality (diagnostics, version checks, collision detection)
|
||||||
// but the system works without it. Failures here degrade gracefully - we warn but continue.
|
// 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)
|
// Check if git has existing issues to import (fresh clone scenario)
|
||||||
// With --from-jsonl: import from local file instead of git history
|
// With --from-jsonl: import from local file instead of git history
|
||||||
if fromJSONL {
|
if fromJSONL {
|
||||||
|
|||||||
@@ -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
|
// TestSetupGlobalGitIgnore_ReadOnly verifies graceful handling when the
|
||||||
// gitignore file cannot be written (prints manual instructions instead of failing).
|
// gitignore file cannot be written (prints manual instructions instead of failing).
|
||||||
func TestSetupGlobalGitIgnore_ReadOnly(t *testing.T) {
|
func TestSetupGlobalGitIgnore_ReadOnly(t *testing.T) {
|
||||||
|
|||||||
@@ -193,13 +193,29 @@ func getConfigFromDB(dbPath string, key string) string {
|
|||||||
return value
|
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 {
|
func Set(ctx context.Context, store storage.Storage, branch string) error {
|
||||||
// GH#807: Use sync-specific validation that rejects main/master
|
// GH#807: Use sync-specific validation that rejects main/master
|
||||||
if err := ValidateSyncBranchName(branch); err != nil {
|
if err := ValidateSyncBranchName(branch); err != nil {
|
||||||
return err
|
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)
|
return store.SetConfig(ctx, ConfigKey, branch)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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) {
|
func TestUnset(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user