diff --git a/cmd/bd/sync_git.go b/cmd/bd/sync_git.go index 5ef9d4f4..f7f162f3 100644 --- a/cmd/bd/sync_git.go +++ b/cmd/bd/sync_git.go @@ -512,33 +512,6 @@ func checkMergeDriverConfig() { } } -// restoreBeadsDirFromBranch restores .beads/ directory from the current branch's committed state. -// This is used after sync when sync.branch is configured to keep the working directory clean. -// The actual beads data lives on the sync branch; the main branch's .beads/ is just a snapshot. -// Uses RepoContext to ensure git commands run in the correct repository. -func restoreBeadsDirFromBranch(ctx context.Context) error { - rc, err := beads.GetRepoContext() - if err != nil { - return fmt.Errorf("getting repo context: %w", err) - } - - // Skip restore when beads directory is redirected (bd-lmqhe) - // When redirected, the beads directory is in a different repo, so - // git checkout from the current repo won't work for paths outside it. - if rc.IsRedirected { - return nil - } - - // Restore .beads/ from HEAD (current branch's committed state) - // Using -- to ensure .beads/ is treated as a path, not a branch name - cmd := rc.GitCmd(ctx, "checkout", "HEAD", "--", rc.BeadsDir) //nolint:gosec // G204: beadsDir from RepoContext - output, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("git checkout failed: %w\n%s", err, output) - } - return nil -} - // gitHasUncommittedBeadsChanges checks if .beads/issues.jsonl has uncommitted changes. // This detects the failure mode where a previous sync exported but failed before commit. // Returns true if the JSONL file has staged or unstaged changes (M or A status). diff --git a/cmd/bd/sync_git_test.go b/cmd/bd/sync_git_test.go index 8db9e220..f702aabe 100644 --- a/cmd/bd/sync_git_test.go +++ b/cmd/bd/sync_git_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "github.com/steveyegge/beads/internal/beads" @@ -579,6 +580,124 @@ func TestGitBranchHasUpstream(t *testing.T) { // TestGetCurrentBranchOrHEAD tests getCurrentBranchOrHEAD which returns "HEAD" // when in detached HEAD state (e.g., jj/jujutsu) instead of failing. +// TestConfigPreservedDuringSync is a regression test for GH#1100. +// It verifies that config.yaml in .beads/ is not overwritten during sync operations. +// The bug was caused by restoreBeadsDirFromBranch() which ran: +// git checkout HEAD -- .beads/ +// This restored the ENTIRE .beads/ directory, including user's uncommitted config.yaml. +// The function was removed in PR #918 (pull-first refactor). +// This test ensures similar restoration logic is never reintroduced. +func TestConfigPreservedDuringSync(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + t.Run("uncommitted config.yaml not overwritten by git operations", func(t *testing.T) { + // Setup: Create git repo with .beads directory + repoPath, cleanup := setupGitRepoWithBeads(t) + defer cleanup() + + beadsDir := filepath.Join(repoPath, ".beads") + + // Create config.yaml and commit it + configPath := filepath.Join(beadsDir, "config.yaml") + originalContent := "sync:\n branch: beads-sync\n" + if err := os.WriteFile(configPath, []byte(originalContent), 0644); err != nil { + t.Fatalf("failed to write config.yaml: %v", err) + } + + // Commit the config + _ = exec.Command("git", "add", ".beads/config.yaml").Run() + if err := exec.Command("git", "commit", "-m", "add config").Run(); err != nil { + t.Fatalf("failed to commit config: %v", err) + } + + // Now modify config.yaml with UNCOMMITTED changes (the bug scenario) + modifiedContent := originalContent + "# User's uncommitted customization\ntest-marker: preserved\n" + if err := os.WriteFile(configPath, []byte(modifiedContent), 0644); err != nil { + t.Fatalf("failed to modify config.yaml: %v", err) + } + + // Verify the modification exists before any operations + beforeSync, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("failed to read config.yaml: %v", err) + } + if !strings.Contains(string(beforeSync), "test-marker: preserved") { + t.Fatal("expected test-marker in config before operations") + } + + // Simulate what the old buggy code did: git checkout HEAD -- .beads/ + // This should NOT happen during normal sync, but we test that IF it did, + // it would restore committed state (losing uncommitted changes). + // The fact that no code calls this anymore is the fix. + + // Verify config.yaml still has uncommitted changes + // (This passes because nothing calls restoreBeadsDirFromBranch anymore) + afterContent, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("failed to read config.yaml after operations: %v", err) + } + + if !strings.Contains(string(afterContent), "test-marker: preserved") { + t.Errorf("REGRESSION: uncommitted config.yaml changes were lost!\n"+ + "Before: %s\nAfter: %s", beforeSync, afterContent) + } + + // Also verify that git status shows the file as modified (uncommitted) + cmd := exec.Command("git", "status", "--porcelain", ".beads/config.yaml") + output, err := cmd.Output() + if err != nil { + t.Fatalf("git status failed: %v", err) + } + + // Should show " M" (modified, unstaged) + if !strings.Contains(string(output), "M") { + t.Errorf("expected config.yaml to show as modified in git status, got: %q", output) + } + }) + + t.Run("restoreBeadsDirFromBranch function does not exist", func(t *testing.T) { + // This test documents that the function was intentionally removed. + // If someone adds it back, they should also update this test with justification. + + // Read the sync_git.go source file + syncGitPath := filepath.Join(getProjectRoot(t), "cmd", "bd", "sync_git.go") + content, err := os.ReadFile(syncGitPath) + if err != nil { + t.Fatalf("failed to read sync_git.go: %v", err) + } + + // The function should NOT exist + if strings.Contains(string(content), "func restoreBeadsDirFromBranch") { + t.Error("REGRESSION: restoreBeadsDirFromBranch function was reintroduced!\n" + + "This function caused GH#1100 by restoring entire .beads/ directory.\n" + + "If you need this functionality, use selective restoration that excludes config.yaml.") + } + }) +} + +// getProjectRoot returns the project root directory for test file access. +func getProjectRoot(t *testing.T) string { + t.Helper() + // Find project root by looking for go.mod + dir, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get working directory: %v", err) + } + + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatal("could not find project root (go.mod)") + } + dir = parent + } +} + func TestGetCurrentBranchOrHEAD(t *testing.T) { ctx := context.Background()