fix(sync): protect local issues from git-history-backfill during sync (#485)
Fix sync bug where newly created issues were incorrectly tombstoned during bd sync. The root cause was git-history-backfill finding issues in local commits on the sync branch, then tombstoning them when they weren't in the merged JSONL. The fix protects issues from the left snapshot (local export) from git-history-backfill. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
@@ -98,6 +98,7 @@ NOTE: Import requires direct database access and does not work with daemon mode.
|
|||||||
force, _ := cmd.Flags().GetBool("force")
|
force, _ := cmd.Flags().GetBool("force")
|
||||||
noGitHistory, _ := cmd.Flags().GetBool("no-git-history")
|
noGitHistory, _ := cmd.Flags().GetBool("no-git-history")
|
||||||
ignoreDeletions, _ := cmd.Flags().GetBool("ignore-deletions")
|
ignoreDeletions, _ := cmd.Flags().GetBool("ignore-deletions")
|
||||||
|
protectLeftSnapshot, _ := cmd.Flags().GetBool("protect-left-snapshot")
|
||||||
|
|
||||||
// Check if stdin is being used interactively (not piped)
|
// Check if stdin is being used interactively (not piped)
|
||||||
if input == "" && term.IsTerminal(int(os.Stdin.Fd())) {
|
if input == "" && term.IsTerminal(int(os.Stdin.Fd())) {
|
||||||
@@ -260,6 +261,23 @@ NOTE: Import requires direct database access and does not work with daemon mode.
|
|||||||
IgnoreDeletions: ignoreDeletions,
|
IgnoreDeletions: ignoreDeletions,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If --protect-left-snapshot is set, read the left snapshot and build ID set
|
||||||
|
// This protects locally exported issues from git-history-backfill (bd-sync-deletion fix)
|
||||||
|
if protectLeftSnapshot && input != "" {
|
||||||
|
beadsDir := filepath.Dir(input)
|
||||||
|
leftSnapshotPath := filepath.Join(beadsDir, "beads.left.jsonl")
|
||||||
|
if _, err := os.Stat(leftSnapshotPath); err == nil {
|
||||||
|
sm := NewSnapshotManager(input)
|
||||||
|
leftIDs, err := sm.BuildIDSet(leftSnapshotPath)
|
||||||
|
if err != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "Warning: failed to read left snapshot: %v\n", err)
|
||||||
|
} else if len(leftIDs) > 0 {
|
||||||
|
opts.ProtectLocalExportIDs = leftIDs
|
||||||
|
fmt.Fprintf(os.Stderr, "Protecting %d issue(s) from left snapshot\n", len(leftIDs))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
result, err := importIssuesCore(ctx, dbPath, store, allIssues, opts)
|
result, err := importIssuesCore(ctx, dbPath, store, allIssues, opts)
|
||||||
|
|
||||||
// Check for uncommitted changes in JSONL after import
|
// Check for uncommitted changes in JSONL after import
|
||||||
@@ -774,6 +792,7 @@ func init() {
|
|||||||
importCmd.Flags().Bool("force", false, "Force metadata update even when database is already in sync with JSONL")
|
importCmd.Flags().Bool("force", false, "Force metadata update even when database is already in sync with JSONL")
|
||||||
importCmd.Flags().Bool("no-git-history", false, "Skip git history backfill for deletions (use during JSONL filename migrations)")
|
importCmd.Flags().Bool("no-git-history", false, "Skip git history backfill for deletions (use during JSONL filename migrations)")
|
||||||
importCmd.Flags().Bool("ignore-deletions", false, "Import issues even if they're in the deletions manifest")
|
importCmd.Flags().Bool("ignore-deletions", false, "Import issues even if they're in the deletions manifest")
|
||||||
|
importCmd.Flags().Bool("protect-left-snapshot", false, "Protect issues in left snapshot from git-history-backfill (bd-sync-deletion fix)")
|
||||||
importCmd.Flags().BoolVar(&jsonOutput, "json", false, "Output import statistics in JSON format")
|
importCmd.Flags().BoolVar(&jsonOutput, "json", false, "Output import statistics in JSON format")
|
||||||
rootCmd.AddCommand(importCmd)
|
rootCmd.AddCommand(importCmd)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -158,15 +158,16 @@ func issueDataChanged(existing *types.Issue, updates map[string]interface{}) boo
|
|||||||
|
|
||||||
// ImportOptions configures how the import behaves
|
// ImportOptions configures how the import behaves
|
||||||
type ImportOptions struct {
|
type ImportOptions struct {
|
||||||
DryRun bool // Preview changes without applying them
|
DryRun bool // Preview changes without applying them
|
||||||
SkipUpdate bool // Skip updating existing issues (create-only mode)
|
SkipUpdate bool // Skip updating existing issues (create-only mode)
|
||||||
Strict bool // Fail on any error (dependencies, labels, etc.)
|
Strict bool // Fail on any error (dependencies, labels, etc.)
|
||||||
RenameOnImport bool // Rename imported issues to match database prefix
|
RenameOnImport bool // Rename imported issues to match database prefix
|
||||||
SkipPrefixValidation bool // Skip prefix validation (for auto-import)
|
SkipPrefixValidation bool // Skip prefix validation (for auto-import)
|
||||||
ClearDuplicateExternalRefs bool // Clear duplicate external_ref values instead of erroring
|
ClearDuplicateExternalRefs bool // Clear duplicate external_ref values instead of erroring
|
||||||
OrphanHandling string // Orphan handling mode: strict/resurrect/skip/allow (empty = use config)
|
OrphanHandling string // Orphan handling mode: strict/resurrect/skip/allow (empty = use config)
|
||||||
NoGitHistory bool // Skip git history backfill for deletions (prevents spurious deletion during JSONL migrations)
|
NoGitHistory bool // Skip git history backfill for deletions (prevents spurious deletion during JSONL migrations)
|
||||||
IgnoreDeletions bool // Import issues even if they're in the deletions manifest
|
IgnoreDeletions bool // Import issues even if they're in the deletions manifest
|
||||||
|
ProtectLocalExportIDs map[string]bool // IDs from left snapshot to protect from git-history-backfill (bd-sync-deletion fix)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ImportResult contains statistics about the import operation
|
// ImportResult contains statistics about the import operation
|
||||||
@@ -186,6 +187,8 @@ type ImportResult struct {
|
|||||||
PurgedIDs []string // IDs that were purged
|
PurgedIDs []string // IDs that were purged
|
||||||
SkippedDeleted int // Issues skipped because they're in deletions manifest
|
SkippedDeleted int // Issues skipped because they're in deletions manifest
|
||||||
SkippedDeletedIDs []string // IDs that were skipped due to deletions manifest
|
SkippedDeletedIDs []string // IDs that were skipped due to deletions manifest
|
||||||
|
PreservedLocalExport int // Issues preserved because they were in local export (bd-sync-deletion fix)
|
||||||
|
PreservedLocalIDs []string // IDs that were preserved from local export
|
||||||
}
|
}
|
||||||
|
|
||||||
// importIssuesCore handles the core import logic used by both manual and auto-import.
|
// importIssuesCore handles the core import logic used by both manual and auto-import.
|
||||||
@@ -227,6 +230,7 @@ func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage,
|
|||||||
OrphanHandling: importer.OrphanHandling(orphanHandling),
|
OrphanHandling: importer.OrphanHandling(orphanHandling),
|
||||||
NoGitHistory: opts.NoGitHistory,
|
NoGitHistory: opts.NoGitHistory,
|
||||||
IgnoreDeletions: opts.IgnoreDeletions,
|
IgnoreDeletions: opts.IgnoreDeletions,
|
||||||
|
ProtectLocalExportIDs: opts.ProtectLocalExportIDs,
|
||||||
}
|
}
|
||||||
|
|
||||||
// Delegate to the importer package
|
// Delegate to the importer package
|
||||||
@@ -252,6 +256,8 @@ func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage,
|
|||||||
PurgedIDs: result.PurgedIDs,
|
PurgedIDs: result.PurgedIDs,
|
||||||
SkippedDeleted: result.SkippedDeleted,
|
SkippedDeleted: result.SkippedDeleted,
|
||||||
SkippedDeletedIDs: result.SkippedDeletedIDs,
|
SkippedDeletedIDs: result.SkippedDeletedIDs,
|
||||||
|
PreservedLocalExport: result.PreservedLocalExport,
|
||||||
|
PreservedLocalIDs: result.PreservedLocalIDs,
|
||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -520,8 +520,10 @@ Use --merge to merge the sync branch back to main branch.`,
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Step 4: Import updated JSONL after pull
|
// Step 4: Import updated JSONL after pull
|
||||||
|
// Enable --protect-left-snapshot to prevent git-history-backfill from
|
||||||
|
// tombstoning issues that were in our local export but got lost during merge (bd-sync-deletion fix)
|
||||||
fmt.Println("→ Importing updated JSONL...")
|
fmt.Println("→ Importing updated JSONL...")
|
||||||
if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory); err != nil {
|
if err := importFromJSONL(ctx, jsonlPath, renameOnImport, noGitHistory, true); err != nil {
|
||||||
fmt.Fprintf(os.Stderr, "Error importing: %v\n", err)
|
fmt.Fprintf(os.Stderr, "Error importing: %v\n", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
}
|
}
|
||||||
@@ -1455,23 +1457,37 @@ func mergeSyncBranch(ctx context.Context, dryRun bool) error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// importFromJSONL imports the JSONL file by running the import command
|
// importFromJSONL imports the JSONL file by running the import command
|
||||||
func importFromJSONL(ctx context.Context, jsonlPath string, renameOnImport bool, noGitHistory ...bool) error {
|
// Optional parameters: noGitHistory, protectLeftSnapshot (bd-sync-deletion fix)
|
||||||
|
func importFromJSONL(ctx context.Context, jsonlPath string, renameOnImport bool, opts ...bool) error {
|
||||||
// Get current executable path to avoid "./bd" path issues
|
// Get current executable path to avoid "./bd" path issues
|
||||||
exe, err := os.Executable()
|
exe, err := os.Executable()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("cannot resolve current executable: %w", err)
|
return fmt.Errorf("cannot resolve current executable: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Parse optional parameters
|
||||||
|
noGitHistory := false
|
||||||
|
protectLeftSnapshot := false
|
||||||
|
if len(opts) > 0 {
|
||||||
|
noGitHistory = opts[0]
|
||||||
|
}
|
||||||
|
if len(opts) > 1 {
|
||||||
|
protectLeftSnapshot = opts[1]
|
||||||
|
}
|
||||||
|
|
||||||
// Build args for import command
|
// Build args for import command
|
||||||
// Use --no-daemon to ensure subprocess uses direct mode, avoiding daemon connection issues
|
// Use --no-daemon to ensure subprocess uses direct mode, avoiding daemon connection issues
|
||||||
args := []string{"--no-daemon", "import", "-i", jsonlPath}
|
args := []string{"--no-daemon", "import", "-i", jsonlPath}
|
||||||
if renameOnImport {
|
if renameOnImport {
|
||||||
args = append(args, "--rename-on-import")
|
args = append(args, "--rename-on-import")
|
||||||
}
|
}
|
||||||
// Handle optional noGitHistory parameter
|
if noGitHistory {
|
||||||
if len(noGitHistory) > 0 && noGitHistory[0] {
|
|
||||||
args = append(args, "--no-git-history")
|
args = append(args, "--no-git-history")
|
||||||
}
|
}
|
||||||
|
// Add --protect-left-snapshot flag for post-pull imports (bd-sync-deletion fix)
|
||||||
|
if protectLeftSnapshot {
|
||||||
|
args = append(args, "--protect-left-snapshot")
|
||||||
|
}
|
||||||
|
|
||||||
// Run import command
|
// Run import command
|
||||||
cmd := exec.CommandContext(ctx, exe, args...) // #nosec G204 - bd import command from trusted binary
|
cmd := exec.CommandContext(ctx, exe, args...) // #nosec G204 - bd import command from trusted binary
|
||||||
|
|||||||
@@ -44,6 +44,7 @@ type Options struct {
|
|||||||
ClearDuplicateExternalRefs bool // Clear duplicate external_ref values instead of erroring
|
ClearDuplicateExternalRefs bool // Clear duplicate external_ref values instead of erroring
|
||||||
NoGitHistory bool // Skip git history backfill for deletions (prevents spurious deletion during JSONL migrations)
|
NoGitHistory bool // Skip git history backfill for deletions (prevents spurious deletion during JSONL migrations)
|
||||||
IgnoreDeletions bool // Import issues even if they're in the deletions manifest
|
IgnoreDeletions bool // Import issues even if they're in the deletions manifest
|
||||||
|
ProtectLocalExportIDs map[string]bool // IDs from left snapshot to protect from git-history-backfill (bd-sync-deletion fix)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Result contains statistics about the import operation
|
// Result contains statistics about the import operation
|
||||||
@@ -65,6 +66,8 @@ type Result struct {
|
|||||||
SkippedDeletedIDs []string // IDs that were skipped due to deletions manifest
|
SkippedDeletedIDs []string // IDs that were skipped due to deletions manifest
|
||||||
ConvertedToTombstone int // Legacy deletions.jsonl entries converted to tombstones (bd-wucl)
|
ConvertedToTombstone int // Legacy deletions.jsonl entries converted to tombstones (bd-wucl)
|
||||||
ConvertedTombstoneIDs []string // IDs that were converted to tombstones
|
ConvertedTombstoneIDs []string // IDs that were converted to tombstones
|
||||||
|
PreservedLocalExport int // Issues preserved because they were in local export (bd-sync-deletion fix)
|
||||||
|
PreservedLocalIDs []string // IDs that were preserved from local export
|
||||||
}
|
}
|
||||||
|
|
||||||
// ImportIssues handles the core import logic used by both manual and auto-import.
|
// ImportIssues handles the core import logic used by both manual and auto-import.
|
||||||
@@ -894,6 +897,17 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage,
|
|||||||
// This could be:
|
// This could be:
|
||||||
// 1. Local work (new issue not yet exported)
|
// 1. Local work (new issue not yet exported)
|
||||||
// 2. Deletion was pruned from manifest (check git history)
|
// 2. Deletion was pruned from manifest (check git history)
|
||||||
|
// 3. Issue was in local export but lost during pull/merge (bd-sync-deletion fix)
|
||||||
|
|
||||||
|
// Check if this issue was in our local export (left snapshot)
|
||||||
|
// If so, it's local work that got lost during merge - preserve it!
|
||||||
|
if opts.ProtectLocalExportIDs != nil && opts.ProtectLocalExportIDs[dbIssue.ID] {
|
||||||
|
fmt.Fprintf(os.Stderr, "Preserving %s (was in local export, lost during merge)\n", dbIssue.ID)
|
||||||
|
result.PreservedLocalExport++
|
||||||
|
result.PreservedLocalIDs = append(result.PreservedLocalIDs, dbIssue.ID)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
needGitCheck = append(needGitCheck, dbIssue.ID)
|
needGitCheck = append(needGitCheck, dbIssue.ID)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -188,6 +188,114 @@ func TestPurgeDeletedIssues_NoDeletionsManifest(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestPurgeDeletedIssues_ProtectLocalExportIDs tests that issues in ProtectLocalExportIDs
|
||||||
|
// are not tombstoned even if they're not in the JSONL (bd-sync-deletion fix)
|
||||||
|
func TestPurgeDeletedIssues_ProtectLocalExportIDs(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
|
||||||
|
// Create database
|
||||||
|
dbPath := filepath.Join(tmpDir, "beads.db")
|
||||||
|
store, err := sqlite.New(ctx, dbPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create database: %v", err)
|
||||||
|
}
|
||||||
|
defer store.Close()
|
||||||
|
|
||||||
|
// Initialize prefix
|
||||||
|
if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
|
||||||
|
t.Fatalf("failed to set prefix: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create issues in the database:
|
||||||
|
// - issue1: in JSONL (should survive)
|
||||||
|
// - issue2: NOT in JSONL, but in ProtectLocalExportIDs (should survive - this is the fix)
|
||||||
|
// - issue3: NOT in JSONL, NOT protected (would be checked by git-history, but we skip that)
|
||||||
|
issue1 := &types.Issue{
|
||||||
|
ID: "test-abc",
|
||||||
|
Title: "Issue 1 (in JSONL)",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
issue2 := &types.Issue{
|
||||||
|
ID: "test-def",
|
||||||
|
Title: "Issue 2 (protected local export)",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
issue3 := &types.Issue{
|
||||||
|
ID: "test-ghi",
|
||||||
|
Title: "Issue 3 (unprotected)",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, iss := range []*types.Issue{issue1, issue2, issue3} {
|
||||||
|
if err := store.CreateIssue(ctx, iss, "test"); err != nil {
|
||||||
|
t.Fatalf("failed to create issue %s: %v", iss.ID, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Simulate import where JSONL only has issue1 (issue2 was in our local export but lost during merge)
|
||||||
|
jsonlIssues := []*types.Issue{issue1}
|
||||||
|
|
||||||
|
result := &Result{
|
||||||
|
IDMapping: make(map[string]string),
|
||||||
|
MismatchPrefixes: make(map[string]int),
|
||||||
|
}
|
||||||
|
|
||||||
|
// Set ProtectLocalExportIDs to protect issue2 (simulates left snapshot protection)
|
||||||
|
opts := Options{
|
||||||
|
ProtectLocalExportIDs: map[string]bool{
|
||||||
|
"test-def": true, // Protect issue2
|
||||||
|
},
|
||||||
|
NoGitHistory: true, // Skip git history check for this test
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call purgeDeletedIssues
|
||||||
|
if err := purgeDeletedIssues(ctx, store, dbPath, jsonlIssues, opts, result); err != nil {
|
||||||
|
t.Fatalf("purgeDeletedIssues failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify issue2 was preserved (the fix!)
|
||||||
|
if result.PreservedLocalExport != 1 {
|
||||||
|
t.Errorf("expected 1 preserved issue, got %d", result.PreservedLocalExport)
|
||||||
|
}
|
||||||
|
if len(result.PreservedLocalIDs) != 1 || result.PreservedLocalIDs[0] != "test-def" {
|
||||||
|
t.Errorf("expected PreservedLocalIDs to contain 'test-def', got %v", result.PreservedLocalIDs)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify issue1 still exists (was in JSONL)
|
||||||
|
iss1, err := store.GetIssue(ctx, "test-abc")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetIssue failed: %v", err)
|
||||||
|
}
|
||||||
|
if iss1 == nil {
|
||||||
|
t.Errorf("expected issue1 to still exist")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify issue2 still exists (was protected)
|
||||||
|
iss2, err := store.GetIssue(ctx, "test-def")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetIssue failed: %v", err)
|
||||||
|
}
|
||||||
|
if iss2 == nil {
|
||||||
|
t.Errorf("expected issue2 (protected local export) to still exist - THIS IS THE FIX")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify issue3 still exists (not in deletions, git history check skipped)
|
||||||
|
iss3, err := store.GetIssue(ctx, "test-ghi")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetIssue failed: %v", err)
|
||||||
|
}
|
||||||
|
if iss3 == nil {
|
||||||
|
t.Errorf("expected issue3 to still exist (git history check skipped)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestPurgeDeletedIssues_EmptyDeletionsManifest tests that import works with empty deletions manifest
|
// TestPurgeDeletedIssues_EmptyDeletionsManifest tests that import works with empty deletions manifest
|
||||||
func TestPurgeDeletedIssues_EmptyDeletionsManifest(t *testing.T) {
|
func TestPurgeDeletedIssues_EmptyDeletionsManifest(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|||||||
Reference in New Issue
Block a user