diff --git a/cmd/bd/sync.go b/cmd/bd/sync.go index 15742824..e5c01830 100644 --- a/cmd/bd/sync.go +++ b/cmd/bd/sync.go @@ -400,8 +400,8 @@ Use --merge to merge the sync branch back to main branch.`, // bd-4u8: Handle safety check with confirmation requirement if pullResult.SafetyCheckTriggered && !pullResult.Pushed { + // bd-dmd: Don't duplicate SafetyCheckDetails - it's already in SafetyWarnings // Prompt for confirmation - fmt.Fprintf(os.Stderr, "\n⚠️ Mass deletion detected: %s\n", pullResult.SafetyCheckDetails) fmt.Fprintf(os.Stderr, "Push these changes to remote? [y/N]: ") var response string diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 47ad0462..e4125b1f 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -186,6 +186,8 @@ Configuration keys use dot-notation namespaces to organize settings: - `export.skip_encoding_errors` - Skip issues that fail JSON encoding (default: false) - `export.write_manifest` - Write .manifest.json with export metadata (default: false) - `auto_export.error_policy` - Override error policy for auto-exports (default: `best-effort`) +- `sync.branch` - Name of the dedicated sync branch for beads data (see docs/PROTECTED_BRANCHES.md) +- `sync.require_confirmation_on_mass_delete` - Require interactive confirmation before pushing when >50% of issues vanish during a merge (default: `false`) ### Integration Namespaces @@ -325,6 +327,33 @@ bd sync # Respects import.orphan_handling setting - Use `strict` only for controlled imports where you need to guarantee parent existence - Use `skip` rarely - only when you want to selectively import a subset +### Example: Sync Safety Options + +Controls for the sync branch workflow (see docs/PROTECTED_BRANCHES.md): + +```bash +# Configure sync branch (required for protected branch workflow) +bd config set sync.branch beads-metadata + +# Enable mass deletion protection (optional, default: false) +# When enabled, if >50% of issues vanish during a merge, bd sync will: +# 1. Show forensic info about vanished issues +# 2. Prompt for confirmation before pushing +bd config set sync.require_confirmation_on_mass_delete "true" +``` + +**When to enable `sync.require_confirmation_on_mass_delete`:** + +- Multi-user workflows where accidental mass deletions could propagate +- Critical projects where data loss prevention is paramount +- When you want manual review before pushing large changes + +**When to keep it disabled (default):** + +- Single-user workflows where you trust your local changes +- CI/CD pipelines that need non-interactive sync +- When you want hands-free automation + ### Example: Jira Integration ```bash diff --git a/internal/merge/merge.go b/internal/merge/merge.go index 6c617805..5f720ccb 100644 --- a/internal/merge/merge.go +++ b/internal/merge/merge.go @@ -427,7 +427,7 @@ func mergeNotes(base, left, right string) string { // mergePriority handles priority merging - on conflict, higher priority wins (lower number) // Special case: 0 is treated as "unset/no priority" due to Go's zero value. -// Any explicitly set priority (>0) wins over 0. (bd-d0t fix) +// Any explicitly set priority (!=0) wins over 0. (bd-d0t fix, bd-1kf fix) func mergePriority(base, left, right int) int { // Standard 3-way merge for non-conflict cases if base == left && base != right { @@ -442,10 +442,11 @@ func mergePriority(base, left, right int) int { // True conflict: both sides changed to different values // bd-d0t fix: Treat 0 as "unset" - explicitly set priority wins over unset - if left == 0 && right > 0 { + // bd-1kf fix: Use != 0 instead of > 0 to handle negative priorities + if left == 0 && right != 0 { return right // right has explicit priority, left is unset } - if right == 0 && left > 0 { + if right == 0 && left != 0 { return left // left has explicit priority, right is unset } diff --git a/internal/merge/merge_test.go b/internal/merge/merge_test.go index 0f68ba49..79c2c050 100644 --- a/internal/merge/merge_test.go +++ b/internal/merge/merge_test.go @@ -563,6 +563,35 @@ func TestMergePriority(t *testing.T) { right: 2, expected: 2, // right changed from 0 to 2 }, + // bd-1kf fix: negative priorities should be handled consistently + { + name: "bd-1kf: negative priority should win over unset (0)", + base: 2, + left: 0, + right: -1, + expected: -1, // negative priority is explicit, should win over unset + }, + { + name: "bd-1kf: negative priority on left should win over unset (0) on right", + base: 2, + left: -1, + right: 0, + expected: -1, // negative priority is explicit, should win over unset + }, + { + name: "bd-1kf: conflict between negative priorities - lower wins", + base: 2, + left: -2, + right: -1, + expected: -2, // -2 is higher priority (more urgent) than -1 + }, + { + name: "bd-1kf: negative vs positive priority conflict", + base: 2, + left: -1, + right: 1, + expected: -1, // -1 is higher priority (lower number) than 1 + }, } for _, tt := range tests { diff --git a/internal/syncbranch/worktree.go b/internal/syncbranch/worktree.go index 39969bf8..3488c662 100644 --- a/internal/syncbranch/worktree.go +++ b/internal/syncbranch/worktree.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "sort" "strconv" "strings" "time" @@ -301,8 +302,9 @@ func PullFromSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str // bd-7ch: Extract local content before merge for safety check localContent, extractErr := extractJSONLFromCommit(ctx, worktreePath, "HEAD", jsonlRelPath) if extractErr != nil { - // bd-feh: Log warning so users know safety check may be skipped - fmt.Fprintf(os.Stderr, "⚠️ Warning: Could not extract local content for safety check: %v\n", extractErr) + // bd-feh: Add warning to result so callers can display appropriately (bd-dtm fix) + result.SafetyWarnings = append(result.SafetyWarnings, + fmt.Sprintf("⚠️ Warning: Could not extract local content for safety check: %v", extractErr)) } mergedContent, err := performContentMerge(ctx, worktreePath, syncBranch, remote, jsonlRelPath) @@ -716,9 +718,10 @@ func PushSyncBranch(ctx context.Context, repoRoot, syncBranch string) error { // Worktree path is under .git/beads-worktrees/ worktreePath := filepath.Join(repoRoot, ".git", "beads-worktrees", syncBranch) - // Verify worktree exists - if _, err := os.Stat(worktreePath); os.IsNotExist(err) { - return fmt.Errorf("sync branch worktree not found at %s", worktreePath) + // bd-k2n: Recreate worktree if it was cleaned up, using the same pattern as CommitToSyncBranch + wtMgr := git.NewWorktreeManager(repoRoot) + if err := wtMgr.CreateBeadsWorktree(syncBranch, worktreePath); err != nil { + return fmt.Errorf("failed to ensure worktree exists: %w", err) } return pushFromWorktree(ctx, worktreePath, syncBranch) @@ -799,18 +802,23 @@ func formatVanishedIssues(localIssues, mergedIssues map[string]issueSummary, loc lines = append(lines, fmt.Sprintf(" After merge: %d issues", mergedCount)) lines = append(lines, " Vanished issues:") - vanishedCount := 0 - for id, summary := range localIssues { + // bd-ciu: Collect vanished IDs first, then sort for deterministic output + var vanishedIDs []string + for id := range localIssues { if _, exists := mergedIssues[id]; !exists { - vanishedCount++ - title := summary.Title - if len(title) > 60 { - title = title[:57] + "..." - } - lines = append(lines, fmt.Sprintf(" - %s: %s", id, title)) + vanishedIDs = append(vanishedIDs, id) } } - lines = append(lines, fmt.Sprintf(" Total vanished: %d\n", vanishedCount)) + sort.Strings(vanishedIDs) + + for _, id := range vanishedIDs { + title := localIssues[id].Title + if len(title) > 60 { + title = title[:57] + "..." + } + lines = append(lines, fmt.Sprintf(" - %s: %s", id, title)) + } + lines = append(lines, fmt.Sprintf(" Total vanished: %d\n", len(vanishedIDs))) return lines } diff --git a/internal/syncbranch/worktree_divergence_test.go b/internal/syncbranch/worktree_divergence_test.go index 40eae27a..96289e57 100644 --- a/internal/syncbranch/worktree_divergence_test.go +++ b/internal/syncbranch/worktree_divergence_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "testing" ) @@ -523,9 +524,10 @@ func TestSafetyCheckMassDeletion(t *testing.T) { t.Run("safety check triggers when >50% issues vanish and >5 existed", func(t *testing.T) { // Test the countIssuesInContent and formatVanishedIssues functions // Create local content with 10 issues + // bd-c5m: Use strconv.Itoa instead of string(rune()) which only works for single digits var localLines []string for i := 1; i <= 10; i++ { - localLines = append(localLines, `{"id":"test-`+string(rune('0'+i))+`","title":"Issue `+string(rune('0'+i))+`"}`) + localLines = append(localLines, `{"id":"test-`+strconv.Itoa(i)+`","title":"Issue `+strconv.Itoa(i)+`"}`) } localContent := []byte(strings.Join(localLines, "\n")) @@ -561,20 +563,40 @@ func TestSafetyCheckMassDeletion(t *testing.T) { if len(forensicLines) == 0 { t.Error("formatVanishedIssues returned empty lines") } + + // bd-8uk: Verify SafetyWarnings would be populated correctly + // Simulate what PullFromSyncBranch does when safety check triggers + var safetyWarnings []string + safetyWarnings = append(safetyWarnings, + "⚠️ Warning: "+strconv.FormatFloat(vanishedPercent, 'f', 0, 64)+"% of issues vanished during merge ("+ + strconv.Itoa(localCount)+" → "+strconv.Itoa(mergedCount)+" issues)") + safetyWarnings = append(safetyWarnings, forensicLines...) + + // Verify warnings contains expected content + if len(safetyWarnings) < 2 { + t.Errorf("SafetyWarnings should have at least 2 entries (warning + forensics), got %d", len(safetyWarnings)) + } + if !strings.Contains(safetyWarnings[0], "Warning") { + t.Error("First SafetyWarning should contain 'Warning'") + } + if !strings.Contains(safetyWarnings[0], "60%") { + t.Errorf("First SafetyWarning should contain '60%%', got: %s", safetyWarnings[0]) + } }) t.Run("safety check does NOT trigger when <50% issues vanish", func(t *testing.T) { // 10 issues, 6 remain = 40% vanished (should NOT trigger) + // bd-c5m: Use strconv.Itoa instead of string(rune()) which only works for single digits var localLines []string for i := 1; i <= 10; i++ { - localLines = append(localLines, `{"id":"test-`+string(rune('0'+i))+`"}`) + localLines = append(localLines, `{"id":"test-`+strconv.Itoa(i)+`"}`) } localContent := []byte(strings.Join(localLines, "\n")) // 6 issues remain (40% vanished) var mergedLines []string for i := 1; i <= 6; i++ { - mergedLines = append(mergedLines, `{"id":"test-`+string(rune('0'+i))+`"}`) + mergedLines = append(mergedLines, `{"id":"test-`+strconv.Itoa(i)+`"}`) } mergedContent := []byte(strings.Join(mergedLines, "\n"))