diff --git a/cmd/bd/export.go b/cmd/bd/export.go index df8cde5f..403cd459 100644 --- a/cmd/bd/export.go +++ b/cmd/bd/export.go @@ -179,7 +179,8 @@ Examples: labelsAny = util.NormalizeLabels(labelsAny) // Build filter - filter := types.IssueFilter{} + // Include tombstones in export for sync propagation (bd-dve) + filter := types.IssueFilter{IncludeTombstones: true} if statusFilter != "" { status := types.Status(statusFilter) filter.Status = &status diff --git a/internal/importer/importer.go b/internal/importer/importer.go index e0f4d6aa..da800a0e 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -117,17 +117,38 @@ func ImportIssues(ctx context.Context, dbPath string, store storage.Storage, iss opts.OrphanHandling = sqliteStore.GetOrphanHandling(ctx) } - // Filter out issues that are in the deletions manifest (bd-4zy) - // Unless IgnoreDeletions is set, skip importing deleted issues + // Handle deletions manifest and tombstones (bd-dve) + // + // Phase 1 (Dual-Write): + // - Tombstones in JSONL are imported as-is (they're issues with status=tombstone) + // - Legacy deletions.jsonl entries are converted to tombstones + // - Non-tombstone issues in deletions manifest are skipped (backwards compat) + // + // Note: Tombstones from JSONL take precedence over legacy deletions.jsonl if !opts.IgnoreDeletions && dbPath != "" { beadsDir := filepath.Dir(dbPath) deletionsPath := deletions.DefaultPath(beadsDir) loadResult, err := deletions.LoadDeletions(deletionsPath) if err == nil && len(loadResult.Records) > 0 { + // Build a map of existing tombstones from JSONL for quick lookup + tombstoneIDs := make(map[string]bool) + for _, issue := range issues { + if issue.IsTombstone() { + tombstoneIDs[issue.ID] = true + } + } + var filteredIssues []*types.Issue for _, issue := range issues { + // Tombstones are always imported (they represent deletions in the new format) + if issue.IsTombstone() { + filteredIssues = append(filteredIssues, issue) + continue + } + if del, found := loadResult.Records[issue.ID]; found { - // Issue is in deletions manifest - skip it + // Non-tombstone issue is in deletions manifest - skip it + // (this maintains backward compatibility during transition) result.SkippedDeleted++ result.SkippedDeletedIDs = append(result.SkippedDeletedIDs, issue.ID) fmt.Fprintf(os.Stderr, "Skipping %s (in deletions manifest: deleted %s by %s)\n", @@ -136,6 +157,19 @@ func ImportIssues(ctx context.Context, dbPath string, store storage.Storage, iss filteredIssues = append(filteredIssues, issue) } } + + // Convert legacy deletions.jsonl entries to tombstones if not already in JSONL + for id, del := range loadResult.Records { + if tombstoneIDs[id] { + // Already have a tombstone for this ID in JSONL, skip + continue + } + // Check if we skipped this issue above (it was in JSONL but filtered out) + // If so, we should create a tombstone for it + tombstone := convertDeletionToTombstone(id, del) + filteredIssues = append(filteredIssues, tombstone) + } + issues = filteredIssues } } @@ -786,10 +820,15 @@ func importComments(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, issu return nil } -// purgeDeletedIssues removes issues from the DB that are in the deletions manifest -// but not in the incoming JSONL. This enables deletion propagation across clones. +// purgeDeletedIssues converts DB issues to tombstones if they are in the deletions +// manifest but not in the incoming JSONL. This enables deletion propagation across clones. // Also uses git history fallback for deletions that were pruned from the manifest, // unless opts.NoGitHistory is set (useful during JSONL filename migrations). +// +// Note (bd-dve): With inline tombstones, most deletions are now handled during import +// via convertDeletionToTombstone. This function primarily handles: +// 1. DB-only issues that need to be tombstoned (not in JSONL at all) +// 2. Git history fallback for pruned deletions func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, dbPath string, jsonlIssues []*types.Issue, opts Options, result *Result) error { // Get deletions manifest path (same directory as database) beadsDir := filepath.Dir(dbPath) @@ -812,7 +851,7 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, jsonlIDs[issue.ID] = true } - // Get all DB issues + // Get all DB issues (exclude existing tombstones - they're already deleted) dbIssues, err := sqliteStore.SearchIssues(ctx, "", types.IssueFilter{}) if err != nil { return fmt.Errorf("failed to get DB issues: %w", err) @@ -824,21 +863,22 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, // Find DB issues that: // 1. Are NOT in the JSONL (not synced from remote) // 2. ARE in the deletions manifest (were deleted elsewhere) + // 3. Are NOT already tombstones for _, dbIssue := range dbIssues { if jsonlIDs[dbIssue.ID] { - // Issue is in JSONL, keep it + // Issue is in JSONL, keep it (tombstone or not) continue } if del, found := loadResult.Records[dbIssue.ID]; found { - // Issue is in deletions manifest - purge it from DB - if err := sqliteStore.DeleteIssue(ctx, dbIssue.ID); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to purge %s: %v\n", dbIssue.ID, err) + // Issue is in deletions manifest - convert to tombstone (bd-dve) + if err := sqliteStore.CreateTombstone(ctx, dbIssue.ID, del.Actor, del.Reason); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to create tombstone for %s: %v\n", dbIssue.ID, err) continue } - // Log the purge with metadata - fmt.Fprintf(os.Stderr, "Purged %s (deleted %s by %s", dbIssue.ID, del.Timestamp.Format("2006-01-02 15:04:05"), del.Actor) + // Log the tombstone creation with metadata + fmt.Fprintf(os.Stderr, "Tombstoned %s (deleted %s by %s", dbIssue.ID, del.Timestamp.Format("2006-01-02 15:04:05"), del.Actor) if del.Reason != "" { fmt.Fprintf(os.Stderr, ", reason: %s", del.Reason) } @@ -872,7 +912,7 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, // Abort if would delete >50% of issues - this is almost certainly a reset if deletePercent > 50 { - fmt.Fprintf(os.Stderr, "Warning: git-history-backfill would delete %d of %d issues (%.1f%%) - aborting\n", + fmt.Fprintf(os.Stderr, "Warning: git-history-backfill would tombstone %d of %d issues (%.1f%%) - aborting\n", deleteCount, totalDBIssues, deletePercent) fmt.Fprintf(os.Stderr, "This usually means the JSONL was reset (git reset, branch switch, etc.)\n") fmt.Fprintf(os.Stderr, "If these are legitimate deletions, add them to deletions.jsonl manually\n") @@ -881,7 +921,7 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, deletedViaGit = nil } else if deleteCount > 10 { // Warn (but proceed) if deleting >10 issues - fmt.Fprintf(os.Stderr, "Warning: git-history-backfill will delete %d issues (%.1f%% of %d total)\n", + fmt.Fprintf(os.Stderr, "Warning: git-history-backfill will tombstone %d issues (%.1f%% of %d total)\n", deleteCount, deletePercent, totalDBIssues) } } @@ -898,13 +938,13 @@ func purgeDeletedIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, fmt.Fprintf(os.Stderr, "Warning: failed to backfill deletion record for %s: %v\n", id, err) } - // Delete from DB - if err := sqliteStore.DeleteIssue(ctx, id); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to purge %s (git-recovered): %v\n", id, err) + // Convert to tombstone (bd-dve) + if err := sqliteStore.CreateTombstone(ctx, id, "git-history-backfill", "recovered from git history (pruned from manifest)"); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to create tombstone for %s (git-recovered): %v\n", id, err) continue } - fmt.Fprintf(os.Stderr, "Purged %s (recovered from git history, pruned from manifest)\n", id) + fmt.Fprintf(os.Stderr, "Tombstoned %s (recovered from git history, pruned from manifest)\n", id) result.Purged++ result.PurgedIDs = append(result.PurgedIDs, id) } @@ -1069,6 +1109,26 @@ func batchCheckGitHistory(repoRoot, jsonlPath string, ids []string) []string { // Helper functions +// convertDeletionToTombstone converts a legacy DeletionRecord to a tombstone Issue. +// This is used during import to migrate from deletions.jsonl to inline tombstones (bd-dve). +func convertDeletionToTombstone(id string, del deletions.DeletionRecord) *types.Issue { + deletedAt := del.Timestamp + return &types.Issue{ + ID: id, + Title: "(deleted)", + Description: "", + Status: types.StatusTombstone, + Priority: 2, // Default priority + IssueType: types.TypeTask, // Default type (original_type unknown from deletions.jsonl) + CreatedAt: del.Timestamp, + UpdatedAt: del.Timestamp, + DeletedAt: &deletedAt, + DeletedBy: del.Actor, + DeleteReason: del.Reason, + OriginalType: "", // Not available in legacy deletions.jsonl + } +} + func GetPrefixList(prefixes map[string]int) []string { var result []string keys := make([]string, 0, len(prefixes)) diff --git a/internal/importer/importer_test.go b/internal/importer/importer_test.go index ba332b81..c9a1f09b 100644 --- a/internal/importer/importer_test.go +++ b/internal/importer/importer_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/steveyegge/beads/internal/deletions" "github.com/steveyegge/beads/internal/storage/sqlite" "github.com/steveyegge/beads/internal/types" ) @@ -1107,3 +1108,161 @@ func TestBatchCheckGitHistory_NonGitDir(t *testing.T) { t.Errorf("Expected empty result for non-git dir, got %v", result) } } + +func TestConvertDeletionToTombstone(t *testing.T) { + ts := time.Date(2025, 12, 5, 14, 30, 0, 0, time.UTC) + del := deletions.DeletionRecord{ + ID: "bd-test", + Timestamp: ts, + Actor: "alice", + Reason: "no longer needed", + } + + tombstone := convertDeletionToTombstone("bd-test", del) + + if tombstone.ID != "bd-test" { + t.Errorf("Expected ID 'bd-test', got %q", tombstone.ID) + } + if tombstone.Status != types.StatusTombstone { + t.Errorf("Expected status 'tombstone', got %q", tombstone.Status) + } + if tombstone.Title != "(deleted)" { + t.Errorf("Expected title '(deleted)', got %q", tombstone.Title) + } + if tombstone.DeletedAt == nil || !tombstone.DeletedAt.Equal(ts) { + t.Errorf("Expected DeletedAt to be %v, got %v", ts, tombstone.DeletedAt) + } + if tombstone.DeletedBy != "alice" { + t.Errorf("Expected DeletedBy 'alice', got %q", tombstone.DeletedBy) + } + if tombstone.DeleteReason != "no longer needed" { + t.Errorf("Expected DeleteReason 'no longer needed', got %q", tombstone.DeleteReason) + } + if tombstone.OriginalType != "" { + t.Errorf("Expected empty OriginalType, got %q", tombstone.OriginalType) + } +} + +func TestImportIssues_TombstoneFromJSONL(t *testing.T) { + ctx := context.Background() + + tmpDB := t.TempDir() + "/test.db" + store, err := sqlite.New(context.Background(), tmpDB) + if err != nil { + t.Fatalf("Failed to create store: %v", err) + } + defer store.Close() + + if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Create a tombstone issue (as it would appear in JSONL) + deletedAt := time.Now().Add(-time.Hour) + tombstone := &types.Issue{ + ID: "test-abc123", + Title: "(deleted)", + Status: types.StatusTombstone, + Priority: 2, + IssueType: types.TypeTask, + CreatedAt: time.Now().Add(-24 * time.Hour), + UpdatedAt: deletedAt, + DeletedAt: &deletedAt, + DeletedBy: "bob", + DeleteReason: "test deletion", + OriginalType: "bug", + } + + result, err := ImportIssues(ctx, tmpDB, store, []*types.Issue{tombstone}, Options{}) + if err != nil { + t.Fatalf("Import failed: %v", err) + } + + if result.Created != 1 { + t.Errorf("Expected 1 created, got %d", result.Created) + } + + // Verify tombstone was imported with all fields + // Need to use IncludeTombstones filter to retrieve it + issues, err := store.SearchIssues(ctx, "", types.IssueFilter{IncludeTombstones: true}) + if err != nil { + t.Fatalf("Failed to search issues: %v", err) + } + + var retrieved *types.Issue + for _, i := range issues { + if i.ID == "test-abc123" { + retrieved = i + break + } + } + + if retrieved == nil { + t.Fatal("Tombstone issue not found after import") + } + if retrieved.Status != types.StatusTombstone { + t.Errorf("Expected status 'tombstone', got %q", retrieved.Status) + } + if retrieved.DeletedBy != "bob" { + t.Errorf("Expected DeletedBy 'bob', got %q", retrieved.DeletedBy) + } + if retrieved.DeleteReason != "test deletion" { + t.Errorf("Expected DeleteReason 'test deletion', got %q", retrieved.DeleteReason) + } +} + +func TestImportIssues_TombstoneNotFilteredByDeletionsManifest(t *testing.T) { + ctx := context.Background() + + tmpDir := t.TempDir() + tmpDB := tmpDir + "/test.db" + store, err := sqlite.New(context.Background(), tmpDB) + if err != nil { + t.Fatalf("Failed to create store: %v", err) + } + defer store.Close() + + if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Create a deletions manifest entry + deletionsPath := deletions.DefaultPath(tmpDir) + delRecord := deletions.DeletionRecord{ + ID: "test-abc123", + Timestamp: time.Now().Add(-time.Hour), + Actor: "alice", + Reason: "old deletion", + } + if err := deletions.AppendDeletion(deletionsPath, delRecord); err != nil { + t.Fatalf("Failed to write deletion record: %v", err) + } + + // Create a tombstone in JSONL for the same issue + deletedAt := time.Now() + tombstone := &types.Issue{ + ID: "test-abc123", + Title: "(deleted)", + Status: types.StatusTombstone, + Priority: 2, + IssueType: types.TypeTask, + CreatedAt: time.Now().Add(-24 * time.Hour), + UpdatedAt: deletedAt, + DeletedAt: &deletedAt, + DeletedBy: "bob", + DeleteReason: "JSONL tombstone", + } + + result, err := ImportIssues(ctx, tmpDB, store, []*types.Issue{tombstone}, Options{}) + if err != nil { + t.Fatalf("Import failed: %v", err) + } + + // The tombstone should be imported (not filtered by deletions manifest) + if result.Created != 1 { + t.Errorf("Expected 1 created (tombstone), got %d", result.Created) + } + if result.SkippedDeleted != 0 { + t.Errorf("Expected 0 skipped deleted (tombstone should not be filtered), got %d", result.SkippedDeleted) + } +} diff --git a/internal/importer/purge_test.go b/internal/importer/purge_test.go index be63a968..1e97b897 100644 --- a/internal/importer/purge_test.go +++ b/internal/importer/purge_test.go @@ -12,7 +12,7 @@ import ( "github.com/steveyegge/beads/internal/types" ) -// TestPurgeDeletedIssues tests that issues in the deletions manifest are purged during import +// TestPurgeDeletedIssues tests that issues in the deletions manifest are converted to tombstones during import func TestPurgeDeletedIssues(t *testing.T) { ctx := context.Background() tmpDir := t.TempDir() @@ -84,7 +84,7 @@ func TestPurgeDeletedIssues(t *testing.T) { t.Fatalf("purgeDeletedIssues failed: %v", err) } - // Verify issue2 was purged + // Verify issue2 was tombstoned (bd-dve: now converts to tombstone instead of hard-delete) if result.Purged != 1 { t.Errorf("expected 1 purged issue, got %d", result.Purged) } @@ -92,13 +92,23 @@ func TestPurgeDeletedIssues(t *testing.T) { t.Errorf("expected PurgedIDs to contain 'test-def', got %v", result.PurgedIDs) } - // Verify issue2 is gone from database - iss2, err := store.GetIssue(ctx, "test-def") + // Verify issue2 is now a tombstone (not hard-deleted) + // GetIssue returns nil for tombstones by default, so use IncludeTombstones filter + issues, err := store.SearchIssues(ctx, "", types.IssueFilter{IncludeTombstones: true}) if err != nil { - t.Fatalf("GetIssue failed: %v", err) + t.Fatalf("SearchIssues failed: %v", err) } - if iss2 != nil { - t.Errorf("expected issue2 to be deleted, but it still exists") + var iss2 *types.Issue + for _, iss := range issues { + if iss.ID == "test-def" { + iss2 = iss + break + } + } + if iss2 == nil { + t.Errorf("expected issue2 to exist as tombstone, but it was hard-deleted") + } else if iss2.Status != types.StatusTombstone { + t.Errorf("expected issue2 to be a tombstone, got status %q", iss2.Status) } // Verify issue1 still exists (in JSONL) diff --git a/internal/storage/sqlite/multirepo_export.go b/internal/storage/sqlite/multirepo_export.go index cadd53ad..40f1c6f4 100644 --- a/internal/storage/sqlite/multirepo_export.go +++ b/internal/storage/sqlite/multirepo_export.go @@ -26,8 +26,8 @@ func (s *SQLiteStorage) ExportToMultiRepo(ctx context.Context) (map[string]int, return nil, nil } - // Get all issues - allIssues, err := s.SearchIssues(ctx, "", types.IssueFilter{}) + // Get all issues including tombstones for sync propagation (bd-dve) + allIssues, err := s.SearchIssues(ctx, "", types.IssueFilter{IncludeTombstones: true}) if err != nil { return nil, fmt.Errorf("failed to query issues: %w", err) }